RE: [PATCH 0/6] Make set_dev_pasid op supportting domain replacement

Tian, Kevin
2024-07-15T08:16:52Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, July 12, 2024 2:41 AM
>
> On Wed, Jul 10, 2024 at 08:24:16AM +0000, Tian, Kevin wrote:
> > > From: Liu, Yi L <yi.l.liu@intel.com>
> > > Sent: Friday, June 28, 2024 4:56 PM
> > >
> > > This splits the preparation works of the iommu and the Intel iommu
> driver
> > > out from the iommufd pasid attach/replace series. [1]
> > >
> > > To support domain replacement, the definition of the set_dev_pasid op
> > > needs to be enhanced. Meanwhile, the existing set_dev_pasid callbacks
> > > should be extended as well to suit the new definition.
> > >
> > > pasid attach/replace is mandatory on Intel VT-d given the PASID table
> > > locates in the physical address space hence must be managed by the
> kernel,
> > > both for supporting vSVA and coming SIOV. But it's optional on
> ARM/AMD
> > > which allow configuring the PASID/CD table either in host physical
> address
> > > space or nested on top of an GPA address space. This series only extends
> > > the Intel iommu driver as the minimal requirement.
> >
> > Looks above is only within VFIO/IOMMUFD context (copied from the old
> > series). But this series is all in IOMMU and pasid attach is certainly not
> > optional for SVA on all platforms. this needs to be revised.
>
> I feel like we should explicitly block replace on AMD by checking if
> old_domain is !NULL and failing.

this is what patch06 does.

>
> Then the description is sort of like
>
> Replace is useful for iommufd/VFIO to provide perfect HW emulation in
> case the VM is expecting to be able to change a PASID on the fly. As
> AMD will only support PASID in VM's using nested translation where we
> don't use the set_dev_pasid API leave it disabled for now.
>

yes that's clearer.

btw I don't remember whether we have discussed the rationale behind
the different driver semantics between RID and PASID. Currently RID
replace is same as RID attach, with the driver simply blocking the old
translation from the start and then no rollback upon failure when
switching to the new domain (expecting the iommu core to recover),
while for PASID replace we expect the driver to implement the hitless
switch.

Is it because there is no need of perfect HW emulation for RID or just
to be cleaned up later?

This difference at least starts bringing some puzzle to me when
reading the related code in the intel-iommu driver.