From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C02A2C32750 for ; Tue, 13 Aug 2019 03:51:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8AAC120644 for ; Tue, 13 Aug 2019 03:51:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565668313; bh=HuT+OglXnLSHgrFdcHs/jiQsLDXIOqJOqXN8r0ibKLc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=jYZzghOnQ7NzYMi9Q71JL6FMU2Gz7GL5zkGAout0O0GvMnXvDqh8kPS5nwYRRZLQm tc9zT7voJ3xr+YseYRdT5GXAJpnJweSYz6bY9qZBcdXF8q23bxfN/UJHXJGDCg/Dp3 O0QqrJNtgzLmPPYBKZLj8cubgYVl94HXhnwOVkfg= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726316AbfHMDvx (ORCPT ); Mon, 12 Aug 2019 23:51:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:46536 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726296AbfHMDvw (ORCPT ); Mon, 12 Aug 2019 23:51:52 -0400 Received: from localhost (c-73-15-1-175.hsd1.ca.comcast.net [73.15.1.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8930D20644; Tue, 13 Aug 2019 03:51:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565668311; bh=HuT+OglXnLSHgrFdcHs/jiQsLDXIOqJOqXN8r0ibKLc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lJ5k+JKoa8E1MKlqOGRqEB1XCz1OSkGZrGGFDkEtfXFiLHtCnXRQ099L3jsmjx190 JcX0sCRWCyXEFGA9/Jne2EGozyWhYS+GLxbvw895WpMmuSHghi0VK5hdYZim0Bly1p 1Oqeuq5c0L5ELXNPZ1UYVv4VOhUS+ez7Vgo/pDL0= Date: Mon, 12 Aug 2019 22:51:48 -0500 From: Bjorn Helgaas To: sathyanarayanan kuppuswamy 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 Message-ID: <20190813035148.GI7302@google.com> References: <0d7e0e0d079c438897f4da8cdca4b55994b1233b.1564702313.git.sathyanarayanan.kuppuswamy@linux.intel.com> <20190812200418.GJ11785@google.com> <09a2faf0-a26f-6374-130a-3b33b1b712d5@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <09a2faf0-a26f-6374-130a-3b33b1b712d5@linux.intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org 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 > > > > > > 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 > > > --- > > > 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 >