From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754105AbdBNPHk (ORCPT ); Tue, 14 Feb 2017 10:07:40 -0500 Received: from mail.kernel.org ([198.145.29.136]:44164 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752573AbdBNPHa (ORCPT ); Tue, 14 Feb 2017 10:07:30 -0500 Date: Tue, 14 Feb 2017 09:07:22 -0600 From: Bjorn Helgaas To: Alex Williamson Cc: Manish Jaggi , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Tirumalesh Chalamarla , "Richter, Robert" , Bjorn Helgaas Subject: Re: [PATCH] PCI: Add cavium acs pci quirk Message-ID: <20170214150722.GC8080@bhelgaas-glaptop.roam.corp.google.com> References: <56ABC5AE.1050209@caviumnetworks.com> <20170213214457.248084de@t450s.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170213214457.248084de@t450s.home> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 13, 2017 at 09:44:57PM -0700, Alex Williamson wrote: > On Sat, 30 Jan 2016 01:33:58 +0530 > Manish Jaggi wrote: > > > 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. > > > > Acked-by: Tirumalesh Chalamarla > > Signed-off-by: Manish Jaggi > > --- > > drivers/pci/quirks.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index 7e32730..a300fa6 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -3814,6 +3814,19 @@ 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) > > +{ > > + /* > > + * 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. > > + */ > > + 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 > > @@ -3966,6 +3979,8 @@ static const struct pci_dev_acs_enabled { > > { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_pch_acs }, > > { 0x19a2, 0x710, pci_quirk_mf_endpoint_acs }, /* Emulex BE3-R */ > > { 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */ > > + /* Cavium ThunderX */ > > + { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs }, > > { 0 } > > }; > > > > Apologies for not catching this, but what sort of crystal ball do you > have that can predict not only current devices, but future devices will > not support peer-to-peer features? Is there an internal design > guidelines reference specification for Cavium that we can realistically > expect this to remain consistent, or is this just an attempt to never > think about ACS again at the customer's peril? What about the existing > non-ThunderX products with Cavium vendor ID, does this really apply to > those? I would strongly suggest taking the device ID into account. > See examples like the pci_quirk_intel_pch_acs quirk where the initial > filter is PCI_ANY_ID, but specific device types and ranges of device > IDs are identified by the function for evaluation. This seems reckless > to me and I'd advise that it be reverted. Thanks, I'd be happy to revert this, but it would be easier if somebody sent a patch and a changelog.