Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger <Larry.Finger@lwfinger.net>]

[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]

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

[-- Attachment #2: b43legacy_tests --]
[-- Type: text/plain, Size: 4133 bytes --]

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;