Re: [PATCH v4 09/16] iommu/amd: Always enable GCR3TRPMode when supported.

[Suthikulpanit, Suravee]

On 11/9/2025 12:39 AM, Vasant Hegde wrote:
> On 10/21/2025 7:13 AM, Suravee Suthikulpanit wrote:
> > The GCR3TRPMode feature allows the DTE[GCR3TRP] field to be configured
> > with GPA (instead of SPA). This simplifies the implementation, and is
> > a pre-requisite for nested translation support.
> >
> > Therefore, always enable this feature if available.
> >
> > Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> > ---
> >   drivers/iommu/amd/amd_iommu_types.h | 1 +
> >   drivers/iommu/amd/init.c            | 3 +++
> >   2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> > index 556f1df32d53..9226edd8af69 100644
> > --- a/drivers/iommu/amd/amd_iommu_types.h
> > +++ b/drivers/iommu/amd/amd_iommu_types.h
> > @@ -185,6 +185,7 @@
> >   #define CONTROL_EPH_EN		45
> >   #define CONTROL_XT_EN		50
> >   #define CONTROL_INTCAPXT_EN	51
> > +#define CONTROL_GCR3TRPMODE	58
> >   #define CONTROL_IRTCACHEDIS	59
> >   #define CONTROL_SNPAVIC_EN	61
> >   
> > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > index f2991c11867c..c45a4bd89569 100644
> > --- a/drivers/iommu/amd/init.c
> > +++ b/drivers/iommu/amd/init.c
> > @@ -1122,6 +1122,9 @@ static void iommu_enable_gt(struct amd_iommu *iommu)
> >   		return;
> >   
> >   	iommu_feature_enable(iommu, CONTROL_GT_EN);
> > +
> > +	if (check_feature2(FEATURE_GCR3TRPMODE))
> > +		iommu_feature_enable(iommu, CONTROL_GCR3TRPMODE);
>
> This works for now as iommu_snp_enable() enable is getting called later.
> But how about moving it to iommu_init_flags() ? 

I put it here because the GCR3TRP feature depends on GT supported / 
enabled, which is checked in this function. This avoid duplicate checks.
> Also we should probably add a comment here.

I will note in the comment regarding this needs to be called prior to 
iommu_snp_enable()

Thanks,
Suravee