diff mbox series

[v2,4/6] iommu/vt-d: Add set_dev_pasid callback for nested domain

Message ID 20240912130427.10119-5-yi.l.liu@intel.com
State New
Headers show
Series Make set_dev_pasid op supporting domain replacement | expand

Commit Message

Liu, Yi L Sept. 12, 2024, 1:04 p.m. UTC 
From: Lu Baolu <baolu.lu@linux.intel.com>

Extend intel_iommu_set_dev_pasid() to set a nested type domain to a PASID
of a device.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Co-developed-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/intel/iommu.c  | 22 +++++++++++++++++-----
 drivers/iommu/intel/iommu.h  |  3 +++
 drivers/iommu/intel/nested.c |  1 +
 3 files changed, 21 insertions(+), 5 deletions(-)

Comments

Baolu Lu Sept. 13, 2024, 1:52 a.m. UTC | #1 
On 9/12/24 9:04 PM, Yi Liu wrote:
> @@ -4299,7 +4304,12 @@ domain_prepare_dev_pasid(struct iommu_domain *domain,
>   	unsigned long flags;
>   	int ret;
>   
> -	ret = prepare_domain_attach_device(domain, dev);
> +	/* Nested type domain should prepare its parent domain */
> +	if (domain_type_is_nested(dmar_domain))
> +		ret = prepare_domain_attach_device(
> +				&dmar_domain->s2_domain->domain, dev);
> +	else
> +		ret = prepare_domain_attach_device(domain, dev);
>   	if (ret)
>   		return ERR_PTR(ret);
>   

'prepare' is a bit confusing in this context. It actually means checking
whether a domain is compatible with the hardware capabilities of RID or
PASID. Or not?

I know this confusion comes from the naming of
prepare_domain_attach_device(). Hence, do you mind renaming this helper
in a small patch?

Thanks,
baolu
Liu, Yi L Sept. 13, 2024, 12:22 p.m. UTC | #2 
On 2024/9/13 09:52, Baolu Lu wrote:
> On 9/12/24 9:04 PM, Yi Liu wrote:
>> @@ -4299,7 +4304,12 @@ domain_prepare_dev_pasid(struct iommu_domain *domain,
>>       unsigned long flags;
>>       int ret;
>> -    ret = prepare_domain_attach_device(domain, dev);
>> +    /* Nested type domain should prepare its parent domain */
>> +    if (domain_type_is_nested(dmar_domain))
>> +        ret = prepare_domain_attach_device(
>> +                &dmar_domain->s2_domain->domain, dev);
>> +    else
>> +        ret = prepare_domain_attach_device(domain, dev);
>>       if (ret)
>>           return ERR_PTR(ret);
> 
> 'prepare' is a bit confusing in this context. It actually means checking
> whether a domain is compatible with the hardware capabilities of RID or
> PASID. Or not?
> 
> I know this confusion comes from the naming of
> prepare_domain_attach_device(). Hence, do you mind renaming this helper
> in a small patch?

good suggestion.
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 6f5a8e549f3f..749ee7741ec4 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -284,6 +284,11 @@  static int __init intel_iommu_setup(char *str)
 }
 __setup("intel_iommu=", intel_iommu_setup);
 
+static int domain_type_is_nested(struct dmar_domain *domain)
+{
+	return domain->domain.type == IOMMU_DOMAIN_NESTED;
+}
+
 static int domain_pfn_supported(struct dmar_domain *domain, unsigned long pfn)
 {
 	int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
@@ -4299,7 +4304,12 @@  domain_prepare_dev_pasid(struct iommu_domain *domain,
 	unsigned long flags;
 	int ret;
 
-	ret = prepare_domain_attach_device(domain, dev);
+	/* Nested type domain should prepare its parent domain */
+	if (domain_type_is_nested(dmar_domain))
+		ret = prepare_domain_attach_device(
+				&dmar_domain->s2_domain->domain, dev);
+	else
+		ret = prepare_domain_attach_device(domain, dev);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -4329,9 +4339,9 @@  domain_prepare_dev_pasid(struct iommu_domain *domain,
 	return ERR_PTR(ret);
 }
 
-static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
-				     struct device *dev, ioasid_t pasid,
-				     struct iommu_domain *old)
+int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+			      struct device *dev, ioasid_t pasid,
+			      struct iommu_domain *old)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
@@ -4356,7 +4366,9 @@  static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
 	intel_pasid_tear_down_entry(iommu, dev, pasid,
 				    INTEL_PASID_TEARDOWN_DRAIN_PRQ);
 
-	if (dmar_domain->use_first_level)
+	if (domain_type_is_nested(dmar_domain))
+		ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain);
+	else if (dmar_domain->use_first_level)
 		ret = domain_setup_first_level(iommu, dmar_domain,
 					       dev, pasid);
 	else
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index b3b295e60626..36cf59bfe4ea 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1109,6 +1109,9 @@  void device_block_translation(struct device *dev);
 int prepare_domain_attach_device(struct iommu_domain *domain,
 				 struct device *dev);
 void domain_update_iommu_cap(struct dmar_domain *domain);
+int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+			      struct device *dev, ioasid_t pasid,
+			      struct iommu_domain *old);
 
 int dmar_ir_support(void);
 
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index 36a91b1b52be..c5a94c00b396 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -131,6 +131,7 @@  static int intel_nested_cache_invalidate_user(struct iommu_domain *domain,
 
 static const struct iommu_domain_ops intel_nested_domain_ops = {
 	.attach_dev		= intel_nested_attach_dev,
+	.set_dev_pasid		= intel_iommu_set_dev_pasid,
 	.free			= intel_nested_domain_free,
 	.cache_invalidate_user	= intel_nested_cache_invalidate_user,
 };