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

Yi Liu
2024-07-15T08:19:16Z

On 2024/7/12 02:41, Jason Gunthorpe wrote:
> 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.

yes, patch 6 of this series has made it fail in AMD's set_dev_pasid
callback.

> 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.

Does it apply to SMMUv3 as well? IIRC. ARM SMMUv3 also has the CD table
(a.k.a PASID table) within the guest. And I think this is the major reason
for your above statement. right?

--
Regards,
Yi Liu