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

[Yi Liu]

On 2024/8/13 20:30, Jason Gunthorpe wrote:
> 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?

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?

>>> @@ -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.

>>> +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