Re: [PATCH 1/2] iommu: Report domain nesting info for arm-smmu-v3

[Vivek Kumar Gautam]

Hi Eric,


On 2/12/21 11:43 PM, Auger Eric wrote:
> Hi Vivek,
> On 2/12/21 11:58 AM, Vivek Gautam wrote:
> > Add a vendor specific structure for domain nesting info for
> > arm smmu-v3, and necessary info fields required to populate
> > stage1 page tables.
> >
> > Signed-off-by: Vivek Gautam <vivek.gautam@xxxxxxx>
> > ---
> >   include/uapi/linux/iommu.h | 31 +++++++++++++++++++++++++------
> >   1 file changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> > index 4d3d988fa353..5f059bcf7720 100644
> > --- a/include/uapi/linux/iommu.h
> > +++ b/include/uapi/linux/iommu.h
> > @@ -323,7 +323,8 @@ struct iommu_gpasid_bind_data {
> >   #define IOMMU_GPASID_BIND_VERSION_1	1
> >   	__u32 version;
> >   #define IOMMU_PASID_FORMAT_INTEL_VTD	1
> > -#define IOMMU_PASID_FORMAT_LAST		2
> > +#define IOMMU_PASID_FORMAT_ARM_SMMU_V3	2
> > +#define IOMMU_PASID_FORMAT_LAST		3
> >   	__u32 format;
> >   	__u32 addr_width;
> >   #define IOMMU_SVA_GPASID_VAL	(1 << 0) /* guest PASID valid */
> > @@ -409,6 +410,21 @@ struct iommu_nesting_info_vtd {
> >   	__u64	ecap_reg;
> >   };
> >   
> > +/*
> > + * struct iommu_nesting_info_arm_smmuv3 - Arm SMMU-v3 nesting info.
> > + */
> > +struct iommu_nesting_info_arm_smmuv3 {
> > +	__u32	flags;
> > +	__u16	asid_bits;
> > +
> > +	/* Arm LPAE page table format as per kernel */
> > +#define ARM_PGTBL_32_LPAE_S1		(0x0)
> > +#define ARM_PGTBL_64_LPAE_S1		(0x2)

Thanks for reviewing and I am terribly sorry for coming to it with delay.

> Shouldn't it be a bitfield instead as both can be supported (the actual
> driver only supports 64b table format though). Does it match matches
> IDR0.TTF?

Yes, it should be a bitfield rather, and it doesn't match with IDR0.TTF. 
This is
 to hint the stage1 table allocations from viommu.
 Please see viommu_setup_pgtable() in the patch at [1].

> > +	__u8	pgtbl_fmt;
> So I understand this API is supposed to allow VFIO to expose those info
> early enough to the userspace to help configuring the viommu and avoid
> errors later on. I wonder how far we want to go on this path. What about
> those other caps that impact the STE/CD validity. There may be others...
>
> SMMU_IDR0.CD2L (support of 2 stage CD)
> SMMU_IDR0.TTENDIAN (endianness)
> SMMU_IDR0.HTTU (if 0 forbids HA/HD setting in the CD)
> SMMU_IDR3.STT (impacts T0SZ)

Right. The idea was to start with a minimal set of configuration.

But as you rightly pointed out we need a scalable solution to this problem

for arm-smmu-v3. I am now thinking if we could even use the nesting_info

for arm. We don't want to end up adding flags for all the feature bits.

Let me know if you have any suggestions.

Best regards
Vivek

[1] 
https://lore.kernel.org/linux-arm-kernel/20210115121342.15093-14-vivek.gautam@xxxxxxx/

[snip]