1. Patchew
  2. linux
  3. [PATCH v6 0/8] Add Intel VT-d nested translation (part 1/2)
  4. View patch

[PATCH v6 7/8] iommu/vt-d: Add nested domain allocation

Yi Liu posted 8 patches 2 years, 1 month ago
There is a newer version of this series
[PATCH v6 7/8] iommu/vt-d: Add nested domain allocation
Posted by Yi Liu 2 years, 1 month ago 
From: Lu Baolu <baolu.lu@linux.intel.com>

This adds the support for IOMMU_HWPT_DATA_VTD_S1 type.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/intel/iommu.c | 39 ++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index b47025fbdea4..c7704e7efd4a 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4076,7 +4076,7 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
 			      struct iommu_domain *parent,
 			      const struct iommu_user_data *user_data)
 {
-	struct iommu_domain *domain;
+	bool request_nest_parent = flags & IOMMU_HWPT_ALLOC_NEST_PARENT;
 	struct intel_iommu *iommu;
 
 	if (flags & (~IOMMU_HWPT_ALLOC_NEST_PARENT))
@@ -4086,18 +4086,35 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
 	if (!iommu)
 		return ERR_PTR(-ENODEV);
 
-	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) && !ecap_nest(iommu->ecap))
+	if (!user_data) { /* Must be PAGING domain */
+		struct iommu_domain *domain;
+
+		if (request_nest_parent && !ecap_nest(iommu->ecap))
+			return ERR_PTR(-EOPNOTSUPP);
+		if (parent)
+			return ERR_PTR(-EINVAL);
+		/*
+		 * domain_alloc_user op needs to fully initialize a domain
+		 * before return, so uses iommu_domain_alloc() here for
+		 * simple.
+		 */
+		domain = iommu_domain_alloc(dev->bus);
+		if (!domain)
+			return ERR_PTR(-ENOMEM);
+		return domain;
+	}
+
+	/* Must be nested domain */
+	if (!ecap_nest(iommu->ecap))
+		return ERR_PTR(-EOPNOTSUPP);
+	if (user_data->type != IOMMU_HWPT_DATA_VTD_S1)
 		return ERR_PTR(-EOPNOTSUPP);
+	if (!parent || parent->ops != intel_iommu_ops.default_domain_ops)
+		return ERR_PTR(-EINVAL);
+	if (request_nest_parent)
+		return ERR_PTR(-EINVAL);
 
-	/*
-	 * domain_alloc_user op needs to fully initialize a domain
-	 * before return, so uses iommu_domain_alloc() here for
-	 * simple.
-	 */
-	domain = iommu_domain_alloc(dev->bus);
-	if (!domain)
-		domain = ERR_PTR(-ENOMEM);
-	return domain;
+	return intel_nested_domain_alloc(parent, user_data);
 }
 
 static void intel_iommu_domain_free(struct iommu_domain *domain)
-- 
2.34.1
Re: [PATCH v6 7/8] iommu/vt-d: Add nested domain allocation
Posted by Baolu Lu 2 years, 1 month ago 
On 10/20/23 5:32 PM, Yi Liu wrote:
> From: Lu Baolu <baolu.lu@linux.intel.com>
> 
> This adds the support for IOMMU_HWPT_DATA_VTD_S1 type.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>   drivers/iommu/intel/iommu.c | 39 ++++++++++++++++++++++++++-----------
>   1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index b47025fbdea4..c7704e7efd4a 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4076,7 +4076,7 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
>   			      struct iommu_domain *parent,
>   			      const struct iommu_user_data *user_data)
>   {
> -	struct iommu_domain *domain;
> +	bool request_nest_parent = flags & IOMMU_HWPT_ALLOC_NEST_PARENT;
>   	struct intel_iommu *iommu;
>   
>   	if (flags & (~IOMMU_HWPT_ALLOC_NEST_PARENT))
> @@ -4086,18 +4086,35 @@ intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
>   	if (!iommu)
>   		return ERR_PTR(-ENODEV);
>   
> -	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) && !ecap_nest(iommu->ecap))
> +	if (!user_data) { /* Must be PAGING domain */
> +		struct iommu_domain *domain;
> +
> +		if (request_nest_parent && !ecap_nest(iommu->ecap))

Hardware capability is not sufficient. How about adding below helper:

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index b5f33a7c1973..b04bbabcd696 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -540,6 +540,8 @@ enum {
  #define sm_supported(iommu)    (intel_iommu_sm && 
ecap_smts((iommu)->ecap))
  #define pasid_supported(iommu) (sm_supported(iommu) &&                 \
                                  ecap_pasid((iommu)->ecap))
+#define nested_supported(iommu)        (sm_supported(iommu) && 
        \
+                                ecap_nest((iommu)->ecap))

  struct pasid_entry;
  struct pasid_state_entry;

And, use nested_supported() here and bleow

> +			return ERR_PTR(-EOPNOTSUPP);
> +		if (parent)
> +			return ERR_PTR(-EINVAL);
> +		/*
> +		 * domain_alloc_user op needs to fully initialize a domain
> +		 * before return, so uses iommu_domain_alloc() here for
> +		 * simple.
> +		 */
> +		domain = iommu_domain_alloc(dev->bus);
> +		if (!domain)
> +			return ERR_PTR(-ENOMEM);
> +		return domain;
> +	}
> +
> +	/* Must be nested domain */
> +	if (!ecap_nest(iommu->ecap))

...here.

> +		return ERR_PTR(-EOPNOTSUPP);
> +	if (user_data->type != IOMMU_HWPT_DATA_VTD_S1)
>   		return ERR_PTR(-EOPNOTSUPP);
> +	if (!parent || parent->ops != intel_iommu_ops.default_domain_ops)
> +		return ERR_PTR(-EINVAL);
> +	if (request_nest_parent)
> +		return ERR_PTR(-EINVAL);
>   
> -	/*
> -	 * domain_alloc_user op needs to fully initialize a domain
> -	 * before return, so uses iommu_domain_alloc() here for
> -	 * simple.
> -	 */
> -	domain = iommu_domain_alloc(dev->bus);
> -	if (!domain)
> -		domain = ERR_PTR(-ENOMEM);
> -	return domain;
> +	return intel_nested_domain_alloc(parent, user_data);
>   }
>   
>   static void intel_iommu_domain_free(struct iommu_domain *domain)

Best regards,
baolu
RE: [PATCH v6 7/8] iommu/vt-d: Add nested domain allocation
Posted by Liu, Yi L 2 years, 1 month ago 
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Saturday, October 21, 2023 11:08 AM
> 
> On 10/20/23 5:32 PM, Yi Liu wrote:
> > From: Lu Baolu <baolu.lu@linux.intel.com>
> >
> > This adds the support for IOMMU_HWPT_DATA_VTD_S1 type.
> >
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > ---
> >   drivers/iommu/intel/iommu.c | 39 ++++++++++++++++++++++++++-----------
> >   1 file changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index b47025fbdea4..c7704e7efd4a 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -4076,7 +4076,7 @@ intel_iommu_domain_alloc_user(struct device *dev, u32
> flags,
> >   			      struct iommu_domain *parent,
> >   			      const struct iommu_user_data *user_data)
> >   {
> > -	struct iommu_domain *domain;
> > +	bool request_nest_parent = flags & IOMMU_HWPT_ALLOC_NEST_PARENT;
> >   	struct intel_iommu *iommu;
> >
> >   	if (flags & (~IOMMU_HWPT_ALLOC_NEST_PARENT))
> > @@ -4086,18 +4086,35 @@ intel_iommu_domain_alloc_user(struct device *dev, u32
> flags,
> >   	if (!iommu)
> >   		return ERR_PTR(-ENODEV);
> >
> > -	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) && !ecap_nest(iommu-
> >ecap))
> > +	if (!user_data) { /* Must be PAGING domain */
> > +		struct iommu_domain *domain;
> > +
> > +		if (request_nest_parent && !ecap_nest(iommu->ecap))
> 
> Hardware capability is not sufficient. How about adding below helper:

Yes. But the sm_supported() seems a bit confusing. Is it? actually, it's
a combined check of the sm capability and software choice (use sm or
not). How about renaming it to be sm_enabled()? Although the best
enable status check is to read the SM bit in root address register. No hurry
in this circle, but may be nice to have.

> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b5f33a7c1973..b04bbabcd696 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -540,6 +540,8 @@ enum {
>   #define sm_supported(iommu)    (intel_iommu_sm &&
> ecap_smts((iommu)->ecap))
>   #define pasid_supported(iommu) (sm_supported(iommu) &&                 \
>                                   ecap_pasid((iommu)->ecap))
> +#define nested_supported(iommu)        (sm_supported(iommu) &&
>         \
> +                                ecap_nest((iommu)->ecap))
> 
>   struct pasid_entry;
>   struct pasid_state_entry;
> 
> And, use nested_supported() here and bleow
> 
> > +			return ERR_PTR(-EOPNOTSUPP);
> > +		if (parent)
> > +			return ERR_PTR(-EINVAL);
> > +		/*
> > +		 * domain_alloc_user op needs to fully initialize a domain
> > +		 * before return, so uses iommu_domain_alloc() here for
> > +		 * simple.
> > +		 */
> > +		domain = iommu_domain_alloc(dev->bus);
> > +		if (!domain)
> > +			return ERR_PTR(-ENOMEM);
> > +		return domain;
> > +	}
> > +
> > +	/* Must be nested domain */
> > +	if (!ecap_nest(iommu->ecap))
> 
> ...here.
> 
> > +		return ERR_PTR(-EOPNOTSUPP);
> > +	if (user_data->type != IOMMU_HWPT_DATA_VTD_S1)
> >   		return ERR_PTR(-EOPNOTSUPP);
> > +	if (!parent || parent->ops != intel_iommu_ops.default_domain_ops)
> > +		return ERR_PTR(-EINVAL);
> > +	if (request_nest_parent)
> > +		return ERR_PTR(-EINVAL);
> >
> > -	/*
> > -	 * domain_alloc_user op needs to fully initialize a domain
> > -	 * before return, so uses iommu_domain_alloc() here for
> > -	 * simple.
> > -	 */
> > -	domain = iommu_domain_alloc(dev->bus);
> > -	if (!domain)
> > -		domain = ERR_PTR(-ENOMEM);
> > -	return domain;
> > +	return intel_nested_domain_alloc(parent, user_data);
> >   }
> >
> >   static void intel_iommu_domain_free(struct iommu_domain *domain)
> 
> Best regards,
> baolu

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.