On Tue, Aug 13, 2024 at 11:11:01AM +0800, Yi Liu wrote:
> > 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?
> > @@ -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.
> > +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
Jason
