Re: [PATCH] iommu: Allow ATS to work on VFs when the PF uses IDENTITY

On 2024/8/13 20:30, Jason Gunthorpe wrote:

On Tue, Aug 13, 2024 at 11:11:01AM +0800, Yi Liu wrote:

quoted

quoted

The simplest solution is to have the iommu driver set the ATS STU when it
probes the device. This way the ATS STU is loaded immediately at boot time
to all PFs and there is no issue when a VF comes to use it.

This only sets STU without setting the ATS_CTRL.E bit. Is it possible that
VF considers the PF's STU field as valid only if PF's ATS_CTRL.E bit is
set?

That doesn't seem to be the case. Do you see something in the spec
that says so?

no, I didn't find any word that describe it. So it is highly possible this
is not defined and up to vendors. This means there is possibility for what
I typed in the above :( I'm not a hw guy, but it seems reasonable to treat
something valid only when the cap is enabled?

quoted

quoted

@@ -4091,6 +4091,7 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
   	dev_iommu_priv_set(dev, info);
   	if (pdev && pci_ats_supported(pdev)) {
+		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);

perhaps just do it for PFs? :)

That check is inside pci_perpare_ats(), no reason to duplicate it in
all the callers.

got it.

quoted

quoted

+int pci_prepare_ats(struct pci_dev *dev, int ps)
+{
+	u16 ctrl;
+
+	if (!pci_ats_supported(dev))
+		return -EINVAL;
+
+	if (WARN_ON(dev->ats_enabled))
+		return -EBUSY;
+
+	if (ps < PCI_ATS_MIN_STU)
+		return -EINVAL;
+
+	if (dev->is_virtfn)
+		return 0;
+
+	dev->ats_stu = ps;
+	ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
+	pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);

Is it valuable to have a flag to mark if STU is set or not? Such way can
avoid setting STU multiple times.

We don't because we only do it for the PF due to the is_virtfn check

ok. Doing it only for PF is enough.

-- 
Regards,
Yi Liu