[PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Halil Pasic]

Since commit e67a5ed1f86f ("dma-direct: Force unencrypted DMA under SME
for certain DMA masks"), force_dma_unencrypted() is broken on s390
(under protvirt). Before used to return sev_active(), after it became
practically architecture specific, with the default implementation
always returning false.

Let's restore the old behavior of force_dma_unencrypted().

Note: we still need sev_active() defined because of the reference
in fs/core/vmcore, but this one is likely to go away soon along
with the need for an s390 sev_active().

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Fixes: e67a5ed1f86f ("dma-direct: Force unencrypted DMA under SME for
certain DMA masks")

--

Thiago has a path that gets rid of the fs/core/vmcore reference. Link:
https://patchwork.ozlabs.org/patch/1131571/

Prior discussion:
https://www.spinics.net/lists/kernel/msg3189113.html
---
 arch/s390/Kconfig                   | 1 +
 arch/s390/include/asm/mem_encrypt.h | 2 +-
 arch/s390/mm/init.c                 | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 5d8570ed6cab..a4ad2733eedf 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -189,6 +189,7 @@ config S390
 	select VIRT_CPU_ACCOUNTING
 	select ARCH_HAS_SCALED_CPUTIME
 	select HAVE_NMI
+	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
 	select SWIOTLB
 	select GENERIC_ALLOCATOR
 
diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h
index 3eb018508190..f8453f8cc191 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -7,7 +7,7 @@
 #define sme_me_mask	0ULL
 
 static inline bool sme_active(void) { return false; }
-extern bool sev_active(void);
+static inline bool sev_active(void) { return false; }
 
 int set_memory_encrypted(unsigned long addr, int numpages);
 int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index f0bee6af3960..023ab4221687 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -156,7 +156,7 @@ int set_memory_decrypted(unsigned long addr, int numpages)
 }
 
 /* are we a protected virtualization guest? */
-bool sev_active(void)
+bool force_dma_unencrypted(struct device *dev)
 {
 	return is_prot_virt_guest();
 }
-- 
2.17.1

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Christoph Hellwig]

This looks good to me - if you and Tom are fine with it I'd like to
fold it into his commit so that what I'll send to Linus is bisection
clean.

> Note: we still need sev_active() defined because of the reference
> in fs/core/vmcore, but this one is likely to go away soon along
> with the need for an s390 sev_active().

Any chance we could not change the return value from the function
at least in this patch/fold as that change seems unrelated to the
dma functionality.  If that is what you really wanted and only
the dma code was in the way we can happily merge it as a separate
patch, of couse.

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Lendacky, Thomas]

On 7/15/19 8:20 AM, Christoph Hellwig wrote:
> This looks good to me - if you and Tom are fine with it I'd like to
> fold it into his commit so that what I'll send to Linus is bisection
> clean.

I'm ok with folding it in. Sorry about missing that.

Thanks,
Tom

> 
>> Note: we still need sev_active() defined because of the reference
>> in fs/core/vmcore, but this one is likely to go away soon along
>> with the need for an s390 sev_active().
> 
> Any chance we could not change the return value from the function
> at least in this patch/fold as that change seems unrelated to the
> dma functionality.  If that is what you really wanted and only
> the dma code was in the way we can happily merge it as a separate
> patch, of couse.
> 

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Christoph Hellwig]

On Mon, Jul 15, 2019 at 01:28:19PM +0000, Lendacky, Thomas wrote:
> On 7/15/19 8:20 AM, Christoph Hellwig wrote:
> > This looks good to me - if you and Tom are fine with it I'd like to
> > fold it into his commit so that what I'll send to Linus is bisection
> > clean.
> 
> I'm ok with folding it in. Sorry about missing that.

The s390 changes were queued up in a different tree for 5.2, so you
had no easy way of noticing this.

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Christoph Hellwig]

Ok,

I've folded the minimal s390 fix in, please both double check that this
is ok as the minimally invasive fix:

http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/7bb9bbcee8845af663a7a60df9e2cc24422b3de5

The s390 fix to clean up sev_active can then go into the series that
Thiago is working on.

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Halil Pasic]

On Mon, 15 Jul 2019 06:20:27 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> This looks good to me - if you and Tom are fine with it I'd like to
> fold it into his commit so that what I'll send to Linus is bisection
> clean.

No objections here.

> 
> > Note: we still need sev_active() defined because of the reference
> > in fs/core/vmcore, but this one is likely to go away soon along
> > with the need for an s390 sev_active().
> 
> Any chance we could not change the return value from the function
> at least in this patch/fold as that change seems unrelated to the
> dma functionality.  If that is what you really wanted and only
> the dma code was in the way we can happily merge it as a separate
> patch, of couse.
> 

AFAIU the story form fs/core/vmcore.c boils down to the same on s390. I
explained this in an email I've sent a moment ago (to Thiago). I expect
sev_active() on s390 to go away soon as it really does not make sense
for us (any more).

Thus yes, we can restore the pre- e67a5ed1f86f ("dma-direct: Force
unencrypted DMA under SME for certain DMA masks") sev_active() behavior
as well, even if we don't care about it. What I did conveys the semantic
better. Not changing the behavior of however sev_active() makes more
sense if the two are going to be squashed.

The corresponding diff looks like follows. Would you like me to send it
out as v2?

Regards,
Halil

----------------------------8<---------------------------------------

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 5d8570ed6cab..a4ad2733eedf 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -189,6 +189,7 @@ config S390
        select VIRT_CPU_ACCOUNTING
        select ARCH_HAS_SCALED_CPUTIME
        select HAVE_NMI
+       select ARCH_HAS_FORCE_DMA_UNENCRYPTED
        select SWIOTLB
        select GENERIC_ALLOCATOR
 
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index f0bee6af3960..dfe47a22480a 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -155,12 +155,17 @@ int set_memory_decrypted(unsigned long addr, int numpages)
        return 0;
 }
 
-/* are we a protected virtualization guest? */
 bool sev_active(void)
 {
        return is_prot_virt_guest();
 }
 
+/* are we a protected virtualization guest? */
+bool force_dma_unencrypted(struct device *dev)
+{
+       return is_prot_virt_guest();
+}
+
 /* protected virtualization */
 static void pv_init(void)
 {

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Halil Pasic]

On Mon, 15 Jul 2019 07:20:45 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> Ok,
> 
> I've folded the minimal s390 fix in, please both double check that this
> is ok as the minimally invasive fix:
> 
> http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/7bb9bbcee8845af663a7a60df9e2cc24422b3de5
> 
> The s390 fix to clean up sev_active can then go into the series that
> Thiago is working on.
> 

Works with me.

Regards,
Halil

Re: [PATCH 1/1] s390/protvirt: restore force_dma_unencrypted()

[Lendacky, Thomas]

On 7/15/19 9:20 AM, Christoph Hellwig wrote:
> Ok,
> 
> I've folded the minimal s390 fix in, please both double check that this
> is ok as the minimally invasive fix:

Looks good to me.

Thanks,
Tom

> 
> http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/7bb9bbcee8845af663a7a60df9e2cc24422b3de5
> 
> The s390 fix to clean up sev_active can then go into the series that
> Thiago is working on.
>