On Fri, 28 Jul 2017 11:50:43 -0700 Feng Kan <fkan@xxxxxxx> wrote: > The APM X-Gene PCIe root port does not support ACS at this point. > However, the hw provides isolation and source validation through > the SMMU. The stream ID generated by the PCIe ports contain both > the BDF as well as the port ID in its 3 most significant bits. > Turn on ACS but disable all the peer to peer features. > > Signed-off-by: Feng Kan <fkan@xxxxxxx> > --- > V3 Change: Add comment regarding unique port id in stream ID > V2 Change: Move XGene ACS quirk to unique XGene function > > drivers/pci/quirks.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 085fb78..0f8f1cd 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -4120,6 +4120,19 @@ static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags) > return acs_flags ? 0 : 1; > } > > +static int pci_quirk_xgene_acs(struct pci_dev *dev, u16 acs_flags) > +{ > + /* > + * XGene root matching this quirk do not allow peer-to-peer > + * transactions with others, allowing masking out these bits as if they > + * were unimplemented in the ACS capability. > + */ > + acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | > + PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT); > + > + return acs_flags ? 0 : 1; > +} > + > /* > * Many Intel PCH root ports do provide ACS-like features to disable peer > * transactions and validate bus numbers in requests, but do not provide an > @@ -4368,6 +4381,8 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags) > { 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */ > /* Cavium ThunderX */ > { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs }, > + /* APM XGene */ > + { PCI_VENDOR_ID_AMCC, 0xE004, pci_quirk_xgene_acs }, > { 0 } > }; > Hi Feng, Sorry, I have one more question as I happened to spend some time looking at PCI_ACS_DT this week. DT specifies that peer-to-peer should occur normally between egress ports for transactions which are pre-translated by an ATS unit on the endpoint. Therefore if a root port doesn't allow peer-to-peer, it seems like it should not claim to support PCI_ACS_DT. I know your quirk is just a copy of the Cavium one, but we should also go back and verify this question with them, or perhaps I'm misinterpreting this capability. AIUI this is also a performance capability, not an isolation capability, so it shouldn't affect the ability to consider a device isolated, it only gets confusing if we expect a performance benefit from this but don't actually see one. Does your root port have this ability to selectively allow peer-to-peer of pre-translated transactions? Thanks, Alex