From: Bjorn Helgaas <helgaas@kernel.org> To: sathyanarayanan kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com> Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, ashok.raj@intel.com, keith.busch@intel.com Subject: Re: [PATCH v5 1/7] PCI/ATS: Fix pci_prg_resp_pasid_required() dependency issues Date: Mon, 12 Aug 2019 22:51:48 -0500 Message-ID: <20190813035148.GI7302@google.com> (raw) In-Reply-To: <09a2faf0-a26f-6374-130a-3b33b1b712d5@linux.intel.com> On Mon, Aug 12, 2019 at 01:20:55PM -0700, sathyanarayanan kuppuswamy wrote: > On 8/12/19 1:04 PM, Bjorn Helgaas wrote: > > On Thu, Aug 01, 2019 at 05:05:58PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote: > > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > > > > > Since pci_prg_resp_pasid_required() function has dependency on both > > > PASID and PRI, define it only if both CONFIG_PCI_PRI and > > > CONFIG_PCI_PASID config options are enabled. > > I don't really like this. It makes the #ifdefs more complicated and I > > don't think it really buys us anything. Will anything break if we > > just drop this patch? > Yes, this function uses "pri_lock" mutex which is only defined if > CONFIG_PCI_PRI is enabled. So not protecting this function within > CONFIG_PCI_PRI will lead to compilation issues. Ah, OK. That helps a lot. "pri_lock" doesn't exist at this point in the series, so the patch makes no sense without knowing that. I'm still not convinced this is the right thing because I'm not sure the lock is necessary. I'll respond to the patch that adds the lock. > > > Fixes: e5567f5f6762 ("PCI/ATS: Add pci_prg_resp_pasid_required() > > > interface.") > > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > > --- > > > drivers/pci/ats.c | 10 ++++++---- > > > include/linux/pci-ats.h | 12 +++++++++--- > > > 2 files changed, 15 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > > > index e18499243f84..cdd936d10f68 100644 > > > --- a/drivers/pci/ats.c > > > +++ b/drivers/pci/ats.c > > > @@ -395,6 +395,8 @@ int pci_pasid_features(struct pci_dev *pdev) > > > } > > > EXPORT_SYMBOL_GPL(pci_pasid_features); > > > +#ifdef CONFIG_PCI_PRI > > > + > > > /** > > > * pci_prg_resp_pasid_required - Return PRG Response PASID Required bit > > > * status. > > > @@ -402,10 +404,8 @@ EXPORT_SYMBOL_GPL(pci_pasid_features); > > > * > > > * Returns 1 if PASID is required in PRG Response Message, 0 otherwise. > > > * > > > - * Even though the PRG response PASID status is read from PRI Status > > > - * Register, since this API will mainly be used by PASID users, this > > > - * function is defined within #ifdef CONFIG_PCI_PASID instead of > > > - * CONFIG_PCI_PRI. > > > + * Since this API has dependency on both PRI and PASID, protect it > > > + * with both CONFIG_PCI_PRI and CONFIG_PCI_PASID. > > > */ > > > int pci_prg_resp_pasid_required(struct pci_dev *pdev) > > > { > > > @@ -425,6 +425,8 @@ int pci_prg_resp_pasid_required(struct pci_dev *pdev) > > > } > > > EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required); > > > +#endif > > > + > > > #define PASID_NUMBER_SHIFT 8 > > > #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) > > > /** > > > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > > > index 1ebb88e7c184..1a0bdaee2f32 100644 > > > --- a/include/linux/pci-ats.h > > > +++ b/include/linux/pci-ats.h > > > @@ -40,7 +40,6 @@ void pci_disable_pasid(struct pci_dev *pdev); > > > void pci_restore_pasid_state(struct pci_dev *pdev); > > > int pci_pasid_features(struct pci_dev *pdev); > > > int pci_max_pasids(struct pci_dev *pdev); > > > -int pci_prg_resp_pasid_required(struct pci_dev *pdev); > > > #else /* CONFIG_PCI_PASID */ > > > @@ -67,11 +66,18 @@ static inline int pci_max_pasids(struct pci_dev *pdev) > > > return -EINVAL; > > > } > > > +#endif /* CONFIG_PCI_PASID */ > > > + > > > +#if defined(CONFIG_PCI_PRI) && defined(CONFIG_PCI_PASID) > > > + > > > +int pci_prg_resp_pasid_required(struct pci_dev *pdev); > > > + > > > +#else /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */ > > > + > > > static inline int pci_prg_resp_pasid_required(struct pci_dev *pdev) > > > { > > > return 0; > > > } > > > -#endif /* CONFIG_PCI_PASID */ > > > - > > > +#endif > > > #endif /* LINUX_PCI_ATS_H*/ > > > -- > > > 2.21.0 > > > > -- > Sathyanarayanan Kuppuswamy > Linux kernel developer >
next prev parent reply index Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-02 0:05 [PATCH v5 0/7] Fix PF/VF dependency issue sathyanarayanan.kuppuswamy 2019-08-02 0:05 ` [PATCH v5 1/7] PCI/ATS: Fix pci_prg_resp_pasid_required() dependency issues sathyanarayanan.kuppuswamy 2019-08-12 20:04 ` Bjorn Helgaas 2019-08-12 20:20 ` sathyanarayanan kuppuswamy 2019-08-13 3:51 ` Bjorn Helgaas [this message] 2019-08-16 18:06 ` Kuppuswamy Sathyanarayanan 2019-08-02 0:05 ` [PATCH v5 2/7] PCI/ATS: Initialize PRI in pci_ats_init() sathyanarayanan.kuppuswamy 2019-08-12 20:04 ` Bjorn Helgaas 2019-08-12 21:35 ` sathyanarayanan kuppuswamy 2019-08-13 4:10 ` Bjorn Helgaas 2019-08-15 4:46 ` Bjorn Helgaas 2019-08-15 17:30 ` Kuppuswamy Sathyanarayanan 2019-08-16 17:31 ` Bjorn Helgaas 2019-08-02 0:06 ` [PATCH v5 3/7] PCI/ATS: Initialize PASID " sathyanarayanan.kuppuswamy 2019-08-12 20:04 ` Bjorn Helgaas 2019-08-15 4:48 ` Bjorn Helgaas 2019-08-15 4:56 ` Bjorn Helgaas 2019-08-15 17:31 ` Kuppuswamy Sathyanarayanan 2019-08-02 0:06 ` [PATCH v5 4/7] PCI/ATS: Add PRI support for PCIe VF devices sathyanarayanan.kuppuswamy 2019-08-12 20:04 ` Bjorn Helgaas 2019-08-12 21:40 ` sathyanarayanan kuppuswamy 2019-08-13 4:16 ` Bjorn Helgaas 2019-08-15 22:20 ` Bjorn Helgaas 2019-08-15 22:39 ` Kuppuswamy Sathyanarayanan 2019-08-19 14:15 ` Bjorn Helgaas 2019-08-19 22:53 ` Kuppuswamy Sathyanarayanan 2019-08-19 23:19 ` Bjorn Helgaas 2019-08-28 18:21 ` Kuppuswamy Sathyanarayanan 2019-08-28 18:57 ` Bjorn Helgaas 2019-08-02 0:06 ` [PATCH v5 5/7] PCI/ATS: Add PASID " sathyanarayanan.kuppuswamy 2019-08-12 20:05 ` Bjorn Helgaas 2019-08-13 22:19 ` Kuppuswamy Sathyanarayanan 2019-08-15 5:04 ` Bjorn Helgaas 2019-08-16 1:21 ` Kuppuswamy Sathyanarayanan 2019-08-02 0:06 ` [PATCH v5 6/7] PCI/ATS: Disable PF/VF ATS service independently sathyanarayanan.kuppuswamy 2019-08-02 0:06 ` [PATCH v5 7/7] PCI: Skip Enhanced Allocation (EA) initialization for VF device sathyanarayanan.kuppuswamy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190813035148.GI7302@google.com \ --to=helgaas@kernel.org \ --cc=ashok.raj@intel.com \ --cc=keith.busch@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=sathyanarayanan.kuppuswamy@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-PCI Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \ linux-pci@vger.kernel.org public-inbox-index linux-pci Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci AGPL code for this site: git clone https://public-inbox.org/public-inbox.git