Thanks for your review. It is indeed accurate. > 2024年11月13日 01:48,Bjorn Helgaas <helgaas@xxxxxxxxxx> 写道: > > 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@xxxxxxxxxxxxx> > > 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