On 6/10/19 3:18 AM, Christoph Hellwig wrote:
On Sat, Jun 08, 2019 at 04:52:24PM -0500, Larry Finger wrote:
On 6/7/19 12:29 PM, Christoph Hellwig wrote:
I don't think we should work around this in the driver, we need to fix
it in the core. I'm curious why my previous patch didn't work. Can
you throw in a few printks what failed? I.e. did dma_direct_supported
return false? Did the actual allocation fail?
Routine dma_direct_supported() returns true.
The failure is in routine dma_set_mask() in the following if test:
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
For b43legacy, dev->dma_mask is 0xc265684800000000.
dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
the routine returns -EIO.
For b43, dev->dma_mask is 0xc265684800000001,
dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
the routine returns 0.
I don't fully understand what values the above map to. Can you send
me your actual debugging patch as well?
I do not understand why the if statement returns true as neither of the values
is zero. After seeing the x86 output shown below, I also do not understand all
the trailing zeros.
My entire patch is attached. That output came from this section:
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask,
dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
On a 32-bit x86 computer with 1GB of RAM, that same output was
For b43legacy, dev->dma_mask is 0x01f4029044.
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0x3fffffff, and
the routine returns 0. 30-bit DMA works.
For b43, dev->dma_mask is 0x01f4029044,
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0xffffffff, and
the routine also returns 0. This card supports 32-bit DMA.
Larry
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2..7a367ce 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ extern void copy_user_page(void *to, void *from, unsigned long vaddr,
#endif /* __ASSEMBLY__ */
#include <asm/slice.h>
+#if 1 /* XXX: pmac? dynamic discovery? */
+#define ARCH_ZONE_DMA_BITS 30
+#else
#define ARCH_ZONE_DMA_BITS 31
+#endif
#endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 09231ef..761d951 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -20,6 +20,8 @@
*/
static inline bool dma_iommu_alloc_bypass(struct device *dev)
{
+ pr_info("dev->archdata.iommu_bypass %d, !iommu_fixed_is_weak %d\n",
+ dev->archdata.iommu_bypass, !iommu_fixed_is_weak)
return dev->archdata.iommu_bypass && !iommu_fixed_is_weak &&
dma_direct_supported(dev, dev->coherent_dma_mask);
}
@@ -27,6 +29,8 @@ static inline bool dma_iommu_alloc_bypass(struct device *dev)
static inline bool dma_iommu_map_bypass(struct device *dev,
unsigned long attrs)
{
+ pr_info("(attrs & DMA_ATTR_WEAK_ORDERING) %d\n",
+ (attrs & DMA_ATTR_WEAK_ORDERING));
return dev->archdata.iommu_bypass &&
(!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index cba2913..2540d3b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -248,7 +248,8 @@ void __init paging_init(void)
(long int)((top_of_ram - total_ram) >> 20));
#ifdef CONFIG_ZONE_DMA
- max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
+ max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
+ ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
#endif
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
#ifdef CONFIG_HIGHMEM
diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 806406a..e0270da 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1053,6 +1053,7 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f4..c625ffc 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -794,6 +794,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e..b716e62 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -391,6 +391,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
+ pr_info("min_mask 0x%x. max_pfn 0x%x, __phys_to_dma 0x%x, mask 0x%x\n", min_mask,
+ max_pfn, __phys_to_dma(dev, min_mask), mask);
return mask >= __phys_to_dma(dev, min_mask);
}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask, dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
