[BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
not work anymore:

[   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
[   42.184837] b43legacy-phy0 debug: Chip initialized
[   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask

The same happens with the current mainline.

Bisected to:

	commit 65a21b71f948406201e4f62e41f06513350ca390
	Author: Christoph Hellwig <hch@lst.de>
	Date:   Wed Feb 13 08:01:26 2019 +0100

	    powerpc/dma: remove dma_nommu_dma_supported

	    This function is largely identical to the generic version used
	    everywhere else.  Replace it with the generic version.

	    Signed-off-by: Christoph Hellwig <hch@lst.de>
	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

A.

[BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
not work anymore:

[   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
[   42.184837] b43legacy-phy0 debug: Chip initialized
[   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask

The same happens with the current mainline.

Bisected to:

	commit 65a21b71f948406201e4f62e41f06513350ca390
	Author: Christoph Hellwig <hch@lst.de>
	Date:   Wed Feb 13 08:01:26 2019 +0100

	    powerpc/dma: remove dma_nommu_dma_supported

	    This function is largely identical to the generic version used
	    everywhere else.  Replace it with the generic version.

	    Signed-off-by: Christoph Hellwig <hch@lst.de>
	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.

How much RAM do you have ?

Ben.

> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.

How much RAM do you have ?

Ben.

> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/5/19 5:50 PM, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.
> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Aaro,

First of all, you have my sympathy for the laborious bisection on a PowerBook 
G4. I have done several myself. Thank you.

I confirm your results.

The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will fail. 
Why the 30-bit fallback fails in b43legacy fails while it works in b43 is a mystery.

Although dma_nommu_dma_supported() may be "largely identical" to 
dma_direct_supported(), they obviously differ. Routine dma_nommu_dma_supported() 
returns 1 for 32-bit systems, but I do not know what dma_direct_supported() returns.

I am trying to find a patch.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/5/19 5:50 PM, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.
> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Aaro,

First of all, you have my sympathy for the laborious bisection on a PowerBook 
G4. I have done several myself. Thank you.

I confirm your results.

The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will fail. 
Why the 30-bit fallback fails in b43legacy fails while it works in b43 is a mystery.

Although dma_nommu_dma_supported() may be "largely identical" to 
dma_direct_supported(), they obviously differ. Routine dma_nommu_dma_supported() 
returns 1 for 32-bit systems, but I do not know what dma_direct_supported() returns.

I am trying to find a patch.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Wed, Jun 05, 2019 at 10:06:18PM -0500, Larry Finger wrote:
> First of all, you have my sympathy for the laborious bisection on a 
> PowerBook G4. I have done several myself. Thank you.
>
> I confirm your results.
>
> The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will 
> fail. Why the 30-bit fallback fails in b43legacy fails while it works in 
> b43 is a mystery.
>
> Although dma_nommu_dma_supported() may be "largely identical" to 
> dma_direct_supported(), they obviously differ. Routine 
> dma_nommu_dma_supported() returns 1 for 32-bit systems, but I do not know 
> what dma_direct_supported() returns.
>
> I am trying to find a patch.

	if (IS_ENABLED(CONFIG_ZONE_DMA))
		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
	else
		min_mask = DMA_BIT_MASK(32);

	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
	return mask >= __phys_to_dma(dev, min_mask);

So the smaller or:

 (1) 32-bit
 (2) ARCH_ZONE_DMA_BITS
 (3) the actual amount of memory in the system

modolo any DMA offsets that come into play.

No offsets should exists on pmac, and ARCH_ZONE_DMA_BITS is 31 on
powerpc.  So unless the system has 1GB or less memory it will probably
return false for b43, because it can't actually guarantee reliable
allocation.  It will work fine on x86 with the smaller ZONE_DMA.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Wed, Jun 05, 2019 at 10:06:18PM -0500, Larry Finger wrote:
> First of all, you have my sympathy for the laborious bisection on a 
> PowerBook G4. I have done several myself. Thank you.
>
> I confirm your results.
>
> The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will 
> fail. Why the 30-bit fallback fails in b43legacy fails while it works in 
> b43 is a mystery.
>
> Although dma_nommu_dma_supported() may be "largely identical" to 
> dma_direct_supported(), they obviously differ. Routine 
> dma_nommu_dma_supported() returns 1 for 32-bit systems, but I do not know 
> what dma_direct_supported() returns.
>
> I am trying to find a patch.

	if (IS_ENABLED(CONFIG_ZONE_DMA))
		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
	else
		min_mask = DMA_BIT_MASK(32);

	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
	return mask >= __phys_to_dma(dev, min_mask);

So the smaller or:

 (1) 32-bit
 (2) ARCH_ZONE_DMA_BITS
 (3) the actual amount of memory in the system

modolo any DMA offsets that come into play.

No offsets should exists on pmac, and ARCH_ZONE_DMA_BITS is 31 on
powerpc.  So unless the system has 1GB or less memory it will probably
return false for b43, because it can't actually guarantee reliable
allocation.  It will work fine on x86 with the smaller ZONE_DMA.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> > not work anymore:
> > 
> > [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> > 
> > The same happens with the current mainline.
> 
> How much RAM do you have ?

The system has 1129 MB RAM. Booting with mem=1G makes it work.

A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> > not work anymore:
> > 
> > [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> > 
> > The same happens with the current mainline.
> 
> How much RAM do you have ?

The system has 1129 MB RAM. Booting with mem=1G makes it work.

A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> wrote:
> > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > Hi,
> > > 
> > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > does
> > > not work anymore:
> > > 
> > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > patch level 14 (2005-04-18 02:36:27)
> > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > support the required 30-bit DMA mask
> > > 
> > > The same happens with the current mainline.
> > 
> > How much RAM do you have ?
> 
> The system has 1129 MB RAM. Booting with mem=1G makes it work.

Wow... that's an odd amount. One thing we could possibly do is add code
to limit the amount of RAM when we detect that device....

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> wrote:
> > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > Hi,
> > > 
> > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > does
> > > not work anymore:
> > > 
> > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > patch level 14 (2005-04-18 02:36:27)
> > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > support the required 30-bit DMA mask
> > > 
> > > The same happens with the current mainline.
> > 
> > How much RAM do you have ?
> 
> The system has 1129 MB RAM. Booting with mem=1G makes it work.

Wow... that's an odd amount. One thing we could possibly do is add code
to limit the amount of RAM when we detect that device....

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 20:56 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> > wrote:
> > > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > > Hi,
> > > > 
> > > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > > does
> > > > not work anymore:
> > > > 
> > > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > > patch level 14 (2005-04-18 02:36:27)
> > > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > > support the required 30-bit DMA mask
> > > > 
> > > > The same happens with the current mainline.
> > > 
> > > How much RAM do you have ?
> > 
> > The system has 1129 MB RAM. Booting with mem=1G makes it work.
> 
> Wow... that's an odd amount. One thing we could possibly do is add code
> to limit the amount of RAM when we detect that device....

Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
on detecting the presence of that device in the device-tree.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Thu, 2019-06-06 at 20:56 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> > wrote:
> > > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > > Hi,
> > > > 
> > > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > > does
> > > > not work anymore:
> > > > 
> > > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > > patch level 14 (2005-04-18 02:36:27)
> > > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > > support the required 30-bit DMA mask
> > > > 
> > > > The same happens with the current mainline.
> > > 
> > > How much RAM do you have ?
> > 
> > The system has 1129 MB RAM. Booting with mem=1G makes it work.
> 
> Wow... that's an odd amount. One thing we could possibly do is add code
> to limit the amount of RAM when we detect that device....

Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
on detecting the presence of that device in the device-tree.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
> > Wow... that's an odd amount. One thing we could possibly do is add code
> > to limit the amount of RAM when we detect that device....
> 
> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
> on detecting the presence of that device in the device-tree.

swiotlb doesn't really help you, as these days swiotlb on matters for
the dma_map* case.  What would help is a ZONE_DMA that covers these
devices.  No need to do the 24-bit x86 does, but 30-bit would do it.

WIP patch for testing below:

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2013b4..7a367ce87c41 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ struct vm_area_struct;
 #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/mm/mem.c b/arch/powerpc/mm/mem.c
index cba29131bccc..2540d3b2588c 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

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
> > Wow... that's an odd amount. One thing we could possibly do is add code
> > to limit the amount of RAM when we detect that device....
> 
> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
> on detecting the presence of that device in the device-tree.

swiotlb doesn't really help you, as these days swiotlb on matters for
the dma_map* case.  What would help is a ZONE_DMA that covers these
devices.  No need to do the 24-bit x86 does, but 30-bit would do it.

WIP patch for testing below:

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2013b4..7a367ce87c41 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ struct vm_area_struct;
 #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/mm/mem.c b/arch/powerpc/mm/mem.c
index cba29131bccc..2540d3b2588c 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

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
> 
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case.  What would help is a ZONE_DMA that covers these
> devices.  No need to do the 24-bit x86 does, but 30-bit would do it.
> 
> WIP patch for testing below:
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
>   #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/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 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
> 

I am generating a test kernel with this patch.

FYI, the "free" command on my machine shows 1.5+ G of memory. That likely means 
I have 2G installed.

I have tested a patched kernel in which b43legacy falls back to a 31-bit DMA 
mask when the 32-bit one failed. That worked, but would likely kill the x86 
version. Let me know if think a fix in the driver rather than the kernel would 
be better. I still need to understand why the same setup works in b43 and fails 
in b43legacy. :(

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
> 
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case.  What would help is a ZONE_DMA that covers these
> devices.  No need to do the 24-bit x86 does, but 30-bit would do it.
> 
> WIP patch for testing below:
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
>   #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/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 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
> 

I am generating a test kernel with this patch.

FYI, the "free" command on my machine shows 1.5+ G of memory. That likely means 
I have 2G installed.

I have tested a patched kernel in which b43legacy falls back to a 31-bit DMA 
mask when the 32-bit one failed. That worked, but would likely kill the x86 
version. Let me know if think a fix in the driver rather than the kernel would 
be better. I still need to understand why the same setup works in b43 and fails 
in b43legacy. :(

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
> 
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case.  What would help is a ZONE_DMA that covers these
> devices.  No need to do the 24-bit x86 does, but 30-bit would do it.
> 
> WIP patch for testing below:
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
>   #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/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 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
> 

This trial patch failed.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
> 
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case.  What would help is a ZONE_DMA that covers these
> devices.  No need to do the 24-bit x86 does, but 30-bit would do it.
> 
> WIP patch for testing below:
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
>   #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/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 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
> 

This trial patch failed.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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

On 6/5/19 5:50 PM, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.
> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Aaro,

Please try the attached patch. I'm not really pleased with it and I will 
continue to determine why the fallback to a 30-bit mask fails, but at least this 
one works for me.

Larry



[-- Attachment #2: 0001-b43legacy-Fix-DMA-breakage-from-commit-commit-65a21b.patch --]
[-- Type: text/x-patch, Size: 2674 bytes --]

From 25e2f50273e785598b6bd9a8aee28cf825d3fe9f Mon Sep 17 00:00:00 2001
From: Larry Finger <Larry.Finger@lwfinger.net>
Date: Fri, 7 Jun 2019 12:04:16 -0500
Subject: [PATCH] b43legacy: Fix DMA breakage from commit commit 65a21b71f948
To: kvalo@codeaurora.org
Cc: linux-wireless@vger.kernel.org,
    pkshih@realtek.com

Following commit 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported"),
this driver errors with a message that "The machine/kernel does not
support the required 30-bit DMA mask." Indeed, the hardware only allows
31-bit masks. This solution is to change the fallback mask from 30-
to 31-bits for 32-bit PPC systems.

Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f44dd9a..75613f516e50 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -27,6 +27,15 @@
 #include <linux/slab.h>
 #include <net/dst.h>
 
+/* Special handling for PPC32 - The maximum DMA mask is 31 bits, and
+ * the fallback to 30 bits fails. Set the fallback at 31.
+ */
+#ifdef CONFIG_PPC32
+#define FB_DMA	31
+#else
+#define FB_DMA	30
+#endif
+
 /* 32bit DMA ops. */
 static
 struct b43legacy_dmadesc32 *op32_idx2desc(struct b43legacy_dmaring *ring,
@@ -418,7 +427,7 @@ static bool b43legacy_dma_mapping_error(struct b43legacy_dmaring *ring,
 
 	switch (ring->type) {
 	case B43legacy_DMA_30BIT:
-		if ((u64)addr + buffersize > (1ULL << 30))
+		if ((u64)addr + buffersize > (1ULL << FB_DMA))
 			goto address_error;
 		break;
 	case B43legacy_DMA_32BIT:
@@ -617,12 +626,12 @@ static u64 supported_dma_mask(struct b43legacy_wldev *dev)
 	if (tmp & B43legacy_DMA32_TXADDREXT_MASK)
 		return DMA_BIT_MASK(32);
 
-	return DMA_BIT_MASK(30);
+	return DMA_BIT_MASK(FB_DMA);
 }
 
 static enum b43legacy_dmatype dma_mask_to_engine_type(u64 dmamask)
 {
-	if (dmamask == DMA_BIT_MASK(30))
+	if (dmamask == DMA_BIT_MASK(FB_DMA))
 		return B43legacy_DMA_30BIT;
 	if (dmamask == DMA_BIT_MASK(32))
 		return B43legacy_DMA_32BIT;
@@ -802,7 +811,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
 			continue;
 		}
 		if (mask == DMA_BIT_MASK(32)) {
-			mask = DMA_BIT_MASK(30);
+			mask = DMA_BIT_MASK(FB_DMA);
 			fallback = true;
 			continue;
 		}
-- 
2.21.0

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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

On 6/5/19 5:50 PM, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.
> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Aaro,

Please try the attached patch. I'm not really pleased with it and I will 
continue to determine why the fallback to a 30-bit mask fails, but at least this 
one works for me.

Larry



[-- Attachment #2: 0001-b43legacy-Fix-DMA-breakage-from-commit-commit-65a21b.patch --]
[-- Type: text/x-patch, Size: 2674 bytes --]

From 25e2f50273e785598b6bd9a8aee28cf825d3fe9f Mon Sep 17 00:00:00 2001
From: Larry Finger <Larry.Finger@lwfinger.net>
Date: Fri, 7 Jun 2019 12:04:16 -0500
Subject: [PATCH] b43legacy: Fix DMA breakage from commit commit 65a21b71f948
To: kvalo@codeaurora.org
Cc: linux-wireless@vger.kernel.org,
    pkshih@realtek.com

Following commit 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported"),
this driver errors with a message that "The machine/kernel does not
support the required 30-bit DMA mask." Indeed, the hardware only allows
31-bit masks. This solution is to change the fallback mask from 30-
to 31-bits for 32-bit PPC systems.

Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f44dd9a..75613f516e50 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -27,6 +27,15 @@
 #include <linux/slab.h>
 #include <net/dst.h>
 
+/* Special handling for PPC32 - The maximum DMA mask is 31 bits, and
+ * the fallback to 30 bits fails. Set the fallback at 31.
+ */
+#ifdef CONFIG_PPC32
+#define FB_DMA	31
+#else
+#define FB_DMA	30
+#endif
+
 /* 32bit DMA ops. */
 static
 struct b43legacy_dmadesc32 *op32_idx2desc(struct b43legacy_dmaring *ring,
@@ -418,7 +427,7 @@ static bool b43legacy_dma_mapping_error(struct b43legacy_dmaring *ring,
 
 	switch (ring->type) {
 	case B43legacy_DMA_30BIT:
-		if ((u64)addr + buffersize > (1ULL << 30))
+		if ((u64)addr + buffersize > (1ULL << FB_DMA))
 			goto address_error;
 		break;
 	case B43legacy_DMA_32BIT:
@@ -617,12 +626,12 @@ static u64 supported_dma_mask(struct b43legacy_wldev *dev)
 	if (tmp & B43legacy_DMA32_TXADDREXT_MASK)
 		return DMA_BIT_MASK(32);
 
-	return DMA_BIT_MASK(30);
+	return DMA_BIT_MASK(FB_DMA);
 }
 
 static enum b43legacy_dmatype dma_mask_to_engine_type(u64 dmamask)
 {
-	if (dmamask == DMA_BIT_MASK(30))
+	if (dmamask == DMA_BIT_MASK(FB_DMA))
 		return B43legacy_DMA_30BIT;
 	if (dmamask == DMA_BIT_MASK(32))
 		return B43legacy_DMA_32BIT;
@@ -802,7 +811,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
 			continue;
 		}
 		if (mask == DMA_BIT_MASK(32)) {
-			mask = DMA_BIT_MASK(30);
+			mask = DMA_BIT_MASK(FB_DMA);
 			fallback = true;
 			continue;
 		}
-- 
2.21.0

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

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?

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

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?

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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?

I agree that that patch should not be sent upstream. I posted it only so that 
anyone running into the problem would have a work around.

I will try to see why your patch failed.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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?

I agree that that patch should not be sent upstream. I posted it only so that 
anyone running into the problem would have a work around.

I will try to see why your patch failed.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

> Please try the attached patch. I'm not really pleased with it and I will 
> continue to determine why the fallback to a 30-bit mask fails, but at least this 
> one works for me.

Your patch only makes sense if the device is indeed capable of
addressing 31-bits.

So either the driver is buggy and asks for a too small mask in which
case your patch is ok, or it's not and you're just going to cause all
sort of interesting random problems including possible memory
corruption.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

> Please try the attached patch. I'm not really pleased with it and I will 
> continue to determine why the fallback to a 30-bit mask fails, but at least this 
> one works for me.

Your patch only makes sense if the device is indeed capable of
addressing 31-bits.

So either the driver is buggy and asks for a too small mask in which
case your patch is ok, or it's not and you're just going to cause all
sort of interesting random problems including possible memory
corruption.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Sat, Jun 08, 2019 at 02:21:23PM +1000, Benjamin Herrenschmidt wrote:
> 
> > Please try the attached patch. I'm not really pleased with it and I will 
> > continue to determine why the fallback to a 30-bit mask fails, but at least this 
> > one works for me.
> 
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
> 
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.

Well, my patches changes ZONE_DMA to be 30-bits instead of 31, and
thus should allow requesting a 30-bit mask to succeed.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Sat, Jun 08, 2019 at 02:21:23PM +1000, Benjamin Herrenschmidt wrote:
> 
> > Please try the attached patch. I'm not really pleased with it and I will 
> > continue to determine why the fallback to a 30-bit mask fails, but at least this 
> > one works for me.
> 
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
> 
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.

Well, my patches changes ZONE_DMA to be 30-bits instead of 31, and
thus should allow requesting a 30-bit mask to succeed.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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.

Thus far I have not found what sets the low-order bit of dev->dma_mask. 
Suggestions are welcome.

These tests have all been with your patch that sets ARCH_ZONE_DMA_BITS to 30.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

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.

Thus far I have not found what sets the low-order bit of dev->dma_mask. 
Suggestions are welcome.

These tests have all been with your patch that sets ARCH_ZONE_DMA_BITS to 30.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

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?

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

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?

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

[-- 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;

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

[-- 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;

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> 
>> Please try the attached patch. I'm not really pleased with it and I will
>> continue to determine why the fallback to a 30-bit mask fails, but at least this
>> one works for me.
> 
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
> 
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.

Of course the driver may be buggy, but it asks for the correct mask.

This particular device is not capable of handling 32-bit DMA. The driver detects 
the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32 
until 5.1. As Christoph said, it should always be possible to use fewer bits 
than the maximum.

Similar devices that are new enough to use b43 rather than b43legacy work with 
new kernels; however, they have and use 32-bit DMA.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> 
>> Please try the attached patch. I'm not really pleased with it and I will
>> continue to determine why the fallback to a 30-bit mask fails, but at least this
>> one works for me.
> 
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
> 
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.

Of course the driver may be buggy, but it asks for the correct mask.

This particular device is not capable of handling 32-bit DMA. The driver detects 
the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32 
until 5.1. As Christoph said, it should always be possible to use fewer bits 
than the maximum.

Similar devices that are new enough to use b43 rather than b43legacy work with 
new kernels; however, they have and use 32-bit DMA.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Mon, 2019-06-10 at 13:44 -0500, Larry Finger wrote:
> On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> > 
> > > Please try the attached patch. I'm not really pleased with it and I will
> > > continue to determine why the fallback to a 30-bit mask fails, but at least this
> > > one works for me.
> > 
> > Your patch only makes sense if the device is indeed capable of
> > addressing 31-bits.
> > 
> > So either the driver is buggy and asks for a too small mask in which
> > case your patch is ok, or it's not and you're just going to cause all
> > sort of interesting random problems including possible memory
> > corruption.
> 
> Of course the driver may be buggy, but it asks for the correct mask.
> 
> This particular device is not capable of handling 32-bit DMA. The driver detects 
> the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32 
> until 5.1. As Christoph said, it should always be possible to use fewer bits 
> than the maximum.

No, I don't think it *worked* on ppc32 before Christoph patch. I think
it "mostly sort-of worked" :-)

The reason I'm saying that is if your system has more than 1GB of RAM,
then you'll have chunks of memory that the device simply cannot
address.

Before Christoph patches, we had no ZONE_DMA or ZONE_DMA32 covering the
30-bit limited space, so any memory allocation could in theory land
above 30-bits, causing all sort of horrible things to happen with that
driver.

The reason I think it sort-of-mostly-worked is that to get more than
1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
buffers aren't allocated in Highmem.... so you got lucky.

That said, there is such as thing as no-copy send on network, so I
wouldn't be surprised if some things would still have failed, just not
frequent enough for you to notice.

> Similar devices that are new enough to use b43 rather than b43legacy work with 
> new kernels; however, they have and use 32-bit DMA.

Cheres,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Mon, 2019-06-10 at 13:44 -0500, Larry Finger wrote:
> On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> > 
> > > Please try the attached patch. I'm not really pleased with it and I will
> > > continue to determine why the fallback to a 30-bit mask fails, but at least this
> > > one works for me.
> > 
> > Your patch only makes sense if the device is indeed capable of
> > addressing 31-bits.
> > 
> > So either the driver is buggy and asks for a too small mask in which
> > case your patch is ok, or it's not and you're just going to cause all
> > sort of interesting random problems including possible memory
> > corruption.
> 
> Of course the driver may be buggy, but it asks for the correct mask.
> 
> This particular device is not capable of handling 32-bit DMA. The driver detects 
> the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32 
> until 5.1. As Christoph said, it should always be possible to use fewer bits 
> than the maximum.

No, I don't think it *worked* on ppc32 before Christoph patch. I think
it "mostly sort-of worked" :-)

The reason I'm saying that is if your system has more than 1GB of RAM,
then you'll have chunks of memory that the device simply cannot
address.

Before Christoph patches, we had no ZONE_DMA or ZONE_DMA32 covering the
30-bit limited space, so any memory allocation could in theory land
above 30-bits, causing all sort of horrible things to happen with that
driver.

The reason I think it sort-of-mostly-worked is that to get more than
1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
buffers aren't allocated in Highmem.... so you got lucky.

That said, there is such as thing as no-copy send on network, so I
wouldn't be surprised if some things would still have failed, just not
frequent enough for you to notice.

> Similar devices that are new enough to use b43 rather than b43legacy work with 
> new kernels; however, they have and use 32-bit DMA.

Cheres,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>>>                  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:

What might be confusing in your output is that dev->dma_mask is a pointer,
and we are setting it in dma_set_mask.  That is before we only check
if the pointer is set, and later we override it.  Of course this doesn't
actually explain the failure.  But what is even more strange to me
is that you get a return value from dma_supported() that isn't 0 or 1,
as that function is supposed to return a boolean, and I really can't see
how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
or 1.  Does the output change if you use the correct printk specifiers?

i.e. with a debug patch like this:


diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..9e5b30b12b10 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
 int dma_direct_supported(struct device *dev, u64 mask)
 {
 	u64 min_mask;
+	bool ret;
 
 	if (IS_ENABLED(CONFIG_ZONE_DMA))
 		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
@@ -391,7 +392,12 @@ 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.
 	 */
-	return mask >= __phys_to_dma(dev, min_mask);
+	ret = (mask >= __phys_to_dma(dev, min_mask));
+	if (!ret)
+		dev_info(dev,
+			"%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
+			__func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
+	return ret;
 }
 
 size_t dma_direct_max_mapping_size(struct device *dev)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..6c57ccdee2ae 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
 
 int dma_set_mask(struct device *dev, u64 mask)
 {
-	if (!dev->dma_mask || !dma_supported(dev, mask))
+	if (!dev->dma_mask) {
+		dev_info(dev, "no DMA mask set!\n");
 		return -EIO;
+	}
+	if (!dma_supported(dev, mask)) {
+		printk("DMA not supported\n");
+		return -EIO;
+	}
 
 	arch_dma_set_mask(dev, mask);
 	dma_check_mask(dev, mask);

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>>>                  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:

What might be confusing in your output is that dev->dma_mask is a pointer,
and we are setting it in dma_set_mask.  That is before we only check
if the pointer is set, and later we override it.  Of course this doesn't
actually explain the failure.  But what is even more strange to me
is that you get a return value from dma_supported() that isn't 0 or 1,
as that function is supposed to return a boolean, and I really can't see
how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
or 1.  Does the output change if you use the correct printk specifiers?

i.e. with a debug patch like this:


diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..9e5b30b12b10 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
 int dma_direct_supported(struct device *dev, u64 mask)
 {
 	u64 min_mask;
+	bool ret;
 
 	if (IS_ENABLED(CONFIG_ZONE_DMA))
 		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
@@ -391,7 +392,12 @@ 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.
 	 */
-	return mask >= __phys_to_dma(dev, min_mask);
+	ret = (mask >= __phys_to_dma(dev, min_mask));
+	if (!ret)
+		dev_info(dev,
+			"%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
+			__func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
+	return ret;
 }
 
 size_t dma_direct_max_mapping_size(struct device *dev)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..6c57ccdee2ae 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
 
 int dma_set_mask(struct device *dev, u64 mask)
 {
-	if (!dev->dma_mask || !dma_supported(dev, mask))
+	if (!dev->dma_mask) {
+		dev_info(dev, "no DMA mask set!\n");
 		return -EIO;
+	}
+	if (!dma_supported(dev, mask)) {
+		printk("DMA not supported\n");
+		return -EIO;
+	}
 
 	arch_dma_set_mask(dev, mask);
 	dma_check_mask(dev, mask);

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt wrote:
> The reason I think it sort-of-mostly-worked is that to get more than
> 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> buffers aren't allocated in Highmem.... so you got lucky.
> 
> That said, there is such as thing as no-copy send on network, so I
> wouldn't be surprised if some things would still have failed, just not
> frequent enough for you to notice.

Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
will bounce buffer highmem pages for the driver under all circumstances.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt wrote:
> The reason I think it sort-of-mostly-worked is that to get more than
> 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> buffers aren't allocated in Highmem.... so you got lucky.
> 
> That said, there is such as thing as no-copy send on network, so I
> wouldn't be surprised if some things would still have failed, just not
> frequent enough for you to notice.

Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
will bounce buffer highmem pages for the driver under all circumstances.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> wrote:
> > The reason I think it sort-of-mostly-worked is that to get more
> > than
> > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > buffers aren't allocated in Highmem.... so you got lucky.
> > 
> > That said, there is such as thing as no-copy send on network, so I
> > wouldn't be surprised if some things would still have failed, just
> > not
> > frequent enough for you to notice.
> 
> Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> will bounce buffer highmem pages for the driver under all
> circumstances.

 ... which b43legacy doesn't set to the best of my knowledge ...

Which makes me wonder how come it didn't work even with your patches ?
AFAIK, we have less than 1GB of lowmem unless the config has been
tweaked....

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> wrote:
> > The reason I think it sort-of-mostly-worked is that to get more
> > than
> > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > buffers aren't allocated in Highmem.... so you got lucky.
> > 
> > That said, there is such as thing as no-copy send on network, so I
> > wouldn't be surprised if some things would still have failed, just
> > not
> > frequent enough for you to notice.
> 
> Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> will bounce buffer highmem pages for the driver under all
> circumstances.

 ... which b43legacy doesn't set to the best of my knowledge ...

Which makes me wonder how come it didn't work even with your patches ?
AFAIK, we have less than 1GB of lowmem unless the config has been
tweaked....

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 16:58 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> > On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> > wrote:
> > > The reason I think it sort-of-mostly-worked is that to get more
> > > than
> > > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > > buffers aren't allocated in Highmem.... so you got lucky.
> > > 
> > > That said, there is such as thing as no-copy send on network, so I
> > > wouldn't be surprised if some things would still have failed, just
> > > not
> > > frequent enough for you to notice.
> > 
> > Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> > will bounce buffer highmem pages for the driver under all
> > circumstances.
> 
>  ... which b43legacy doesn't set to the best of my knowledge ...
> 
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....

Ah stupid me ... it's dma_set_mask that failed, since it has no idea
that the calling driver is limited to lowmem.

That's also why the "wrong" patch worked.

So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 16:58 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> > On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> > wrote:
> > > The reason I think it sort-of-mostly-worked is that to get more
> > > than
> > > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > > buffers aren't allocated in Highmem.... so you got lucky.
> > > 
> > > That said, there is such as thing as no-copy send on network, so I
> > > wouldn't be surprised if some things would still have failed, just
> > > not
> > > frequent enough for you to notice.
> > 
> > Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> > will bounce buffer highmem pages for the driver under all
> > circumstances.
> 
>  ... which b43legacy doesn't set to the best of my knowledge ...
> 
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....

Ah stupid me ... it's dma_set_mask that failed, since it has no idea
that the calling driver is limited to lowmem.

That's also why the "wrong" patch worked.

So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 04:58:12PM +1000, Benjamin Herrenschmidt wrote:
>  ... which b43legacy doesn't set to the best of my knowledge ...
> 
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....

It needs to bounce to somewhere.  And the dma-direct code is pretty
strict to require a zone it can do allocations from when setting the
dma mask.  As was the old ppc64 code, but not the ppc32 code that
allowed setting any DMA mask.  And something about the more strict
validation seem to trip up now.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 04:58:12PM +1000, Benjamin Herrenschmidt wrote:
>  ... which b43legacy doesn't set to the best of my knowledge ...
> 
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....

It needs to bounce to somewhere.  And the dma-direct code is pretty
strict to require a zone it can do allocations from when setting the
dma mask.  As was the old ppc64 code, but not the ppc32 code that
allowed setting any DMA mask.  And something about the more strict
validation seem to trip up now.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 04:59:54PM +1000, Benjamin Herrenschmidt wrote:
> Ah stupid me ... it's dma_set_mask that failed, since it has no idea
> that the calling driver is limited to lowmem.
> 
> That's also why the "wrong" patch worked.
> 
> So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.

Well, according to Larry it doesn't actually work, which is odd.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 04:59:54PM +1000, Benjamin Herrenschmidt wrote:
> Ah stupid me ... it's dma_set_mask that failed, since it has no idea
> that the calling driver is limited to lowmem.
> 
> That's also why the "wrong" patch worked.
> 
> So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.

Well, according to Larry it doesn't actually work, which is odd.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 09:54 +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 04:59:54PM +1000, Benjamin Herrenschmidt
> wrote:
> > Ah stupid me ... it's dma_set_mask that failed, since it has no
> > idea
> > that the calling driver is limited to lowmem.
> > 
> > That's also why the "wrong" patch worked.
> > 
> > So yes, a ZONE_DMA at 30-bits will work, though it's somewhat
> > overkill.
> 
> Well, according to Larry it doesn't actually work, which is odd.

Oh I assume that's just a glitch in the patch :-)

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 09:54 +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 04:59:54PM +1000, Benjamin Herrenschmidt
> wrote:
> > Ah stupid me ... it's dma_set_mask that failed, since it has no
> > idea
> > that the calling driver is limited to lowmem.
> > 
> > That's also why the "wrong" patch worked.
> > 
> > So yes, a ZONE_DMA at 30-bits will work, though it's somewhat
> > overkill.
> 
> Well, according to Larry it doesn't actually work, which is odd.

Oh I assume that's just a glitch in the patch :-)

Cheers,
Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Andreas Schwab]

On Jun 10 2019, Larry Finger <Larry.Finger@lwfinger.net> wrote:

> I do not understand why the if statement returns true as neither of the
> values is zero.

That's because the format string does not make any sense.  You are
printing garbage.

> 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));

None of the format directives match the type of the arguments.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Andreas Schwab]

On Jun 10 2019, Larry Finger <Larry.Finger@lwfinger.net> wrote:

> I do not understand why the if statement returns true as neither of the
> values is zero.

That's because the format string does not make any sense.  You are
printing garbage.

> 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));

None of the format directives match the type of the arguments.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 1:05 AM, Christoph Hellwig wrote:
> On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
> 
> What might be confusing in your output is that dev->dma_mask is a pointer,
> and we are setting it in dma_set_mask.  That is before we only check
> if the pointer is set, and later we override it.  Of course this doesn't
> actually explain the failure.  But what is even more strange to me
> is that you get a return value from dma_supported() that isn't 0 or 1,
> as that function is supposed to return a boolean, and I really can't see
> how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
> or 1.  Does the output change if you use the correct printk specifiers?
> 
> i.e. with a debug patch like this:
> 
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..9e5b30b12b10 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
>   int dma_direct_supported(struct device *dev, u64 mask)
>   {
>   	u64 min_mask;
> +	bool ret;
>   
>   	if (IS_ENABLED(CONFIG_ZONE_DMA))
>   		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> @@ -391,7 +392,12 @@ 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.
>   	 */
> -	return mask >= __phys_to_dma(dev, min_mask);
> +	ret = (mask >= __phys_to_dma(dev, min_mask));
> +	if (!ret)
> +		dev_info(dev,
> +			"%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
> +			__func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
> +	return ret;
>   }
>   
>   size_t dma_direct_max_mapping_size(struct device *dev)
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..6c57ccdee2ae 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>   
>   int dma_set_mask(struct device *dev, u64 mask)
>   {
> -	if (!dev->dma_mask || !dma_supported(dev, mask))
> +	if (!dev->dma_mask) {
> +		dev_info(dev, "no DMA mask set!\n");
>   		return -EIO;
> +	}
> +	if (!dma_supported(dev, mask)) {
> +		printk("DMA not supported\n");
> +		return -EIO;
> +	}
>   
>   	arch_dma_set_mask(dev, mask);
>   	dma_check_mask(dev, mask);
> 

After I got the correct formatting, the output with this patch only gives the 
following in dmesg:

b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fffffff, 
min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
DMA not supported
b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit 
DMA mask

Your first patch did not work as the configuration does not have 
CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts at 32 
bits and is taken down to 31 with the maximum pfn minimization. When I forced 
the initial value of min_mask to 30 bits, the device worked.

It is obvious that the case of a mask smaller than min_mask should be handled by 
the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG 
variables containing IOMMU are not selected. When dma_direct_supported() fails, 
should the system not try for an IOMMU solution? Is the driver asking for the 
wrong type of memory? It is doing a dma_and_set_mask_coherent() call.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 1:05 AM, Christoph Hellwig wrote:
> On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
> 
> What might be confusing in your output is that dev->dma_mask is a pointer,
> and we are setting it in dma_set_mask.  That is before we only check
> if the pointer is set, and later we override it.  Of course this doesn't
> actually explain the failure.  But what is even more strange to me
> is that you get a return value from dma_supported() that isn't 0 or 1,
> as that function is supposed to return a boolean, and I really can't see
> how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
> or 1.  Does the output change if you use the correct printk specifiers?
> 
> i.e. with a debug patch like this:
> 
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..9e5b30b12b10 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
>   int dma_direct_supported(struct device *dev, u64 mask)
>   {
>   	u64 min_mask;
> +	bool ret;
>   
>   	if (IS_ENABLED(CONFIG_ZONE_DMA))
>   		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> @@ -391,7 +392,12 @@ 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.
>   	 */
> -	return mask >= __phys_to_dma(dev, min_mask);
> +	ret = (mask >= __phys_to_dma(dev, min_mask));
> +	if (!ret)
> +		dev_info(dev,
> +			"%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
> +			__func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
> +	return ret;
>   }
>   
>   size_t dma_direct_max_mapping_size(struct device *dev)
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..6c57ccdee2ae 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>   
>   int dma_set_mask(struct device *dev, u64 mask)
>   {
> -	if (!dev->dma_mask || !dma_supported(dev, mask))
> +	if (!dev->dma_mask) {
> +		dev_info(dev, "no DMA mask set!\n");
>   		return -EIO;
> +	}
> +	if (!dma_supported(dev, mask)) {
> +		printk("DMA not supported\n");
> +		return -EIO;
> +	}
>   
>   	arch_dma_set_mask(dev, mask);
>   	dma_check_mask(dev, mask);
> 

After I got the correct formatting, the output with this patch only gives the 
following in dmesg:

b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fffffff, 
min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
DMA not supported
b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit 
DMA mask

Your first patch did not work as the configuration does not have 
CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts at 32 
bits and is taken down to 31 with the maximum pfn minimization. When I forced 
the initial value of min_mask to 30 bits, the device worked.

It is obvious that the case of a mask smaller than min_mask should be handled by 
the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG 
variables containing IOMMU are not selected. When dma_direct_supported() fails, 
should the system not try for an IOMMU solution? Is the driver asking for the 
wrong type of memory? It is doing a dma_and_set_mask_coherent() call.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
> It is obvious that the case of a mask smaller than min_mask should be
> handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All
> other CONFIG variables containing IOMMU are not selected. When
> dma_direct_supported() fails, should the system not try for an IOMMU
> solution? Is the driver asking for the wrong type of memory? It is doing a
> dma_and_set_mask_coherent() call.

I don't think we have IOMMU on G4. On G5 it should work (I remember fixing
b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43
are dead and waiting for re-capping).

A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Aaro Koskinen]

Hi,

On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
> It is obvious that the case of a mask smaller than min_mask should be
> handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All
> other CONFIG variables containing IOMMU are not selected. When
> dma_direct_supported() fails, should the system not try for an IOMMU
> solution? Is the driver asking for the wrong type of memory? It is doing a
> dma_and_set_mask_coherent() call.

I don't think we have IOMMU on G4. On G5 it should work (I remember fixing
b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43
are dead and waiting for re-capping).

A.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
> 0x3fffffff, 
> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f

Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
bogus.

Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
> 0x3fffffff, 
> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f

Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
bogus.

Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
>> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
>> 0x3fffffff,
>> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
> 
> Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
> bogus.

I agree, but that is not likely serious as most systems will have enough memory 
that the max_pfn term will be much larger than the initial min_mask, and 
min_mask will be unchanged by the min function. In addition, min_mask is not 
used beyond this routine, and then only to decide if direct dma is supported. 
The following patch generates masks with no holes, but I cannot see that it is 
needed.

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..e3edd4f29e80 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
         else
                 min_mask = DMA_BIT_MASK(32);

-       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+       min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) |
+                                        DMA_BIT_MASK(PAGE_SHIFT));

         /*
          * This check needs to be against the actual bit mask value, so


Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
>> b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
>> 0x3fffffff,
>> min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
> 
> Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
> bogus.

I agree, but that is not likely serious as most systems will have enough memory 
that the max_pfn term will be much larger than the initial min_mask, and 
min_mask will be unchanged by the min function. In addition, min_mask is not 
used beyond this routine, and then only to decide if direct dma is supported. 
The following patch generates masks with no holes, but I cannot see that it is 
needed.

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..e3edd4f29e80 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
         else
                 min_mask = DMA_BIT_MASK(32);

-       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+       min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) |
+                                        DMA_BIT_MASK(PAGE_SHIFT));

         /*
          * This check needs to be against the actual bit mask value, so


Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 5:46 PM, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
>> It is obvious that the case of a mask smaller than min_mask should be
>> handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All
>> other CONFIG variables containing IOMMU are not selected. When
>> dma_direct_supported() fails, should the system not try for an IOMMU
>> solution? Is the driver asking for the wrong type of memory? It is doing a
>> dma_and_set_mask_coherent() call.
> 
> I don't think we have IOMMU on G4. On G5 it should work (I remember fixing
> b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43
> are dead and waiting for re-capping).

You are right. My configuration has CONFIG_IOMMU_SUPPORT=y, but there is no 
mention of an IOMMU in the log.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/11/19 5:46 PM, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
>> It is obvious that the case of a mask smaller than min_mask should be
>> handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All
>> other CONFIG variables containing IOMMU are not selected. When
>> dma_direct_supported() fails, should the system not try for an IOMMU
>> solution? Is the driver asking for the wrong type of memory? It is doing a
>> dma_and_set_mask_coherent() call.
> 
> I don't think we have IOMMU on G4. On G5 it should work (I remember fixing
> b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43
> are dead and waiting for re-capping).

You are right. My configuration has CONFIG_IOMMU_SUPPORT=y, but there is no 
mention of an IOMMU in the log.

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 20:52 -0500, Larry Finger wrote:
> On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote:
> > On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
> > > b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
> > > 0x3fffffff,
> > > min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
> > 
> > Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
> > bogus.
> 
> I agree, but that is not likely serious as most systems will have enough memory 
> that the max_pfn term will be much larger than the initial min_mask, and 
> min_mask will be unchanged by the min function. 

Well no... it's too much memory that is the problem. If min_mask is
bogus though it will cause problem later too, so one should look into
it.

> In addition, min_mask is not 
> used beyond this routine, and then only to decide if direct dma is supported. 
> The following patch generates masks with no holes, but I cannot see that it is 
> needed.

The right fix is to round up max_pfn to a power of 2, something like

min_mask = min_t(u64, min_mask, (roundup_pow_of_two(max_pfn - 1)) <<
		PAGE_SHIFT) 

> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..e3edd4f29e80 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
>          else
>                  min_mask = DMA_BIT_MASK(32);
> 
> -       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +       min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) |
> +                                        DMA_BIT_MASK(PAGE_SHIFT));
> 
>          /*
>           * This check needs to be against the actual bit mask value, so
> 
> 
> Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Tue, 2019-06-11 at 20:52 -0500, Larry Finger wrote:
> On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote:
> > On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote:
> > > b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask =
> > > 0x3fffffff,
> > > min_mask = 0x5ffff000/0x5ffff000, dma bits = 0x1f
> > 
> > Ugh ? A mask with holes in it ? That's very wrong... That min_mask is
> > bogus.
> 
> I agree, but that is not likely serious as most systems will have enough memory 
> that the max_pfn term will be much larger than the initial min_mask, and 
> min_mask will be unchanged by the min function. 

Well no... it's too much memory that is the problem. If min_mask is
bogus though it will cause problem later too, so one should look into
it.

> In addition, min_mask is not 
> used beyond this routine, and then only to decide if direct dma is supported. 
> The following patch generates masks with no holes, but I cannot see that it is 
> needed.

The right fix is to round up max_pfn to a power of 2, something like

min_mask = min_t(u64, min_mask, (roundup_pow_of_two(max_pfn - 1)) <<
		PAGE_SHIFT) 

> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 2c2772e9702a..e3edd4f29e80 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
>          else
>                  min_mask = DMA_BIT_MASK(32);
> 
> -       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +       min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) |
> +                                        DMA_BIT_MASK(PAGE_SHIFT));
> 
>          /*
>           * This check needs to be against the actual bit mask value, so
> 
> 
> Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
> Your first patch did not work as the configuration does not have 
> CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts 
> at 32 bits and is taken down to 31 with the maximum pfn minimization. When 
> I forced the initial value of min_mask to 30 bits, the device worked.

Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
powerpc.  Crude enablement hack below:

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8c1c636308c8..1dd71a98b70c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
 
 config ZONE_DMA
 	bool
-	default y if PPC_BOOK3E_64
+	default y
 
 config PGTABLE_LEVELS
 	int

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote:
> Your first patch did not work as the configuration does not have 
> CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts 
> at 32 bits and is taken down to 31 with the maximum pfn minimization. When 
> I forced the initial value of min_mask to 30 bits, the device worked.

Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
powerpc.  Crude enablement hack below:

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8c1c636308c8..1dd71a98b70c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
 
 config ZONE_DMA
 	bool
-	default y if PPC_BOOK3E_64
+	default y
 
 config PGTABLE_LEVELS
 	int

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/12/19 1:55 AM, Christoph Hellwig wrote:
> 
> Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
> powerpc.  Crude enablement hack below:
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8c1c636308c8..1dd71a98b70c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
>   
>   config ZONE_DMA
>   	bool
> -	default y if PPC_BOOK3E_64
> +	default y
>   
>   config PGTABLE_LEVELS
>   	int
> 

With the patch for Kconfig above, and the original patch setting 
ARCH_ZONE_DMA_BITS to 30, everything works.

Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Larry Finger]

On 6/12/19 1:55 AM, Christoph Hellwig wrote:
> 
> Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
> powerpc.  Crude enablement hack below:
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8c1c636308c8..1dd71a98b70c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
>   
>   config ZONE_DMA
>   	bool
> -	default y if PPC_BOOK3E_64
> +	default y
>   
>   config PGTABLE_LEVELS
>   	int
> 

With the patch for Kconfig above, and the original patch setting 
ARCH_ZONE_DMA_BITS to 30, everything works.

Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?

Larry

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Wed, 2019-06-12 at 14:41 -0500, Larry Finger wrote:
> On 6/12/19 1:55 AM, Christoph Hellwig wrote:
> > 
> > Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
> > powerpc.  Crude enablement hack below:
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 8c1c636308c8..1dd71a98b70c 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
> >    
> >    config ZONE_DMA
> >        bool
> > -     default y if PPC_BOOK3E_64
> > +     default y
> >    
> >    config PGTABLE_LEVELS
> >        int
> > 
> 
> With the patch for Kconfig above, and the original patch setting 
> ARCH_ZONE_DMA_BITS to 30, everything works.
> 
> Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?

I think CONFIG_PPC32 is fine

Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Benjamin Herrenschmidt]

On Wed, 2019-06-12 at 14:41 -0500, Larry Finger wrote:
> On 6/12/19 1:55 AM, Christoph Hellwig wrote:
> > 
> > Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
> > powerpc.  Crude enablement hack below:
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 8c1c636308c8..1dd71a98b70c 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
> >    
> >    config ZONE_DMA
> >        bool
> > -     default y if PPC_BOOK3E_64
> > +     default y
> >    
> >    config PGTABLE_LEVELS
> >        int
> > 
> 
> With the patch for Kconfig above, and the original patch setting 
> ARCH_ZONE_DMA_BITS to 30, everything works.
> 
> Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?

I think CONFIG_PPC32 is fine

Ben.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Thu, Jun 13, 2019 at 07:59:51AM +1000, Benjamin Herrenschmidt wrote:
> > With the patch for Kconfig above, and the original patch setting 
> > ARCH_ZONE_DMA_BITS to 30, everything works.
> > 
> > Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> > Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?
> 
> I think CONFIG_PPC32 is fine

I'll cook up a patch unless someone beats me to it.

Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook

[Christoph Hellwig]

On Thu, Jun 13, 2019 at 07:59:51AM +1000, Benjamin Herrenschmidt wrote:
> > With the patch for Kconfig above, and the original patch setting 
> > ARCH_ZONE_DMA_BITS to 30, everything works.
> > 
> > Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> > Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?
> 
> I think CONFIG_PPC32 is fine

I'll cook up a patch unless someone beats me to it.