From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-by2nam03on0060.outbound.protection.outlook.com ([104.47.42.60]:22987 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751554AbdI0SDa (ORCPT ); Wed, 27 Sep 2017 14:03:30 -0400 Date: Wed, 27 Sep 2017 11:03:23 -0700 From: Vadim Lomovtsev To: bhelgaas@google.com Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Wilson.Snyder@cavium.com, jcm@redhat.com, Vadim.Lomovtsev@cavium.com, alex.williamson@redhat.com Subject: Re: [PATCH v5] PCI: quirks: update Cavium ThunderX ACS quirk implementation Message-ID: <20170927180323.GB28672@localhost.localdomain> References: <1505724481-28413-1-git-send-email-Vadim.Lomovtsev@caviumnetworks.com> <1506344920-24016-1-git-send-email-Vadim.Lomovtsev@caviumnetworks.com> <20170926094343.3e0d8d90@w520.home> <20170926160026.GA26639@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20170926160026.GA26639@localhost.localdomain> Sender: linux-pci-owner@vger.kernel.org List-ID: 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 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 > > > --- > > > drivers/pci/quirks.c | 29 +++++++++++++++++++++-------- > > > 1 file changed, 21 insertions(+), 8 deletions(-) > > > > > > Reviewed-by: Alex Williamson > > > > 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) > >