From: Baolu Lu <baolu.lu@linux.intel.com> To: Jason Gunthorpe <jgg@nvidia.com> Cc: Joerg Roedel <joro@8bytes.org>, Christoph Hellwig <hch@infradead.org>, Kevin Tian <kevin.tian@intel.com>, Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Jean-Philippe Brucker <jean-philippe@linaro.com>, Dave Jiang <dave.jiang@intel.com>, Vinod Koul <vkoul@kernel.org>, Eric Auger <eric.auger@redhat.com>, Liu Yi L <yi.l.liu@intel.com>, Jacob jun Pan <jacob.jun.pan@intel.com>, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Jean-Philippe Brucker <jean-philippe@linaro.org> Subject: Re: [PATCH v6 02/12] iommu: Add pasid_bits field in struct dev_iommu Date: Wed, 11 May 2022 10:25:48 +0800 [thread overview] Message-ID: <3fe05f18-6726-276a-8c42-79e0b134dfdc@linux.intel.com> (raw) In-Reply-To: <20220510143405.GE49344@nvidia.com> On 2022/5/10 22:34, Jason Gunthorpe wrote: > On Tue, May 10, 2022 at 02:17:28PM +0800, Lu Baolu wrote: > >> int iommu_device_register(struct iommu_device *iommu, >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 627a3ed5ee8f..afc63fce6107 100644 >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -2681,6 +2681,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev) >> smmu->features & ARM_SMMU_FEAT_STALL_FORCE) >> master->stall_enabled = true; >> >> + dev->iommu->pasid_bits = master->ssid_bits; >> return &smmu->iommu; >> >> err_free_master: >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 2990f80c5e08..99643f897f26 100644 >> +++ b/drivers/iommu/intel/iommu.c >> @@ -4624,8 +4624,11 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev) >> if (pasid_supported(iommu)) { >> int features = pci_pasid_features(pdev); >> >> - if (features >= 0) >> + if (features >= 0) { >> info->pasid_supported = features | 1; >> + dev->iommu->pasid_bits = >> + fls(pci_max_pasids(pdev)) - 1; >> + } > > It is not very nice that both the iommu drivers have to duplicate the > code to read the pasid capability out of the PCI device. > > IMHO it would make more sense for the iommu layer to report the > capability of its own HW block only, and for the core code to figure > out the master's limitation using a bus-specific approach. Fair enough. The iommu hardware capability could be reported in /** * struct iommu_device - IOMMU core representation of one IOMMU hardware * instance * @list: Used by the iommu-core to keep a list of registered iommus * @ops: iommu-ops for talking to this iommu * @dev: struct device for sysfs handling */ struct iommu_device { struct list_head list; const struct iommu_ops *ops; struct fwnode_handle *fwnode; struct device *dev; }; I haven't checked ARM code yet, but it works for x86 as far as I can see. > > It is also unfortunate that the enable/disable pasid is inside the > iommu driver as well - ideally the PCI driver itself would do this > when it knows it wants to use PASIDs. > > The ordering interaction with ATS makes this look quite annoying > though. :( > > I'm also not convinced individual IOMMU drivers should be forcing ATS > on, there are performance and functional implications here. Using ATS > or not is possibly best left as an administrator policy controlled by > the core code. Again we seem to have some mess. Agreed with you. This has already been in my task list. I will start to solve it after the iommufd tasks. Best regards, baolu
WARNING: multiple messages have this Message-ID (diff)
From: Baolu Lu <baolu.lu@linux.intel.com> To: Jason Gunthorpe <jgg@nvidia.com> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>, Kevin Tian <kevin.tian@intel.com>, Dave Jiang <dave.jiang@intel.com>, Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Christoph Hellwig <hch@infradead.org>, Jean-Philippe Brucker <jean-philippe@linaro.com>, Vinod Koul <vkoul@kernel.org>, Jacob jun Pan <jacob.jun.pan@intel.com>, Robin Murphy <robin.murphy@arm.com> Subject: Re: [PATCH v6 02/12] iommu: Add pasid_bits field in struct dev_iommu Date: Wed, 11 May 2022 10:25:48 +0800 [thread overview] Message-ID: <3fe05f18-6726-276a-8c42-79e0b134dfdc@linux.intel.com> (raw) In-Reply-To: <20220510143405.GE49344@nvidia.com> On 2022/5/10 22:34, Jason Gunthorpe wrote: > On Tue, May 10, 2022 at 02:17:28PM +0800, Lu Baolu wrote: > >> int iommu_device_register(struct iommu_device *iommu, >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 627a3ed5ee8f..afc63fce6107 100644 >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -2681,6 +2681,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev) >> smmu->features & ARM_SMMU_FEAT_STALL_FORCE) >> master->stall_enabled = true; >> >> + dev->iommu->pasid_bits = master->ssid_bits; >> return &smmu->iommu; >> >> err_free_master: >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 2990f80c5e08..99643f897f26 100644 >> +++ b/drivers/iommu/intel/iommu.c >> @@ -4624,8 +4624,11 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev) >> if (pasid_supported(iommu)) { >> int features = pci_pasid_features(pdev); >> >> - if (features >= 0) >> + if (features >= 0) { >> info->pasid_supported = features | 1; >> + dev->iommu->pasid_bits = >> + fls(pci_max_pasids(pdev)) - 1; >> + } > > It is not very nice that both the iommu drivers have to duplicate the > code to read the pasid capability out of the PCI device. > > IMHO it would make more sense for the iommu layer to report the > capability of its own HW block only, and for the core code to figure > out the master's limitation using a bus-specific approach. Fair enough. The iommu hardware capability could be reported in /** * struct iommu_device - IOMMU core representation of one IOMMU hardware * instance * @list: Used by the iommu-core to keep a list of registered iommus * @ops: iommu-ops for talking to this iommu * @dev: struct device for sysfs handling */ struct iommu_device { struct list_head list; const struct iommu_ops *ops; struct fwnode_handle *fwnode; struct device *dev; }; I haven't checked ARM code yet, but it works for x86 as far as I can see. > > It is also unfortunate that the enable/disable pasid is inside the > iommu driver as well - ideally the PCI driver itself would do this > when it knows it wants to use PASIDs. > > The ordering interaction with ATS makes this look quite annoying > though. :( > > I'm also not convinced individual IOMMU drivers should be forcing ATS > on, there are performance and functional implications here. Using ATS > or not is possibly best left as an administrator policy controlled by > the core code. Again we seem to have some mess. Agreed with you. This has already been in my task list. I will start to solve it after the iommufd tasks. Best regards, baolu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-05-11 2:26 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-10 6:17 [PATCH v6 00/12] iommu: SVA and IOPF refactoring Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 01/12] dmaengine: idxd: Separate user and kernel pasid enabling Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 02/12] iommu: Add pasid_bits field in struct dev_iommu Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:34 ` Jason Gunthorpe via iommu 2022-05-10 14:34 ` Jason Gunthorpe 2022-05-11 2:25 ` Baolu Lu [this message] 2022-05-11 2:25 ` Baolu Lu 2022-05-11 8:00 ` Jean-Philippe Brucker 2022-05-11 8:00 ` Jean-Philippe Brucker 2022-05-11 11:59 ` Jason Gunthorpe 2022-05-11 11:59 ` Jason Gunthorpe via iommu 2022-05-10 6:17 ` [PATCH v6 03/12] iommu: Add attach/detach_dev_pasid domain ops Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:02 ` Jason Gunthorpe via iommu 2022-05-10 14:02 ` Jason Gunthorpe 2022-05-11 2:32 ` Baolu Lu 2022-05-11 2:32 ` Baolu Lu 2022-05-11 4:09 ` Tian, Kevin 2022-05-11 4:09 ` Tian, Kevin 2022-05-11 7:54 ` Jean-Philippe Brucker 2022-05-11 7:54 ` Jean-Philippe Brucker 2022-05-11 12:02 ` Jason Gunthorpe 2022-05-11 12:02 ` Jason Gunthorpe via iommu 2022-05-12 7:00 ` Jean-Philippe Brucker 2022-05-12 7:00 ` Jean-Philippe Brucker 2022-05-12 11:51 ` Jason Gunthorpe 2022-05-12 11:51 ` Jason Gunthorpe via iommu 2022-05-16 2:03 ` Baolu Lu 2022-05-16 2:03 ` Baolu Lu 2022-05-10 6:17 ` [PATCH v6 04/12] iommu/sva: Basic data structures for SVA Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 05/12] iommu/vt-d: Remove SVM_FLAG_SUPERVISOR_MODE support Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 14:35 ` Jason Gunthorpe via iommu 2022-05-10 14:35 ` Jason Gunthorpe 2022-05-10 6:17 ` [PATCH v6 06/12] iommu/vt-d: Add SVA domain support Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 07/12] arm-smmu-v3/sva: " Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 08/12] iommu/sva: Use attach/detach_pasid_dev in SVA interfaces Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 15:23 ` Jason Gunthorpe via iommu 2022-05-10 15:23 ` Jason Gunthorpe 2022-05-11 7:21 ` Baolu Lu 2022-05-11 7:21 ` Baolu Lu 2022-05-11 14:53 ` Jason Gunthorpe 2022-05-11 14:53 ` Jason Gunthorpe via iommu 2022-05-12 3:02 ` Baolu Lu 2022-05-12 3:02 ` Baolu Lu 2022-05-12 5:01 ` Tian, Kevin 2022-05-12 5:01 ` Tian, Kevin 2022-05-12 5:17 ` Baolu Lu 2022-05-12 5:17 ` Baolu Lu 2022-05-12 5:44 ` Tian, Kevin 2022-05-12 5:44 ` Tian, Kevin 2022-05-12 6:16 ` Baolu Lu 2022-05-12 6:16 ` Baolu Lu 2022-05-12 11:48 ` Jason Gunthorpe 2022-05-12 11:48 ` Jason Gunthorpe via iommu 2022-05-12 11:59 ` Baolu Lu 2022-05-12 11:59 ` Baolu Lu 2022-05-12 12:03 ` Jason Gunthorpe 2022-05-12 12:03 ` Jason Gunthorpe via iommu 2022-05-12 12:47 ` Baolu Lu 2022-05-12 12:47 ` Baolu Lu 2022-05-12 11:51 ` Jason Gunthorpe 2022-05-12 11:51 ` Jason Gunthorpe via iommu 2022-05-12 12:39 ` Baolu Lu 2022-05-12 12:39 ` Baolu Lu 2022-05-10 6:17 ` [PATCH v6 09/12] iommu: Remove SVA related callbacks from iommu ops Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 10/12] iommu: Prepare IOMMU domain for IOPF Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 11/12] iommu: Per-domain I/O page fault handling Lu Baolu 2022-05-10 6:17 ` Lu Baolu 2022-05-10 6:17 ` [PATCH v6 12/12] iommu: Rename iommu-sva-lib.{c,h} Lu Baolu 2022-05-10 6:17 ` Lu Baolu
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=3fe05f18-6726-276a-8c42-79e0b134dfdc@linux.intel.com \ --to=baolu.lu@linux.intel.com \ --cc=ashok.raj@intel.com \ --cc=dave.jiang@intel.com \ --cc=eric.auger@redhat.com \ --cc=hch@infradead.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@intel.com \ --cc=jean-philippe@linaro.com \ --cc=jean-philippe@linaro.org \ --cc=jgg@nvidia.com \ --cc=joro@8bytes.org \ --cc=kevin.tian@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=robin.murphy@arm.com \ --cc=vkoul@kernel.org \ --cc=will@kernel.org \ --cc=yi.l.liu@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.