Re: [PATCH RFCv1 05/14] iommufd: Add IOMMUFD_OBJ_VIOMMU and IOMMUFD_CMD_VIOMMU_ALLOC

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, May 14, 2024 at 12:38:57PM -0300, Jason Gunthorpe wrote:
> > > > +
> > > > +/**
> > > > + * enum iommu_viommu_type - VIOMMU Type
> > > > + * @IOMMU_VIOMMU_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension for SMMUv3
> > > > + */
> > > > +enum iommu_viommu_type {
> > > > +	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> > > > +};
> > > 
> > > At least the 241 line should be in a following patch
> > 
> > It's for the "enum iommu_viommu_type" mentioned in the following
> > structure. Yi told me that you don't like an empty enum, and he
> > did something like this in HWPT_INVALIDATE series:
> > https://lore.kernel.org/linux-iommu/20240111041015.47920-3-yi.l.liu@intel.com/
> 
> I suspect 0 should be reserved as a non-set value for some
> basic sanity in all these driver type enums.

We have an IOMMU_HWPT_DATA_NONE for HWPT_ALLOC to compatible
with an S2 hwpt, since it doesn't need a data.

Maybe we can have an IOMMU_VIOMMU_TYPE_DEFAULT to be 0, for
an IOMMU driver (e.g. VT-d) that doesn't need to handle nor
be aware of any viommu object?

So, VMM can have a unified "attach-to-viommu" practice with
different IOMMUs, v.s. some still doing "attach-to-s2"?

> > > So, to make this all work perfectly we need approx the following
> > >  - S2 sharing across instances in ARM - meaning the VMID is allocated
> > >    at attach not domain alloc
> > >  - S2 hwpt is refcounted by the VIOMMU in the iommufd layer
> > >  - VIOMMU is refcounted by every nesting child in the iommufd layer
> > >  - The nesting child holds a pointer to both the S2 and the VIOMMU
> > >    (viommu optional)
> > >  - When the nesting child attaches to a device the STE will source the
> > >    VMID from the VIOMMU if present otherwise from the S2
> > >  - "RID" attach (ie naked S2) will have to be done with a Nesting
> > >    Child using a vSTE that indicates Identity. Then the attach logic
> > >    will have enough information to get the VMID from the VIOMMU
> > 
> > What is this RID attach (naked S2) case? S1DSS_BYPASS + SVA?
> 
> No, when the guest installs a vSTE that simply says bypass with no CD
> table pointer. That should result in a pSTE that is the S2 with on CD
> pointer.
> 
> I was originally thinking that the VMM would simply directly attach
> the S2 HWPT in this caes, but given the above issue with the VMID lifetime
> it makes more sense to 'attach' the viommu which holds the correct
> VMID. 
> 
> The issue with direct attach the S2 HWPT is the VMID lifetime, as it
> would have to borrow the VMID from the viommu but then the lifetime
> becomes more complex as it has to live beyond VIOMMU destruction. Not
> unsolvable but it seems easier to just avoid it entirely.

That makes a lot sense. I'd need to go through QEMU code and
see how we will accommodate these two more naturally: likely
the QEMU core should allocate an S2 HWPT for a VM, while the
viommu code should allocate a VIOMMU for each instance.

Thanks
Nicolin