[PATCH 2.6.34] ehci-hcd: add option to enable 64-bit DMA support

[Robert Hancock]

Add a module parameter to allow the user to enable 64-bit DMA support in EHCI,
which has been forcibly disabled since 2003 - see:

http://www.mail-archive.com/linux-usb-devel-5NWGOfrQmneRv+***@public.gmane.org/msg17230.html

At that time the comment was "it'd only matter on a few big Intel boxes anyway",
however the situation is much different today when many new machines have 4GB
or more of RAM and IOMMU/SWIOTLB are thus needlessly required for USB transfers.
For now, the support remains disabled by default and is controlled by an
allow_64bit module parameter.

Note that some USB device drivers may require updates to pass the DMA
capabilities up to their higher layers to avoid unnecessary IOMMU or bounce-
buffer use (i.e. networking layer NETIF_F_HIGHDMA). Some of these checks were
disabled by the patch listed above, and more may be required again today.
However, those previous checks were done incorrectly using dma_supported,
which checks to see whether a device's DMA mask can be validly set to a given
mask, not whether its previously set mask will accomodate the mask passed in.

Signed-off-by: Robert Hancock <hancockrwd-***@public.gmane.org>

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1ec3857..f527e15 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -101,6 +101,10 @@ static int ignore_oc = 0;
module_param (ignore_oc, bool, S_IRUGO);
MODULE_PARM_DESC (ignore_oc, "ignore bogus hardware overcurrent indications");

+static int allow_64bit;
+module_param(allow_64bit, bool, S_IRUGO);
+MODULE_PARM_DESC(allow_64bit, "allow 64-bit DMA");
+
#define INTR_MASK (STS_IAA | STS_FATAL | STS_PCD | STS_ERR | STS_INT)

/*-------------------------------------------------------------------------*/
@@ -644,11 +648,9 @@ static int ehci_run (struct usb_hcd *hcd)
hcc_params = ehci_readl(ehci, &ehci->caps->hcc_params);
if (HCC_64BIT_ADDR(hcc_params)) {
ehci_writel(ehci, 0, &ehci->regs->segment);
-#if 0
-// this is deeply broken on almost all architectures
- if (!dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)))
+ if (allow_64bit &&
+ !dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)))
ehci_info(ehci, "enabled 64bit DMA\n");
-#endif
}


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

[Greg KH]

Post by Robert Hancock
Add a module parameter to allow the user to enable 64-bit DMA support in EHCI,
At that time the comment was "it'd only matter on a few big Intel boxes anyway",
however the situation is much different today when many new machines have 4GB
or more of RAM and IOMMU/SWIOTLB are thus needlessly required for USB transfers.
For now, the support remains disabled by default and is controlled by an
allow_64bit module parameter.
Note that some USB device drivers may require updates to pass the DMA
capabilities up to their higher layers to avoid unnecessary IOMMU or bounce-
buffer use (i.e. networking layer NETIF_F_HIGHDMA). Some of these checks were
disabled by the patch listed above, and more may be required again today.
However, those previous checks were done incorrectly using dma_supported,
which checks to see whether a device's DMA mask can be validly set to a given
mask, not whether its previously set mask will accomodate the mask passed in.

What is the "advantage" that setting this option would allow people to
do that the code currently does not? Is such an advantage measurable at
the slow rates that the EHCI driver controls?

Is there any way to dynamically figure out if we can enable this or not?
Adding module paramaters sucks, as they are hard to configure for most
users, and they tend to be ignored.

And are you really ok with enabling this on a system-wide level, and not
on a per-controller level? Does that work properly on all systems?

And if the system does not support it, and a user enables it, who is
going to support their broken system? :)

thanks,

greg k-h

[Robert Hancock]

Add a module parameter to allow the user to enable 64-bit DMA suppor=

t in EHCI,
g17230.html

At that time the comment was "it'd only matter on a few big Intel bo=

xes anyway",

however the situation is much different today when many new machines=

have 4GB

or more of RAM and IOMMU/SWIOTLB are thus needlessly required for US=

B transfers.

For now, the support remains disabled by default and is controlled b=

y an

allow_64bit module parameter.
Note that some USB device drivers may require updates to pass the DM=

A

capabilities up to their higher layers to avoid unnecessary IOMMU or=

bounce-

buffer use (i.e. networking layer NETIF_F_HIGHDMA). Some of these ch=

ecks were

disabled by the patch listed above, and more may be required again t=

oday.

However, those previous checks were done incorrectly using dma_suppo=

rted,

which checks to see whether a device's DMA mask can be validly set t=

o a given

mask, not whether its previously set mask will accomodate the mask p=

assed in.

What is the "advantage" that setting this option would allow people t=

o

do that the code currently does not? =A0Is such an advantage measurab=

le at

the slow rates that the EHCI driver controls?

I expect it would likely be quite system-dependent. However,
particularly with devices like high-speed USB storage on systems with
lots of RAM, especially 8GB or more (where more of the memory can't be
addressed with 32-bit than can), I suspect it would likely be
measurable in terms of CPU usage, throughput or both, especially if
there's no hardware IOMMU and software bounce-buffering is required.

Is there any way to dynamically figure out if we can enable this or n=

ot?

Adding module paramaters sucks, as they are hard to configure for mos=

t

users, and they tend to be ignored.

Well, the option only has an effect if the controller indicates the
64-bit addressing feature flag in the first place. The only issue
would be with potential controllers that report the feature flag but
it doesn't work. Based on the libata experience with AHCI which has a
similar 64-bit feature flag, there are some controllers that didn't do
it properly, but not many (only ATI SB600 it appears, and only on
certain boards, as it seems it was a BIOS configuration issue).

Having the ability to turn it on manually for testing would likely be
the first step towards turning it on for all controllers that indicate
support, not the end goal in itself.

And are you really ok with enabling this on a system-wide level, and =

not

on a per-controller level? =A0Does that work properly on all systems?

As above, the only issue is if the controller reports the capability
but it doesn't function properly. If a user runs into the case where
one of their controllers doesn't work with it enabled, that likely
means that we need to add blacklisting for that device ID, etc. (or
blacklist 64-bit DMA for their platform in general).

And if the system does not support it, and a user enables it, who is
going to support their broken system? =A0:)

That would be me/us :-) We need those reports though, to know how to
proceed, and better to be from those who opted into testing for now..
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Greg KH]

Post by Robert Hancock

Post by Greg KH

Post by Robert Hancock
Add a module parameter to allow the user to enable 64-bit DMA support in EHCI,
At that time the comment was "it'd only matter on a few big Intel boxes anyway",
however the situation is much different today when many new machines have 4GB
or more of RAM and IOMMU/SWIOTLB are thus needlessly required for USB transfers.
For now, the support remains disabled by default and is controlled by an
allow_64bit module parameter.
Note that some USB device drivers may require updates to pass the DMA
capabilities up to their higher layers to avoid unnecessary IOMMU or bounce-
buffer use (i.e. networking layer NETIF_F_HIGHDMA). Some of these checks were
disabled by the patch listed above, and more may be required again today.
However, those previous checks were done incorrectly using dma_supported,
which checks to see whether a device's DMA mask can be validly set to a given
mask, not whether its previously set mask will accomodate the mask passed in.

What is the "advantage" that setting this option would allow people to
do that the code currently does not? ?Is such an advantage measurable at
the slow rates that the EHCI driver controls?

I expect it would likely be quite system-dependent. However,
particularly with devices like high-speed USB storage on systems with
lots of RAM, especially 8GB or more (where more of the memory can't be
addressed with 32-bit than can), I suspect it would likely be
measurable in terms of CPU usage, throughput or both, especially if
there's no hardware IOMMU and software bounce-buffering is required.

So you did not measure it?

Hm, I guess this change must not be necessary :)

Post by Robert Hancock

Post by Greg KH
Is there any way to dynamically figure out if we can enable this or not?
Adding module paramaters sucks, as they are hard to configure for most
users, and they tend to be ignored.

Well, the option only has an effect if the controller indicates the
64-bit addressing feature flag in the first place. The only issue
would be with potential controllers that report the feature flag but
it doesn't work. Based on the libata experience with AHCI which has a
similar 64-bit feature flag, there are some controllers that didn't do
it properly, but not many (only ATI SB600 it appears, and only on
certain boards, as it seems it was a BIOS configuration issue).
Having the ability to turn it on manually for testing would likely be
the first step towards turning it on for all controllers that indicate
support, not the end goal in itself.

But we disabled it on purpose, because of problems, do we want those
problems again?

Post by Robert Hancock

Post by Greg KH
And are you really ok with enabling this on a system-wide level, and not
on a per-controller level? ?Does that work properly on all systems?

As above, the only issue is if the controller reports the capability
but it doesn't function properly. If a user runs into the case where
one of their controllers doesn't work with it enabled, that likely
means that we need to add blacklisting for that device ID, etc. (or
blacklist 64-bit DMA for their platform in general).

blacklists are hard to keep up to date, you are always handling bug
reports after-the-fact. That's not something you should add lightly.

Post by Robert Hancock

Post by Greg KH
And if the system does not support it, and a user enables it, who is
going to support their broken system? ?:)

That would be me/us :-) We need those reports though, to know how to
proceed, and better to be from those who opted into testing for now..

But we need a _reason_ to enable it.

What user needs this?

And if there is no speed benifit, why take the risk?

thanks,

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

[Yuhong Bao]

Post by Robert Hancock
Add a module parameter to allow the user to enable 64-bit DMA support in EHCI,
At that time the comment was "it'd only matter on a few big Intel boxes anyway",

That months *after* AMD released the Opteron and Athlon 64 processors with x86_64 support, with an x86_64 version of Linux.

Yuhong Bao

_________________________________________________________________
Your E-mail and More On-the-Go. Get Windows Live Hotmail Free.
http://clk.atdmt.com/GBL/go/201469229/direct/01/--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/