[PATCH v4 6/7] iommu/vt-d: Add support for static identity domain

Lu Baolu posted 7 patches 1 year, 4 months ago
[PATCH v4 6/7] iommu/vt-d: Add support for static identity domain
Posted by Lu Baolu 1 year, 4 months ago
Software determines VT-d hardware support for passthrough translation by
inspecting the capability register. If passthrough translation is not
supported, the device is instructed to use DMA domain for its default
domain.

Add a global static identity domain with guaranteed attach semantics for
IOMMUs that support passthrough translation mode.

The global static identity domain is a dummy domain without corresponding
dmar_domain structure. Consequently, the device's info->domain will be
NULL with the identity domain is attached. Refactor the code accordingly.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 114 ++++++++++++++++++++++++++++++++++--
 1 file changed, 110 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7950152bb4e6..14f1fcf17152 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3691,11 +3691,9 @@ int prepare_domain_attach_device(struct iommu_domain *domain,
 static int intel_iommu_attach_device(struct iommu_domain *domain,
 				     struct device *dev)
 {
-	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	int ret;
 
-	if (info->domain)
-		device_block_translation(dev);
+	device_block_translation(dev);
 
 	ret = prepare_domain_attach_device(domain, dev);
 	if (ret)
@@ -4301,11 +4299,17 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
 					 struct iommu_domain *domain)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
-	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct dev_pasid_info *curr, *dev_pasid = NULL;
 	struct intel_iommu *iommu = info->iommu;
+	struct dmar_domain *dmar_domain;
 	unsigned long flags;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		intel_pasid_tear_down_entry(iommu, dev, pasid, false);
+		return;
+	}
+
+	dmar_domain = to_dmar_domain(domain);
 	spin_lock_irqsave(&dmar_domain->lock, flags);
 	list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
 		if (curr->dev == dev && curr->pasid == pasid) {
@@ -4532,9 +4536,111 @@ static const struct iommu_dirty_ops intel_dirty_ops = {
 	.read_and_clear_dirty = intel_iommu_read_and_clear_dirty,
 };
 
+static int context_setup_pass_through(struct device *dev, u8 bus, u8 devfn)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu = info->iommu;
+	struct context_entry *context;
+
+	spin_lock(&iommu->lock);
+	context = iommu_context_addr(iommu, bus, devfn, 1);
+	if (!context) {
+		spin_unlock(&iommu->lock);
+		return -ENOMEM;
+	}
+
+	if (context_present(context) && !context_copied(iommu, bus, devfn)) {
+		spin_unlock(&iommu->lock);
+		return 0;
+	}
+
+	copied_context_tear_down(iommu, context, bus, devfn);
+	context_clear_entry(context);
+	context_set_domain_id(context, FLPT_DEFAULT_DID);
+
+	/*
+	 * In pass through mode, AW must be programmed to indicate the largest
+	 * AGAW value supported by hardware. And ASR is ignored by hardware.
+	 */
+	context_set_address_width(context, iommu->msagaw);
+	context_set_translation_type(context, CONTEXT_TT_PASS_THROUGH);
+	context_set_fault_enable(context);
+	context_set_present(context);
+	if (!ecap_coherent(iommu->ecap))
+		clflush_cache_range(context, sizeof(*context));
+	context_present_cache_flush(iommu, FLPT_DEFAULT_DID, bus, devfn);
+	spin_unlock(&iommu->lock);
+
+	return 0;
+}
+
+static int context_setup_pass_through_cb(struct pci_dev *pdev, u16 alias, void *data)
+{
+	struct device *dev = data;
+
+	if (dev != &pdev->dev)
+		return 0;
+
+	return context_setup_pass_through(dev, PCI_BUS_NUM(alias), alias & 0xff);
+}
+
+static int device_setup_pass_through(struct device *dev)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+
+	if (!dev_is_pci(dev))
+		return context_setup_pass_through(dev, info->bus, info->devfn);
+
+	return pci_for_each_dma_alias(to_pci_dev(dev),
+				      context_setup_pass_through_cb, dev);
+}
+
+static int identity_domain_attach_dev(struct iommu_domain *domain, struct device *dev)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu = info->iommu;
+	int ret;
+
+	device_block_translation(dev);
+
+	if (dev_is_real_dma_subdevice(dev))
+		return 0;
+
+	if (sm_supported(iommu)) {
+		ret = intel_pasid_setup_pass_through(iommu, dev, IOMMU_NO_PASID);
+		if (!ret)
+			iommu_enable_pci_caps(info);
+	} else {
+		ret = device_setup_pass_through(dev);
+	}
+
+	return ret;
+}
+
+static int identity_domain_set_dev_pasid(struct iommu_domain *domain,
+					 struct device *dev, ioasid_t pasid)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu = info->iommu;
+
+	if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+		return -EOPNOTSUPP;
+
+	return intel_pasid_setup_pass_through(iommu, dev, pasid);
+}
+
+static struct iommu_domain identity_domain = {
+	.type = IOMMU_DOMAIN_IDENTITY,
+	.ops = &(const struct iommu_domain_ops) {
+		.attach_dev	= identity_domain_attach_dev,
+		.set_dev_pasid	= identity_domain_set_dev_pasid,
+	},
+};
+
 const struct iommu_ops intel_iommu_ops = {
 	.blocked_domain		= &blocking_domain,
 	.release_domain		= &blocking_domain,
+	.identity_domain	= &identity_domain,
 	.capable		= intel_iommu_capable,
 	.hw_info		= intel_iommu_hw_info,
 	.domain_alloc		= intel_iommu_domain_alloc,
-- 
2.34.1
RE: [PATCH v4 6/7] iommu/vt-d: Add support for static identity domain
Posted by Tian, Kevin 1 year, 4 months ago
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, August 9, 2024 1:55 PM
> 
> +static int context_setup_pass_through(struct device *dev, u8 bus, u8 devfn)
> +{
> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> +	struct intel_iommu *iommu = info->iommu;
> +	struct context_entry *context;
> +
> +	spin_lock(&iommu->lock);
> +	context = iommu_context_addr(iommu, bus, devfn, 1);
> +	if (!context) {
> +		spin_unlock(&iommu->lock);
> +		return -ENOMEM;
> +	}
> +
> +	if (context_present(context) && !context_copied(iommu, bus, devfn))
> {
> +		spin_unlock(&iommu->lock);
> +		return 0;
> +	}

Is it a valid case to setup passthrough on a present entry?
Re: [PATCH v4 6/7] iommu/vt-d: Add support for static identity domain
Posted by Baolu Lu 1 year, 4 months ago
On 2024/8/9 16:29, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Friday, August 9, 2024 1:55 PM
>>
>> +static int context_setup_pass_through(struct device *dev, u8 bus, u8 devfn)
>> +{
>> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
>> +	struct intel_iommu *iommu = info->iommu;
>> +	struct context_entry *context;
>> +
>> +	spin_lock(&iommu->lock);
>> +	context = iommu_context_addr(iommu, bus, devfn, 1);
>> +	if (!context) {
>> +		spin_unlock(&iommu->lock);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	if (context_present(context) && !context_copied(iommu, bus, devfn))
>> {
>> +		spin_unlock(&iommu->lock);
>> +		return 0;
>> +	}
> 
> Is it a valid case to setup passthrough on a present entry?

It's valid but unnecessary.

Since the context is present, it indicates that the configuration has
already been setup by a PCI aliased device. The iommu group mandates
that all PCI aliased devices must be attached to the same iommu domain.
Consequently, there's no need for additional configuration.

While it's feasible to remove this line of code due to the check in the
pci_for_each_dma_alias() callback:

static int context_setup_pass_through_cb(struct pci_dev *pdev, u16 
alias, void *data)
{
         struct device *dev = data;

         if (dev != &pdev->dev)
                 return 0;

         return context_setup_pass_through(dev, PCI_BUS_NUM(alias), 
alias & 0xff);
}

But it's in the original code, I've retained it to prevent any potential
regression.

Thanks,
baolu
RE: [PATCH v4 6/7] iommu/vt-d: Add support for static identity domain
Posted by Tian, Kevin 1 year, 4 months ago
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Saturday, August 10, 2024 4:02 PM
> 
> On 2024/8/9 16:29, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Friday, August 9, 2024 1:55 PM
> >>
> >> +static int context_setup_pass_through(struct device *dev, u8 bus, u8
> devfn)
> >> +{
> >> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> >> +	struct intel_iommu *iommu = info->iommu;
> >> +	struct context_entry *context;
> >> +
> >> +	spin_lock(&iommu->lock);
> >> +	context = iommu_context_addr(iommu, bus, devfn, 1);
> >> +	if (!context) {
> >> +		spin_unlock(&iommu->lock);
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	if (context_present(context) && !context_copied(iommu, bus, devfn))
> >> {
> >> +		spin_unlock(&iommu->lock);
> >> +		return 0;
> >> +	}
> >
> > Is it a valid case to setup passthrough on a present entry?
> 
> It's valid but unnecessary.
> 
> Since the context is present, it indicates that the configuration has
> already been setup by a PCI aliased device. The iommu group mandates
> that all PCI aliased devices must be attached to the same iommu domain.
> Consequently, there's no need for additional configuration.
> 
> While it's feasible to remove this line of code due to the check in the
> pci_for_each_dma_alias() callback:
> 
> static int context_setup_pass_through_cb(struct pci_dev *pdev, u16
> alias, void *data)
> {
>          struct device *dev = data;
> 
>          if (dev != &pdev->dev)
>                  return 0;
> 
>          return context_setup_pass_through(dev, PCI_BUS_NUM(alias),
> alias & 0xff);
> }
> 
> But it's in the original code, I've retained it to prevent any potential
> regression.
> 

I failed to find the original code which is why I asked above question.

But now I got that you were talking about the check in
domain_context_mapping_one() which applied to the si domain before.

So,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>