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

From: Vivek Kumar Gautam <vivek.gautam@arm.com>
To: Auger Eric <eric.auger@redhat.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	iommu@lists.linux-foundation.org,
	virtualization@lists.linux-foundation.org
Cc: jean-philippe@linaro.org, kevin.tian@intel.com, mst@redhat.com,
	will.deacon@arm.com, alex.williamson@redhat.com,
	robin.murphy@arm.com
Subject: Re: [PATCH 1/2] iommu: Report domain nesting info for arm-smmu-v3
Date: Wed, 3 Mar 2021 15:25:05 +0530	[thread overview]
Message-ID: <203f0aa1-e7d7-f27f-fec8-6583c0425ebd@arm.com> (raw)
In-Reply-To: <28c0204d-2a5e-4eb9-d869-9dbd6edab55a@redhat.com>

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@arm.com>
>> ---
>>   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@arm.com/

[snip]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu