2010-02-18 03:11:17

by Robert Hancock

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

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/[email protected]/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 <[email protected]>

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
}


2010-02-18 04:27:12

by Greg KH

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

On Wed, Feb 17, 2010 at 09:10:13PM -0600, Robert Hancock wrote:
> 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/[email protected]/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 <[email protected]>

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

2010-02-18 05:13:10

by Robert Hancock

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

On Wed, Feb 17, 2010 at 10:26 PM, Greg KH <[email protected]> wrote:
> On Wed, Feb 17, 2010 at 09:10:13PM -0600, Robert Hancock wrote:
>> 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/[email protected]/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 <[email protected]>
>
> 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.

>
> 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.

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

>
> 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..

2010-02-18 05:22:20

by Greg KH

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

On Wed, Feb 17, 2010 at 11:13:05PM -0600, Robert Hancock wrote:
> On Wed, Feb 17, 2010 at 10:26 PM, Greg KH <[email protected]> wrote:
> > On Wed, Feb 17, 2010 at 09:10:13PM -0600, Robert Hancock wrote:
> >> 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/[email protected]/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 <[email protected]>
> >
> > 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 :)

> > 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?

> > 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.

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

2010-02-19 00:33:46

by Robert Hancock

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

On Wed, Feb 17, 2010 at 11:22 PM, Greg KH <[email protected]> wrote:
> On Wed, Feb 17, 2010 at 11:13:05PM -0600, Robert Hancock wrote:
>> On Wed, Feb 17, 2010 at 10:26 PM, Greg KH <[email protected]> wrote:
>> > On Wed, Feb 17, 2010 at 09:10:13PM -0600, Robert Hancock wrote:
>> >> 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/[email protected]/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 <[email protected]>
>> >
>> > 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 :)

I'll try and run some tests and see what I can quantify. However, I
only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
random memory allocation only has a 25% chance of ending up in the
area where it would make a difference, so it may take a bit of doing.

>
>> > 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?

AFAICS, it was disabled because of problems with kernel code, not with
hardware (and it appears the issue was with the code that detected the
expanded DMA mask in the USB device driver code, not the HCD driver).
CCing David Brownell who may know more.

>
>> > 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.

I'm not a fan of blacklists in general (we do have too many of them),
but I don't expect a lot of entries in here. Having a device that
explicitly indicates it does something it can't do seems unlikely
(especially when the capability flag is most likely implemented in
silicon and not by the BIOS writer..)

>
>> > 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?

I expect there will be a speed benefit on some configurations. The
only question is how much..

2010-02-19 00:47:47

by Greg KH

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

On Thu, Feb 18, 2010 at 06:33:42PM -0600, Robert Hancock wrote:
> On Wed, Feb 17, 2010 at 11:22 PM, Greg KH <[email protected]> wrote:
> > On Wed, Feb 17, 2010 at 11:13:05PM -0600, Robert Hancock wrote:
> >> On Wed, Feb 17, 2010 at 10:26 PM, Greg KH <[email protected]> wrote:
> >> > On Wed, Feb 17, 2010 at 09:10:13PM -0600, Robert Hancock wrote:
> >> >> 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/[email protected]/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 <[email protected]>
> >> >
> >> > 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 :)
>
> I'll try and run some tests and see what I can quantify. However, I
> only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
> random memory allocation only has a 25% chance of ending up in the
> area where it would make a difference, so it may take a bit of doing.

Without any good justification, including real tests being run, I can't
take this patch, the risk is just too high.

And really, for USB 2.0 speeds, I doubt you are going to even notice
this kind of overhead, it's in the noise. Especially given that almost
always the limiting factor is the device itself, not the host.

thanks,

greg k-h

2010-02-19 03:46:46

by Robert Hancock

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

On Thu, Feb 18, 2010 at 6:47 PM, Greg KH <[email protected]> wrote:
>> > So you did not measure it?
>> >
>> > Hm, I guess this change must not be necessary :)
>>
>> I'll try and run some tests and see what I can quantify. However, I
>> only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
>> random memory allocation only has a 25% chance of ending up in the
>> area where it would make a difference, so it may take a bit of doing.
>
> Without any good justification, including real tests being run, I can't
> take this patch, the risk is just too high.

Again, this particular patch has essentially zero risk for anyone that
doesn't choose to experiment with the option. One can hardly say it
presents much of a long-term maintenance burden either..

>
> And really, for USB 2.0 speeds, I doubt you are going to even notice
> this kind of overhead, it's in the noise. ?Especially given that almost
> always the limiting factor is the device itself, not the host.

Well, I do have some results. This is from running this "dd
if=/dev/sdg of=/dev/null bs=3800M iflag=direct" against an OCZ Rally2
USB flash drive, which gets about 30 MB/sec on read, with CPU-burning
tasks on all cores in the background. (The huge block size and
iflag=direct is to try to force more of the IO to happen to memory
above the 4GB mark.) With that workload, swiotlb_bounce shows up as
between 1.5 to 4% of the CPU time spent in the kernel according to
oprofile. Obviously with the 64-bit DMA enabled, that disappears. Of
course, the overall kernel time is only around 2% of the total time,
so that's a pretty small overall percentage.

I'll try some tests later with a faster SATA-to-IDE device that should
stress things a bit more, but a huge difference doesn't seem likely.
One thing that's uncertain is just how much of the IO is needing to be
bounced - an even distribution of the buffer across all of physical
RAM would suggest 25% in this case, but I don't know an easy way to
verify that.

Aside from speed considerations though, I should point out another
factor: IOMMU/SWIOTLB space is in many cases a limited resource for
all IO in flight at a particular time (SWIOTLB is typically 64MB). The
number of hits when Googling for "Out of IOMMU space" indicates it is
a problem that people do hit from time to time. From that perspective,
anything that prevents unnecessary use of bounce buffers is a good
thing.

2010-02-19 14:24:57

by Greg KH

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

On Thu, Feb 18, 2010 at 09:46:29PM -0600, Robert Hancock wrote:
> On Thu, Feb 18, 2010 at 6:47 PM, Greg KH <[email protected]> wrote:
> >> > So you did not measure it?
> >> >
> >> > Hm, I guess this change must not be necessary :)
> >>
> >> I'll try and run some tests and see what I can quantify. However, I
> >> only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
> >> random memory allocation only has a 25% chance of ending up in the
> >> area where it would make a difference, so it may take a bit of doing.
> >
> > Without any good justification, including real tests being run, I can't
> > take this patch, the risk is just too high.
>
> Again, this particular patch has essentially zero risk for anyone that
> doesn't choose to experiment with the option. One can hardly say it
> presents much of a long-term maintenance burden either..

Then don't give them the option, as it doesn't seem needed :)

Again, it is tough to remove options once you add them, so not adding
them at all is the best thing to do.

> > And really, for USB 2.0 speeds, I doubt you are going to even notice
> > this kind of overhead, it's in the noise. ?Especially given that almost
> > always the limiting factor is the device itself, not the host.
>
> Well, I do have some results. This is from running this "dd
> if=/dev/sdg of=/dev/null bs=3800M iflag=direct" against an OCZ Rally2
> USB flash drive, which gets about 30 MB/sec on read, with CPU-burning
> tasks on all cores in the background. (The huge block size and
> iflag=direct is to try to force more of the IO to happen to memory
> above the 4GB mark.) With that workload, swiotlb_bounce shows up as
> between 1.5 to 4% of the CPU time spent in the kernel according to
> oprofile. Obviously with the 64-bit DMA enabled, that disappears. Of
> course, the overall kernel time is only around 2% of the total time,
> so that's a pretty small overall percentage.

2% is noise, right? So overall you have not really shown any
improvement.

> I'll try some tests later with a faster SATA-to-IDE device that should
> stress things a bit more, but a huge difference doesn't seem likely.
> One thing that's uncertain is just how much of the IO is needing to be
> bounced - an even distribution of the buffer across all of physical
> RAM would suggest 25% in this case, but I don't know an easy way to
> verify that.
>
> Aside from speed considerations though, I should point out another
> factor: IOMMU/SWIOTLB space is in many cases a limited resource for
> all IO in flight at a particular time (SWIOTLB is typically 64MB). The
> number of hits when Googling for "Out of IOMMU space" indicates it is
> a problem that people do hit from time to time. From that perspective,
> anything that prevents unnecessary use of bounce buffers is a good
> thing.

Sure, but again, for USB 2.0 stuff, we don't have many I/O in flight, as
they are pretty slow devices.

USB 3.0 is different, and that's a different driver, and hopefully that
is all addressed already :)

thanks,

greg k-h

2010-02-20 01:30:36

by Robert Hancock

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

On Thu, Feb 18, 2010 at 9:54 PM, Greg KH <[email protected]> wrote:
> On Thu, Feb 18, 2010 at 09:46:29PM -0600, Robert Hancock wrote:
>> On Thu, Feb 18, 2010 at 6:47 PM, Greg KH <[email protected]> wrote:
>> >> > So you did not measure it?
>> >> >
>> >> > Hm, I guess this change must not be necessary :)
>> >>
>> >> I'll try and run some tests and see what I can quantify. However, I
>> >> only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
>> >> random memory allocation only has a 25% chance of ending up in the
>> >> area where it would make a difference, so it may take a bit of doing.
>> >
>> > Without any good justification, including real tests being run, I can't
>> > take this patch, the risk is just too high.
>>
>> Again, this particular patch has essentially zero risk for anyone that
>> doesn't choose to experiment with the option. One can hardly say it
>> presents much of a long-term maintenance burden either..
>
> Then don't give them the option, as it doesn't seem needed :)
>
> Again, it is tough to remove options once you add them, so not adding
> them at all is the best thing to do.

I don't know why you would remove the option. Even if you someday
changed the default to 1, it would likely be a still idea to keep it
around for debugging purposes at least.

If you're complaining about options, ehci-hcd already has some which
are quite a bit more nebulous in usefulness than this one..

>
>> > And really, for USB 2.0 speeds, I doubt you are going to even notice
>> > this kind of overhead, it's in the noise. ?Especially given that almost
>> > always the limiting factor is the device itself, not the host.
>>
>> Well, I do have some results. This is from running this "dd
>> if=/dev/sdg of=/dev/null bs=3800M iflag=direct" against an OCZ Rally2
>> USB flash drive, which gets about 30 MB/sec on read, with CPU-burning
>> tasks on all cores in the background. (The huge block size and
>> iflag=direct is to try to force more of the IO to happen to memory
>> above the 4GB mark.) With that workload, swiotlb_bounce shows up as
>> between 1.5 to 4% of the CPU time spent in the kernel according to
>> oprofile. Obviously with the 64-bit DMA enabled, that disappears. Of
>> course, the overall kernel time is only around 2% of the total time,
>> so that's a pretty small overall percentage.
>
> 2% is noise, right? ?So overall you have not really shown any
> improvement.

What threshold of performance improvement would you rather see? It's
pretty clear that there will be a performance upside, even if small,
and no downside.

I honestly didn't expect as much resistance to a simple hardware
feature enablement patch, that has zero impact on anyone that doesn't
opt-in..

>
>> I'll try some tests later with a faster SATA-to-IDE device that should
>> stress things a bit more, but a huge difference doesn't seem likely.
>> One thing that's uncertain is just how much of the IO is needing to be
>> bounced - an even distribution of the buffer across all of physical
>> RAM would suggest 25% in this case, but I don't know an easy way to
>> verify that.
>>
>> Aside from speed considerations though, I should point out another
>> factor: IOMMU/SWIOTLB space is in many cases a limited resource for
>> all IO in flight at a particular time (SWIOTLB is typically 64MB). The
>> number of hits when Googling for "Out of IOMMU space" indicates it is
>> a problem that people do hit from time to time. From that perspective,
>> anything that prevents unnecessary use of bounce buffers is a good
>> thing.
>
> Sure, but again, for USB 2.0 stuff, we don't have many I/O in flight, as
> they are pretty slow devices.

Think that's a bit simplistic, if you have multiple devices active at
once, or multiple controllers (not at all uncommon these days, newer
Intel chipset machines have two EHCI controllers, with USB 1.x devices
handled through a logical hub with TT connected to each of them) that
can chew up more space.

>
> USB 3.0 is different, and that's a different driver, and hopefully that
> is all addressed already :)

Doesn't look like it, from the version in current -git anyway - I
don't see any calls to set DMA masks in the XHCI code so it will just
default to 32-bit. I imagine that'll hurt performance at 4.8 Gbps if
you've got lots of RAM..

2010-02-20 04:31:34

by Greg KH

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

On Fri, Feb 19, 2010 at 07:30:30PM -0600, Robert Hancock wrote:
> On Thu, Feb 18, 2010 at 9:54 PM, Greg KH <[email protected]> wrote:
> > On Thu, Feb 18, 2010 at 09:46:29PM -0600, Robert Hancock wrote:
> >> On Thu, Feb 18, 2010 at 6:47 PM, Greg KH <[email protected]> wrote:
> >> >> > So you did not measure it?
> >> >> >
> >> >> > Hm, I guess this change must not be necessary :)
> >> >>
> >> >> I'll try and run some tests and see what I can quantify. However, I
> >> >> only have 4GB of RAM on my machine (with a 1GB memory hole) and so a
> >> >> random memory allocation only has a 25% chance of ending up in the
> >> >> area where it would make a difference, so it may take a bit of doing.
> >> >
> >> > Without any good justification, including real tests being run, I can't
> >> > take this patch, the risk is just too high.
> >>
> >> Again, this particular patch has essentially zero risk for anyone that
> >> doesn't choose to experiment with the option. One can hardly say it
> >> presents much of a long-term maintenance burden either..
> >
> > Then don't give them the option, as it doesn't seem needed :)
> >
> > Again, it is tough to remove options once you add them, so not adding
> > them at all is the best thing to do.
>
> I don't know why you would remove the option. Even if you someday
> changed the default to 1, it would likely be a still idea to keep it
> around for debugging purposes at least.
>
> If you're complaining about options, ehci-hcd already has some which
> are quite a bit more nebulous in usefulness than this one..

Yeah, and adding another one, for no known benifit, and a known
detriment for some machines, is not a good idea.

> >> > And really, for USB 2.0 speeds, I doubt you are going to even notice
> >> > this kind of overhead, it's in the noise. ?Especially given that almost
> >> > always the limiting factor is the device itself, not the host.
> >>
> >> Well, I do have some results. This is from running this "dd
> >> if=/dev/sdg of=/dev/null bs=3800M iflag=direct" against an OCZ Rally2
> >> USB flash drive, which gets about 30 MB/sec on read, with CPU-burning
> >> tasks on all cores in the background. (The huge block size and
> >> iflag=direct is to try to force more of the IO to happen to memory
> >> above the 4GB mark.) With that workload, swiotlb_bounce shows up as
> >> between 1.5 to 4% of the CPU time spent in the kernel according to
> >> oprofile. Obviously with the 64-bit DMA enabled, that disappears. Of
> >> course, the overall kernel time is only around 2% of the total time,
> >> so that's a pretty small overall percentage.
> >
> > 2% is noise, right? ?So overall you have not really shown any
> > improvement.
>
> What threshold of performance improvement would you rather see? It's
> pretty clear that there will be a performance upside, even if small,
> and no downside.

I thought the downside was that this would break on some machines. And
debugging this is quite difficult.

> I honestly didn't expect as much resistance to a simple hardware
> feature enablement patch, that has zero impact on anyone that doesn't
> opt-in..

Again, people will opt-in, if we want them to or not, and then if
problems happen, will blame us for it. Determining that they did enable
this option, is quite difficult, right?

> >> I'll try some tests later with a faster SATA-to-IDE device that should
> >> stress things a bit more, but a huge difference doesn't seem likely.
> >> One thing that's uncertain is just how much of the IO is needing to be
> >> bounced - an even distribution of the buffer across all of physical
> >> RAM would suggest 25% in this case, but I don't know an easy way to
> >> verify that.
> >>
> >> Aside from speed considerations though, I should point out another
> >> factor: IOMMU/SWIOTLB space is in many cases a limited resource for
> >> all IO in flight at a particular time (SWIOTLB is typically 64MB). The
> >> number of hits when Googling for "Out of IOMMU space" indicates it is
> >> a problem that people do hit from time to time. From that perspective,
> >> anything that prevents unnecessary use of bounce buffers is a good
> >> thing.
> >
> > Sure, but again, for USB 2.0 stuff, we don't have many I/O in flight, as
> > they are pretty slow devices.
>
> Think that's a bit simplistic, if you have multiple devices active at
> once, or multiple controllers (not at all uncommon these days, newer
> Intel chipset machines have two EHCI controllers, with USB 1.x devices
> handled through a logical hub with TT connected to each of them) that
> can chew up more space.

I see machines with 4+ EHCI controllers quite common, and lots have a
number more than that. But I don't see how that matters here, they
aren't doing RAID over USB :)

> > USB 3.0 is different, and that's a different driver, and hopefully that
> > is all addressed already :)
>
> Doesn't look like it, from the version in current -git anyway - I
> don't see any calls to set DMA masks in the XHCI code so it will just
> default to 32-bit. I imagine that'll hurt performance at 4.8 Gbps if
> you've got lots of RAM..

Ok, we can worry about that when we get there, so far there is almost no
USB 3.0 devices out there to test with.

thanks,

greg k-h

2010-02-20 05:39:50

by David Brownell

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

On Thursday 18 February 2010, Robert Hancock wrote:
> >
> > But we disabled it on purpose, because of problems, do we want those
> > problems again?
>
> AFAICS, it was disabled because of problems with kernel code, not with
> hardware (and it appears the issue was with the code that detected the
> expanded DMA mask in the USB device driver code, not the HCD driver).
> CCing David Brownell who may know more.

That's a good summary of the high points. Testing was potentially an
issue, but it never quite got that far. So I have no idea if there are
systems where EHCI advertises 64-bit DMA support but that support is
broken (e.g. "Yet Another Hardware Mechanism MS-Windows Ignores", so that
only Linux would ever trip over the relevant BIOS breakage).

I won't attempt to go into details, but I recall a few basic issues:

* Not all clients or implementors of the "dma mask" mechanism agreed
on what it was supposed to achieve. Few, for example, really used
it as a mask ... and it rarely affects allocation of buffers that
will later get used for DMA.

* Confusing semantics for the various types of DMA restriction which
hardware may impose, and upper layers in driver stacks would thus
need (in some cases) to cope with.

* How to pass such restrictions up the driver stack ... as for example
that NETIF_* flag. ISTR there was some block layer issue too, but
at this remove I can't remember any details at all. (If networking
and the block layer can use 64-bit DMA, I can't imagine many other
subsystems would deliver wins as big.) For example, how would one
pass up the knowledge that a driver for a particular USB peripheral
across a few hubs) can do DMA to/from address 0x1234567890abcdef, but
the same driver can't do that for an otherwise identical peripheral
connected through a different HCD?

* There were probably a few PCI-specific issues too. I don't think
at that time there were many users of 64-bit DMA which weren't
specific to PCI. Wanting to use the generic DMA calls for such
stuff wasn't really done back then. But ... the USB stack uses
the generic calls, and drivers sitting on top of usbcore (and its
tightly coupled HCDs) will never issue PCI-specific calls, since
they need to work on systems that don't even have PCI.

I basically think that if the controller can do 64-bit DMA, it should
be enabling it by default ... assuming the software stack can handle
that. (What would be the benefit of adding needless restrictions,
and making systems needlessly apply bounce buffering.) So while I'd
like to see the 64-bit DMA working, it should IMO be done without any
options to cause trouble/confusion.

But at that time it wasn't straightforward to manage 64-bit DMA except
in the very lowest level PCI drivers. That is, EHCI could do it ...
but driver layers on top of it had no good way to do their part. For
example, when they manage DMA mappings themselves.)

- Dave

2010-02-20 07:15:39

by Robert Hancock

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

On Fri, Feb 19, 2010 at 11:39 PM, David Brownell <[email protected]> wrote:
> On Thursday 18 February 2010, Robert Hancock wrote:
>> >
>> > But we disabled it on purpose, because of problems, do we want those
>> > problems again?
>>
>> AFAICS, it was disabled because of problems with kernel code, not with
>> hardware (and it appears the issue was with the code that detected the
>> expanded DMA mask in the USB device driver code, not the HCD driver).
>> CCing David Brownell who may know more.
>
> That's a good summary of the high points. ?Testing was potentially an
> issue, but it never quite got that far. ?So I have no idea if there are
> systems where EHCI advertises 64-bit DMA support but that support is
> broken (e.g. "Yet Another Hardware Mechanism MS-Windows Ignores", so that
> only Linux would ever trip over the relevant BIOS breakage).

According to one Microsoft page I saw, Windows XP did not implement
the 64-bit addressing feature in EHCI. I haven't found any information
on whether any newer Windows versions do or not.

>
> I won't attempt to go into details, but I recall a few basic issues:
>
> ?* Not all clients or implementors of the "dma mask" mechanism agreed
> ? on what it was supposed to achieve. ?Few, for example, really used
> ? it as a mask ... and it rarely affects allocation of buffers that
> ? will later get used for DMA.
>
> ?* Confusing semantics for the various types of DMA restriction which
> ? hardware may impose, and upper layers in driver stacks would thus
> ? need (in some cases) to cope with.

I think this is pretty well nailed down at this point. I suspect the
confusion partly comes from the expectation that driver code should be
able to use dma_supported to test a particular mask against what a
device had been configured for. This function is really meant for use
in arch code, not for drivers. It's supposed to indicate if it's legal
to set a device's mask to the given value, not whether a given DMA
mask is compatible with the previously set mask. Calling this on a
device that arch code doesn't know about (like a USB device) will
quite likely give a non-useful result or possibly even blow up. If a
driver needs to check a device's mask, it can just check against it
directly.

>
> ?* How to pass such restrictions up the driver stack ... as for example
> ? that NETIF_* flag. ?ISTR there was some block layer issue too, but
> ? at this remove I can't remember any details at all. ?(If networking
> ? and the block layer can use 64-bit DMA, I can't imagine many other
> ? subsystems would deliver wins as big.) ?For example, how would one
> ? pass up the knowledge that a driver for a particular USB peripheral
> ? across a few hubs) can do DMA to/from address 0x1234567890abcdef, but
> ? the same driver can't do that for an otherwise identical peripheral
> ? connected through a different HCD?

I think this logic is also in place, for the most part. The DMA mask
from the HCD appears to be propagated into the USB device, and then
into the interface objects. For usb-storage, the SCSI layer
automatically sets the bounce limit based on the device passed into
it, so the right thing gets done. The networking layer seems like it
would need explicit handling in the drivers - I think basically a
check if the device interface's DMA mask was set to DMA_BIT_MASK(64)
and if so, set the HIGHDMA flag.

>
> ?* There were probably a few PCI-specific issues too. ?I don't think
> ? at that time there were many users of 64-bit DMA which weren't
> ? specific to PCI. ?Wanting to use the generic DMA calls for such
> ? stuff wasn't really done back then. ?But ... the USB stack uses
> ? the generic calls, and drivers sitting on top of usbcore (and its
> ? tightly coupled HCDs) will never issue PCI-specific calls, since
> ? they need to work on systems that don't even have PCI.
>
> I basically think that if the controller can do 64-bit DMA, it should
> be enabling it by default ... assuming the software stack can handle
> that. ?(What would be the benefit of adding needless restrictions,
> and making systems needlessly apply bounce buffering.) ?So while I'd
> like to see the 64-bit DMA working, it should IMO be done without any
> options to cause trouble/confusion.
>
> But at that time it wasn't straightforward to manage 64-bit DMA except
> in the very lowest level PCI drivers. ?That is, EHCI could do it ...
> but driver layers on top of it had no good way to do their part. ?For
> example, when they manage DMA mappings themselves.)
>
> - Dave
>
>

2010-02-20 08:08:06

by David Brownell

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

On Friday 19 February 2010, Robert Hancock wrote:
> > That's a good summary of the high points. ?Testing was potentially an
> > issue, but it never quite got that far. ?So I have no idea if there are
> > systems where EHCI advertises 64-bit DMA support but that support is
> > broken (e.g. "Yet Another Hardware Mechanism MS-Windows Ignores", so that
> > only Linux would ever trip over the relevant BIOS breakage).
>
> According to one Microsoft page I saw, Windows XP did not implement
> the 64-bit addressing feature in EHCI. I haven't found any information
> on whether any newer Windows versions do or not.

Note that it's pure speculation on my part whether or not any such
BIOS setup is needed. One would hope it wouldn't be required ...
but then engineers have been known to create all sorts of options
that require tweaking ... and trigger errors when the options aren't
stroked in the right way.


> > I won't attempt to go into details, but I recall a few basic issues:
> >
> > ?* Not all clients or implementors of the "dma mask" mechanism agreed
> > ? on what it was supposed to achieve. ?Few, for example, really used
> > ? it as a mask ... and it rarely affects allocation of buffers that
> > ? will later get used for DMA.
> >
> > ?* Confusing semantics for the various types of DMA restriction which
> > ? hardware may impose, and upper layers in driver stacks would thus
> > ? need (in some cases) to cope with.
>
> I think this is pretty well nailed down at this point. I suspect the
> confusion partly comes from the expectation that driver code should be
> able to use dma_supported to test a particular mask against what a
> device had been configured for. This function is really meant for use
> in arch code, not for drivers.

If so, that suggests a minor hole in the DMA interface, since drivers
do need such info.

As you note, mask manipulation can be done in drivers ... but on the flip
side, such things are a bit error prone and deserve API wrappers. (Plus,
there's the whole confusion about whether it's really a "mask", where a
given bit flags whether that address line is valid. Seems more like using
a bitstring of N ones as a representation of N, where only N matters.)


> > ?* How to pass such restrictions up the driver stack ... as for example
> > ? that NETIF_* flag. ?ISTR there was some block layer issue too, but
> > ? at this remove I can't remember any details at all. ?(If networking
> > ? and the block layer can use 64-bit DMA, I can't imagine many other
> > ? subsystems would deliver wins as big.) ?For example, how would one
> > ? pass up the knowledge that a driver for a particular USB peripheral
> > ? across a few hubs) can do DMA to/from address 0x1234567890abcdef, but
> > ? the same driver can't do that for an otherwise identical peripheral
> > ? connected through a different HCD?
>
> I think this logic is also in place, for the most part. The DMA mask
> from the HCD appears to be propagated into the USB device, and then
> into the interface objects.

Yeah, I recall thinking about that stuff way back when... intended to
set that up correctly. IT was at least partially tested.


> For usb-storage, the SCSI layer
> automatically sets the bounce limit based on the device passed into
> it, so the right thing gets done. The networking layer seems like it
> would need explicit handling in the drivers - I think basically a
> check if the device interface's DMA mask was set to DMA_BIT_MASK(64)
> and if so, set the HIGHDMA flag.

Another example of how roundabout all that stuff is. "64" being the
relevant number, in contrast to something less. So if for example the
DMA address bus width is 48 bits, things will be strange.

I wonder why the two layers don't adopt the same approach ... seemingly
they're making different assumptions about driver behavior, suggesting
that one of them may well be overly optimistic.

- Dave

2010-02-20 18:13:55

by Robert Hancock

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

On Sat, Feb 20, 2010 at 2:07 AM, David Brownell <[email protected]> wrote:
> On Friday 19 February 2010, Robert Hancock wrote:
>> > That's a good summary of the high points. ?Testing was potentially an
>> > issue, but it never quite got that far. ?So I have no idea if there are
>> > systems where EHCI advertises 64-bit DMA support but that support is
>> > broken (e.g. "Yet Another Hardware Mechanism MS-Windows Ignores", so that
>> > only Linux would ever trip over the relevant BIOS breakage).
>>
>> According to one Microsoft page I saw, Windows XP did not implement
>> the 64-bit addressing feature in EHCI. I haven't found any information
>> on whether any newer Windows versions do or not.
>
> Note that it's pure speculation on my part whether or not any such
> BIOS setup is needed. ?One would hope it wouldn't be required ...
> but then engineers have been known to create all sorts of options
> that require tweaking ... and trigger errors when the options aren't
> stroked in the right way.
>
>
>> > I won't attempt to go into details, but I recall a few basic issues:
>> >
>> > ?* Not all clients or implementors of the "dma mask" mechanism agreed
>> > ? on what it was supposed to achieve. ?Few, for example, really used
>> > ? it as a mask ... and it rarely affects allocation of buffers that
>> > ? will later get used for DMA.
>> >
>> > ?* Confusing semantics for the various types of DMA restriction which
>> > ? hardware may impose, and upper layers in driver stacks would thus
>> > ? need (in some cases) to cope with.
>>
>> I think this is pretty well nailed down at this point. I suspect the
>> confusion partly comes from the expectation that driver code should be
>> able to use dma_supported to test a particular mask against what a
>> device had been configured for. This function is really meant for use
>> in arch code, not for drivers.
>
> If so, that suggests a minor hole in the DMA interface, since drivers
> do need such info.

Well, if you need to test a mask against a device's existing one, then
all you really need is something like:

if( *dev->dma_mask & mymask == mymask)

A wrapper for that might not be a bad idea, but it's fairly trivial..

>
> As you note, mask manipulation can be done in drivers ... but on the flip
> side, such things are a bit error prone and deserve API wrappers. ?(Plus,
> there's the whole confusion about whether it's really a "mask", where a
> given bit flags whether that address line is valid. ?Seems more like using
> a bitstring of N ones as a representation of N, where only N matters.)

Yeah, that's the de-facto valid definition.. I'm sure not much kernel
code copes well with somebody setting a mask like "allow 64-bit
addresses, except not where bits 48 and 53 are set"..

>
>
>> > ?* How to pass such restrictions up the driver stack ... as for example
>> > ? that NETIF_* flag. ?ISTR there was some block layer issue too, but
>> > ? at this remove I can't remember any details at all. ?(If networking
>> > ? and the block layer can use 64-bit DMA, I can't imagine many other
>> > ? subsystems would deliver wins as big.) ?For example, how would one
>> > ? pass up the knowledge that a driver for a particular USB peripheral
>> > ? across a few hubs) can do DMA to/from address 0x1234567890abcdef, but
>> > ? the same driver can't do that for an otherwise identical peripheral
>> > ? connected through a different HCD?
>>
>> I think this logic is also in place, for the most part. The DMA mask
>> from the HCD appears to be propagated into the USB device, and then
>> into the interface objects.
>
> Yeah, I recall thinking about that stuff way back when... intended to
> set that up correctly. ?IT was at least partially tested.
>
>
>> For usb-storage, the SCSI layer
>> automatically sets the bounce limit based on the device passed into
>> it, so the right thing gets done. The networking layer seems like it
>> would need explicit handling in the drivers - I think basically a
>> check if the device interface's DMA mask was set to DMA_BIT_MASK(64)
>> and if so, set the HIGHDMA flag.
>
> Another example of how roundabout all that stuff is. ?"64" being the
> relevant number, in contrast to something less. ?So if for example the
> DMA address bus width is 48 bits, things will be strange.
>
> I wonder why the two layers don't adopt the same approach ... seemingly
> they're making different assumptions about driver behavior, suggesting
> that one of them may well be overly optimistic.

Hmm, it seems I was wrong about the usage of NETIF_F_HIGHDMA. I was
thinking it indicated 64-bit addressing support, but actually it just
indicates whether the driver can access highmem addresses and has
nothing to do with 64-bit at all. Essentially all network devices
should set that flag unless they can't access highmem, which would
only realistically happen if somebody was using PIO. (In this USB
networking case, it appears that would mean it should be set unless
the DMA mask for the device is set to NULL.) On configurations like
x86_64 where there's no highmem, it has no effect at all.

Unfortunately it appears that a lot of networking drivers' authors had
similar confusion and use it to indicate 64-bit DMA support, which
means the flag's not set in a lot of cases where it should be. Ugh..
think I'll start a new thread about that one.

2010-02-23 06:28:56

by Yuhong Bao

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


> 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/[email protected]/msg17230.html
>
> 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/-

2010-02-23 06:48:39

by Yuhong Bao

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


> According to one Microsoft page I saw, Windows XP did not implement
> the 64-bit addressing feature in EHCI. I haven't found any information
> on whether any newer Windows versions do or not.
Windows 7 seems to support it, and it seems like they have to already do a workaround for broken NVIDIA chipsets:
http://support.microsoft.com/?kbid=976972
http://www.windows7tipsonline.com/USB_Problem.htm
http://social.technet.microsoft.com/Forums/en-US/w7itprohardware/thread/3aae3b66-6a1a-47e8-ad1b-b20b68eaecf8#79ea3219-d76e-40bc-b910-c7d347002e66
http://social.technet.microsoft.com/Forums/en-US/w7itprohardware/thread/38f25c1d-de67-4c74-8845-2cd3a15d8e41

Yuhong Bao

_________________________________________________________________
Hotmail: Powerful Free email with security by Microsoft.
http://clk.atdmt.com/GBL/go/201469230/direct/01/-

2010-02-24 00:26:32

by Robert Hancock

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

On Tue, Feb 23, 2010 at 12:48 AM, Yuhong Bao <[email protected]> wrote:
>
>> According to one Microsoft page I saw, Windows XP did not implement
>> the 64-bit addressing feature in EHCI. I haven't found any information
>> on whether any newer Windows versions do or not.
> Windows 7 seems to support it, and it seems like they have to already do a workaround for broken NVIDIA chipsets:
> http://support.microsoft.com/?kbid=976972
> http://www.windows7tipsonline.com/USB_Problem.htm
> http://social.technet.microsoft.com/Forums/en-US/w7itprohardware/thread/3aae3b66-6a1a-47e8-ad1b-b20b68eaecf8#79ea3219-d76e-40bc-b910-c7d347002e66
> http://social.technet.microsoft.com/Forums/en-US/w7itprohardware/thread/38f25c1d-de67-4c74-8845-2cd3a15d8e41

That's good news actually.. it means they've apparently already
debugged the feature for us :-) I suppose the next step would be to
contact NVIDIA and see what they say (i.e. which particular chipsets
have the problem), but from my experience with them, requests of the
form "can you confirm X is a bug in your chipset" to those guys seem
to go into the bit bucket. Does anyone have any good contacts at
NVIDIA for USB or other chipset issues? I see there's some existing
NVIDIA-specific EHCI quirks so I'm assuming someone was in contact
with them before..

The fact that Windows 7 is now using the feature also means that there
aren't likely to be too many machines where the 64-bit addressing is
reported but doesn't work. Which means that aside from the NVIDIA
quirk, I think enabling 64-bit addressing should be relatively safe.

2010-02-24 03:53:08

by Yuhong Bao

[permalink] [raw]
Subject: SB600 64-bit DMA BIOS misconfiguration (formerly RE: [PATCH 2.6.34] ehci-hcd: add option to enable 64-bit DMA support)


> 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).
It seems it was solved by a DMI whitelist, but if it is a BIOS configuration issue, I think it would be better if it could be detected directly instead by reading the configuration directly from the device and correcting it if possible. Is that possible?

Yuhong Bao

_________________________________________________________________
Your E-mail and More On-the-Go. Get Windows Live Hotmail Free.
http://clk.atdmt.com/GBL/go/201469229/direct/01/-

2010-02-24 04:30:19

by Robert Hancock

[permalink] [raw]
Subject: Re: SB600 64-bit DMA BIOS misconfiguration (formerly RE: [PATCH 2.6.34] ehci-hcd: add option to enable 64-bit DMA support)

On Tue, Feb 23, 2010 at 9:53 PM, Yuhong Bao <[email protected]> wrote:
>
>> 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).
> It seems it was solved by a DMI whitelist, but if it is a BIOS configuration issue, I think it would be better if it could be detected directly instead by reading the configuration directly from the device and correcting it if possible. Is that possible?

That would likely be the ideal solution, but only AMD would likely
have the info needed to do that, if it's possible..

2010-02-24 04:33:11

by Yuhong Bao

[permalink] [raw]
Subject: RE: SB600 64-bit DMA BIOS misconfiguration (formerly RE: [PATCH 2.6.34] ehci-hcd: add option to enable 64-bit DMA support)


> That would likely be the ideal solution, but only AMD would likely
> have the info needed to do that, if it's possible..
That is why I am CCing [email protected], maybe they can help here.

Yuhong Bao

_________________________________________________________________
Hotmail: Powerful Free email with security by Microsoft.
http://clk.atdmt.com/GBL/go/201469230/direct/01/-

2010-02-24 13:30:52

by Huang, Shane

[permalink] [raw]
Subject: RE: SB600 64-bit DMA BIOS misconfiguration (formerly RE: [PATCH 2.6.34] ehci-hcd: add option to enable 64-bit DMA support)

Hi Robert and Yuhong,

> >> 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).
> > It seems it was solved by a DMI whitelist, but if it is a BIOS
configuration issue, I think it would be better if it could be detected
directly
> instead by reading the configuration directly from the device and
correcting it if possible. Is that possible?
>
> That would likely be the ideal solution, but only AMD would likely
> have the info needed to do that, if it's possible..

Yes, I did see SB600 64 bit DMA issue on several SB600 platforms,
especially the SATA controller and HD audio controller.
But we do not know the root cause, which seems to be BIOS related
after debug. So far what I know is:

1. The 64bit DMA issue only appears on few SB600 platforms, while
We do NOT see it on SB700, SB800 and our own SB600 reference boards.

2. According to the debug to ASUS M2A-VM SATA controller, some
BIOS change between version 1404 and 1501 lead to the issue:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commi
t;h=58a09b38cfcd700b796ea07ae3d2e0efbb28b561

3. I do not get any related SB600 errata or known issue confirmation
from our HW team, we do not think it is HW issue.

So I do not have too much suggestion but one:
If you have any contact window to ASUS, is it possible for you to
ask ASUS to check the difference between 1404 and 1501?
We might get clue from the change list.


Thanks,
Shane

2010-02-25 02:18:14

by Tejun Heo

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

Hello,

On 02/24/2010 09:26 AM, Robert Hancock wrote:
> The fact that Windows 7 is now using the feature also means that there
> aren't likely to be too many machines where the 64-bit addressing is
> reported but doesn't work. Which means that aside from the NVIDIA
> quirk, I think enabling 64-bit addressing should be relatively safe.

(following thread from the ATA side) I'm with Greg on this. USB2.0 is
quite slow on today's standard and most 64bit machines now have IOMMU
of some kind anyway. If we have problems on IOMMU (sw or hw)
allocation, I think it should be fixed there.

64bit support even in libata was quite painful with quirky BIOSen, PCI
host controllers, bridges and the controllers themselves. ATA being a
major secondary storage subsystem, libata had to do it but even then
for many those enabling trials, I think we've lost more than we
gained. There just isn't much point in trying to enable a shiny new
feature on an aging platform or technology. The gain might be there
but the downside is we end up catching fallout cases where boot fails
or data corrupts after many months later if we're very lucky. It's
just not worth it.

Thanks.

--
tejun

2010-02-25 02:41:44

by Yuhong Bao

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


> gained. There just isn't much point in trying to enable a shiny new
> feature on an aging platform or technology.
You mean we should focus on enabling 64-bit DMA for USB 3.0 (xHCI)?

Yuhong Bao

_________________________________________________________________
Hotmail: Trusted email with Microsoft?s powerful SPAM protection.
http://clk.atdmt.com/GBL/go/201469226/direct/01/-

2010-02-25 02:48:20

by Tejun Heo

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

Hello,

On 02/25/2010 11:41 AM, Yuhong Bao wrote:
>
>> gained. There just isn't much point in trying to enable a shiny new
>> feature on an aging platform or technology.
> You mean we should focus on enabling 64-bit DMA for USB 3.0 (xHCI)?

Yeah, probably, in this context, but I was just making a general
statement. Trying out a new feature which hasn't been used widely on
old things doesn't go too well in many cases - the hw vendor and BIOS
writer already moved on, people end up experiencing regressions on
configurations which used to work fine and when that happens, new
comers try out and find out it's broken in strange ways and when that
happens they're much less likely to bother reporting and trying to get
the problem fixed. And, most importantly, when things aren't used
widely, that usually is for a reason - it just doesn't make that much
of a difference.

Thanks.

--
tejun

2010-02-25 03:15:21

by Yuhong Bao

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



> (following thread from the ATA side) I'm with Greg on this. USB2.0 is
> quite slow on today's standard and most 64bit machines now have IOMMU
> of some kind anyway.

Except that unfortunately, Intel with Lynnfield and Clarkdale decided
to do market segmentation on the VT-d feature.
This could be done because both have an integrated northbridge.
With Lynnfield, the very common Core i5 750 CPU do not support VT-d,
but the Core i7 800 series does.
With Clarkdale, the very common Core i3 500 series do not support VT-d,
but the Core i5 600 series does, except the 661.
Not to mention that in the Core 2 generation, VT-d was available only
on Q35/Q45/X38/X48 chipsets.

Yuhong Bao

_________________________________________________________________
Hotmail: Trusted email with powerful SPAM protection.
http://clk.atdmt.com/GBL/go/201469227/direct/01/-

2010-02-25 03:19:20

by Tejun Heo

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

Hello,

On 02/25/2010 12:15 PM, Yuhong Bao wrote:
>> (following thread from the ATA side) I'm with Greg on this. USB2.0 is
>> quite slow on today's standard and most 64bit machines now have IOMMU
>> of some kind anyway.
>
> Except that unfortunately, Intel with Lynnfield and Clarkdale decided
> to do market segmentation on the VT-d feature.
> This could be done because both have an integrated northbridge.
> With Lynnfield, the very common Core i5 750 CPU do not support VT-d,
> but the Core i7 800 series does.
> With Clarkdale, the very common Core i3 500 series do not support VT-d,
> but the Core i5 600 series does, except the 661.
> Not to mention that in the Core 2 generation, VT-d was available only
> on Q35/Q45/X38/X48 chipsets.

Arghhh.... I hate when intel pulls this type of 'product
differentiation' stunts. At any rate, I'm doubtful swiotlb'ing for
usb2.0 would be noticeable at all.

Thanks.

--
tejun

2010-02-25 04:03:35

by Oliver Neukum

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

Am Donnerstag, 25. Februar 2010 04:29:12 schrieb Tejun Heo:
> Arghhh.... I hate when intel pulls this type of 'product
> differentiation' stunts. At any rate, I'm doubtful swiotlb'ing for
> usb2.0 would be noticeable at all.

That can and should be tested.

Regards
Oliver

2010-02-25 05:15:41

by Tejun Heo

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

On 02/25/2010 01:03 PM, Oliver Neukum wrote:
> Am Donnerstag, 25. Februar 2010 04:29:12 schrieb Tejun Heo:
>> Arghhh.... I hate when intel pulls this type of 'product
>> differentiation' stunts. At any rate, I'm doubtful swiotlb'ing for
>> usb2.0 would be noticeable at all.
>
> That can and should be tested.

Robert did already. Given that USB2.0 devices aren't usually used for
high-perf storage subsystem, the benefit looks marginal to me but
everyone is entitled to own opinion.

Thanks.

Thanks.

--
tejun

2010-02-25 16:21:26

by Greg KH

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

On Thu, Feb 25, 2010 at 02:25:34PM +0900, Tejun Heo wrote:
> On 02/25/2010 01:03 PM, Oliver Neukum wrote:
> > Am Donnerstag, 25. Februar 2010 04:29:12 schrieb Tejun Heo:
> >> Arghhh.... I hate when intel pulls this type of 'product
> >> differentiation' stunts. At any rate, I'm doubtful swiotlb'ing for
> >> usb2.0 would be noticeable at all.
> >
> > That can and should be tested.
>
> Robert did already. Given that USB2.0 devices aren't usually used for
> high-perf storage subsystem, the benefit looks marginal to me but
> everyone is entitled to own opinion.

For now, I'd like to not enable this, given the huge risks and no known
benifit.

For USB 3.0, we can reconsider it, as it might start to make a
difference there.

thanks,

greg k-h

2010-02-25 23:15:51

by Robert Hancock

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

On Thu, Feb 25, 2010 at 10:14 AM, Greg KH <[email protected]> wrote:
> On Thu, Feb 25, 2010 at 02:25:34PM +0900, Tejun Heo wrote:
>> On 02/25/2010 01:03 PM, Oliver Neukum wrote:
>> > Am Donnerstag, 25. Februar 2010 04:29:12 schrieb Tejun Heo:
>> >> Arghhh.... I hate when intel pulls this type of 'product
>> >> differentiation' stunts. ?At any rate, I'm doubtful swiotlb'ing for
>> >> usb2.0 would be noticeable at all.
>> >
>> > That can and should be tested.
>>
>> Robert did already. ?Given that USB2.0 devices aren't usually used for
>> high-perf storage subsystem, the benefit looks marginal to me but
>> everyone is entitled to own opinion.

Those tests shouldn't be given too much weight, other than indicating
there is some measurable benefit. A more useful test would be on a
machine with more RAM and with device(s) that are pushing closer to
the limits of the USB bus.

>
> For now, I'd like to not enable this, given the huge risks and no known
> benifit.

"Huge risks" is likely a bit melodramatic, we've seen from the Windows
7 case that there are some problematic cases, but you can be fairly
aggressive about blacklisting controllers there. If Windows hadn't
adopted the feature one might argue there would be some risk going
into the future of this breaking on new machines, but we've seen that
Windows using the feature generally indicates it works..

>
> For USB 3.0, we can reconsider it, as it might start to make a
> difference there.

I'd say that's pretty certain..

2010-02-26 00:13:22

by Greg KH

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

On Thu, Feb 25, 2010 at 05:15:49PM -0600, Robert Hancock wrote:
> > For now, I'd like to not enable this, given the huge risks and no known
> > benifit.
>
> "Huge risks" is likely a bit melodramatic

No it is not. This has the ability to break USB functionality that
currently works just fine for a user. That's not acceptable.

And yes, we can fix things after-the-fact with blacklists, but as Tejun
so aptly described, it is a major pain to do so, and lots of users just
get frustrated and never tell us that problems happen.

thanks,

greg k-h

2010-02-26 07:01:42

by Oliver Neukum

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

Am Freitag, 26. Februar 2010 00:25:29 schrieb Greg KH:
> And yes, we can fix things after-the-fact with blacklists, but as Tejun
> so aptly described, it is a major pain to do so, and lots of users just
> get frustrated and never tell us that problems happen.

Exactly. We'd be better off with a whitelist in this case.

Regards
Oliver