[PATCH PCI] PCI: Add ACS quirk for Wangxun FF5XXX NICS

[PATCH PCI] PCI: Add ACS quirk for Wangxun FF5XXX NICS

[Mengyuan Lou]

Wangxun FF5XXX NICs are same as selection of SFxxx, RP1000 and
RP2000 NICS. They may be multi-function devices, but the hardware
does not advertise ACS capability.

Add this ACS quirk for FF5XXX NICs in pci_quirk_wangxun_nic_acs
so the functions can be in independent IOMMU groups.

Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
 drivers/pci/quirks.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index dccb60c1d9cc..d1973a8fd70c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4996,18 +4996,20 @@ static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags)
 }
 
 /*
- * Wangxun 10G/1G NICs have no ACS capability, and on multi-function
+ * Wangxun 40G/25G/10G/1G NICs have no ACS capability, and on multi-function
  * devices, peer-to-peer transactions are not be used between the functions.
  * So add an ACS quirk for below devices to isolate functions.
  * SFxxx 1G NICs(em).
  * RP1000/RP2000 10G NICs(sp).
+ * FF5xxx 40G/25G/10G NICs(aml).
  */
 static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags)
 {
 	switch (dev->device) {
-	case 0x0100 ... 0x010F:
-	case 0x1001:
-	case 0x2001:
+	case 0x0100 ... 0x010F: /* EM */
+	case 0x1001: case 0x2001: /* SP */
+	case 0x5010: case 0x5025: case 0x5040: /* AML */
+	case 0x5110: case 0x5125: case 0x5140: /* AML */
 		return pci_acs_ctrl_enabled(acs_flags,
 			PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
 	}
-- 
2.43.2

Re: [PATCH PCI] PCI: Add ACS quirk for Wangxun FF5XXX NICS

[Bjorn Helgaas]

On Tue, Nov 12, 2024 at 07:18:16PM +0800, Mengyuan Lou wrote:
> Wangxun FF5XXX NICs are same as selection of SFxxx, RP1000 and
> RP2000 NICS. They may be multi-function devices, but the hardware
> does not advertise ACS capability.
>
> Add this ACS quirk for FF5XXX NICs in pci_quirk_wangxun_nic_acs
> so the functions can be in independent IOMMU groups.
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>

I propose the following commit log and comment updates to be clear
that the hardware actually enforces this isolation.  Please
confirm that they are accurate.

  PCI: Add ACS quirk for Wangxun FF5xxx NICs

  Wangxun FF5xxx NICs are similar to SFxxx, RP1000 and RP2000 NICs.
  They may be multi-function devices, but they do not advertise an ACS
  capability.

  But the hardware does isolate FF5xxx functions as though it had an
  ACS capability and PCI_ACS_RR and PCI_ACS_CR were set in the ACS
  Control register, i.e., all peer-to-peer traffic is directed
  upstream instead of being routed internally.

  Add ACS quirk for FF5xxx NICs in pci_quirk_wangxun_nic_acs() so the
  functions can be in independent IOMMU groups.

> ---
>  drivers/pci/quirks.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index dccb60c1d9cc..d1973a8fd70c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4996,18 +4996,20 @@ static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags)
>  }
>  
>  /*
> - * Wangxun 10G/1G NICs have no ACS capability, and on multi-function
> + * Wangxun 40G/25G/10G/1G NICs have no ACS capability, and on multi-function
>   * devices, peer-to-peer transactions are not be used between the functions.
>   * So add an ACS quirk for below devices to isolate functions.

  Wangxun 40G/25G/10G/1G NICs have no ACS capability, but on
  multi-function devices, the hardware isolates the functions by
  directing all peer-to-peer traffic upstream as though PCI_ACS_RR and
  PCI_ACS_CR were set.

>   * SFxxx 1G NICs(em).
>   * RP1000/RP2000 10G NICs(sp).
> + * FF5xxx 40G/25G/10G NICs(aml).
>   */
>  static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags)
>  {
>  	switch (dev->device) {
> -	case 0x0100 ... 0x010F:
> -	case 0x1001:
> -	case 0x2001:
> +	case 0x0100 ... 0x010F: /* EM */
> +	case 0x1001: case 0x2001: /* SP */
> +	case 0x5010: case 0x5025: case 0x5040: /* AML */
> +	case 0x5110: case 0x5125: case 0x5140: /* AML */
>  		return pci_acs_ctrl_enabled(acs_flags,
>  			PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
>  	}
> -- 
> 2.43.2
> 

Re: [PATCH PCI] PCI: Add ACS quirk for Wangxun FF5XXX NICS

[mengyuanlou]

Thanks for your review. It is indeed accurate.

> 2024年11月13日 01:48，Bjorn Helgaas <helgaas@kernel.org> 写道：
> 
> On Tue, Nov 12, 2024 at 07:18:16PM +0800, Mengyuan Lou wrote:
>> Wangxun FF5XXX NICs are same as selection of SFxxx, RP1000 and
>> RP2000 NICS. They may be multi-function devices, but the hardware
>> does not advertise ACS capability.
>> 
>> Add this ACS quirk for FF5XXX NICs in pci_quirk_wangxun_nic_acs
>> so the functions can be in independent IOMMU groups.
>> 
>> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
> 
> I propose the following commit log and comment updates to be clear
> that the hardware actually enforces this isolation.  Please
> confirm that they are accurate.
> 
>  PCI: Add ACS quirk for Wangxun FF5xxx NICs
> 
>  Wangxun FF5xxx NICs are similar to SFxxx, RP1000 and RP2000 NICs.
>  They may be multi-function devices, but they do not advertise an ACS
>  capability.
> 
>  But the hardware does isolate FF5xxx functions as though it had an
>  ACS capability and PCI_ACS_RR and PCI_ACS_CR were set in the ACS
>  Control register, i.e., all peer-to-peer traffic is directed
>  upstream instead of being routed internally.
> 
>  Add ACS quirk for FF5xxx NICs in pci_quirk_wangxun_nic_acs() so the
>  functions can be in independent IOMMU groups.
> 
>> ---
>> drivers/pci/quirks.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index dccb60c1d9cc..d1973a8fd70c 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -4996,18 +4996,20 @@ static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags)
>> }
>> 
>> /*
>> - * Wangxun 10G/1G NICs have no ACS capability, and on multi-function
>> + * Wangxun 40G/25G/10G/1G NICs have no ACS capability, and on multi-function
>>  * devices, peer-to-peer transactions are not be used between the functions.
>>  * So add an ACS quirk for below devices to isolate functions.
> 
>  Wangxun 40G/25G/10G/1G NICs have no ACS capability, but on
>  multi-function devices, the hardware isolates the functions by
>  directing all peer-to-peer traffic upstream as though PCI_ACS_RR and
>  PCI_ACS_CR were set.
> 
>>  * SFxxx 1G NICs(em).
>>  * RP1000/RP2000 10G NICs(sp).
>> + * FF5xxx 40G/25G/10G NICs(aml).
>>  */
>> static int  pci_quirk_wangxun_nic_acs(struct pci_dev *dev, u16 acs_flags)
>> {
>> switch (dev->device) {
>> - case 0x0100 ... 0x010F:
>> - case 0x1001:
>> - case 0x2001:
>> + case 0x0100 ... 0x010F: /* EM */
>> + case 0x1001: case 0x2001: /* SP */
>> + case 0x5010: case 0x5025: case 0x5040: /* AML */
>> + case 0x5110: case 0x5125: case 0x5140: /* AML */
>> return pci_acs_ctrl_enabled(acs_flags,
>> PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF);
>> }
>> -- 
>> 2.43.2