linux-kernel - Re: [PATCH v6] PCI: quirks: update Cavium ThunderX ACS quirk implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171009161454.GA20316@localhost.localdomain>
Date:   Mon, 9 Oct 2017 09:14:54 -0700
From:   Vadim Lomovtsev <Vadim.Lomovtsev@...iumnetworks.com>
To:     bhelgaas@...gle.com
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wilson.Snyder@...ium.com, vadim.lomovtsev@...ium.com,
        jcm@...hat.com, alex.williamson@...hat.com
Subject: Re: [PATCH v6] PCI: quirks: update Cavium ThunderX ACS quirk
 implementation

Hi Bjorn,

Would you please consider to take this patch ?
It is required to provide correct ACS mask for CN8xxx PCI root ports.

WBR,
Vadim

On Wed, Sep 27, 2017 at 02:18:54PM -0600, Alex Williamson wrote:
> On Wed, 27 Sep 2017 11:20:39 -0700
> Vadim Lomovtsev <Vadim.Lomovtsev@...iumnetworks.com> 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@...iumnetworks.com>
> > ---
> > 	v5 -> v6: comment typo fix: change 0xa00 to 0xa000
> 
> Reviewed-by: Alex Williamson <alex.williamson@...hat.com>
> 
>  
> >  drivers/pci/quirks.c | 29 +++++++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index a4d3361..ed6c76d 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 0xa000 mask (which represents 8 SoCs), while the lower bits of device ID
> > +	 * 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)
>