[RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Hanjun Guo]

From: Hanjun Guo <hanjun.guo@linaro.org>

We met an issue that when we update the IORT table to revision D,
and the kernel update to 4.19, the USB on D06 (ARM64 based server)
will probe fail:

[   13.495751] CPU: 0 PID: 15 Comm: kworker/0:1 Not tainted 4.19.0-00115-gb2b5200 #5
[   13.503219] Hardware name: Huawei D06/D06, BIOS Hisilicon D06 UEFI RC0 - V1.09.02 12/25/2018
[   13.511645] Workqueue: events work_for_cpu_fn
[   13.515989] pstate: a0c00009 (NzCv daif +PAN +UAO)
[   13.520767] pc : dma_pool_alloc+0x218/0x270
[   13.524937] lr : dma_pool_alloc+0xa0/0x270
[   13.529019] sp : ffff000009e23b20
[   13.532320] x29: ffff000009e23b20 x28: ffff8027c58ad098 
[   13.537619] x27: 0000000000001000 x26: ffff8027d7a790a8 
[   13.542918] x25: ffff000008fa7000 x24: ffff000009e23bc0 
[   13.548216] x23: 00000000006000c0 x22: ffff8027c58ad010 
[   13.553515] x21: ffff0000097e1000 x20: ffff8027c58ad000 
[   13.558814] x19: ffff8027c58ad080 x18: ffffffffffffffff 
[   13.564112] x17: 0000000000000000 x16: 0000000000007fff 
[   13.569411] x15: ffff0000097e16c8 x14: ffff8027c5d39885 
[   13.574709] x13: ffff8027c5d39884 x12: 0000000000000038 
[   13.580008] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f 
[   13.585307] x9 : 0000000000000000 x8 : ffff8027c587c400 
[   13.590605] x7 : 0000000000000000 x6 : 000000000000003f 
[   13.595904] x5 : ffff8027dc5b8000 x4 : ffff8027e09b91e0 
[   13.601202] x3 : 00000000008d2280 x2 : ffff8027c58ad100 
[   13.606501] x1 : 0000000000000028 x0 : 0000000000000000 
[   13.611800] Call trace:
[   13.614234]  dma_pool_alloc+0x218/0x270
[   13.617710] ata1: SATA link down (SStatus 0 SControl 300)
[   13.618059]  ehci_qh_alloc+0x5c/0xf8
[   13.627002]  ehci_setup+0x17c/0x4b8
[   13.630478]  ehci_pci_setup+0x18c/0x5b8
[   13.634301]  usb_add_hcd+0x290/0x7a0
[   13.637863]  usb_hcd_pci_probe+0x2cc/0x3e8
[   13.641946]  ehci_pci_probe+0x34/0x48
[   13.645596]  local_pci_probe+0x3c/0xb0
[   13.649331]  work_for_cpu_fn+0x18/0x28
[   13.653067]  process_one_work+0x1e4/0x458
[   13.657063]  worker_thread+0x228/0x450
[   13.660798]  kthread+0x12c/0x130
[   13.664014]  ret_from_fork+0x10/0x18
[   13.667577] ---[ end trace 6f8757456e2ec456 ]---

It turns out the the IORT revision D introduce the DMA address
limit size for PCI RC and in commit 5ac65e8c8941 ("ACPI/IORT: Support
address size limit for root complexes"), will set the DMA mask
for the RC and that will be inherited by device under the RC.

D06 only enables 1 RC but has EPs with different DMA address sizes,
for USB it use 32bit DMA, and 64bit for HNS and SAS, so this will
cause probe failure if we use 64bit DMA for USB controllers.

Set the DMA mask to 32bit for PCI based USB controllers,
EHCI and OHCI USB controllers are using 32bit DMA address,
XHCI will set the DMA mask in its probe after the pci probe,
so it's safe just add dma_coerce_mask_and_coherent() in
usb_hcd_pci_probe().

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
Hi all,

This is the RFC version, I'm not sure this is the best solution,
comments are warmly welcomed.

Thanks
Hanjun

 drivers/usb/core/hcd-pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 0343246..a9c33e6 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (pci_enable_device(dev) < 0)
 		return -ENODEV;
 
+	retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+	if (retval)
+		return retval;
+
 	/*
 	 * The xHCI driver has its own irq management
 	 * make sure irq setup is not touched for xhci in generic hcd code
-- 
1.7.12.4

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Christoph Hellwig]

On Wed, Jan 30, 2019 at 03:01:54PM +0800, Hanjun Guo wrote:
> This is the RFC version, I'm not sure this is the best solution,
> comments are warmly welcomed.
> 
> Thanks
> Hanjun
> 
>  drivers/usb/core/hcd-pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 0343246..a9c33e6 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	if (pci_enable_device(dev) < 0)
>  		return -ENODEV;
>  
> +	retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
> +	if (retval)
> +		return retval;

dma_coerce_mask_and_coherent is only for platform devices (and I'm
not sure it is a good idea to start with, but that is a different
story).

PCI device should have the dma_mask pointer set already, so you should
use dma_set_mask_and_coherent here.

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Hanjun Guo]

Hi Christoph,

On 2019/1/30 15:40, Christoph Hellwig wrote:
> On Wed, Jan 30, 2019 at 03:01:54PM +0800, Hanjun Guo wrote:
>> This is the RFC version, I'm not sure this is the best solution,
>> comments are warmly welcomed.
>>
>> Thanks
>> Hanjun
>>
>>  drivers/usb/core/hcd-pci.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>> index 0343246..a9c33e6 100644
>> --- a/drivers/usb/core/hcd-pci.c
>> +++ b/drivers/usb/core/hcd-pci.c
>> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>  	if (pci_enable_device(dev) < 0)
>>  		return -ENODEV;
>>  
>> +	retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
>> +	if (retval)
>> +		return retval;
> 
> dma_coerce_mask_and_coherent is only for platform devices (and I'm
> not sure it is a good idea to start with, but that is a different
> story).

Yes, that's why this is the RFC version.

> 
> PCI device should have the dma_mask pointer set already, so you should
> use dma_set_mask_and_coherent here.

I will wait for a while to see if more comments, if not, I will update
my patch as you suggested.

Thanks
Hanjun

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[John Garry]

On 30/01/2019 07:01, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
>
> We met an issue that when we update the IORT table to revision D,
> and the kernel update to 4.19, the USB on D06 (ARM64 based server)
> will probe fail:
>
> [   13.495751] CPU: 0 PID: 15 Comm: kworker/0:1 Not tainted 4.19.0-00115-gb2b5200 #5
> [   13.503219] Hardware name: Huawei D06/D06, BIOS Hisilicon D06 UEFI RC0 - V1.09.02 12/25/2018
> [   13.511645] Workqueue: events work_for_cpu_fn
> [   13.515989] pstate: a0c00009 (NzCv daif +PAN +UAO)
> [   13.520767] pc : dma_pool_alloc+0x218/0x270
> [   13.524937] lr : dma_pool_alloc+0xa0/0x270
> [   13.529019] sp : ffff000009e23b20
> [   13.532320] x29: ffff000009e23b20 x28: ffff8027c58ad098
> [   13.537619] x27: 0000000000001000 x26: ffff8027d7a790a8
> [   13.542918] x25: ffff000008fa7000 x24: ffff000009e23bc0
> [   13.548216] x23: 00000000006000c0 x22: ffff8027c58ad010
> [   13.553515] x21: ffff0000097e1000 x20: ffff8027c58ad000
> [   13.558814] x19: ffff8027c58ad080 x18: ffffffffffffffff
> [   13.564112] x17: 0000000000000000 x16: 0000000000007fff
> [   13.569411] x15: ffff0000097e16c8 x14: ffff8027c5d39885
> [   13.574709] x13: ffff8027c5d39884 x12: 0000000000000038
> [   13.580008] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> [   13.585307] x9 : 0000000000000000 x8 : ffff8027c587c400
> [   13.590605] x7 : 0000000000000000 x6 : 000000000000003f
> [   13.595904] x5 : ffff8027dc5b8000 x4 : ffff8027e09b91e0
> [   13.601202] x3 : 00000000008d2280 x2 : ffff8027c58ad100
> [   13.606501] x1 : 0000000000000028 x0 : 0000000000000000
> [   13.611800] Call trace:
> [   13.614234]  dma_pool_alloc+0x218/0x270
> [   13.617710] ata1: SATA link down (SStatus 0 SControl 300)
> [   13.618059]  ehci_qh_alloc+0x5c/0xf8
> [   13.627002]  ehci_setup+0x17c/0x4b8
> [   13.630478]  ehci_pci_setup+0x18c/0x5b8
> [   13.634301]  usb_add_hcd+0x290/0x7a0
> [   13.637863]  usb_hcd_pci_probe+0x2cc/0x3e8
> [   13.641946]  ehci_pci_probe+0x34/0x48
> [   13.645596]  local_pci_probe+0x3c/0xb0
> [   13.649331]  work_for_cpu_fn+0x18/0x28
> [   13.653067]  process_one_work+0x1e4/0x458
> [   13.657063]  worker_thread+0x228/0x450
> [   13.660798]  kthread+0x12c/0x130
> [   13.664014]  ret_from_fork+0x10/0x18
> [   13.667577] ---[ end trace 6f8757456e2ec456 ]---
>
> It turns out the the IORT revision D introduce the DMA address
> limit size for PCI RC and in commit 5ac65e8c8941 ("ACPI/IORT: Support
> address size limit for root complexes"), will set the DMA mask
> for the RC and that will be inherited by device under the RC.
>
> D06 only enables 1 RC but has EPs with different DMA address sizes,
> for USB it use 32bit DMA, and 64bit for HNS and SAS, so this will
> cause probe failure if we use 64bit DMA for USB controllers.
>
> Set the DMA mask to 32bit for PCI based USB controllers,
> EHCI and OHCI USB controllers are using 32bit DMA address,
> XHCI will set the DMA mask in its probe after the pci probe,
> so it's safe just add dma_coerce_mask_and_coherent() in
> usb_hcd_pci_probe().
>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
> Hi all,
>
> This is the RFC version, I'm not sure this is the best solution,
> comments are warmly welcomed.
>
> Thanks
> Hanjun
>
>  drivers/usb/core/hcd-pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 0343246..a9c33e6 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	if (pci_enable_device(dev) < 0)
>  		return -ENODEV;
>
> +	retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
> +	if (retval)
> +		return retval;

Hi Hanjun,

You're missing tidy-up upon failure.

thanks,
John

> +
>  	/*
>  	 * The xHCI driver has its own irq management
>  	 * make sure irq setup is not touched for xhci in generic hcd code
>

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Hanjun Guo]

Hi John,

On 2019/1/31 17:54, John Garry wrote:
> On 30/01/2019 07:01, Hanjun Guo wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
[...]
>>
>>  drivers/usb/core/hcd-pci.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>> index 0343246..a9c33e6 100644
>> --- a/drivers/usb/core/hcd-pci.c
>> +++ b/drivers/usb/core/hcd-pci.c
>> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>      if (pci_enable_device(dev) < 0)
>>          return -ENODEV;
>>
>> +    retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
>> +    if (retval)
>> +        return retval;
> 
> Hi Hanjun,
> 
> You're missing tidy-up upon failure.

Good catch, needs to disable pci for failure, I will send
a updated version to address the comments from you and Christoph.

Thanks
Hanjun

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Hanjun Guo]

On 2019/2/1 13:55, Hanjun Guo wrote:
> Hi John,
> 
> On 2019/1/31 17:54, John Garry wrote:
>> On 30/01/2019 07:01, Hanjun Guo wrote:
>>> From: Hanjun Guo <hanjun.guo@linaro.org>
> [...]
>>>
>>>  drivers/usb/core/hcd-pci.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>>> index 0343246..a9c33e6 100644
>>> --- a/drivers/usb/core/hcd-pci.c
>>> +++ b/drivers/usb/core/hcd-pci.c
>>> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>      if (pci_enable_device(dev) < 0)
>>>          return -ENODEV;
>>>
>>> +    retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
>>> +    if (retval)
>>> +        return retval;
>>
>> Hi Hanjun,
>>
>> You're missing tidy-up upon failure.
> 
> Good catch, needs to disable pci for failure, I will send
> a updated version to address the comments from you and Christoph.

There is a _DMA method which was introduced in ACPI 6.2 to cover
this, I will try that solution then report back.

Thanks
Hanjun

Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers

[Hanjun Guo]

On 2019/2/1 17:13, Hanjun Guo wrote:
> On 2019/2/1 13:55, Hanjun Guo wrote:
>> Hi John,
>>
>> On 2019/1/31 17:54, John Garry wrote:
>>> On 30/01/2019 07:01, Hanjun Guo wrote:
>>>> From: Hanjun Guo <hanjun.guo@linaro.org>
>> [...]
>>>>
>>>>  drivers/usb/core/hcd-pci.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>>>> index 0343246..a9c33e6 100644
>>>> --- a/drivers/usb/core/hcd-pci.c
>>>> +++ b/drivers/usb/core/hcd-pci.c
>>>> @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>>      if (pci_enable_device(dev) < 0)
>>>>          return -ENODEV;
>>>>
>>>> +    retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
>>>> +    if (retval)
>>>> +        return retval;
>>>
>>> Hi Hanjun,
>>>
>>> You're missing tidy-up upon failure.
>>
>> Good catch, needs to disable pci for failure, I will send
>> a updated version to address the comments from you and Christoph.
> 
> There is a _DMA method which was introduced in ACPI 6.2 to cover
> this, I will try that solution then report back.

We add a _DMA method under the hostbridge which the EHCI/OHCI being
connected to, the calltrace is gone and EHCI works as expected.

No need for kernel change, so this patch is dropped.

Thanks
Hanjun