2024-01-30 11:43:14

by Lennert Buytenhek

[permalink] [raw]
Subject: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

ASMedia have confirmed that all ASM106x parts currently listed in
ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
into on the ASM1061, and therefore, we need to apply the quirk added by
commit 20730e9b2778 to the other supported ASM106x parts as well.

Signed-off-by: Lennert Buytenhek <[email protected]>
---
drivers/ata/ahci.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index d2460fa985b7..da2e74fce2d9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */

/* ASMedia */
- { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
- { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
+ { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
+ { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
{ PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
{ PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
- { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
- { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
- { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
+ { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
+ { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
+ { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
{ PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
{ PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
{ PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */
--
2.43.0


2024-01-30 11:50:55

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

On 1/30/24 20:41, Lennert Buytenhek wrote:
> ASMedia have confirmed that all ASM106x parts currently listed in
> ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
> into on the ASM1061, and therefore, we need to apply the quirk added by
> commit 20730e9b2778 to the other supported ASM106x parts as well.
>
> Signed-off-by: Lennert Buytenhek <[email protected]>

I think this needs a cc: stable tag.

> ---
> drivers/ata/ahci.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index d2460fa985b7..da2e74fce2d9 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */
>
> /* ASMedia */
> - { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
> - { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
> { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
> { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
> - { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
> - { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
> - { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
> + { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
> + { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
> + { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
> { PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
> { PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
> { PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */

--
Damien Le Moal
Western Digital Research


2024-01-30 11:59:43

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

On Tue, Jan 30, 2024 at 08:46:23PM +0900, Damien Le Moal wrote:

> > ASMedia have confirmed that all ASM106x parts currently listed in
> > ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
> > into on the ASM1061, and therefore, we need to apply the quirk added by
> > commit 20730e9b2778 to the other supported ASM106x parts as well.
> >
> > Signed-off-by: Lennert Buytenhek <[email protected]>
>
> I think this needs a cc: stable tag.

The commit that is likely responsible for surfacing this issue is
791c2b17fb40 which went into v6.6 -- so would this then be appropriate,
or do you think this should be backported to older versions as well?

Cc: [email protected] # 6.6.x


> > ---
> > drivers/ata/ahci.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> > index d2460fa985b7..da2e74fce2d9 100644
> > --- a/drivers/ata/ahci.c
> > +++ b/drivers/ata/ahci.c
> > @@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> > { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */
> >
> > /* ASMedia */
> > - { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
> > - { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
> > + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
> > + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
> > { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
> > { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
> > - { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
> > - { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
> > - { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
> > + { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
> > + { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
> > + { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
> > { PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
> > { PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
> > { PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */

2024-01-30 12:24:14

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

On Tue, Jan 30, 2024 at 01:41:02PM +0200, Lennert Buytenhek wrote:
> ASMedia have confirmed that all ASM106x parts currently listed in
> ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
> into on the ASM1061, and therefore, we need to apply the quirk added by
> commit 20730e9b2778 to the other supported ASM106x parts as well.

Lennert, thanks a lot for sending this follow up patch.

However, checkpatch.pl complains:

ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 20730e9b2778 ("ahci: add 43-bit DMA address quirk for ASMedia ASM1061 controllers")'
#44:
commit 20730e9b2778 to the other supported ASM106x parts as well.

Perhaps send a v2 with this fixed,
and Cc: stable like Damien requested.


Kind regards,
Niklas

>
> Signed-off-by: Lennert Buytenhek <[email protected]>
> ---
> drivers/ata/ahci.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index d2460fa985b7..da2e74fce2d9 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */
>
> /* ASMedia */
> - { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
> - { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
> { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
> { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
> - { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
> - { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
> - { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
> + { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
> + { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
> + { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
> { PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
> { PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
> { PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */
> --
> 2.43.0

2024-01-30 12:35:04

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

On 30/01/2024 12:26 pm, Damien Le Moal wrote:
> On 1/30/24 20:59, Lennert Buytenhek wrote:
>> On Tue, Jan 30, 2024 at 08:46:23PM +0900, Damien Le Moal wrote:
>>
>>>> ASMedia have confirmed that all ASM106x parts currently listed in
>>>> ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
>>>> into on the ASM1061, and therefore, we need to apply the quirk added by
>>>> commit 20730e9b2778 to the other supported ASM106x parts as well.
>>>>
>>>> Signed-off-by: Lennert Buytenhek <[email protected]>
>>>
>>> I think this needs a cc: stable tag.
>>
>> The commit that is likely responsible for surfacing this issue is
>> 791c2b17fb40 which went into v6.6 -- so would this then be appropriate,
>> or do you think this should be backported to older versions as well?
>
> Hmmm... given this is a hardware "bug", it seems safer to backport to all stable
> & lts. From what I understand, the device may be doing bad DMA, regardless of
> what the iommu is doing. Niklas ? you followed this more carefully than I did :)

Yes, that would be the case; in practice though it's likely that people
just don't tend to use these particular controllers in big systems which
actually have RAM at >43-bit physical addresses.

Thanks,
Robin.

>
>>
>> Cc: [email protected] # 6.6.x
>>
>>
>>>> ---
>>>> drivers/ata/ahci.c | 10 +++++-----
>>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>>>> index d2460fa985b7..da2e74fce2d9 100644
>>>> --- a/drivers/ata/ahci.c
>>>> +++ b/drivers/ata/ahci.c
>>>> @@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>>>> { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */
>>>>
>>>> /* ASMedia */
>>>> - { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
>>>> - { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
>>>> + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
>>>> + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
>>>> { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
>>>> { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
>>>> - { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
>>>> - { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
>>>> - { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
>>>> + { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
>>>> + { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
>>>> + { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
>>>> { PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
>>>> { PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
>>>> { PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */
>

2024-01-30 12:50:26

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

On 1/30/24 20:59, Lennert Buytenhek wrote:
> On Tue, Jan 30, 2024 at 08:46:23PM +0900, Damien Le Moal wrote:
>
>>> ASMedia have confirmed that all ASM106x parts currently listed in
>>> ahci_pci_tbl[] suffer from the 43-bit DMA address limitation that we ran
>>> into on the ASM1061, and therefore, we need to apply the quirk added by
>>> commit 20730e9b2778 to the other supported ASM106x parts as well.
>>>
>>> Signed-off-by: Lennert Buytenhek <[email protected]>
>>
>> I think this needs a cc: stable tag.
>
> The commit that is likely responsible for surfacing this issue is
> 791c2b17fb40 which went into v6.6 -- so would this then be appropriate,
> or do you think this should be backported to older versions as well?

Hmmm... given this is a hardware "bug", it seems safer to backport to all stable
& lts. From what I understand, the device may be doing bad DMA, regardless of
what the iommu is doing. Niklas ? you followed this more carefully than I did :)

>
> Cc: [email protected] # 6.6.x
>
>
>>> ---
>>> drivers/ata/ahci.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>>> index d2460fa985b7..da2e74fce2d9 100644
>>> --- a/drivers/ata/ahci.c
>>> +++ b/drivers/ata/ahci.c
>>> @@ -606,13 +606,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>>> { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */
>>>
>>> /* ASMedia */
>>> - { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
>>> - { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
>>> + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci_43bit_dma }, /* ASM1060 */
>>> + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci_43bit_dma }, /* ASM1060 */
>>> { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci_43bit_dma }, /* ASM1061 */
>>> { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci_43bit_dma }, /* ASM1061/1062 */
>>> - { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci }, /* ASM1061R */
>>> - { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci }, /* ASM1062R */
>>> - { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci }, /* ASM1062+JMB575 */
>>> + { PCI_VDEVICE(ASMEDIA, 0x0621), board_ahci_43bit_dma }, /* ASM1061R */
>>> + { PCI_VDEVICE(ASMEDIA, 0x0622), board_ahci_43bit_dma }, /* ASM1062R */
>>> + { PCI_VDEVICE(ASMEDIA, 0x0624), board_ahci_43bit_dma }, /* ASM1062+JMB575 */
>>> { PCI_VDEVICE(ASMEDIA, 0x1062), board_ahci }, /* ASM1062A */
>>> { PCI_VDEVICE(ASMEDIA, 0x1064), board_ahci }, /* ASM1064 */
>>> { PCI_VDEVICE(ASMEDIA, 0x1164), board_ahci }, /* ASM1164 */

--
Damien Le Moal
Western Digital Research