[PATCH v2 09/12] iommu/amd: Add support for nest parent domain allocation

Suravee Suthikulpanit posted 12 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v2 09/12] iommu/amd: Add support for nest parent domain allocation
Posted by Suravee Suthikulpanit 4 months, 1 week ago
To support nested translation, the nest parent domain is allocated with
IOMMU_HWPT_ALLOC_NEST_PARENT flag, and stores information of the v1 page
table for stage 2 (i.e. GPA->SPA).

Also, only support nest parent domain on AMD system, which can support
the Guest CR3 Table (GCR3TRPMode) feature. This feature is required in
order to program DTE[GCR3 Table Root Pointer] with the GPA.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd/amd_iommu_types.h |  1 +
 drivers/iommu/amd/iommu.c           | 27 +++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 556f1df32d53..d8c755b2045d 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -107,6 +107,7 @@
 
 
 /* Extended Feature 2 Bits */
+#define FEATURE_GCR3TRPMODE	BIT_ULL(3)
 #define FEATURE_SNPAVICSUP	GENMASK_ULL(7, 5)
 #define FEATURE_SNPAVICSUP_GAM(x) \
 	(FIELD_GET(FEATURE_SNPAVICSUP, x) == 0x1)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index e0bfcda678a8..facee0f7a131 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2584,6 +2584,18 @@ do_iommu_domain_alloc(struct device *dev, u32 flags,
 	return &domain->domain;
 }
 
+static inline bool is_nest_parent_supported(u32 flags)
+{
+	/* Only allow nest parent when these features are supported */
+	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) &&
+	    (!check_feature(FEATURE_GT) ||
+	     !check_feature(FEATURE_GIOSUP) ||
+	     !check_feature2(FEATURE_GCR3TRPMODE)))
+		return false;
+
+	return true;
+}
+
 static struct iommu_domain *
 amd_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
 				    const struct iommu_user_data *user_data)
@@ -2591,15 +2603,22 @@ amd_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
 {
 	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
 	const u32 supported_flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
-						IOMMU_HWPT_ALLOC_PASID;
+						IOMMU_HWPT_ALLOC_PASID |
+						IOMMU_HWPT_ALLOC_NEST_PARENT;
 
-	if ((flags & ~supported_flags) || user_data)
+	if ((flags & ~supported_flags) || user_data || !is_nest_parent_supported(flags))
 		return ERR_PTR(-EOPNOTSUPP);
 
 	switch (flags & supported_flags) {
 	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING:
-		/* Allocate domain with v1 page table for dirty tracking */
-		if (!amd_iommu_hd_support(iommu))
+	case IOMMU_HWPT_ALLOC_NEST_PARENT:
+	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING | IOMMU_HWPT_ALLOC_NEST_PARENT:
+		/*
+		 * Allocate domain with v1 page table for dirty tracking
+		 * and/or Nest parent.
+		 */
+		if ((flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING) &&
+		    !amd_iommu_hd_support(iommu))
 			break;
 		return do_iommu_domain_alloc(dev, flags, PD_MODE_V1);
 	case IOMMU_HWPT_ALLOC_PASID:
-- 
2.34.1
Re: [PATCH v2 09/12] iommu/amd: Add support for nest parent domain allocation
Posted by Nicolin Chen 4 months, 1 week ago
On Wed, Oct 01, 2025 at 06:09:51AM +0000, Suravee Suthikulpanit wrote:
> To support nested translation, the nest parent domain is allocated with
> IOMMU_HWPT_ALLOC_NEST_PARENT flag, and stores information of the v1 page
> table for stage 2 (i.e. GPA->SPA).
> 
> Also, only support nest parent domain on AMD system, which can support
> the Guest CR3 Table (GCR3TRPMode) feature. This feature is required in
> order to program DTE[GCR3 Table Root Pointer] with the GPA.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

> +static inline bool is_nest_parent_supported(u32 flags)
> +{
> +	/* Only allow nest parent when these features are supported */
> +	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) &&
> +	    (!check_feature(FEATURE_GT) ||
> +	     !check_feature(FEATURE_GIOSUP) ||
> +	     !check_feature2(FEATURE_GCR3TRPMODE)))
> +		return false;
> +
> +	return true;

If the "flags" doesn't have IOMMU_HWPT_ALLOC_NEST_PARENT while one
of the features is missing, this would return true, indicating the
HW supports nesting parent, which will be logically wrong although
it does validate the nest parent flag.

Following the existing coding style, I think this could be just:

static inline bool is_nest_parent_supported(void)
{
	/* Only allow nest parent when these features are supported */
	return check_feature(FEATURE_GT) && check_feature(FEATURE_GIOSUP) &&
	       check_feature2(FEATURE_GCR3TRPMODE);
}

> @@ -2591,15 +2603,22 @@ amd_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
>  {
>  	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
>  	const u32 supported_flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
> -						IOMMU_HWPT_ALLOC_PASID;
> +						IOMMU_HWPT_ALLOC_PASID |
> +						IOMMU_HWPT_ALLOC_NEST_PARENT;
>  
> -	if ((flags & ~supported_flags) || user_data)
> +	if ((flags & ~supported_flags) || user_data || !is_nest_parent_supported(flags))
>  		return ERR_PTR(-EOPNOTSUPP);

Un-change this, given the code below is already validating flags.

>  	switch (flags & supported_flags) {
>  	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING:
> -		/* Allocate domain with v1 page table for dirty tracking */
> -		if (!amd_iommu_hd_support(iommu))
> +	case IOMMU_HWPT_ALLOC_NEST_PARENT:
> +	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING | IOMMU_HWPT_ALLOC_NEST_PARENT:
> +		/*
> +		 * Allocate domain with v1 page table for dirty tracking
> +		 * and/or Nest parent.
> +		 */
> +		if ((flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING) &&
> +		    !amd_iommu_hd_support(iommu))
>  			break;

And add here:
		if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT)) &&
		    !is_nest_parent_supported())
			    break;

Otherwise, this looks good to me:

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Re: [PATCH v2 09/12] iommu/amd: Add support for nest parent domain allocation
Posted by Suthikulpanit, Suravee 4 months ago

On 10/2/2025 1:00 PM, Nicolin Chen wrote:
> On Wed, Oct 01, 2025 at 06:09:51AM +0000, Suravee Suthikulpanit wrote:
>> To support nested translation, the nest parent domain is allocated with
>> IOMMU_HWPT_ALLOC_NEST_PARENT flag, and stores information of the v1 page
>> table for stage 2 (i.e. GPA->SPA).
>>
>> Also, only support nest parent domain on AMD system, which can support
>> the Guest CR3 Table (GCR3TRPMode) feature. This feature is required in
>> order to program DTE[GCR3 Table Root Pointer] with the GPA.
>>
>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>> +static inline bool is_nest_parent_supported(u32 flags)
>> +{
>> +	/* Only allow nest parent when these features are supported */
>> +	if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) &&
>> +	    (!check_feature(FEATURE_GT) ||
>> +	     !check_feature(FEATURE_GIOSUP) ||
>> +	     !check_feature2(FEATURE_GCR3TRPMODE)))
>> +		return false;
>> +
>> +	return true;
> If the "flags" doesn't have IOMMU_HWPT_ALLOC_NEST_PARENT while one
> of the features is missing, this would return true, indicating the
> HW supports nesting parent, which will be logically wrong although
> it does validate the nest parent flag.
> 
> Following the existing coding style, I think this could be just:
> 
> static inline bool is_nest_parent_supported(void)
> {
> 	/* Only allow nest parent when these features are supported */
> 	return check_feature(FEATURE_GT) && check_feature(FEATURE_GIOSUP) &&
> 	       check_feature2(FEATURE_GCR3TRPMODE);
> }

Ahh, I missed this part.

Thanks,
Suravee
Re: [PATCH v2 09/12] iommu/amd: Add support for nest parent domain allocation
Posted by Jason Gunthorpe 4 months ago
On Thu, Oct 02, 2025 at 11:00:14AM -0700, Nicolin Chen wrote:

> >  	switch (flags & supported_flags) {
> >  	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING:
> > -		/* Allocate domain with v1 page table for dirty tracking */
> > -		if (!amd_iommu_hd_support(iommu))
> > +	case IOMMU_HWPT_ALLOC_NEST_PARENT:
> > +	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING | IOMMU_HWPT_ALLOC_NEST_PARENT:
> > +		/*
> > +		 * Allocate domain with v1 page table for dirty tracking
> > +		 * and/or Nest parent.
> > +		 */
> > +		if ((flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING) &&
> > +		    !amd_iommu_hd_support(iommu))
> >  			break;
> 
> And add here:
> 		if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT)) &&
> 		    !is_nest_parent_supported())
> 			    break;

Yeah this looks better

On VT-D we have V1/V2 specific alloc functions and these tests were
put at the top of those functions in this form. This is close enough,
it is helpful to be consistent.

Jason