Re: [PATCH 02/11] iommu/amd: Introduce struct protection_domain.pd_mode

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Aug 11, 2023 at 03:34:46PM +0530, Vasant Hegde wrote:
> Jason,
> 
> 
> On 8/8/2023 9:00 PM, Jason Gunthorpe wrote:
> > On Tue, Aug 08, 2023 at 10:02:23AM +0000, Vasant Hegde wrote:
> >> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >>
> >> This enum variable is used to track the type of page table used by the
> >> protection domain. It will replace the protection_domain.flags in
> >> subsequent series.
> >>
> >> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> >> ---
> >>  drivers/iommu/amd/amd_iommu_types.h | 7 +++++++
> >>  drivers/iommu/amd/iommu.c           | 3 +++
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> >> index 0d339e022572..5e89032b3dee 100644
> >> --- a/drivers/iommu/amd/amd_iommu_types.h
> >> +++ b/drivers/iommu/amd/amd_iommu_types.h
> >> @@ -555,6 +555,12 @@ struct amd_io_pgtable {
> >>  	u64			*pgd;		/* v2 pgtable pgd pointer */
> >>  };
> >>  
> >> +enum protection_domain_mode {
> >> +	PD_MODE_PT = 0,
> >> +	PD_MODE_V1,
> >> +	PD_MODE_V2,
> >> +};
> > 
> > PD_MODE_PT is redundant because the domain->type ==
> > IOMMU_DOMAIN_IDENTITY already indicates identity. It is better not to
> > denormalize the data structures.
> 
> We need to track how we have setup the hardware page table here. We can use
> `IOMMU_DOMAIN_IDENTITY` for PT mode, but we don't have anything for other two
> modes. So having macros makes it easy. Hence we have introduced new macros for
> protection domain.

I think this was immediately abused in the next patch, so I'd prefer
not to see this kind of duplication. It should describe the format of
the IOPTEs and if the domain doesn't have IOPTEs then it shouldn't
need to ever look at this value.

Jason