From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 511FE79EF for ; Mon, 27 Feb 2023 19:29:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1677526190; x=1709062190; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=v3mSa++3w5di4VuVyxMCx7ivKLHS0LlRNUH1BB+AgB8=; b=ftgu/xX5wOyUGfALo4kLPFZC8jnVJq/Bc9p9bqj7XsFJ5EHh09ssZhrg iitgtwL+lh9fCRmE/L1FaORsKVwsgznKuhhOy13JqPX+R6yRqWvf9VtPM rJHRt0MxFVhKWkX/Fm7gVRlIMazuYDcnI9AF8U8mV6xy40eA94SNhMh4C f6Zpq6n9YMghHN8NyRuylX7wJUDv55a0eec3bnnjfwUBfOyeAFVk3rXHp 5nWppZFn7jcC+ExLUA8hICQ7Xp7xG5NqzUM0uO1LsHl7bW+qS2gieAujX LSBvQ7ZFNGZHue4e1Js6pBL75kQoDxGdrF5Qzmt4AlotBltXMltMEgT7q Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="313619218" X-IronPort-AV: E=Sophos;i="5.98,219,1673942400"; d="scan'208";a="313619218" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Feb 2023 11:29:49 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="737856265" X-IronPort-AV: E=Sophos;i="5.98,219,1673942400"; d="scan'208";a="737856265" Received: from cpalit-mobl2.amr.corp.intel.com (HELO [10.212.235.220]) ([10.212.235.220]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Feb 2023 11:29:48 -0800 Message-ID: Date: Mon, 27 Feb 2023 11:29:47 -0800 Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.4.2 Subject: Re: [PATCH 1/2] PCI/ATS: Add a helper function to configure ATS STU of a PF. Content-Language: en-US To: Ganapatrao Kulkarni , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev, joro@8bytes.org, bhelgaas@google.com, robin.murphy@arm.com, will@kernel.org Cc: jean-philippe@linaro.org, darren@os.amperecomputing.com, scott@os.amperecomputing.com References: <20230227132151.1907480-1-gankulkarni@os.amperecomputing.com> <20230227132151.1907480-2-gankulkarni@os.amperecomputing.com> From: Sathyanarayanan Kuppuswamy In-Reply-To: <20230227132151.1907480-2-gankulkarni@os.amperecomputing.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi, On 2/27/23 5:21 AM, Ganapatrao Kulkarni wrote: > As per PCI specification (PCI Express Base Specification Revision > 6.0, Section 10.5) both PF and VFs of a PCI EP are permitted to be enabled > independently for ATS capability, however the STU(Smallest Translation > Unit) is shared between PF and VFs. For VFs, it is hardwired to Zero and > the associated PF's value applies to VFs. > > In the current code, the STU is being configured while enabling the PF ATS. > Hence, it is not able to enable ATS for VFs, if it is not enabled on the > associated PF already.> > Adding a function pci_ats_stu_configure(), which can be called to > configure the STU during PF enumeration. > Latter enumerations of VFs can successfully enable ATS independently. Why not enable ATS in PF before enabling it in VF? Just updating STU of PF and not enabling it seem odd. > > Signed-off-by: Ganapatrao Kulkarni > --- > drivers/pci/ats.c | 32 ++++++++++++++++++++++++++++++-- > include/linux/pci-ats.h | 1 + > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index f9cc2e10b676..70e1982efdb4 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -46,6 +46,34 @@ bool pci_ats_supported(struct pci_dev *dev) > } > EXPORT_SYMBOL_GPL(pci_ats_supported); > > +/** > + * pci_ats_stu_configure - Configure STU of a PF. > + * @dev: the PCI device > + * @ps: the IOMMU page shift > + * > + * Returns 0 on success, or negative on failure. > + */ > +int pci_ats_stu_configure(struct pci_dev *dev, int ps) > +{ > + u16 ctrl; > + > + if (dev->ats_enabled || dev->is_virtfn) > + return 0; > + > + if (!pci_ats_supported(dev)) > + return -EINVAL; > + > + if (ps < PCI_ATS_MIN_STU) > + return -EINVAL; > + > + dev->ats_stu = ps; > + ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU); > + pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl); If you just want to update the STU, don't overwrite other fields. > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pci_ats_stu_configure); > + > /** > * pci_enable_ats - enable the ATS capability > * @dev: the PCI device > @@ -68,8 +96,8 @@ int pci_enable_ats(struct pci_dev *dev, int ps) > return -EINVAL; > > /* > - * Note that enabling ATS on a VF fails unless it's already enabled > - * with the same STU on the PF. > + * Note that enabling ATS on a VF fails unless it's already > + * configured with the same STU on the PF. > */ > ctrl = PCI_ATS_CTRL_ENABLE; > if (dev->is_virtfn) { > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > index df54cd5b15db..9b40eb555124 100644 > --- a/include/linux/pci-ats.h > +++ b/include/linux/pci-ats.h > @@ -8,6 +8,7 @@ > /* Address Translation Service */ > bool pci_ats_supported(struct pci_dev *dev); > int pci_enable_ats(struct pci_dev *dev, int ps); > +int pci_ats_stu_configure(struct pci_dev *dev, int ps); What about dummy declaration for !CONFIG_PCI_ATS case? > void pci_disable_ats(struct pci_dev *dev); > int pci_ats_queue_depth(struct pci_dev *dev); > int pci_ats_page_aligned(struct pci_dev *dev); -- Sathyanarayanan Kuppuswamy Linux Kernel Developer 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EB609C64ED6 for ; Mon, 27 Feb 2023 19:30:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Yw2H+0nswwYrQy3cluflZTVE29FTvatb/ZXXYz8Re5E=; b=xjvQRMlUcYqrNi NvO+FdE31iKUKu8eeqP+c/PHqkNAIieZClVYRxEPcVQmvG5zrtShKjExiAd0sHK8qubSFNArRnZOS 6QxP5iiUms80wIqwuIhH5SfhoS6bozNiivYAJygXgsR2ZyawiAnCZfWI6uhYt34nvOSSY5HcYEJkK iOXgX/ePT3rCypoFxGcl+smup0T85x5G7MW4XOZYfxJnRgrkIbgAmvPFRWXUSnQrlL/sjiCsaATqj l4TZxNmNTJ9gq5RRvIi9ZUqOyL0Aar+AGUTQcc/xLoWdEHzqPjKNqdMijg3FGQCjGGt8q4zPX7NDF /VqhC4PjAi/fZH90uEGQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pWjC7-00AxM9-Pa; Mon, 27 Feb 2023 19:29:55 +0000 Received: from mga12.intel.com ([192.55.52.136]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pWjC5-00AxKn-2I for linux-arm-kernel@lists.infradead.org; Mon, 27 Feb 2023 19:29:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1677526193; x=1709062193; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=v3mSa++3w5di4VuVyxMCx7ivKLHS0LlRNUH1BB+AgB8=; b=FhNUse/kGQ0ctbTwADBBK3ywNOZFTvN2osOrFJpWYnmDfeduTTGVDZNr gpvNYqNjdLaos3gZC1HuK+mHcUKkpc+kMUste1A6xZPDUjuz/l4uCR+UN lIZb8m7muVgA1+ZSuq8RWuir0/unPUzSHevIuXaeg0IzZdQv4Qk1fm7/w 1D0XWQUYrMy1zXhnCGyACIJ7dMLfwC024pRFKUNOPUkxAidILoRjjtj5T 0DigQ4ItvvhFLua0GxRiTwLQOT7N/jWMe0x7NjGTTy72Um1QByaQmI+VZ k1bpmd81mUBDlTKNT63ZfgZLgC/HdCF8lJL9dm3Rx3pnN26le1TKnyPa2 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="313619216" X-IronPort-AV: E=Sophos;i="5.98,219,1673942400"; d="scan'208";a="313619216" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Feb 2023 11:29:49 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="737856265" X-IronPort-AV: E=Sophos;i="5.98,219,1673942400"; d="scan'208";a="737856265" Received: from cpalit-mobl2.amr.corp.intel.com (HELO [10.212.235.220]) ([10.212.235.220]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Feb 2023 11:29:48 -0800 Message-ID: Date: Mon, 27 Feb 2023 11:29:47 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.4.2 Subject: Re: [PATCH 1/2] PCI/ATS: Add a helper function to configure ATS STU of a PF. Content-Language: en-US To: Ganapatrao Kulkarni , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev, joro@8bytes.org, bhelgaas@google.com, robin.murphy@arm.com, will@kernel.org Cc: jean-philippe@linaro.org, darren@os.amperecomputing.com, scott@os.amperecomputing.com References: <20230227132151.1907480-1-gankulkarni@os.amperecomputing.com> <20230227132151.1907480-2-gankulkarni@os.amperecomputing.com> From: Sathyanarayanan Kuppuswamy In-Reply-To: <20230227132151.1907480-2-gankulkarni@os.amperecomputing.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230227_112953_158952_2EA3D824 X-CRM114-Status: GOOD ( 24.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On 2/27/23 5:21 AM, Ganapatrao Kulkarni wrote: > As per PCI specification (PCI Express Base Specification Revision > 6.0, Section 10.5) both PF and VFs of a PCI EP are permitted to be enabled > independently for ATS capability, however the STU(Smallest Translation > Unit) is shared between PF and VFs. For VFs, it is hardwired to Zero and > the associated PF's value applies to VFs. > > In the current code, the STU is being configured while enabling the PF ATS. > Hence, it is not able to enable ATS for VFs, if it is not enabled on the > associated PF already.> > Adding a function pci_ats_stu_configure(), which can be called to > configure the STU during PF enumeration. > Latter enumerations of VFs can successfully enable ATS independently. Why not enable ATS in PF before enabling it in VF? Just updating STU of PF and not enabling it seem odd. > > Signed-off-by: Ganapatrao Kulkarni > --- > drivers/pci/ats.c | 32 ++++++++++++++++++++++++++++++-- > include/linux/pci-ats.h | 1 + > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index f9cc2e10b676..70e1982efdb4 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -46,6 +46,34 @@ bool pci_ats_supported(struct pci_dev *dev) > } > EXPORT_SYMBOL_GPL(pci_ats_supported); > > +/** > + * pci_ats_stu_configure - Configure STU of a PF. > + * @dev: the PCI device > + * @ps: the IOMMU page shift > + * > + * Returns 0 on success, or negative on failure. > + */ > +int pci_ats_stu_configure(struct pci_dev *dev, int ps) > +{ > + u16 ctrl; > + > + if (dev->ats_enabled || dev->is_virtfn) > + return 0; > + > + if (!pci_ats_supported(dev)) > + return -EINVAL; > + > + if (ps < PCI_ATS_MIN_STU) > + return -EINVAL; > + > + dev->ats_stu = ps; > + ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU); > + pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl); If you just want to update the STU, don't overwrite other fields. > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pci_ats_stu_configure); > + > /** > * pci_enable_ats - enable the ATS capability > * @dev: the PCI device > @@ -68,8 +96,8 @@ int pci_enable_ats(struct pci_dev *dev, int ps) > return -EINVAL; > > /* > - * Note that enabling ATS on a VF fails unless it's already enabled > - * with the same STU on the PF. > + * Note that enabling ATS on a VF fails unless it's already > + * configured with the same STU on the PF. > */ > ctrl = PCI_ATS_CTRL_ENABLE; > if (dev->is_virtfn) { > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > index df54cd5b15db..9b40eb555124 100644 > --- a/include/linux/pci-ats.h > +++ b/include/linux/pci-ats.h > @@ -8,6 +8,7 @@ > /* Address Translation Service */ > bool pci_ats_supported(struct pci_dev *dev); > int pci_enable_ats(struct pci_dev *dev, int ps); > +int pci_ats_stu_configure(struct pci_dev *dev, int ps); What about dummy declaration for !CONFIG_PCI_ATS case? > void pci_disable_ats(struct pci_dev *dev); > int pci_ats_queue_depth(struct pci_dev *dev); > int pci_ats_page_aligned(struct pci_dev *dev); -- Sathyanarayanan Kuppuswamy Linux Kernel Developer _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel