Re: [PATCH v5 08/12] iommufd: Enforce pasid compatible domain for PASID-capable device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/12/13 15:52, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Friday, December 13, 2024 3:20 PM

On 2024/12/13 10:43, Tian, Kevin wrote:

Here is my full picture:

At domain allocation the driver should decide whether the setting of
ALLOC_PASID is compatible to the given domain type.

If paging and iommu supports pasid then ALLOC_PASID is allowed. This
applies to all drivers. AMD driver will further select V1 vs. V2 according
to the flag bit.

If nesting, AMR/ARM drivers will reject the bit as a CD/PASID table
cannot be attached to a PASID. Intel driver allows it if pasid is supported
by iommu.

Following your opinion, I think the enforcement is something like this,
it only checks pasid_compat for the PASID path.

+	if (idev->dev->iommu->max_pasids && pasid != IOMMU_NO_PASID
&&
!hwpt->pasid_compat)
+		return -EINVAL;

shouldn't it be:

	if (!idev->dev->iommu->max_pasids ||
	     pasid == IOMMU_NO_PASID ||
	     !hwpt->pasid_compat)
		return -EINVAL;

?

no, this check is added in a common place shared by RID and PASID path. If
it is added in place only for the PASID, it should be something like you
wrote.


This means the RID path is not surely be attached to pasid-comapt domain
or not. either iommufd or iommu driver should do across check between the
RID and PASID path. It is failing attaching non-pasid compat domain to RID
if PASID has been attached, and vice versa, attaching PASIDs should be
failed if RID has been attached to non pasid comapt domain. I doubt if this
can be done easily as there is no lock between RID and PASID paths.

I'm not sure where that requirement comes from. Does AMD require RID
and PASID to use the same format when nesting is disabled? If yes, that's
still a driver burden to handle, not iommufd's...

yes, I've asked this question [1]. AMD requires the RID and PASID use the
same format. I agree it's a driver's burden but now it's defined in the
ALLOC_PASID. So, I doubt if it becomes a common requirement to all the
iommu drivers. Otherwise the ALLOC_PASID definition is broken. e.g. Intel
may have no need to enforce it, but it would be like Intel is breaking
it.

--
Regards,
Yi Liu




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux