Re: [RFC PATCH v2 14/22] iommufd: Add TIO calls

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Jason Gunthorpe <jgg@ziepe.ca> writes:

> On Fri, Mar 28, 2025 at 10:57:18AM +0530, Aneesh Kumar K.V wrote:
>> > +int iommufd_vdevice_tsm_bind_ioctl(struct iommufd_ucmd *ucmd)
>> > +{
>> > +	struct iommu_vdevice_tsm_bind *cmd = ucmd->cmd;
>> > +	struct iommufd_viommu *viommu;
>> > +	struct iommufd_vdevice *vdev;
>> > +	struct iommufd_device *idev;
>> > +	struct tsm_tdi *tdi;
>> > +	int rc = 0;
>> > +
>> > +	viommu = iommufd_get_viommu(ucmd, cmd->viommu_id);
>> > +	if (IS_ERR(viommu))
>> > +		return PTR_ERR(viommu);
>> >
>> 
>> Would this require an IOMMU_HWPT_ALLOC_NEST_PARENT page table
>> allocation?
>
> Probably. That flag is what forces a S2 page table.
>
>> How would this work in cases where there's no need to set up Stage 1
>> IOMMU tables?
>
> Either attach the raw HWPT of the IOMMU_HWPT_ALLOC_NEST_PARENT or:
>
>> Alternatively, should we allocate an IOMMU_HWPT_ALLOC_NEST_PARENT with a
>> Stage 1 disabled translation config? (In the ARM case, this could mean
>> marking STE entries as Stage 1 bypass and Stage 2 translate.)
>
> For arm you mean IOMMU_HWPT_DATA_ARM_SMMUV3.. But yes, this can work
> too and is mandatory if you want the various viommu linked features to
> work.
>

I was trying to prototype this using kvmtool and I have run into some
issues. First i needed the below change for vIOMMU alloc to work

modified   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4405,6 +4405,8 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR3);
 	if (FIELD_GET(IDR3_RIL, reg))
 		smmu->features |= ARM_SMMU_FEAT_RANGE_INV;
+	if (FIELD_GET(IDR3_FWB, reg))
+		smmu->features |= ARM_SMMU_FEAT_S2FWB;
 
 	/* IDR5 */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);



Also current code don't allow a Stage 1 bypass, Stage2 translation when
allocating HWPT.

arm_vsmmu_alloc_domain_nested -> arm_smmu_validate_vste -> 

	cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(arg->ste[0]));
	if (cfg != STRTAB_STE_0_CFG_ABORT && cfg != STRTAB_STE_0_CFG_BYPASS &&
	    cfg != STRTAB_STE_0_CFG_S1_TRANS)
		return -EIO;


This only allow a abort or bypass or stage1 translate/stage2 bypass config

Also if we don't need stage1 table, what will
iommufd_viommu_alloc_hwpt_nested() return?

>
>> Also, if a particular setup doesn't require creating IOMMU
>> entries because the entire guest RAM is identity-mapped in the IOMMU, do
>> we still need to make tsm_tdi_bind use this abstraction in iommufd?
>
> Even if the viommu will not be exposed to the guest I'm expecting that
> iommufd will have a viommu object, just not use various features. We
> are using viommu as the handle for the KVM, vmid and other things that
> are likely important here.
>
> Jason

-aneesh