Re: [PATCH 1/1] iommu/vt-d: Move PCI PASID enablement to probe path

From: Yi Liu
Date: Tue Aug 20 2024 - 04:51:47 EST


On 2024/8/20 16:30, Vasant Hegde wrote:
Hi All,



On 8/20/2024 9:40 AM, Baolu Lu wrote:
On 2024/8/19 20:34, Jason Gunthorpe wrote:
On Mon, Aug 19, 2024 at 03:09:00PM +0800, Baolu Lu wrote:
On 2024/8/19 14:34, Vasant Hegde wrote:
On 8/16/2024 6:39 PM, Baolu Lu wrote:
On 2024/8/16 20:16, Vasant Hegde wrote:
On 8/16/2024 4:19 PM, Lu Baolu wrote:
Currently, PCI PASID is enabled alongside PCI ATS when an iommu domain is
attached to the device and disabled when the device transitions to block
translation mode. This approach is inappropriate as PCI PASID is a device
feature independent of the type of the attached domain.
Reading through other thread, I thought we want to enable both PASID and
PRI in
device probe path. Did I miss something?
PRI is different. PRI should be enabled when the first iopf-capable
domain is attached to device or its PASID, and disabled when the last
such domain is detached.
Right. That's what AMD driver also does (We enable it when we attach IOPF
capable domain). But looking into pci_enable_pri() :


202         /*
203          * VFs must not implement the PRI Capability.  If their PF
204          * implements PRI, it is shared by the VFs, so if the PF PRI is
205          * enabled, it is also enabled for the VF.
206          */
207         if (pdev->is_virtfn) {
208                 if (pci_physfn(pdev)->pri_enabled)
209                         return 0;
210                 return -EINVAL;
211         }
212


If we try to enable PRI for VF without first enabling it in PF it will fail
right?

Now if PF is attached to non-IOPF capable domain (like in AMD case attaching to
domain with V1 page table) and we try to attach VF to IOPF capable domain  (say
AMD v2 page table -OR- nested domain) it will fail right?
Yeah! So, the iommu driver should basically control the PRI switch on
the PF whenever someone wants to use it on a VF.
PRI enable sounds like PASID enable to me.

The ATS control is per VF/PF, and PRI does nothing unless ATS returns
a non-present indication.

Like PASID, it seems the purpose of PRI caps is to negotiate if the
CPU can process PRI TLPs globally.

So, I'd guess that just like PASID we should turn it on at PF probe
time if the IOMMU can globall handle PRI.

Enabling ATS will cause PRI TLPs to be sent.

Probably more of this code should be lifted out of the iommu drivers..

Some architectures, including VT-d non-scalable mode, doesn't support
ATS translation and translated requests when it is working in the
IDENTITY domain mode. In that case, probably PCI ATS still need to be
disabled when such domain is attached and re-enabled when the domain is
detached.

Does it make sense to move both PASID/PRI enablement to probe() path? something
like below :

[I am assuming ops->dev_enable_feat() interface is going away]

- Enable device side PASID/PRI during ops->probe_device()
- In device attach path (ops->attach_dev()), depending on IOMMU, device and
domain capability configure the features like PASID, IOPF and ATS. That means
ATS enablement is still done at attach device path.

Are we sure that it is ok to enable PRI before enabling ATS?

--
Regards,
Yi Liu