Problem with non aligned DMA in usbnet on ARM

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Hi all,

I have a DLINK DUB-E100 USB / Ethernet adapter with using the Asix
AX88772 chipset.
This device works fine on linux x86 using the usbnet based asix driver
however on ARM (iMX21 SoC, kernel 2.6.35) transmit works but receive
fails with:
"asix_rx_fixup() Bad Header Length"

Digging a bit showed that the driver was submitting a non aligned data
pointer in the URB.
This is due to:
skb_reserve (skb, NET_IP_ALIGN);

in net/usb/usbnet.c rx_submit()

NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
by ARM) hence the data buffer address was offset by 2 whereas the hcd
requires 4 byte alignment to work at all and cache line (32 byte here)
alignment to work reliably.
Note that usbnet uses skb->data as the buffer submitted to USB in the URB.

Removing this call to skb_reserve() fixes the problem for me.

However the comments in skbuff.h make it clear that this is done
deliberately knowing that it will cause non aligned DMA but seem to
suggest that this should work anyway (even if less efficiently).

My understanding, based on previous discussions on -usb a year back
while I was working on the usb hcd is that the hcd does _not_ have to
support unaligned DMA.

So what is the correct fix for this problem?
The options seem to be:
1) Remove skb_reserve() from usbnet
But this will cause the ip header to be non aligned on platforms that
don't have the DMA alignment requirement.

2) Define NET_IP_ALIGN to 0 for ARM
Seems a bit intrusive to me.

3) Change the HCD to copy if not aligned
I'd like to avoid that.

4) Change usbnet to use seperate buffers for the usb side and skb.
Copy between them so both remain aligned
Seems unlikely that the gain further up the stack from ensuring the ip
header is aligned will offset the cost of the copy.

Any ideas?

On a related note, this is the second time I've run into this type of
problem.It would be much easier to debug if the HCD warned (or
refused) misaligned URBs. If HCDs provided information on their
alignment requirements then the USB core could do this in a uniform
way. For example adding an alignment field to struct hc_driver or
struct usb_hcd?

Cheers,

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Hi all,

I have a DLINK DUB-E100 USB / Ethernet adapter with using the Asix
AX88772 chipset.
This device works fine on linux x86 using the usbnet based asix driver
however on ARM (iMX21 SoC, kernel 2.6.35) transmit works but receive
fails with:
"asix_rx_fixup() Bad Header Length"

Digging a bit showed that the driver was submitting a non aligned data
pointer in the URB.
This is due to:
skb_reserve (skb, NET_IP_ALIGN);

in net/usb/usbnet.c rx_submit()

NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
by ARM) hence the data buffer address was offset by 2 whereas the hcd
requires 4 byte alignment to work at all and cache line (32 byte here)
alignment to work reliably.
Note that usbnet uses skb->data as the buffer submitted to USB in the URB.

Removing this call to skb_reserve() fixes the problem for me.

However the comments in skbuff.h make it clear that this is done
deliberately knowing that it will cause non aligned DMA but seem to
suggest that this should work anyway (even if less efficiently).

My understanding, based on previous discussions on -usb a year back
while I was working on the usb hcd is that the hcd does _not_ have to
support unaligned DMA.

So what is the correct fix for this problem?
The options seem to be:
1) Remove skb_reserve() from usbnet
But this will cause the ip header to be non aligned on platforms that
don't have the DMA alignment requirement.

2) Define NET_IP_ALIGN to 0 for ARM
Seems a bit intrusive to me.

3) Change the HCD to copy if not aligned
I'd like to avoid that.

4) Change usbnet to use seperate buffers for the usb side and skb.
Copy between them so both remain aligned
Seems unlikely that the gain further up the stack from ensuring the ip
header is aligned will offset the cost of the copy.

Any ideas?

On a related note, this is the second time I've run into this type of
problem.It would be much easier to debug if the HCD warned (or
refused) misaligned URBs. If HCDs provided information on their
alignment requirements then the USB core could do this in a uniform
way. For example adding an alignment field to struct hc_driver or
struct usb_hcd?

Cheers,

Martin

Re: Problem with non aligned DMA in usbnet on ARM

[Russell King - ARM Linux]

On Wed, Aug 11, 2010 at 11:41:41AM +0200, Martin Fuzzey wrote:
> Digging a bit showed that the driver was submitting a non aligned data
> pointer in the URB.
> This is due to:
> skb_reserve (skb, NET_IP_ALIGN);
> 
> in net/usb/usbnet.c rx_submit()
> 
> NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
> by ARM) hence the data buffer address was offset by 2 whereas the hcd
> requires 4 byte alignment to work at all and cache line (32 byte here)
> alignment to work reliably.

x86 also has NET_IP_ALIGN as 2, so it will also be IP-header aligned there
too, so the restriction won't be from the USB stack.

> Removing this call to skb_reserve() fixes the problem for me.

However, that makes parsing and creating IP headers really inefficient,
especially as we take a trap each time a misaligned access is performed.

> 3) Change the HCD to copy if not aligned
> I'd like to avoid that.

You haven't said what HCD you're using.

> On a related note, this is the second time I've run into this type of
> problem.It would be much easier to debug if the HCD warned (or
> refused) misaligned URBs.

If HCD can't cope with misaligned URBs, it should fail across the board -
or be made to warn across the board - but I suspect it's a specific HCD
that you're using which is only applicable to ARM.

In that case, I'd say the HCD must handle misaligned URBs itself so
behaviour is consistent between all implementations on different
platforms.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Russell King - ARM Linux]

On Wed, Aug 11, 2010 at 11:41:41AM +0200, Martin Fuzzey wrote:
> Digging a bit showed that the driver was submitting a non aligned data
> pointer in the URB.
> This is due to:
> skb_reserve (skb, NET_IP_ALIGN);
> 
> in net/usb/usbnet.c rx_submit()
> 
> NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
> by ARM) hence the data buffer address was offset by 2 whereas the hcd
> requires 4 byte alignment to work at all and cache line (32 byte here)
> alignment to work reliably.

x86 also has NET_IP_ALIGN as 2, so it will also be IP-header aligned there
too, so the restriction won't be from the USB stack.

> Removing this call to skb_reserve() fixes the problem for me.

However, that makes parsing and creating IP headers really inefficient,
especially as we take a trap each time a misaligned access is performed.

> 3) Change the HCD to copy if not aligned
> I'd like to avoid that.

You haven't said what HCD you're using.

> On a related note, this is the second time I've run into this type of
> problem.It would be much easier to debug if the HCD warned (or
> refused) misaligned URBs.

If HCD can't cope with misaligned URBs, it should fail across the board -
or be made to warn across the board - but I suspect it's a specific HCD
that you're using which is only applicable to ARM.

In that case, I'd say the HCD must handle misaligned URBs itself so
behaviour is consistent between all implementations on different
platforms.

Re: Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

Hi,

Martin Fuzzey a écrit :
> Hi all,

> So what is the correct fix for this problem?
> The options seem to be:
> 1) Remove skb_reserve() from usbnet
> But this will cause the ip header to be non aligned on platforms that
> don't have the DMA alignment requirement.
And cause lot's of unaligned access for the IP stack. This will slow 
down arm platform...

> 
> 2) Define NET_IP_ALIGN to 0 for ARM
> Seems a bit intrusive to me.
Again this will generate lot's of unaligned access.
> 
> 3) Change the HCD to copy if not aligned
> I'd like to avoid that.
> 
> 4) Change usbnet to use seperate buffers for the usb side and skb.
> Copy between them so both remain aligned
> Seems unlikely that the gain further up the stack from ensuring the ip
> header is aligned will offset the cost of the copy.
> 
> Any ideas?
I got the same problem on the gadget side : 
http://article.gmane.org/gmane.linux.usb.general/28700

A solution could be the hcd driver to tell it doesn't support unaligned 
transfert and in this case the usb stack/usb driver align it.

It is best to solve in usb driver, because for example in Asix case the 
driver already do copy (see asix_rx_fixup/asix_tx_fixup).


Matthieu

PS : what hcd driver do you use ?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

Hi,

Martin Fuzzey a ?crit :
> Hi all,

> So what is the correct fix for this problem?
> The options seem to be:
> 1) Remove skb_reserve() from usbnet
> But this will cause the ip header to be non aligned on platforms that
> don't have the DMA alignment requirement.
And cause lot's of unaligned access for the IP stack. This will slow 
down arm platform...

> 
> 2) Define NET_IP_ALIGN to 0 for ARM
> Seems a bit intrusive to me.
Again this will generate lot's of unaligned access.
> 
> 3) Change the HCD to copy if not aligned
> I'd like to avoid that.
> 
> 4) Change usbnet to use seperate buffers for the usb side and skb.
> Copy between them so both remain aligned
> Seems unlikely that the gain further up the stack from ensuring the ip
> header is aligned will offset the cost of the copy.
> 
> Any ideas?
I got the same problem on the gadget side : 
http://article.gmane.org/gmane.linux.usb.general/28700

A solution could be the hcd driver to tell it doesn't support unaligned 
transfert and in this case the usb stack/usb driver align it.

It is best to solve in usb driver, because for example in Asix case the 
driver already do copy (see asix_rx_fixup/asix_tx_fixup).


Matthieu

PS : what hcd driver do you use ?

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

> You haven't said what HCD you're using.
>
imx21_hcd  (submitted by me and mainlined in 2.6.34)

> In that case, I'd say the HCD must handle misaligned URBs itself so
> behaviour is consistent between all implementations on different
> platforms.
>
Here is a pointer to the thread where it was stated that HCD's don't
have to handle this.

http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

> You haven't said what HCD you're using.
>
imx21_hcd  (submitted by me and mainlined in 2.6.34)

> In that case, I'd say the HCD must handle misaligned URBs itself so
> behaviour is consistent between all implementations on different
> platforms.
>
Here is a pointer to the thread where it was stated that HCD's don't
have to handle this.

http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

On Wed, Aug 11, 2010 at 11:59 AM, Matthieu CASTET
<matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org> wrote:
> It is best to solve in usb driver, because for example in Asix case the
> driver already do copy (see asix_rx_fixup/asix_tx_fixup).
>
Yes, however those functions are only called for devices which register them.
Looking at the driver_info and product id tables shows that it's only
a subset of the devices that asix supports (88772 and 88178 chips)

In fact my hardware _is_ included
	// DLink DUB-E100 H/W Ver B1 Alternate
	USB_DEVICE (0x2001, 0x3c05),
	.driver_info = (unsigned long) &ax88772_info,

but fixing this in asix_rx_fixup wouldn't solve it for the other
supported devices.

It seems to me these fixup functions are not intended to solve
alignment issues but rather implement device specific framing (such as
when the hardware packs multiple ethernet frames into a single urb)

> PS : what hcd driver do you use ?
imx21_hcd

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

On Wed, Aug 11, 2010 at 11:59 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> It is best to solve in usb driver, because for example in Asix case the
> driver already do copy (see asix_rx_fixup/asix_tx_fixup).
>
Yes, however those functions are only called for devices which register them.
Looking at the driver_info and product id tables shows that it's only
a subset of the devices that asix supports (88772 and 88178 chips)

In fact my hardware _is_ included
	// DLink DUB-E100 H/W Ver B1 Alternate
	USB_DEVICE (0x2001, 0x3c05),
	.driver_info = (unsigned long) &ax88772_info,

but fixing this in asix_rx_fixup wouldn't solve it for the other
supported devices.

It seems to me these fixup functions are not intended to solve
alignment issues but rather implement device specific framing (such as
when the hardware packs multiple ethernet frames into a single urb)

> PS : what hcd driver do you use ?
imx21_hcd

Martin

Re: Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 12:11:52PM +0200, Martin Fuzzey wrote:
> > You haven't said what HCD you're using.
> >
> imx21_hcd  (submitted by me and mainlined in 2.6.34)
> 
> > In that case, I'd say the HCD must handle misaligned URBs itself so
> > behaviour is consistent between all implementations on different
> > platforms.
> >
> Here is a pointer to the thread where it was stated that HCD's don't
> have to handle this.
> 
> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164

No, that thread is about stack vs. heap allocations, not about alignment
issues.

thanks,

greg k-h

Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 12:11:52PM +0200, Martin Fuzzey wrote:
> > You haven't said what HCD you're using.
> >
> imx21_hcd  (submitted by me and mainlined in 2.6.34)
> 
> > In that case, I'd say the HCD must handle misaligned URBs itself so
> > behaviour is consistent between all implementations on different
> > platforms.
> >
> Here is a pointer to the thread where it was stated that HCD's don't
> have to handle this.
> 
> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164

No, that thread is about stack vs. heap allocations, not about alignment
issues.

thanks,

greg k-h

Problem with non aligned DMA in usbnet on ARM

[Gary King]

On 08/11/2010 04:38 AM, Martin Fuzzey wrote:
>
>
> It seems to me these fixup functions are not intended to solve
> alignment issues but rather implement device specific framing (such as
> when the hardware packs multiple ethernet frames into a single urb)
>
I sent a patch to the list about 2 weeks ago that added support to
DMA bounce to bounce for misaligned buffers. We have a similar
problem with URB alignment for usbnet on Tegra's HCD:

http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html

- Gary

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

On Wed, Aug 11, 2010 at 5:04 PM, Greg KH <greg@kroah.com> wrote:
>> Here is a pointer to the thread where it was stated that HCD's don't
>> have to handle this.
>>
>> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
>
> No, that thread is about stack vs. heap allocations, not about alignment
> issues.
>

Well although the issue discussed in that thread was caused by a stack
allocation isn't the issue here the same?

My understanding is that a heap allocation as returned by kmalloc() will be:
1) correctly aligned for DMA
and
2) in a memory zone accessible to DMA

whereas a stack allocation is not guaranteed to have either of these properties.

The problem I described in that thread was due to case 1
(misalignment) rather than the stack memory zone not being accessible
at all to DMA.
To which was the reply was basically "use a heap allocation".

So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?

regards,

Martin









> thanks,
>
> greg k-h
>

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

On Wed, Aug 11, 2010 at 5:04 PM, Greg KH <greg@kroah.com> wrote:
>> Here is a pointer to the thread where it was stated that HCD's don't
>> have to handle this.
>>
>> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
>
> No, that thread is about stack vs. heap allocations, not about alignment
> issues.
>

Well although the issue discussed in that thread was caused by a stack
allocation isn't the issue here the same?

My understanding is that a heap allocation as returned by kmalloc() will be:
1) correctly aligned for DMA
and
2) in a memory zone accessible to DMA

whereas a stack allocation is not guaranteed to have either of these properties.

The problem I described in that thread was due to case 1
(misalignment) rather than the stack memory zone not being accessible
at all to DMA.
To which was the reply was basically "use a heap allocation".

So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?

regards,

Martin









> thanks,
>
> greg k-h
>

Re: Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 06:08:43PM +0200, Martin Fuzzey wrote:
> On Wed, Aug 11, 2010 at 5:04 PM, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
> >> Here is a pointer to the thread where it was stated that HCD's don't
> >> have to handle this.
> >>
> >> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
> >
> > No, that thread is about stack vs. heap allocations, not about alignment
> > issues.
> >
> 
> Well although the issue discussed in that thread was caused by a stack
> allocation isn't the issue here the same?
> 
> My understanding is that a heap allocation as returned by kmalloc() will be:
> 1) correctly aligned for DMA
> and
> 2) in a memory zone accessible to DMA
> 
> whereas a stack allocation is not guaranteed to have either of these properties.
> 
> The problem I described in that thread was due to case 1
> (misalignment) rather than the stack memory zone not being accessible
> at all to DMA.
> To which was the reply was basically "use a heap allocation".
> 
> So the question is are hcds expected to accept arbitarilly aligned but
> heap allocated pointers (such as the result of kmalloc() + 1)?

It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)

If you allocate the urb with a kmalloc() call with no offset, does it
all work properly?  The driver should be calling usb_alloc_urb() which
does this automatically for them, right?  Or is it trying to allocate
things on its own somehow?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 06:08:43PM +0200, Martin Fuzzey wrote:
> On Wed, Aug 11, 2010 at 5:04 PM, Greg KH <greg@kroah.com> wrote:
> >> Here is a pointer to the thread where it was stated that HCD's don't
> >> have to handle this.
> >>
> >> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
> >
> > No, that thread is about stack vs. heap allocations, not about alignment
> > issues.
> >
> 
> Well although the issue discussed in that thread was caused by a stack
> allocation isn't the issue here the same?
> 
> My understanding is that a heap allocation as returned by kmalloc() will be:
> 1) correctly aligned for DMA
> and
> 2) in a memory zone accessible to DMA
> 
> whereas a stack allocation is not guaranteed to have either of these properties.
> 
> The problem I described in that thread was due to case 1
> (misalignment) rather than the stack memory zone not being accessible
> at all to DMA.
> To which was the reply was basically "use a heap allocation".
> 
> So the question is are hcds expected to accept arbitarilly aligned but
> heap allocated pointers (such as the result of kmalloc() + 1)?

It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)

If you allocate the urb with a kmalloc() call with no offset, does it
all work properly?  The driver should be calling usb_alloc_urb() which
does this automatically for them, right?  Or is it trying to allocate
things on its own somehow?

thanks,

greg k-h

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Greg KH wrote:
>> So the question is are hcds expected to accept arbitarilly aligned but
>> heap allocated pointers (such as the result of kmalloc() + 1)?
>>     
>
> It sounds like your HCD doesn't like this, so perhaps we should make
> that rule :)
>
> If you allocate the urb with a kmalloc() call with no offset, does it
> all work properly? 
Yes
>  The driver should be calling usb_alloc_urb() which
> does this automatically for them, right?  Or is it trying to allocate
> things on its own somehow?
>
>   
It's not the URB itself (which is allocated by usb_alloc_urb) but rather
the buffer pointer within the URB that causes the problem.

It's the asix driver (or more exactly the usbnet core used by that driver).
It does (rx_submit() in drivers/net/usb/usbnet.c):

urb = usb_alloc_urb();
skb = alloc_skb (...);
skb_reserve (skb, NET_IP_ALIGN);
usb_fill_bulk_urb (urb,...  skb->data);
usb_submit_urb(urb)

skb->data as returned by alloc_skb() is aligned
but skb_reserve adds 2.

Thus removing the skb_reserve() call makes it work.
BUT if I do that the IP header is no longer aligned so accesses further
up the network stack have to be fixed up by exception handlers which is
expensive (even with hcds which don't require this)

cheers,
Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Greg KH wrote:
>> So the question is are hcds expected to accept arbitarilly aligned but
>> heap allocated pointers (such as the result of kmalloc() + 1)?
>>     
>
> It sounds like your HCD doesn't like this, so perhaps we should make
> that rule :)
>
> If you allocate the urb with a kmalloc() call with no offset, does it
> all work properly? 
Yes
>  The driver should be calling usb_alloc_urb() which
> does this automatically for them, right?  Or is it trying to allocate
> things on its own somehow?
>
>   
It's not the URB itself (which is allocated by usb_alloc_urb) but rather
the buffer pointer within the URB that causes the problem.

It's the asix driver (or more exactly the usbnet core used by that driver).
It does (rx_submit() in drivers/net/usb/usbnet.c):

urb = usb_alloc_urb();
skb = alloc_skb (...);
skb_reserve (skb, NET_IP_ALIGN);
usb_fill_bulk_urb (urb,...  skb->data);
usb_submit_urb(urb)

skb->data as returned by alloc_skb() is aligned
but skb_reserve adds 2.

Thus removing the skb_reserve() call makes it work.
BUT if I do that the IP header is no longer aligned so accesses further
up the network stack have to be fixed up by exception handlers which is
expensive (even with hcds which don't require this)

cheers,
Martin

Re: Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Mittwoch 11 August 2010 19:42:38 schrieb Greg KH:
> > So the question is are hcds expected to accept arbitarilly aligned but
> > heap allocated pointers (such as the result of kmalloc() + 1)?
> 
> It sounds like your HCD doesn't like this, so perhaps we should make
> that rule :)
> 
> If you allocate the urb with a kmalloc() call with no offset, does it
> all work properly?  The driver should be calling usb_alloc_urb() which
> does this automatically for them, right?  Or is it trying to allocate
> things on its own somehow?

The buffer is the problem not the URB. And up to now the alignment
was not specified, but drivers are generally written assuming byte
granularity.

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Mittwoch 11 August 2010 19:42:38 schrieb Greg KH:
> > So the question is are hcds expected to accept arbitarilly aligned but
> > heap allocated pointers (such as the result of kmalloc() + 1)?
> 
> It sounds like your HCD doesn't like this, so perhaps we should make
> that rule :)
> 
> If you allocate the urb with a kmalloc() call with no offset, does it
> all work properly?  The driver should be calling usb_alloc_urb() which
> does this automatically for them, right?  Or is it trying to allocate
> things on its own somehow?

The buffer is the problem not the URB. And up to now the alignment
was not specified, but drivers are generally written assuming byte
granularity.

	Regards
		Oliver

Re: Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 09:07:27PM +0200, Martin Fuzzey wrote:
> Greg KH wrote:
> >> So the question is are hcds expected to accept arbitarilly aligned but
> >> heap allocated pointers (such as the result of kmalloc() + 1)?
> >>     
> >
> > It sounds like your HCD doesn't like this, so perhaps we should make
> > that rule :)
> >
> > If you allocate the urb with a kmalloc() call with no offset, does it
> > all work properly? 
> Yes
> >  The driver should be calling usb_alloc_urb() which
> > does this automatically for them, right?  Or is it trying to allocate
> > things on its own somehow?
> >
> >   
> It's not the URB itself (which is allocated by usb_alloc_urb) but rather
> the buffer pointer within the URB that causes the problem.

Doh, you are right, sorry about that.

> It's the asix driver (or more exactly the usbnet core used by that driver).
> It does (rx_submit() in drivers/net/usb/usbnet.c):
> 
> urb = usb_alloc_urb();
> skb = alloc_skb (...);
> skb_reserve (skb, NET_IP_ALIGN);
> usb_fill_bulk_urb (urb,...  skb->data);
> usb_submit_urb(urb)
> 
> skb->data as returned by alloc_skb() is aligned
> but skb_reserve adds 2.
> 
> Thus removing the skb_reserve() call makes it work.
> BUT if I do that the IP header is no longer aligned so accesses further
> up the network stack have to be fixed up by exception handlers which is
> expensive (even with hcds which don't require this)

Can you fix this in the host controller driver?

thanks,

greg k-h

Problem with non aligned DMA in usbnet on ARM

[Greg KH]

On Wed, Aug 11, 2010 at 09:07:27PM +0200, Martin Fuzzey wrote:
> Greg KH wrote:
> >> So the question is are hcds expected to accept arbitarilly aligned but
> >> heap allocated pointers (such as the result of kmalloc() + 1)?
> >>     
> >
> > It sounds like your HCD doesn't like this, so perhaps we should make
> > that rule :)
> >
> > If you allocate the urb with a kmalloc() call with no offset, does it
> > all work properly? 
> Yes
> >  The driver should be calling usb_alloc_urb() which
> > does this automatically for them, right?  Or is it trying to allocate
> > things on its own somehow?
> >
> >   
> It's not the URB itself (which is allocated by usb_alloc_urb) but rather
> the buffer pointer within the URB that causes the problem.

Doh, you are right, sorry about that.

> It's the asix driver (or more exactly the usbnet core used by that driver).
> It does (rx_submit() in drivers/net/usb/usbnet.c):
> 
> urb = usb_alloc_urb();
> skb = alloc_skb (...);
> skb_reserve (skb, NET_IP_ALIGN);
> usb_fill_bulk_urb (urb,...  skb->data);
> usb_submit_urb(urb)
> 
> skb->data as returned by alloc_skb() is aligned
> but skb_reserve adds 2.
> 
> Thus removing the skb_reserve() call makes it work.
> BUT if I do that the IP header is no longer aligned so accesses further
> up the network stack have to be fixed up by exception handlers which is
> expensive (even with hcds which don't require this)

Can you fix this in the host controller driver?

thanks,

greg k-h

Problem with non aligned DMA in usbnet on ARM

[Russell King - ARM Linux]

On Wed, Aug 11, 2010 at 08:54:27AM -0700, Gary King wrote:
> On 08/11/2010 04:38 AM, Martin Fuzzey wrote:
> > It seems to me these fixup functions are not intended to solve
> > alignment issues but rather implement device specific framing (such as
> > when the hardware packs multiple ethernet frames into a single urb)
> >
> I sent a patch to the list about 2 weeks ago that added support to
> DMA bounce to bounce for misaligned buffers. We have a similar
> problem with URB alignment for usbnet on Tegra's HCD:
> 
> http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html

We don't want to add support for this to DMA bounce.  DMA bounce is already
a pain in the backside and causes its own set of problems - please let it
die a long slow but quite death.

If you want to see the kind of pain dmabounce causes, look at this long
standing and as yet unsolved bug:

  http://bugzilla.kernel.org/show_bug.cgi?id=7760

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

[Gary, you forgot to copy -usb and netdev so adding them again to try to
keep this thread joined up]

Russell King - ARM Linux wrote:
> On Wed, Aug 11, 2010 at 08:54:27AM -0700, Gary King wrote:
>   
>> I sent a patch to the list about 2 weeks ago that added support to
>> DMA bounce to bounce for misaligned buffers. We have a similar
>> problem with URB alignment for usbnet on Tegra's HCD:
>> http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html
>>     
Nice to know someone else has the same problem :)
What is the Tegra hcd?
I can't find it in the kernel sources.

> We don't want to add support for this to DMA bounce.  DMA bounce is already
> a pain in the backside and causes its own set of problems - please let it
> die a long slow but quite death.
>
> If you want to see the kind of pain dmabounce causes, look at this long
> standing and as yet unsolved bug:
>
>   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>
>   
Well I don't know the dmabounce code but why is using it likely to cause
OOM problems (at least why more so than copying the buffer in the HCD or
the usb core). In both cases there will be two copies of the buffer in
memory which could I agree be a problem in memory constrained systems.
But if we _do_ want to accept unaligned buffers from usb drivers I can't
see  a way round that.


> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

[Gary, you forgot to copy -usb and netdev so adding them again to try to
keep this thread joined up]

Russell King - ARM Linux wrote:
> On Wed, Aug 11, 2010 at 08:54:27AM -0700, Gary King wrote:
>   
>> I sent a patch to the list about 2 weeks ago that added support to
>> DMA bounce to bounce for misaligned buffers. We have a similar
>> problem with URB alignment for usbnet on Tegra's HCD:
>> http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html
>>     
Nice to know someone else has the same problem :)
What is the Tegra hcd?
I can't find it in the kernel sources.

> We don't want to add support for this to DMA bounce.  DMA bounce is already
> a pain in the backside and causes its own set of problems - please let it
> die a long slow but quite death.
>
> If you want to see the kind of pain dmabounce causes, look at this long
> standing and as yet unsolved bug:
>
>   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>
>   
Well I don't know the dmabounce code but why is using it likely to cause
OOM problems (at least why more so than copying the buffer in the HCD or
the usb core). In both cases there will be two copies of the buffer in
memory which could I agree be a problem in memory constrained systems.
But if we _do_ want to accept unaligned buffers from usb drivers I can't
see  a way round that.


> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>   

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Greg KH wrote:
> Can you fix this in the host controller driver?
>
>   
Technically yes (by copying the unaligned buffers).

But if we do decide that USB must support unaligned buffers wouldn't it
be better to have the HCD indicate it's alignment requirement to the
core and have the core do the copying?

Gary King sent a message to this thread but only to the arm list saying
he has the same problem with the tegra hcd (which doesn't seem to be in
the tree yet.)  I don't know if any of the other in tree HCDs have this
problem.

Also if this is a new requirement for HCDs usbtest probably needs a new
test...

Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Greg KH wrote:
> Can you fix this in the host controller driver?
>
>   
Technically yes (by copying the unaligned buffers).

But if we do decide that USB must support unaligned buffers wouldn't it
be better to have the HCD indicate it's alignment requirement to the
core and have the core do the copying?

Gary King sent a message to this thread but only to the arm list saying
he has the same problem with the tegra hcd (which doesn't seem to be in
the tree yet.)  I don't know if any of the other in tree HCDs have this
problem.

Also if this is a new requirement for HCDs usbtest probably needs a new
test...

Martin

Re: Problem with non aligned DMA in usbnet on ARM

[Russell King - ARM Linux]

On Thu, Aug 12, 2010 at 12:20:07AM +0200, Martin Fuzzey wrote:
> [Gary, you forgot to copy -usb and netdev so adding them again to try to
> keep this thread joined up]
> 
> Russell King - ARM Linux wrote:
> > On Wed, Aug 11, 2010 at 08:54:27AM -0700, Gary King wrote:
> >   
> >> I sent a patch to the list about 2 weeks ago that added support to
> >> DMA bounce to bounce for misaligned buffers. We have a similar
> >> problem with URB alignment for usbnet on Tegra's HCD:
> >> http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html
>
> Nice to know someone else has the same problem :)
> What is the Tegra hcd?
> I can't find it in the kernel sources.
> 
> > We don't want to add support for this to DMA bounce.  DMA bounce is already
> > a pain in the backside and causes its own set of problems - please let it
> > die a long slow but quite death.
> >
> > If you want to see the kind of pain dmabounce causes, look at this long
> > standing and as yet unsolved bug:
> >
> >   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>
> Well I don't know the dmabounce code but why is using it likely to cause
> OOM problems (at least why more so than copying the buffer in the HCD or
> the usb core).

The problem in that bug appears to be that there's soo much pressure on
the first 64MB of memory that we run out of sufficiently large blocks
of memory to be able to satisfy the requirements to do DMA bouncing via
the dmabounce code.  I don't think anyone got to the bottom of why the
DMA zone was exhausted first before the DMA32 zone ran out of pages.

However, as something using dma_map_single/dma_map_page has to return a
contiguous buffer, we do need to use high order allocations for them - a
driver on the other hand can make a decision about whether its possible
to break up the buffer into smaller (and therefore easier to allocate)
buffers.

> In both cases there will be two copies of the buffer in
> memory which could I agree be a problem in memory constrained systems.
> But if we _do_ want to accept unaligned buffers from usb drivers I can't
> see  a way round that.

One thing we have done in the past (eg, SMC91x network driver on PXA) is
to PIO the first couple of bytes to the device, and DMA the remainder -
the DMA can't do non-word aligned start addresses.  This gives us the
benefit of being able to deal with buffers containing IP packets without
copying them, while preserving the performance of using DMA.

Problem with non aligned DMA in usbnet on ARM

[Russell King - ARM Linux]

On Thu, Aug 12, 2010 at 12:20:07AM +0200, Martin Fuzzey wrote:
> [Gary, you forgot to copy -usb and netdev so adding them again to try to
> keep this thread joined up]
> 
> Russell King - ARM Linux wrote:
> > On Wed, Aug 11, 2010 at 08:54:27AM -0700, Gary King wrote:
> >   
> >> I sent a patch to the list about 2 weeks ago that added support to
> >> DMA bounce to bounce for misaligned buffers. We have a similar
> >> problem with URB alignment for usbnet on Tegra's HCD:
> >> http://lists.arm.linux.org.uk/lurker/message/20100729.005746.b43fa1d9.en.html
>
> Nice to know someone else has the same problem :)
> What is the Tegra hcd?
> I can't find it in the kernel sources.
> 
> > We don't want to add support for this to DMA bounce.  DMA bounce is already
> > a pain in the backside and causes its own set of problems - please let it
> > die a long slow but quite death.
> >
> > If you want to see the kind of pain dmabounce causes, look at this long
> > standing and as yet unsolved bug:
> >
> >   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>
> Well I don't know the dmabounce code but why is using it likely to cause
> OOM problems (at least why more so than copying the buffer in the HCD or
> the usb core).

The problem in that bug appears to be that there's soo much pressure on
the first 64MB of memory that we run out of sufficiently large blocks
of memory to be able to satisfy the requirements to do DMA bouncing via
the dmabounce code.  I don't think anyone got to the bottom of why the
DMA zone was exhausted first before the DMA32 zone ran out of pages.

However, as something using dma_map_single/dma_map_page has to return a
contiguous buffer, we do need to use high order allocations for them - a
driver on the other hand can make a decision about whether its possible
to break up the buffer into smaller (and therefore easier to allocate)
buffers.

> In both cases there will be two copies of the buffer in
> memory which could I agree be a problem in memory constrained systems.
> But if we _do_ want to accept unaligned buffers from usb drivers I can't
> see  a way round that.

One thing we have done in the past (eg, SMC91x network driver on PXA) is
to PIO the first couple of bytes to the device, and DMA the remainder -
the DMA can't do non-word aligned start addresses.  This gives us the
benefit of being able to deal with buffers containing IP packets without
copying them, while preserving the performance of using DMA.

Re: Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

Martin Fuzzey a écrit :
> Greg KH wrote:
>> Can you fix this in the host controller driver?
>>
>>   
> Technically yes (by copying the unaligned buffers).
> 
> But if we do decide that USB must support unaligned buffers wouldn't it
> be better to have the HCD indicate it's alignment requirement to the
> core and have the core do the copying?
> 
> Gary King sent a message to this thread but only to the arm list saying
> he has the same problem with the tegra hcd (which doesn't seem to be in
> the tree yet.)  I don't know if any of the other in tree HCDs have this
> problem.
dwc otg also got this limitation. I don't know if the version submitted 
for inclusion handle unaligned buffers.


Matthieu

Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

Martin Fuzzey a ?crit :
> Greg KH wrote:
>> Can you fix this in the host controller driver?
>>
>>   
> Technically yes (by copying the unaligned buffers).
> 
> But if we do decide that USB must support unaligned buffers wouldn't it
> be better to have the HCD indicate it's alignment requirement to the
> core and have the core do the copying?
> 
> Gary King sent a message to this thread but only to the arm list saying
> he has the same problem with the tegra hcd (which doesn't seem to be in
> the tree yet.)  I don't know if any of the other in tree HCDs have this
> problem.
dwc otg also got this limitation. I don't know if the version submitted 
for inclusion handle unaligned buffers.


Matthieu

Re: Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

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

Martin Fuzzey a écrit :
>> We don't want to add support for this to DMA bounce.  DMA bounce is already
>> a pain in the backside and causes its own set of problems - please let it
>> die a long slow but quite death.
>>
>> If you want to see the kind of pain dmabounce causes, look at this long
>> standing and as yet unsolved bug:
>>
>>   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>>
>>   
> Well I don't know the dmabounce code but why is using it likely to cause
> OOM problems (at least why more so than copying the buffer in the HCD or
> the usb core). In both cases there will be two copies of the buffer in
> memory which could I agree be a problem in memory constrained systems.
> But if we _do_ want to accept unaligned buffers from usb drivers I can't
> see  a way round that.
> 
memmove is our friend :
the buffer allocated in usbnet got an offset.
All you have to do it remove this offset and memmove the data. That what 
I did [1], and why it is better to do it in usb driver.


Matthieu




[1] http://article.gmane.org/gmane.linux.usb.general/28700

[-- Attachment #2: gadget-align.diff --]
[-- Type: text/x-diff, Size: 2251 bytes --]

diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h
index 1edbc12..ed3ee67 100644
--- a/drivers/usb/gadget/gadget_chips.h
+++ b/drivers/usb/gadget/gadget_chips.h
@@ -214,4 +214,14 @@ static inline bool gadget_supports_altsettings(struct usb_gadget *gadget)
 	return true;
 }
 
+/**
+ * gadget_dma32 - return true if we want buffer aligned on 32 bits (for dma)
+ * @gadget: the gadget in question
+ */
+static inline bool gadget_dma32(struct usb_gadget *gadget)
+{
+	if (gadget_is_musbhdrc(gadget))
+		return true;
+	return false;
+}
 #endif /* __GADGET_CHIPS_H */
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 84ca195..697af90 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -249,7 +249,12 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags)
 	 * but on at least one, checksumming fails otherwise.  Note:
 	 * RNDIS headers involve variable numbers of LE32 values.
 	 */
-	skb_reserve(skb, NET_IP_ALIGN);
+	/*
+	 * RX: Do not move data by IP_ALIGN:
+	 * if your DMA controller cannot handle it
+	 */
+	if (!gadget_dma32(dev->gadget))
+		skb_reserve(skb, NET_IP_ALIGN);
 
 	req->buf = skb->data;
 	req->length = size;
@@ -282,6 +287,12 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
 	/* normal completion */
 	case 0:
 		skb_put(skb, req->actual);
+		if (gadget_dma32(dev->gadget) && NET_IP_ALIGN) {
+			u8 *data = skb->data;
+			size_t len = skb_headlen(skb);
+			skb_reserve(skb, NET_IP_ALIGN);
+			memmove(skb->data, data, len);
+		}
 
 		if (dev->unwrap) {
 			unsigned long	flags;
@@ -573,6 +584,24 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 
 		length = skb->len;
 	}
+
+	/*
+	 * Align data to 32bit if the dma controller requires it
+	 */
+	if (gadget_dma32(dev->gadget)) {
+		unsigned long align = (unsigned long)skb->data & 3;
+		if (WARN_ON(skb_headroom(skb) < align)) {
+			dev_kfree_skb_any(skb);
+			goto drop;
+		} else if (align) {
+			u8 *data = skb->data;
+			size_t len = skb_headlen(skb);
+			skb->data -= align;
+			memmove(skb->data, data, len);
+			skb_set_tail_pointer(skb, len);
+		}
+	}
+
 	req->buf = skb->data;
 	req->context = skb;
 	req->complete = tx_complete;

Problem with non aligned DMA in usbnet on ARM

[Matthieu CASTET]

Martin Fuzzey a ?crit :
>> We don't want to add support for this to DMA bounce.  DMA bounce is already
>> a pain in the backside and causes its own set of problems - please let it
>> die a long slow but quite death.
>>
>> If you want to see the kind of pain dmabounce causes, look at this long
>> standing and as yet unsolved bug:
>>
>>   http://bugzilla.kernel.org/show_bug.cgi?id=7760
>>
>>   
> Well I don't know the dmabounce code but why is using it likely to cause
> OOM problems (at least why more so than copying the buffer in the HCD or
> the usb core). In both cases there will be two copies of the buffer in
> memory which could I agree be a problem in memory constrained systems.
> But if we _do_ want to accept unaligned buffers from usb drivers I can't
> see  a way round that.
> 
memmove is our friend :
the buffer allocated in usbnet got an offset.
All you have to do it remove this offset and memmove the data. That what 
I did [1], and why it is better to do it in usb driver.


Matthieu




[1] http://article.gmane.org/gmane.linux.usb.general/28700
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gadget-align.diff
Type: text/x-diff
Size: 2251 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100812/7bf8e096/attachment.bin>

Re: Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Matthieu CASTET wrote:
> memmove is our friend :
> the buffer allocated in usbnet got an offset.
> All you have to do it remove this offset and memmove the data. That
> what I did [1], and why it is better to do it in usb driver.
>
>
> [1] http://article.gmane.org/gmane.linux.usb.general/28700
The problem I see with this approach is that it requires the usb driver
to be aware of the device controller's quirks  (hence the
gadget_dma32()) your patch adds. That appears to be the norm (and maybe
unavoidable) on the gadget side but on the host side things aren't
supposed to work like that.

The expectation for USB hosts is that any properly written usb driver
will work with any properly written host controller driver. I don't
think it's reasonable to expect driver developers to test their code
with all the HCDs, nor expect HCD developers to test their code with all
the drivers.

So I think a policy needs to be defined to ensure this and enforced in
the code. I can see two possible methods:

1) Require that usb drivers submit buffers obtained from kmalloc() and
friends with no extra offsets. If they want some other alignment later
they can use memmove in the completion handler. Enforce this in the core
by checking the buffer pointers are aligned to ARCH_KMALLOC_MINALIGN

or

2) Require that usb_submit_urb() accept byte aligned buffers. Enforce
this by a new test in usbtest (which all HCDs are expected to pass).
Implement it either in individual HCDs that require it or by letting
HCDs inform the core of their requirements and have the core do the
alignment (as it already does the dma mapping). Of course HCDs that can
implement byte aligned transfers (either natively or using tricks such
as the one Russell suggested) should do so.

I think 2) is the better solution because:
a) Solution 1 will impose a runtime overhead even on platforms / HCDs
that don't need it (including the most common cases)
b) There are more drivers than HCDs

Martin

Problem with non aligned DMA in usbnet on ARM

[Martin Fuzzey]

Matthieu CASTET wrote:
> memmove is our friend :
> the buffer allocated in usbnet got an offset.
> All you have to do it remove this offset and memmove the data. That
> what I did [1], and why it is better to do it in usb driver.
>
>
> [1] http://article.gmane.org/gmane.linux.usb.general/28700
The problem I see with this approach is that it requires the usb driver
to be aware of the device controller's quirks  (hence the
gadget_dma32()) your patch adds. That appears to be the norm (and maybe
unavoidable) on the gadget side but on the host side things aren't
supposed to work like that.

The expectation for USB hosts is that any properly written usb driver
will work with any properly written host controller driver. I don't
think it's reasonable to expect driver developers to test their code
with all the HCDs, nor expect HCD developers to test their code with all
the drivers.

So I think a policy needs to be defined to ensure this and enforced in
the code. I can see two possible methods:

1) Require that usb drivers submit buffers obtained from kmalloc() and
friends with no extra offsets. If they want some other alignment later
they can use memmove in the completion handler. Enforce this in the core
by checking the buffer pointers are aligned to ARCH_KMALLOC_MINALIGN

or

2) Require that usb_submit_urb() accept byte aligned buffers. Enforce
this by a new test in usbtest (which all HCDs are expected to pass).
Implement it either in individual HCDs that require it or by letting
HCDs inform the core of their requirements and have the core do the
alignment (as it already does the dma mapping). Of course HCDs that can
implement byte aligned transfers (either natively or using tricks such
as the one Russell suggested) should do so.

I think 2) is the better solution because:
a) Solution 1 will impose a runtime overhead even on platforms / HCDs
that don't need it (including the most common cases)
b) There are more drivers than HCDs

Martin

Re: Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Freitag, 13. August 2010, 12:06:54 schrieb Martin Fuzzey:

> So I think a policy needs to be defined to ensure this and enforced in
> the code. I can see two possible methods:
> 
> 1) Require that usb drivers submit buffers obtained from kmalloc() and
> friends with no extra offsets. If they want some other alignment later
> they can use memmove in the completion handler. Enforce this in the core
> by checking the buffer pointers are aligned to ARCH_KMALLOC_MINALIGN
> 
> or
> 
> 2) Require that usb_submit_urb() accept byte aligned buffers. Enforce
> this by a new test in usbtest (which all HCDs are expected to pass).
> Implement it either in individual HCDs that require it or by letting
> HCDs inform the core of their requirements and have the core do the
> alignment (as it already does the dma mapping). Of course HCDs that can
> implement byte aligned transfers (either natively or using tricks such
> as the one Russell suggested) should do so.
> 
> I think 2) is the better solution because:
> a) Solution 1 will impose a runtime overhead even on platforms / HCDs
> that don't need it (including the most common cases)
> b) There are more drivers than HCDs

Yes. But it doesn't prevent you from publishing this information so
drivers can help the lower levels.
	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Freitag, 13. August 2010, 12:06:54 schrieb Martin Fuzzey:

> So I think a policy needs to be defined to ensure this and enforced in
> the code. I can see two possible methods:
> 
> 1) Require that usb drivers submit buffers obtained from kmalloc() and
> friends with no extra offsets. If they want some other alignment later
> they can use memmove in the completion handler. Enforce this in the core
> by checking the buffer pointers are aligned to ARCH_KMALLOC_MINALIGN
> 
> or
> 
> 2) Require that usb_submit_urb() accept byte aligned buffers. Enforce
> this by a new test in usbtest (which all HCDs are expected to pass).
> Implement it either in individual HCDs that require it or by letting
> HCDs inform the core of their requirements and have the core do the
> alignment (as it already does the dma mapping). Of course HCDs that can
> implement byte aligned transfers (either natively or using tricks such
> as the one Russell suggested) should do so.
> 
> I think 2) is the better solution because:
> a) Solution 1 will impose a runtime overhead even on platforms / HCDs
> that don't need it (including the most common cases)
> b) There are more drivers than HCDs

Yes. But it doesn't prevent you from publishing this information so
drivers can help the lower levels.
	Regards
		Oliver

Re: Problem with non aligned DMA in usbnet on ARM

[David Brownell]

> Subject: Re: Problem with non aligned DMA in usbnet on ARM

I remain unconvinced the bug is anywhere except
in whatever host controller is rejecting the
buffers it's given.

Does anyone have proof the bug is elsewhere?


Think for a moment what chaos the kernel would be
if arbitrary subsystems were allowed to introduce
random DMA alignment "requirements".  It would not
be possible to pass buffers between subsystems with
any success at all...

> > 1) Require that usb drivers submit buffers obtained
> from kmalloc() and > friends

Requirements are already documented with the
Description of what memory is DMA-able.  It's
not kmalloc() specifically that's required.

 with no extra offsets.

Offsets have nothing to do with being DMA-able.
They might have to do with working around hardware
flaws though.  (ISTR PXA255 also had a 32-bit goof,
making its DMA engine unusable for most purposes.)

 
> > 2) Require that usb_submit_urb() accept byte aligned buffers.

I believe we already require that.  Of course,
the requirement is on HCDs, not on that call.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[David Brownell]

> Subject: Re: Problem with non aligned DMA in usbnet on ARM

I remain unconvinced the bug is anywhere except
in whatever host controller is rejecting the
buffers it's given.

Does anyone have proof the bug is elsewhere?


Think for a moment what chaos the kernel would be
if arbitrary subsystems were allowed to introduce
random DMA alignment "requirements".  It would not
be possible to pass buffers between subsystems with
any success at all...

> > 1) Require that usb drivers submit buffers obtained
> from kmalloc() and > friends

Requirements are already documented with the
Description of what memory is DMA-able.  It's
not kmalloc() specifically that's required.

 with no extra offsets.

Offsets have nothing to do with being DMA-able.
They might have to do with working around hardware
flaws though.  (ISTR PXA255 also had a 32-bit goof,
making its DMA engine unusable for most purposes.)

 
> > 2) Require that usb_submit_urb() accept byte aligned buffers.

I believe we already require that.  Of course,
the requirement is on HCDs, not on that call.

Re: Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Freitag, 13. August 2010, 15:42:32 schrieb David Brownell:
> > > 2) Require that usb_submit_urb() accept byte aligned buffers.
> 
> I believe we already require that.  Of course,
> the requirement is on HCDs, not on th

Divers fail if HCDs can't do it. But we've never explicitly said that HCDs
need to be able to do that. We should probably document that requirement.

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with non aligned DMA in usbnet on ARM

[Oliver Neukum]

Am Freitag, 13. August 2010, 15:42:32 schrieb David Brownell:
> > > 2) Require that usb_submit_urb() accept byte aligned buffers.
> 
> I believe we already require that.  Of course,
> the requirement is on HCDs, not on th

Divers fail if HCDs can't do it. But we've never explicitly said that HCDs
need to be able to do that. We should probably document that requirement.

	Regards
		Oliver