ixp4xx eth broken in 3.7.0/3.8-rc5?

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Mikael Pettersson]

When I try to update my ixp4xx machine to a 3.8-rc5 kernel it boots
Ok but the network (CONFIG_IXP4XX_ETH=y) fails to come up, with the
following in the kernel log:

net eth0: coherent DMA mask is unset

With a 3.7.0 kernel the situation is similar, except that the "net
eth0: coherent DMA mask is unset" message repeats itself endlessly
and I can't even get in on the serial console.

With the 3.6.0 kernel everything works perfectly.

I've (quickly) reviewed the ixp4xx changes from 3.6.0 to 3.7.0, but
nothing stands out.

Any ideas?

/Mikael

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Mikael Pettersson]

Mikael Pettersson writes:
 > When I try to update my ixp4xx machine to a 3.8-rc5 kernel it boots
 > Ok but the network (CONFIG_IXP4XX_ETH=y) fails to come up, with the
 > following in the kernel log:
 > 
 > net eth0: coherent DMA mask is unset
 > 
 > With a 3.7.0 kernel the situation is similar, except that the "net
 > eth0: coherent DMA mask is unset" message repeats itself endlessly
 > and I can't even get in on the serial console.
 > 
 > With the 3.6.0 kernel everything works perfectly.

A git bisect identified the following as the culprit:

 > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
 > From: Xi Wang <xi.wang@gmail.com>
 > Date: Sat, 17 Nov 2012 20:25:09 +0000
 > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
 > 
 > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
 > allow NULL dev.
 > 
 > Signed-off-by: Xi Wang <xi.wang@gmail.com>
 > Cc: Andrew Morton <akpm@linux-foundation.org>
 > Signed-off-by: David S. Miller <davem@davemloft.net>
 > ---
 >  drivers/net/ethernet/xscale/ixp4xx_eth.c |    8 +++++---
 >  1 files changed, 5 insertions(+), 3 deletions(-)

Reverting this unbreaks ixp4xx_eth on my ixp4xx machine with kernels
3.7.0 and 3.8-rc5.

(Added cc: author and netdev.)

/Mikael

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Xi Wang]

On 1/29/13 7:11 PM, Mikael Pettersson wrote:
> A git bisect identified the following as the culprit:
> 
>   > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
>   > From: Xi Wang <xi.wang@gmail.com>
>   > Date: Sat, 17 Nov 2012 20:25:09 +0000
>   > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
>   >
>   > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
>   > allow NULL dev.
>   >
>   > Signed-off-by: Xi Wang <xi.wang@gmail.com>
>   > Cc: Andrew Morton <akpm@linux-foundation.org>
>   > Signed-off-by: David S. Miller <davem@davemloft.net>
>   > ---
>   >  drivers/net/ethernet/xscale/ixp4xx_eth.c |    8 +++++---
>   >  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> Reverting this unbreaks ixp4xx_eth on my ixp4xx machine with kernels
> 3.7.0 and 3.8-rc5.

Thanks.  The problem was that in init_queues(), the dma_pool_create()
call requires a non-null dev.

  dma_pool = dma_pool_create(DRV_NAME, ??, ...);

What do you think would work here?

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Ben Hutchings]

On Tue, 2013-01-29 at 19:32 -0500, Xi Wang wrote:
> On 1/29/13 7:11 PM, Mikael Pettersson wrote:
> > A git bisect identified the following as the culprit:
> > 
> >   > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
> >   > From: Xi Wang <xi.wang@gmail.com>
> >   > Date: Sat, 17 Nov 2012 20:25:09 +0000
> >   > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
> >   >
> >   > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
> >   > allow NULL dev.
> >   >
> >   > Signed-off-by: Xi Wang <xi.wang@gmail.com>
> >   > Cc: Andrew Morton <akpm@linux-foundation.org>
> >   > Signed-off-by: David S. Miller <davem@davemloft.net>
> >   > ---
> >   >  drivers/net/ethernet/xscale/ixp4xx_eth.c |    8 +++++---
> >   >  1 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > Reverting this unbreaks ixp4xx_eth on my ixp4xx machine with kernels
> > 3.7.0 and 3.8-rc5.
> 
> Thanks.  The problem was that in init_queues(), the dma_pool_create()
> call requires a non-null dev.
> 
>   dma_pool = dma_pool_create(DRV_NAME, ??, ...);
> 
> What do you think would work here?

For DMA purposes I would expect the right device to be the parent bus
device, i.e. port->netdev->dev.parent.  The driver might also need to
set its DMA masks.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Mikael Pettersson]

Ben Hutchings writes:
 > On Tue, 2013-01-29 at 19:32 -0500, Xi Wang wrote:
 > > On 1/29/13 7:11 PM, Mikael Pettersson wrote:
 > > > A git bisect identified the following as the culprit:
 > > > 
 > > >   > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
 > > >   > From: Xi Wang <xi.wang@gmail.com>
 > > >   > Date: Sat, 17 Nov 2012 20:25:09 +0000
 > > >   > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
 > > >   >
 > > >   > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
 > > >   > allow NULL dev.
 > > >   >
 > > >   > Signed-off-by: Xi Wang <xi.wang@gmail.com>
 > > >   > Cc: Andrew Morton <akpm@linux-foundation.org>
 > > >   > Signed-off-by: David S. Miller <davem@davemloft.net>
 > > >   > ---
 > > >   >  drivers/net/ethernet/xscale/ixp4xx_eth.c |    8 +++++---
 > > >   >  1 files changed, 5 insertions(+), 3 deletions(-)
 > > > 
 > > > Reverting this unbreaks ixp4xx_eth on my ixp4xx machine with kernels
 > > > 3.7.0 and 3.8-rc5.
 > > 
 > > Thanks.  The problem was that in init_queues(), the dma_pool_create()
 > > call requires a non-null dev.
 > > 
 > >   dma_pool = dma_pool_create(DRV_NAME, ??, ...);

I had a look around in the 3.8-rc5 kernel source.

mm/dmapool.c:dma_pool_create() needs a non-NULL dev in the CONFIG_NUMA case,
because it calls dev_to_node(dev) which dereferences dev if CONFIG_NUMA.
But in the non-CONFIG_NUMA case there is no dereference of dev, and in fact
there is an explicit check for dev being NULL or not towards the end of
dma_pool_create().  So that explains why ixp4xx works without the change:
it's not NUMA.

A quick grep found three calls to dma_pool_create() with a NULL dev parameter,
two in drivers/usb/gadget/amd5536udc.c and one in arch/arm/mach-s3c64xx/dma.c.
These would all be broken with CONFIG_NUMA but Ok otherwise.

 > > 
 > > What do you think would work here?
 > 
 > For DMA purposes I would expect the right device to be the parent bus
 > device, i.e. port->netdev->dev.parent.  The driver might also need to
 > set its DMA masks.

At least on ixp4xx the DMA masks are normally set by the platform PCI init
code (it has a 64MB PCI DMA window limitation).

I can give the port->netdev->dev.parent variant a try later this evening.

/Mikael

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Mikael Pettersson]

Mikael Pettersson writes:
 > Ben Hutchings writes:
 >  > On Tue, 2013-01-29 at 19:32 -0500, Xi Wang wrote:
 >  > > On 1/29/13 7:11 PM, Mikael Pettersson wrote:
 >  > > > A git bisect identified the following as the culprit:
 >  > > > 
 >  > > >   > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
 >  > > >   > From: Xi Wang <xi.wang@gmail.com>
 >  > > >   > Date: Sat, 17 Nov 2012 20:25:09 +0000
 >  > > >   > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
 >  > > >   >
 >  > > >   > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
 >  > > >   > allow NULL dev.
 >  > > >   >
 >  > > >   > Signed-off-by: Xi Wang <xi.wang@gmail.com>
 >  > > >   > Cc: Andrew Morton <akpm@linux-foundation.org>
 >  > > >   > Signed-off-by: David S. Miller <davem@davemloft.net>
 >  > > >   > ---
 >  > > >   >  drivers/net/ethernet/xscale/ixp4xx_eth.c |    8 +++++---
 >  > > >   >  1 files changed, 5 insertions(+), 3 deletions(-)
 >  > > > 
 >  > > > Reverting this unbreaks ixp4xx_eth on my ixp4xx machine with kernels
 >  > > > 3.7.0 and 3.8-rc5.
 >  > > 
 >  > > Thanks.  The problem was that in init_queues(), the dma_pool_create()
 >  > > call requires a non-null dev.
 >  > > 
 >  > >   dma_pool = dma_pool_create(DRV_NAME, ??, ...);
 > 
 > I had a look around in the 3.8-rc5 kernel source.
 > 
 > mm/dmapool.c:dma_pool_create() needs a non-NULL dev in the CONFIG_NUMA case,
 > because it calls dev_to_node(dev) which dereferences dev if CONFIG_NUMA.
 > But in the non-CONFIG_NUMA case there is no dereference of dev, and in fact
 > there is an explicit check for dev being NULL or not towards the end of
 > dma_pool_create().  So that explains why ixp4xx works without the change:
 > it's not NUMA.
 > 
 > A quick grep found three calls to dma_pool_create() with a NULL dev parameter,
 > two in drivers/usb/gadget/amd5536udc.c and one in arch/arm/mach-s3c64xx/dma.c.
 > These would all be broken with CONFIG_NUMA but Ok otherwise.
 > 
 >  > > 
 >  > > What do you think would work here?
 >  > 
 >  > For DMA purposes I would expect the right device to be the parent bus
 >  > device, i.e. port->netdev->dev.parent.  The driver might also need to
 >  > set its DMA masks.
 > 
 > At least on ixp4xx the DMA masks are normally set by the platform PCI init
 > code (it has a 64MB PCI DMA window limitation).
 > 
 > I can give the port->netdev->dev.parent variant a try later this evening.

No joy, s/&port->netdev->dev/port->netdev->dev.parent/ in init_queues' call
to dma_pool_create made no difference, it still fails.

/Mikael

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Krzysztof Halasa]

Mikael Pettersson <mikpe@it.uu.se> writes:

>  > When I try to update my ixp4xx machine to a 3.8-rc5 kernel it boots
>  > Ok but the network (CONFIG_IXP4XX_ETH=y) fails to come up, with the
>  > following in the kernel log:
>  >
>  > net eth0: coherent DMA mask is unset
>
> A git bisect identified the following as the culprit:
>
>  > From 1a4901177574083c35fafc24c4d151c2a7c7647c Mon Sep 17 00:00:00 2001
>  > From: Xi Wang <xi.wang@gmail.com>
>  > Date: Sat, 17 Nov 2012 20:25:09 +0000
>  > Subject: [PATCH] ixp4xx_eth: avoid calling dma_pool_create() with NULL dev
>  >
>  > Use &port->netdev->dev instead of NULL since dma_pool_create() doesn't
>  > allow NULL dev.

IIRC NULL dev in dma_pool_create() meant the usual IXP4xx mask (64MB)
was used. Perhaps devices don't get a mask by default anymore. I will
look at this in few days.
-- 
Krzysztof Halasa

ixp4xx eth broken in 3.7.0/3.8-rc5?

[Krzysztof Halasa]

Mikael Pettersson <mikpe@it.uu.se> writes:

> When I try to update my ixp4xx machine to a 3.8-rc5 kernel it boots
> Ok but the network (CONFIG_IXP4XX_ETH=y) fails to come up, with the
> following in the kernel log:
>
> net eth0: coherent DMA mask is unset
>
> With a 3.7.0 kernel the situation is similar, except that the "net
> eth0: coherent DMA mask is unset" message repeats itself endlessly
> and I can't even get in on the serial console.
>
> With the 3.6.0 kernel everything works perfectly.

I haven't yet tried the Ethernet driver but it seems my IXP425 box
doesn't like this while mounting a disk (PCI CS3356-based IDE CF card).

commit e9da6e9905e639b0f842a244bc770b48ad0523e9
Author: Marek Szyprowski <m.szyprowski@samsung.com>
Date:   Mon Jul 30 09:11:33 2012 +0200

    ARM: dma-mapping: remove custom consistent dma region

    This patch changes dma-mapping subsystem to use generic vmalloc areas
    for all consistent dma allocations. This increases the total size limit
    of the consistent allocations and removes platform hacks and a lot of
    duplicated code.

    Atomic allocations are served from special pool preallocated on boot,
    because vmalloc areas cannot be reliably created in atomic context.

This is post-3.5, pre-3.6. Could be a bug in some (not yet upstream)
support code, checking. I wouldn't even mention this except for it
touches this same coherent alloc thing.
-- 
Krzysztof Halasa

ARM coherent allocs, Was: ixp4xx eth broken in 3.7.0/3.8-rc5?

[Mikael Pettersson]

Krzysztof Halasa writes:
 > Hi,
 > 
 > I'm not sure how is it supposed to work. Environment: IXP4xx CPU,
 > only 64 MB (of 256 MB) of RAM is available for PCI bus master DMA,
 > /dev/sda is a PATA CF or SATA SSD using CS5536-based PATA interface
 > (SATA - with a bridge) in DMA (PCI bus master) mode.
 > 
 > It works in PIO mode.
 > The problem seems to be this: pci_dev->dev.coherent_dma_mask is 0x3FFFFFF
 > (64MB-1). Yet __dma_alloc() called with GFP_DMA returns memory
 > physically located (dma_handle) above 64MB region.

Isn't that what the ARM-specific dma bounce allocator is supposed to
handle?  Or did e9da6e9905e639b0f842a244bc770b48ad0523e9 disable that one?

My ixp4xx box only has 64MB RAM so there is never any bouncing there,
in fact I patch my kernel to disable the bounce support entirely.

/Mikael

 > 
 > Bisecting shows this commit broke it:
 > 
 > > I haven't yet tried the Ethernet driver but it seems my IXP425 box
 > > doesn't like this while mounting a disk (PCI CS3356-based IDE CF card).
 > >
 > > commit e9da6e9905e639b0f842a244bc770b48ad0523e9
 > > Author: Marek Szyprowski <m.szyprowski@samsung.com>
 > > Date:   Mon Jul 30 09:11:33 2012 +0200
 > >
 > >     ARM: dma-mapping: remove custom consistent dma region
 > >
 > >     This patch changes dma-mapping subsystem to use generic vmalloc areas
 > >     for all consistent dma allocations. This increases the total size limit
 > >     of the consistent allocations and removes platform hacks and a lot of
 > >     duplicated code.
 > >
 > >     Atomic allocations are served from special pool preallocated on boot,
 > >     because vmalloc areas cannot be reliably created in atomic context.
 > 
 > I'm trying to understand it (current code as of v3.8rc-7):
 > 
 > static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 >                          gfp_t gfp, pgprot_t prot, bool is_coherent, const void *caller)
 > {
 >         u64 mask = get_coherent_dma_mask(dev);
 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This gets 64MB - 1
 >         struct page *page = NULL;
 >         void *addr;
 > 
 > #ifdef CONFIG_DMA_API_DEBUG
 >         u64 limit = (mask + 1) & ~mask;
 >         if (limit && size >= limit) {
 >                 dev_warn(dev, "coherent allocation too big (requested %#x mask %#llx)\n",
 >                         size, mask);
 >                 return NULL;
 >         }
 > #endif
 > 
 >         if (!mask)
 >                 return NULL;
 > 
 >         if (mask < 0xffffffffULL)
 >                 gfp |= GFP_DMA;
 > 
 >         /*
 >          * Following is a work-around (a.k.a. hack) to prevent pages
 >          * with __GFP_COMP being passed to split_page() which cannot
 >          * handle them.  The real problem is that this flag probably
 >          * should be 0 on ARM as it is not supported on this
 >          * platform; see CONFIG_HUGETLBFS.
 >          */
 >         gfp &= ~(__GFP_COMP);
 > 
 >         *handle = DMA_ERROR_CODE;
 >         size = PAGE_ALIGN(size);
 > 
 >         if (is_coherent || nommu())
 >                 addr = __alloc_simple_buffer(dev, size, gfp, &page);
 >         else if (!(gfp & __GFP_WAIT))
 >                 addr = __alloc_from_pool(size, &page);
 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 >         else ...
 > 
 > Is the pool supposed to live in GFP_DMA area or should we use more
 > pools? We call __alloc_from_pool() but it knows nothing about our
 > device's coherent_dma_mask or GFP flags.
 > -- 
 > Krzysztof Halasa
 > 

ARM coherent allocs, Was: ixp4xx eth broken in 3.7.0/3.8-rc5?

[Russell King - ARM Linux]

On Mon, Feb 18, 2013 at 10:08:53AM +0100, Mikael Pettersson wrote:
> Krzysztof Halasa writes:
>  > Hi,
>  > 
>  > I'm not sure how is it supposed to work. Environment: IXP4xx CPU,
>  > only 64 MB (of 256 MB) of RAM is available for PCI bus master DMA,
>  > /dev/sda is a PATA CF or SATA SSD using CS5536-based PATA interface
>  > (SATA - with a bridge) in DMA (PCI bus master) mode.
>  > 
>  > It works in PIO mode.
>  > The problem seems to be this: pci_dev->dev.coherent_dma_mask is 0x3FFFFFF
>  > (64MB-1). Yet __dma_alloc() called with GFP_DMA returns memory
>  > physically located (dma_handle) above 64MB region.
> 
> Isn't that what the ARM-specific dma bounce allocator is supposed to
> handle?  Or did e9da6e9905e639b0f842a244bc770b48ad0523e9 disable that one?

How can it if the coherent DMA allocator is not respecting the DMA mask?

ARM coherent allocs, Was: ixp4xx eth broken in 3.7.0/3.8-rc5?

[Krzysztof Halasa]

Mikael Pettersson <mikpe@it.uu.se> writes:

>  > I'm not sure how is it supposed to work. Environment: IXP4xx CPU,
>  > only 64 MB (of 256 MB) of RAM is available for PCI bus master DMA,
>  > /dev/sda is a PATA CF or SATA SSD using CS5536-based PATA interface
>  > (SATA - with a bridge) in DMA (PCI bus master) mode.
>  >
>  > It works in PIO mode.
>  > The problem seems to be this: pci_dev->dev.coherent_dma_mask is 0x3FFFFFF
>  > (64MB-1). Yet __dma_alloc() called with GFP_DMA returns memory
>  > physically located (dma_handle) above 64MB region.
>
> Isn't that what the ARM-specific dma bounce allocator is supposed to
> handle?  Or did e9da6e9905e639b0f842a244bc770b48ad0523e9 disable that
> one?

The so called "coherent" (aka "consistent") allocations mean that both
CPU and other bus masters can read and write at will, (almost) no
synchronization is needed and no map/unmap/sync calls are issued. This
also means the bounce buffering can't be used.
Streaming allocs (which can bounce) use "dma_mask" and coherent allocs
use "coherent_dma_mask".

> My ixp4xx box only has 64MB RAM so there is never any bouncing there,
> in fact I patch my kernel to disable the bounce support entirely.

Right. The problem is only visible with more than 64 MB RAM.
-- 
Krzysztof Halasa