diff mbox series

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

Message ID	20231020093246.17015-8-yi.l.liu@intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; From: Yi Liu <yi.l.liu@intel.com> To: joro@8bytes.org, alex.williamson@redhat.com, jgg@nvidia.com, kevin.tian@intel.com, robin.murphy@arm.com, baolu.lu@linux.intel.com Cc: cohuck@redhat.com, eric.auger@redhat.com, nicolinc@nvidia.com, kvm@vger.kernel.org, mjrosato@linux.ibm.com, chao.p.peng@linux.intel.com, yi.l.liu@intel.com, yi.y.sun@linux.intel.com, peterx@redhat.com, jasowang@redhat.com, shameerali.kolothum.thodi@huawei.com, lulu@redhat.com, suravee.suthikulpanit@amd.com, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, zhenzhong.duan@intel.com, joao.m.martins@oracle.com, xin.zeng@intel.com Subject: [PATCH v6 7/8] iommu/vt-d: Add nested domain allocation Date: Fri, 20 Oct 2023 02:32:45 -0700 Message-Id: <20231020093246.17015-8-yi.l.liu@intel.com> In-Reply-To: <20231020093246.17015-1-yi.l.liu@intel.com> References: <20231020093246.17015-1-yi.l.liu@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Add Intel VT-d nested translation (part 1/2) \| [v6,0/8] Add Intel VT-d nested translation (part 1/2) [v6,1/8] iommufd: Add data structure for Intel VT-d stage-1 domain allocation [v6,2/8] iommu/vt-d: Extend dmar_domain to support nested domain [v6,3/8] iommu/vt-d: Add helper for nested domain allocation [v6,4/8] iommu/vt-d: Add helper to setup pasid nested translation [v6,5/8] iommu/vt-d: Make domain attach helpers to be extern [v6,6/8] iommu/vt-d: Set the nested domain to a device [v6,7/8] iommu/vt-d: Add nested domain allocation [v6,8/8] iommu/vt-d: Disallow read-only mappings to nest parent domain

Commit Message

Yi Liu Oct. 20, 2023, 9:32 a.m. UTC

  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(-)

Comments

Baolu Lu Oct. 21, 2023, 3:07 a.m. UTC | #1

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

Yi Liu Oct. 23, 2023, 11:11 a.m. UTC | #2

> 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

diff mbox series

Patch

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)