ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts

Message ID ZbjgTmR5FbAnb-Ua@wantstofly.org
State New
Headers
Series ahci: Extend ASM1061 43-bit DMA address quirk to other ASM106x parts |

Commit Message

Lennert Buytenhek Jan. 30, 2024, 11:41 a.m. UTC
  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 <kernel@wantstofly.org>
---
 drivers/ata/ahci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
  

Comments

Damien Le Moal Jan. 30, 2024, 11:46 a.m. UTC | #1
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 <kernel@wantstofly.org>

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 */
  
Lennert Buytenhek Jan. 30, 2024, 11:59 a.m. UTC | #2
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 <kernel@wantstofly.org>
> 
> 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: stable@vger.kernel.org # 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 */
  
Niklas Cassel Jan. 30, 2024, 12:23 p.m. UTC | #3
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 <kernel@wantstofly.org>
> ---
>  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
  
Damien Le Moal Jan. 30, 2024, 12:26 p.m. UTC | #4
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 <kernel@wantstofly.org>
>>
>> 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: stable@vger.kernel.org # 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 */
  
Robin Murphy Jan. 30, 2024, 12:34 p.m. UTC | #5
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 <kernel@wantstofly.org>
>>>
>>> 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: stable@vger.kernel.org # 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 */
>
  

Patch

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