x86-64 kernel dma issue; bisected

x86-64 kernel dma issue; bisected

[Al Farleigh]

re: the dma-direct code commit below

I have bisected the kernel to isolate a PCI board problem on my AMD 
x86-64 ASROCK system. The board worked at (Fedora kernel) 4.18.16 but 
stopped when moving to (Fedora kernel) 5.0. I then used 
(git/torvalds/linux) 4.20-rc4 or so to locate the fault via bisect.

I now have two kernels, good/bad, that straddle the commit.

I was asked to try v5.2 just in case it was fixed; I compiled it and the 
fault appears to still be present.

Simply, mpeg video does not stream from board; no errors, but no video.

My work is documented at
https://bugs.kde.org/show_bug.cgi?id=408004

including dmesgs, lspcis, and (second) git bisect log.

Could I please ask if and where I should submit a bug? and am willing to 
assist if I can.

Thank You!


-----

commit 55897af63091ebc2c3f239c6a6666f748113ac50
Author: Christoph Hellwig <hch@lst.de>
Date:   Mon Dec 3 11:43:54 2018 +0100

     dma-direct: merge swiotlb_dma_ops into the dma_direct code

     While the dma-direct code is (relatively) clean and simple we
     actually have to use the swiotlb ops for the mapping on many
     architectures due to devices with addressing limits.  Instead
     of keeping two implementations around this commit allows the
     dma-direct implementation to call the swiotlb bounce buffering
     functions and thus share the guts of the mapping implementation.
     This also simplified the dma-mapping setup on a few architectures
     where we don't have to differenciate which implementation to use.





_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: x86-64 kernel dma issue; bisected

[Christoph Hellwig]

On Tue, Jul 16, 2019 at 10:43:33PM -0400, Al Farleigh wrote:
> re: the dma-direct code commit below
> 
> I have bisected the kernel to isolate a PCI board problem on my AMD x86-64
> ASROCK system. The board worked at (Fedora kernel) 4.18.16 but stopped when
> moving to (Fedora kernel) 5.0. I then used (git/torvalds/linux) 4.20-rc4 or
> so to locate the fault via bisect.
> 
> I now have two kernels, good/bad, that straddle the commit.
> 
> I was asked to try v5.2 just in case it was fixed; I compiled it and the
> fault appears to still be present.
> 
> Simply, mpeg video does not stream from board; no errors, but no video.
> 
> My work is documented at
> https://bugs.kde.org/show_bug.cgi?id=408004

I've looked through that log but I'm still not sure what driver
you are actually using for video capture.  Am I right in that your
are using the saa7134 driver based on the dmesg?

That driver uses a 32-bit DMA mask, so we should be bounce buffering.

We recently had a bug with the AMD memory encryption and the encrypted
bit, are you using that?  The "Freeing unused decrypted memory:" message
somehow suggests that but I can't find where that would come from by a
simple grep.  If your are using AMD memory encryption or are not sure
please try the patch below:

---
From 9087c37584fb7d8315877bb55f85e4268cc0b4f4 Mon Sep 17 00:00:00 2001
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Wed, 10 Jul 2019 19:01:19 +0000
Subject: dma-direct: Force unencrypted DMA under SME for certain DMA masks

If a device doesn't support DMA to a physical address that includes the
encryption bit (currently bit 47, so 48-bit DMA), then the DMA must
occur to unencrypted memory. SWIOTLB is used to satisfy that requirement
if an IOMMU is not active (enabled or configured in passthrough mode).

However, commit fafadcd16595 ("swiotlb: don't dip into swiotlb pool for
coherent allocations") modified the coherent allocation support in
SWIOTLB to use the DMA direct coherent allocation support. When an IOMMU
is not active, this resulted in dma_alloc_coherent() failing for devices
that didn't support DMA addresses that included the encryption bit.

Addressing this requires changes to the force_dma_unencrypted() function
in kernel/dma/direct.c. Since the function is now non-trivial and
SME/SEV specific, update the DMA direct support to add an arch override
for the force_dma_unencrypted() function. The arch override is selected
when CONFIG_AMD_MEM_ENCRYPT is set. The arch override function resides in
the arch/x86/mm/mem_encrypt.c file and forces unencrypted DMA when either
SEV is active or SME is active and the device does not support DMA to
physical addresses that include the encryption bit.

Fixes: fafadcd16595 ("swiotlb: don't dip into swiotlb pool for coherent allocations")
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
[hch: moved the force_dma_unencrypted declaration to dma-mapping.h,
      fold the s390 fix from Halil Pasic]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/s390/Kconfig          |  1 +
 arch/s390/mm/init.c        |  7 ++++++-
 arch/x86/Kconfig           |  1 +
 arch/x86/mm/mem_encrypt.c  | 30 ++++++++++++++++++++++++++++++
 include/linux/dma-direct.h |  9 +++++++++
 kernel/dma/Kconfig         |  3 +++
 kernel/dma/direct.c        | 16 ++++------------
 7 files changed, 54 insertions(+), 13 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/mm/init.c b/arch/s390/mm/init.c
index f0bee6af3960..78c319c5ce48 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -30,7 +30,7 @@
 #include <linux/export.h>
 #include <linux/cma.h>
 #include <linux/gfp.h>
-#include <linux/dma-mapping.h>
+#include <linux/dma-direct.h>
 #include <asm/processor.h>
 #include <linux/uaccess.h>
 #include <asm/pgtable.h>
@@ -161,6 +161,11 @@ bool sev_active(void)
 	return is_prot_virt_guest();
 }
 
+bool force_dma_unencrypted(struct device *dev)
+{
+	return sev_active();
+}
+
 /* protected virtualization */
 static void pv_init(void)
 {
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 879741336771..d1afe92bf994 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1528,6 +1528,7 @@ config AMD_MEM_ENCRYPT
 	depends on X86_64 && CPU_SUP_AMD
 	select DYNAMIC_PHYSICAL_MASK
 	select ARCH_USE_MEMREMAP_PROT
+	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
 	---help---
 	  Say yes to enable support for the encryption of system memory.
 	  This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index e0df96fdfe46..c805f0a5c16e 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -15,6 +15,10 @@
 #include <linux/dma-direct.h>
 #include <linux/swiotlb.h>
 #include <linux/mem_encrypt.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/tlbflush.h>
 #include <asm/fixmap.h>
@@ -348,6 +352,32 @@ bool sev_active(void)
 }
 EXPORT_SYMBOL(sev_active);
 
+/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
+bool force_dma_unencrypted(struct device *dev)
+{
+	/*
+	 * For SEV, all DMA must be to unencrypted addresses.
+	 */
+	if (sev_active())
+		return true;
+
+	/*
+	 * For SME, all DMA must be to unencrypted addresses if the
+	 * device does not support DMA to addresses that include the
+	 * encryption mask.
+	 */
+	if (sme_active()) {
+		u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask));
+		u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask,
+						dev->bus_dma_mask);
+
+		if (dma_dev_mask <= dma_enc_mask)
+			return true;
+	}
+
+	return false;
+}
+
 /* Architecture __weak replacement functions */
 void __init mem_encrypt_free_decrypted_mem(void)
 {
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index b7338702592a..adf993a3bd58 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -32,6 +32,15 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
 }
 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
 
+#ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
+bool force_dma_unencrypted(struct device *dev);
+#else
+static inline bool force_dma_unencrypted(struct device *dev)
+{
+	return false;
+}
+#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
+
 /*
  * If memory encryption is supported, phys_to_dma will set the memory encryption
  * bit in the DMA address, and dma_to_phys will clear it.  The raw __phys_to_dma
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 70f8f8d9200e..9decbba255fc 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -48,6 +48,9 @@ config ARCH_HAS_DMA_COHERENT_TO_PFN
 config ARCH_HAS_DMA_MMAP_PGPROT
 	bool
 
+config ARCH_HAS_FORCE_DMA_UNENCRYPTED
+	bool
+
 config DMA_NONCOHERENT_CACHE_SYNC
 	bool
 
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index b90e1aede743..d7cec866d16b 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -23,14 +23,6 @@
 #define ARCH_ZONE_DMA_BITS 24
 #endif
 
-/*
- * For AMD SEV all DMA must be to unencrypted addresses.
- */
-static inline bool force_dma_unencrypted(void)
-{
-	return sev_active();
-}
-
 static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
 {
 	if (!dev->dma_mask) {
@@ -46,7 +38,7 @@ static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
 		phys_addr_t phys)
 {
-	if (force_dma_unencrypted())
+	if (force_dma_unencrypted(dev))
 		return __phys_to_dma(dev, phys);
 	return phys_to_dma(dev, phys);
 }
@@ -67,7 +59,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
 	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
 		dma_mask = dev->bus_dma_mask;
 
-	if (force_dma_unencrypted())
+	if (force_dma_unencrypted(dev))
 		*phys_mask = __dma_to_phys(dev, dma_mask);
 	else
 		*phys_mask = dma_to_phys(dev, dma_mask);
@@ -159,7 +151,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 	}
 
 	ret = page_address(page);
-	if (force_dma_unencrypted()) {
+	if (force_dma_unencrypted(dev)) {
 		set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
 		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
 	} else {
@@ -192,7 +184,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
 		return;
 	}
 
-	if (force_dma_unencrypted())
+	if (force_dma_unencrypted(dev))
 		set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
 
 	if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
-- 
2.20.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: x86-64 kernel dma issue; bisected

[Lendacky, Thomas]

On 7/18/19 4:31 AM, Christoph Hellwig wrote:
> On Tue, Jul 16, 2019 at 10:43:33PM -0400, Al Farleigh wrote:
>> re: the dma-direct code commit below
>>
>> I have bisected the kernel to isolate a PCI board problem on my AMD x86-64
>> ASROCK system. The board worked at (Fedora kernel) 4.18.16 but stopped when
>> moving to (Fedora kernel) 5.0. I then used (git/torvalds/linux) 4.20-rc4 or
>> so to locate the fault via bisect.
>>
>> I now have two kernels, good/bad, that straddle the commit.
>>
>> I was asked to try v5.2 just in case it was fixed; I compiled it and the
>> fault appears to still be present.
>>
>> Simply, mpeg video does not stream from board; no errors, but no video.
>>
>> My work is documented at
>> https://bugs.kde.org/show_bug.cgi?id=408004
> 
> I've looked through that log but I'm still not sure what driver
> you are actually using for video capture.  Am I right in that your
> are using the saa7134 driver based on the dmesg?
> 
> That driver uses a 32-bit DMA mask, so we should be bounce buffering.
> 
> We recently had a bug with the AMD memory encryption and the encrypted
> bit, are you using that?  The "Freeing unused decrypted memory:" message
> somehow suggests that but I can't find where that would come from by a
> simple grep.  If your are using AMD memory encryption or are not sure
> please try the patch below:

Based on the dmesg, this is a family 15h processor, so it doesn't support
memory encryption.

The "Freeing unused decrypted memory:" message is a special section in
support of memory encryption, but since memory encryption isn't active,
it is all freed.

So I suspect this is a different issue.

Thanks,
Tom

> 
> ---
> From 9087c37584fb7d8315877bb55f85e4268cc0b4f4 Mon Sep 17 00:00:00 2001
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Wed, 10 Jul 2019 19:01:19 +0000
> Subject: dma-direct: Force unencrypted DMA under SME for certain DMA masks
> 
> If a device doesn't support DMA to a physical address that includes the
> encryption bit (currently bit 47, so 48-bit DMA), then the DMA must
> occur to unencrypted memory. SWIOTLB is used to satisfy that requirement
> if an IOMMU is not active (enabled or configured in passthrough mode).
> 
> However, commit fafadcd16595 ("swiotlb: don't dip into swiotlb pool for
> coherent allocations") modified the coherent allocation support in
> SWIOTLB to use the DMA direct coherent allocation support. When an IOMMU
> is not active, this resulted in dma_alloc_coherent() failing for devices
> that didn't support DMA addresses that included the encryption bit.
> 
> Addressing this requires changes to the force_dma_unencrypted() function
> in kernel/dma/direct.c. Since the function is now non-trivial and
> SME/SEV specific, update the DMA direct support to add an arch override
> for the force_dma_unencrypted() function. The arch override is selected
> when CONFIG_AMD_MEM_ENCRYPT is set. The arch override function resides in
> the arch/x86/mm/mem_encrypt.c file and forces unencrypted DMA when either
> SEV is active or SME is active and the device does not support DMA to
> physical addresses that include the encryption bit.
> 
> Fixes: fafadcd16595 ("swiotlb: don't dip into swiotlb pool for coherent allocations")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> [hch: moved the force_dma_unencrypted declaration to dma-mapping.h,
>       fold the s390 fix from Halil Pasic]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/s390/Kconfig          |  1 +
>  arch/s390/mm/init.c        |  7 ++++++-
>  arch/x86/Kconfig           |  1 +
>  arch/x86/mm/mem_encrypt.c  | 30 ++++++++++++++++++++++++++++++
>  include/linux/dma-direct.h |  9 +++++++++
>  kernel/dma/Kconfig         |  3 +++
>  kernel/dma/direct.c        | 16 ++++------------
>  7 files changed, 54 insertions(+), 13 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/mm/init.c b/arch/s390/mm/init.c
> index f0bee6af3960..78c319c5ce48 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -30,7 +30,7 @@
>  #include <linux/export.h>
>  #include <linux/cma.h>
>  #include <linux/gfp.h>
> -#include <linux/dma-mapping.h>
> +#include <linux/dma-direct.h>
>  #include <asm/processor.h>
>  #include <linux/uaccess.h>
>  #include <asm/pgtable.h>
> @@ -161,6 +161,11 @@ bool sev_active(void)
>  	return is_prot_virt_guest();
>  }
>  
> +bool force_dma_unencrypted(struct device *dev)
> +{
> +	return sev_active();
> +}
> +
>  /* protected virtualization */
>  static void pv_init(void)
>  {
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 879741336771..d1afe92bf994 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1528,6 +1528,7 @@ config AMD_MEM_ENCRYPT
>  	depends on X86_64 && CPU_SUP_AMD
>  	select DYNAMIC_PHYSICAL_MASK
>  	select ARCH_USE_MEMREMAP_PROT
> +	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
>  	---help---
>  	  Say yes to enable support for the encryption of system memory.
>  	  This requires an AMD processor that supports Secure Memory
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index e0df96fdfe46..c805f0a5c16e 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -15,6 +15,10 @@
>  #include <linux/dma-direct.h>
>  #include <linux/swiotlb.h>
>  #include <linux/mem_encrypt.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/dma-mapping.h>
>  
>  #include <asm/tlbflush.h>
>  #include <asm/fixmap.h>
> @@ -348,6 +352,32 @@ bool sev_active(void)
>  }
>  EXPORT_SYMBOL(sev_active);
>  
> +/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> +bool force_dma_unencrypted(struct device *dev)
> +{
> +	/*
> +	 * For SEV, all DMA must be to unencrypted addresses.
> +	 */
> +	if (sev_active())
> +		return true;
> +
> +	/*
> +	 * For SME, all DMA must be to unencrypted addresses if the
> +	 * device does not support DMA to addresses that include the
> +	 * encryption mask.
> +	 */
> +	if (sme_active()) {
> +		u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask));
> +		u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask,
> +						dev->bus_dma_mask);
> +
> +		if (dma_dev_mask <= dma_enc_mask)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  /* Architecture __weak replacement functions */
>  void __init mem_encrypt_free_decrypted_mem(void)
>  {
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index b7338702592a..adf993a3bd58 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -32,6 +32,15 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>  }
>  #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>  
> +#ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
> +bool force_dma_unencrypted(struct device *dev);
> +#else
> +static inline bool force_dma_unencrypted(struct device *dev)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> +
>  /*
>   * If memory encryption is supported, phys_to_dma will set the memory encryption
>   * bit in the DMA address, and dma_to_phys will clear it.  The raw __phys_to_dma
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 70f8f8d9200e..9decbba255fc 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -48,6 +48,9 @@ config ARCH_HAS_DMA_COHERENT_TO_PFN
>  config ARCH_HAS_DMA_MMAP_PGPROT
>  	bool
>  
> +config ARCH_HAS_FORCE_DMA_UNENCRYPTED
> +	bool
> +
>  config DMA_NONCOHERENT_CACHE_SYNC
>  	bool
>  
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index b90e1aede743..d7cec866d16b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -23,14 +23,6 @@
>  #define ARCH_ZONE_DMA_BITS 24
>  #endif
>  
> -/*
> - * For AMD SEV all DMA must be to unencrypted addresses.
> - */
> -static inline bool force_dma_unencrypted(void)
> -{
> -	return sev_active();
> -}
> -
>  static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
>  {
>  	if (!dev->dma_mask) {
> @@ -46,7 +38,7 @@ static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
>  static inline dma_addr_t phys_to_dma_direct(struct device *dev,
>  		phys_addr_t phys)
>  {
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>  		return __phys_to_dma(dev, phys);
>  	return phys_to_dma(dev, phys);
>  }
> @@ -67,7 +59,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>  	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
>  		dma_mask = dev->bus_dma_mask;
>  
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>  		*phys_mask = __dma_to_phys(dev, dma_mask);
>  	else
>  		*phys_mask = dma_to_phys(dev, dma_mask);
> @@ -159,7 +151,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
>  	}
>  
>  	ret = page_address(page);
> -	if (force_dma_unencrypted()) {
> +	if (force_dma_unencrypted(dev)) {
>  		set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
>  		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
>  	} else {
> @@ -192,7 +184,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
>  		return;
>  	}
>  
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>  		set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
>  
>  	if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: x86-64 kernel dma issue; bisected

[Al Farleigh]

On 7/18/19 5:31 AM, Christoph Hellwig wrote:
> On Tue, Jul 16, 2019 at 10:43:33PM -0400, Al Farleigh wrote:
>> re: the dma-direct code commit below
>>
>> I have bisected the kernel to isolate a PCI board problem on my AMD x86-64
>> ASROCK system. The board worked at (Fedora kernel) 4.18.16 but stopped when
>> moving to (Fedora kernel) 5.0. I then used (git/torvalds/linux) 4.20-rc4 or
>> so to locate the fault via bisect.
>>
>> I now have two kernels, good/bad, that straddle the commit.
>>
>> I was asked to try v5.2 just in case it was fixed; I compiled it and the
>> fault appears to still be present.
>>
>> Simply, mpeg video does not stream from board; no errors, but no video.
>>
>> My work is documented at
>> https://bugs.kde.org/show_bug.cgi?id=408004
> 
> I've looked through that log but I'm still not sure what driver
> you are actually using for video capture.  Am I right in that your
> are using the saa7134 driver based on the dmesg?
> 
> That driver uses a 32-bit DMA mask, so we should be bounce buffering.
> 
> We recently had a bug with the AMD memory encryption and the encrypted
> bit, are you using that?  The "Freeing unused decrypted memory:" message
> somehow suggests that but I can't find where that would come from by a
> simple grep.  If your are using AMD memory encryption or are not sure
> please try the patch below:
> 
> ---
>>From 9087c37584fb7d8315877bb55f85e4268cc0b4f4 Mon Sep 17 00:00:00 2001
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Wed, 10 Jul 2019 19:01:19 +0000
> Subject: dma-direct: Force unencrypted DMA under SME for certain DMA masks

Yes, the board  is a KWorld 115 ATSC Tuner using saa7134 driver, here is 
a more detailed lspci -vvnn (while running kern v5.2) below.

<...applied patch and was compiling just as Tom's message about lack of 
memory encryption support on family 15h came through; will follow through>

Thank you for your help.



04:06.0 Multimedia controller [0480]: Philips Semiconductors 
SAA7131/SAA7133/SAA7135 Video Broadcast Decoder [1131:7133] (rev d1)
	Subsystem: KWorld Computer Co. Ltd. ATSC 115 Digital / Analog HDTV 
Tuner [17de:7352]
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 32 (63750ns min, 63750ns max)
	Interrupt: pin A routed to IRQ 21
	NUMA node: 0
	Region 0: Memory at fe700000 (32-bit, non-prefetchable) [size=2K]
	Capabilities: [40] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=3 PME-
	Kernel driver in use: saa7134
	Kernel modules: saa7134





> 
> If a device doesn't support DMA to a physical address that includes the
> encryption bit (currently bit 47, so 48-bit DMA), then the DMA must
> occur to unencrypted memory. SWIOTLB is used to satisfy that requirement
> if an IOMMU is not active (enabled or configured in passthrough mode).
> 
> However, commit fafadcd16595 ("swiotlb: don't dip into swiotlb pool for
> coherent allocations") modified the coherent allocation support in
> SWIOTLB to use the DMA direct coherent allocation support. When an IOMMU
> is not active, this resulted in dma_alloc_coherent() failing for devices
> that didn't support DMA addresses that included the encryption bit.
> 
> Addressing this requires changes to the force_dma_unencrypted() function
> in kernel/dma/direct.c. Since the function is now non-trivial and
> SME/SEV specific, update the DMA direct support to add an arch override
> for the force_dma_unencrypted() function. The arch override is selected
> when CONFIG_AMD_MEM_ENCRYPT is set. The arch override function resides in
> the arch/x86/mm/mem_encrypt.c file and forces unencrypted DMA when either
> SEV is active or SME is active and the device does not support DMA to
> physical addresses that include the encryption bit.
> 
> Fixes: fafadcd16595 ("swiotlb: don't dip into swiotlb pool for coherent allocations")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> [hch: moved the force_dma_unencrypted declaration to dma-mapping.h,
>        fold the s390 fix from Halil Pasic]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   arch/s390/Kconfig          |  1 +
>   arch/s390/mm/init.c        |  7 ++++++-
>   arch/x86/Kconfig           |  1 +
>   arch/x86/mm/mem_encrypt.c  | 30 ++++++++++++++++++++++++++++++
>   include/linux/dma-direct.h |  9 +++++++++
>   kernel/dma/Kconfig         |  3 +++
>   kernel/dma/direct.c        | 16 ++++------------
>   7 files changed, 54 insertions(+), 13 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/mm/init.c b/arch/s390/mm/init.c
> index f0bee6af3960..78c319c5ce48 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -30,7 +30,7 @@
>   #include <linux/export.h>
>   #include <linux/cma.h>
>   #include <linux/gfp.h>
> -#include <linux/dma-mapping.h>
> +#include <linux/dma-direct.h>
>   #include <asm/processor.h>
>   #include <linux/uaccess.h>
>   #include <asm/pgtable.h>
> @@ -161,6 +161,11 @@ bool sev_active(void)
>   	return is_prot_virt_guest();
>   }
>   
> +bool force_dma_unencrypted(struct device *dev)
> +{
> +	return sev_active();
> +}
> +
>   /* protected virtualization */
>   static void pv_init(void)
>   {
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 879741336771..d1afe92bf994 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1528,6 +1528,7 @@ config AMD_MEM_ENCRYPT
>   	depends on X86_64 && CPU_SUP_AMD
>   	select DYNAMIC_PHYSICAL_MASK
>   	select ARCH_USE_MEMREMAP_PROT
> +	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
>   	---help---
>   	  Say yes to enable support for the encryption of system memory.
>   	  This requires an AMD processor that supports Secure Memory
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index e0df96fdfe46..c805f0a5c16e 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -15,6 +15,10 @@
>   #include <linux/dma-direct.h>
>   #include <linux/swiotlb.h>
>   #include <linux/mem_encrypt.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/dma-mapping.h>
>   
>   #include <asm/tlbflush.h>
>   #include <asm/fixmap.h>
> @@ -348,6 +352,32 @@ bool sev_active(void)
>   }
>   EXPORT_SYMBOL(sev_active);
>   
> +/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> +bool force_dma_unencrypted(struct device *dev)
> +{
> +	/*
> +	 * For SEV, all DMA must be to unencrypted addresses.
> +	 */
> +	if (sev_active())
> +		return true;
> +
> +	/*
> +	 * For SME, all DMA must be to unencrypted addresses if the
> +	 * device does not support DMA to addresses that include the
> +	 * encryption mask.
> +	 */
> +	if (sme_active()) {
> +		u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask));
> +		u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask,
> +						dev->bus_dma_mask);
> +
> +		if (dma_dev_mask <= dma_enc_mask)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   /* Architecture __weak replacement functions */
>   void __init mem_encrypt_free_decrypted_mem(void)
>   {
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index b7338702592a..adf993a3bd58 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -32,6 +32,15 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>   }
>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>   
> +#ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
> +bool force_dma_unencrypted(struct device *dev);
> +#else
> +static inline bool force_dma_unencrypted(struct device *dev)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> +
>   /*
>    * If memory encryption is supported, phys_to_dma will set the memory encryption
>    * bit in the DMA address, and dma_to_phys will clear it.  The raw __phys_to_dma
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 70f8f8d9200e..9decbba255fc 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -48,6 +48,9 @@ config ARCH_HAS_DMA_COHERENT_TO_PFN
>   config ARCH_HAS_DMA_MMAP_PGPROT
>   	bool
>   
> +config ARCH_HAS_FORCE_DMA_UNENCRYPTED
> +	bool
> +
>   config DMA_NONCOHERENT_CACHE_SYNC
>   	bool
>   
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index b90e1aede743..d7cec866d16b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -23,14 +23,6 @@
>   #define ARCH_ZONE_DMA_BITS 24
>   #endif
>   
> -/*
> - * For AMD SEV all DMA must be to unencrypted addresses.
> - */
> -static inline bool force_dma_unencrypted(void)
> -{
> -	return sev_active();
> -}
> -
>   static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
>   {
>   	if (!dev->dma_mask) {
> @@ -46,7 +38,7 @@ static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
>   static inline dma_addr_t phys_to_dma_direct(struct device *dev,
>   		phys_addr_t phys)
>   {
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>   		return __phys_to_dma(dev, phys);
>   	return phys_to_dma(dev, phys);
>   }
> @@ -67,7 +59,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>   	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
>   		dma_mask = dev->bus_dma_mask;
>   
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>   		*phys_mask = __dma_to_phys(dev, dma_mask);
>   	else
>   		*phys_mask = dma_to_phys(dev, dma_mask);
> @@ -159,7 +151,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
>   	}
>   
>   	ret = page_address(page);
> -	if (force_dma_unencrypted()) {
> +	if (force_dma_unencrypted(dev)) {
>   		set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
>   		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
>   	} else {
> @@ -192,7 +184,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
>   		return;
>   	}
>   
> -	if (force_dma_unencrypted())
> +	if (force_dma_unencrypted(dev))
>   		set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
>   
>   	if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: x86-64 kernel dma issue; bisected

[Al Farleigh]

On 7/18/19 5:31 AM, Christoph Hellwig wrote:
> On Tue, Jul 16, 2019 at 10:43:33PM -0400, Al Farleigh wrote:
>> re: the dma-direct code commit below
>>
>> I have bisected the kernel to isolate a PCI board problem on my AMD x86-64
>> ASROCK system. The board worked at (Fedora kernel) 4.18.16 but stopped when
>> moving to (Fedora kernel) 5.0. I then used (git/torvalds/linux) 4.20-rc4 or
>> so to locate the fault via bisect.
>>
>> I now have two kernels, good/bad, that straddle the commit.
>>
>> I was asked to try v5.2 just in case it was fixed; I compiled it and the
>> fault appears to still be present.
>>
>> Simply, mpeg video does not stream from board; no errors, but no video.
>>
>> My work is documented at
>> https://bugs.kde.org/show_bug.cgi?id=408004
> 
> I've looked through that log but I'm still not sure what driver
> you are actually using for video capture.  Am I right in that your
> are using the saa7134 driver based on the dmesg?
> 
> That driver uses a 32-bit DMA mask, so we should be bounce buffering.
> 
> We recently had a bug with the AMD memory encryption and the encrypted
> bit, are you using that?  The "Freeing unused decrypted memory:" message
> somehow suggests that but I can't find where that would come from by a
> simple grep.  If your are using AMD memory encryption or are not sure
> please try the patch below:
> 

Hello; I installed the patch on v5.2, verified by eye it got applied, 
recompiled, and found no change; that is, the video stream doesn't display.

I also went back and verified my known good v4.20-rc4 kernel still 
worked, and my known bad v4.20-rc4 stayed bad. No changes.

I will note that the video has once or twice but extremely rarely had 
started on the bad kernels; maybe a timing or race, or somehow being set 
up from a previous boot; I try to power off between tests to make sure 
system starts from a known, uncontaminated environment.

Regards, Al Farleigh


> ---
>>From 9087c37584fb7d8315877bb55f85e4268cc0b4f4 Mon Sep 17 00:00:00 2001
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Wed, 10 Jul 2019 19:01:19 +0000
> Subject: dma-direct: Force unencrypted DMA under SME for certain DMA masks
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: x86-64 kernel dma issue; bisected

[Al Farleigh]

The patch sent in the below thread fixed the problem in kernel v5.2 on 
my system.

A sincere Thank You to everyone who jumped in to help, using their 
valuable time on this obscure issue.

Best Regards,
Alfred Farleigh

----

Re: [PATCH dma 1/1] dma-direct: correct the physical addr in 
dma_direct_sync_sg_for_cpu/device


Fixes: 55897af63091("dma-direct: merge swiotlb_dma_ops into the 
dma_direct code")
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
   kernel/dma/direct.c | 18 +++++++++++-------
   1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index b90e1ae..0e87f86 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -242,12 +242,14 @@ void dma_direct_sync_sg_for_device(struct device *dev,
       int i;
         for_each_sg(sgl, sg, nents, i) {
-        if (unlikely(is_swiotlb_buffer(sg_phys(sg))))
-            swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length,
+        phys_addr_t paddr = dma_to_phys(dev, sg_dma_address(sg));
+
+        if (unlikely(is_swiotlb_buffer(paddr)))
+            swiotlb_tbl_sync_single(dev, paddr, sg->length,
                       dir, SYNC_FOR_DEVICE);
             if (!dev_is_dma_coherent(dev))
-            arch_sync_dma_for_device(dev, sg_phys(sg), sg->length,
+            arch_sync_dma_for_device(dev, paddr, sg->length,
                       dir);
       }
   }
@@ -279,11 +281,13 @@ void dma_direct_sync_sg_for_cpu(struct device *dev,
       int i;
         for_each_sg(sgl, sg, nents, i) {
+        phys_addr_t paddr = dma_to_phys(dev, sg_dma_address(sg));
+
           if (!dev_is_dma_coherent(dev))
-            arch_sync_dma_for_cpu(dev, sg_phys(sg), sg->length, dir);
-
-        if (unlikely(is_swiotlb_buffer(sg_phys(sg))))
-            swiotlb_tbl_sync_single(dev, sg_phys(sg), sg->length, dir,
+            arch_sync_dma_for_cpu(dev, paddr, sg->length, dir);
+
+        if (unlikely(is_swiotlb_buffer(paddr)))
+            swiotlb_tbl_sync_single(dev, paddr, sg->length, dir,
                       SYNC_FOR_CPU);
       }



On 7/16/19 10:43 PM, Al Farleigh wrote:
> re: the dma-direct code commit below
> 
> I have bisected the kernel to isolate a PCI board problem on my AMD 
> x86-64 ASROCK system. The board worked at (Fedora kernel) 4.18.16 but 
> stopped when moving to (Fedora kernel) 5.0. I then used 
> (git/torvalds/linux) 4.20-rc4 or so to locate the fault via bisect.
> 
> I now have two kernels, good/bad, that straddle the commit.
> 
> I was asked to try v5.2 just in case it was fixed; I compiled it and the 
> fault appears to still be present.
> 
> Simply, mpeg video does not stream from board; no errors, but no video.
> 
> My work is documented at
> https://bugs.kde.org/show_bug.cgi?id=408004
> 
> including dmesgs, lspcis, and (second) git bisect log.
> 
> Could I please ask if and where I should submit a bug? and am willing to 
> assist if I can.
> 
> Thank You!
> 
> 
> -----
> 
> commit 55897af63091ebc2c3f239c6a6666f748113ac50
> Author: Christoph Hellwig <hch@lst.de>
> Date:   Mon Dec 3 11:43:54 2018 +0100
> 
>      dma-direct: merge swiotlb_dma_ops into the dma_direct code
> 
>      While the dma-direct code is (relatively) clean and simple we
>      actually have to use the swiotlb ops for the mapping on many
>      architectures due to devices with addressing limits.  Instead
>      of keeping two implementations around this commit allows the
>      dma-direct implementation to call the swiotlb bounce buffering
>      functions and thus share the guts of the mapping implementation.
>      This also simplified the dma-mapping setup on a few architectures
>      where we don't have to differenciate which implementation to use.
> 
> 
> 
> 
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu