| Message ID | 20190715131719.100650-1-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] s390/protvirt: restore force_dma_unencrypted() | expand |
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.
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. >
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.
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.
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) {
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
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. >
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(); }
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(-)