On Tue, Sep 26, 2017 at 09:00:26AM -0700, Vadim Lomovtsev wrote:
> On Tue, Sep 26, 2017 at 09:43:43AM -0600, Alex Williamson wrote:
> > On Mon, 25 Sep 2017 06:08:40 -0700
> > Vadim Lomovtsev <Vadim.Lomovtsev@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > > This commit makes Cavium PCI ACS quirk applicable only to Cavium
> > > ThunderX (CN8XXX) family PCIE Root Ports which has limited PCI capabilities
> > > in terms of no ACS support advertisement. However, the RTL internally
> > > implements similar protection as if ACS had completion/request redirection,
> > > upstream forwarding and validation features enabled.
> > >
> > > Current quirk implementation doesn't take into account PCIERCs which
> > > also needs to be quirked. So the pci device id check mask is updated
> > > and check of device ID moved into separate function.
> > >
> > > Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/pci/quirks.c | 29 +++++++++++++++++++++--------
> > > 1 file changed, 21 insertions(+), 8 deletions(-)
> >
> >
> > Reviewed-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> >
> > Thanks for making the updates
> >
>
> Thank you for your patience and for your time, Alex.
>
>
>
> Bjorn,
>
> Would you mind to pick up this patch ?
>
> WBR,
> Vadim
>
> >
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index a4d3361..0fd2e15 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -4211,20 +4211,33 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
> > > #endif
> > > }
> > >
> > > -static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
> > > +/*
> > > + * The Cavium downstream ports doesn't advertise their ACS capability registers.
> > > + * However, the RTL internally implements similar protection as if
> > > + * ACS had completion redirection, forwarding and validation features enabled.
> > > + * So by this flags we're asserting that the hardware implements and
> > > + * enables equivalent ACS functionality for these flags.
> > > + */
> > > +#define CAVIUM_CN8XXX_ACS_FLAGS (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_SV | PCI_ACS_UF)
> > > +
> > > +static __inline__ bool pci_quirk_cavium_acs_match(struct pci_dev *dev)
> > > {
> > > /*
> > > - * Cavium devices matching this quirk do not perform peer-to-peer
> > > - * with other functions, allowing masking out these bits as if they
> > > - * were unimplemented in the ACS capability.
> > > + * Effectively selects all downstream ports for whole ThunderX 1 family
> > > + * by 0xa00 mask (which represents 8 SoCs), while the lower bits of device ID
Bjorn,
Please ignore this since I found one more typo here: it should be 0xa000, sorry. Will post v6.
Sorry for wasting your time, guys.
WBR,
Vadim
> > > + * are used to indicate which subdevice is used within the SoC.
> > > */
> > > - acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR |
> > > - PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
> > > + return (pci_is_pcie(dev) &&
> > > + (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) &&
> > > + ((dev->device & 0xf800) == 0xa000));
> > > +}
> > >
> > > - if (!((dev->device >= 0xa000) && (dev->device <= 0xa0ff)))
> > > +static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
> > > +{
> > > + if (!pci_quirk_cavium_acs_match(dev))
> > > return -ENOTTY;
> > >
> > > - return acs_flags ? 0 : 1;
> > > + return acs_flags & ~(CAVIUM_CN8XXX_ACS_FLAGS) ? 0 : 1;
> > > }
> > >
> > > static int pci_quirk_xgene_acs(struct pci_dev *dev, u16 acs_flags)
> >
