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

[Jason Gunthorpe <jgg@nvidia.com>]

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