Re: [RFC PATCH v4 03/16] iommu/arm-smmu-v3: Add initial pSMMU realm viommu plumbing

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

Nicolin Chen <nicolinc@nvidia.com> writes:

> On Mon, Apr 27, 2026 at 02:23:31PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> +static const struct iommufd_viommu_ops arm_realm_smmu_v3_ops = {
>> +	.destroy = arm_realm_smmu_v3_destroy,
>> +	.alloc_domain_nested = arm_vsmmu_alloc_domain_nested,
>> +	.cache_invalidate = arm_vsmmu_cache_invalidate,
>
> I don't think realm vsmmu should include NS nested domain ops. I
> wonder if adding here is for some covert reason that prevents us
> from registering viommu/vdevice objects?
>
>> +static int arm_realm_smmu_v3_vdevice_init(struct iommufd_vdevice *vdev)
>> +{
>> +	struct device *dev = iommufd_vdevice_to_device(vdev);
>> +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>> +	// fixme which stream to pick
>> +	/* At this moment, iommufd only supports PCI device that has one SID */
>> +	struct arm_smmu_stream *stream = &master->streams[0];
>> +	struct arm_smmu_device *smmu = master->smmu;
>> +	unsigned long rmi_ret = 0;
>> +	int ret;
>> +
>> +	if (!smmu->realm_initialized)
>> +		return -EINVAL;
>> +
>> +	ret = rmi_psmmu_st_l2_create(smmu->base_phys,
>> +				     ALIGN_DOWN(stream->id, STRTAB_NUM_L2_STES),
>> +				     &rmi_ret);
>
> The "vdevice" is for a PSMMU stream table allocation..
>
>> +int arm_realm_smmu_v3_init(struct iommufd_viommu *viommu,
>> +			   const struct iommu_user_data *user_data)
>> +{
> [...]
>> +psmmu_activate:
>> +	ret = rmi_psmmu_activate(smmu->base_phys, virt_to_phys(params),
>> +				 &rmi_ret);
>
> .. and the "viommu" is also for PSMMU activation...
>
>> +++ b/include/uapi/linux/iommufd.h
>> @@ -1055,6 +1055,7 @@ enum iommu_viommu_type {
>>  	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
>>  	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
>>  	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
>> +	IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 = 3,
>
> .. and we demand userspace (VMM) to use IOMMU_VIOMMU_ALLOC ioctl,
> even if VMM does not actually expose a guest-level SMMU instance.
> Thus, no user_data.
>
> I can get the reasoning behind the flow using this viommu/vdevice.
>
> But, on the other hand, I can imagine that a Realm VSMMU would add
> a new flag with a user_data to this VIOMMU. Then, this flow would
> give some troubles to VMM (QEMU for example):
>
>  - For VM with a guest-level SMMU, QEMU creates a realm instance
>    where IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 (with vsmmu) can be
>    allocated.
>  - For VM w/o a guest-level SMMU, QEMU won't create such a realm
>    instance, while still required to invoke the ioctl (w/o vsmmu). 
>
> Taking a step back, I wonder if we really need to use iommufd for
> PSMMU activation and its stream table allocations?
>
> Here are some facts:
>   - An iommufd has a ctx, that's one per VM. Similarly, a Realm
>     has an RD.
>   - For an RMI command that needs an RD, it makes sense to be per
>     iommufd ctx, e.g. RMI_VSMMU_* or RMI_VDEV_* commands.
>   - PSMMU commands are global; they don't need RD. So they don't
>     seem necessary to tie to an iommufd ctx.
>
> Instead, could the PSMMU activation be done after RMI_PSMMU_INFO
> check? Is there any reason not to do that? A safer timing might
> be at the device assignment stage?
>

One of the reasons I added IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 was to
avoid creating a psmmu object when we are not using a PCI passthrough
VM. That is also the reason for all the refcounting around the psmmu
objects.

If we are okay with creating psmmu objects early, then I guess we can go
with the above approach.

>
> Speaking of which, RMI_PSMMU_ST_L2_CREATE doesn't seem necessary
> to be invoked in a vdevice context either. Maybe it should align
> with iommufd idev's lifecycle?
>

-aneesh