Re: [PATCH v5 1/3] PCI: Add pci_ats_required() for CXL.cache capable devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic intended to allow a Virtual Function (VF) to inherit its Physical Function's (PF) ATS requirements is dead code. As a result, pci_ats_required() will always incorrectly return false for VFs.
--

commit 0af55683d00886be6357cf5cf0cf3df17b4c4b9b
Author: Nicolin Chen <nicolinc@nvidia.com>

PCI: Add pci_ats_required() for CXL.cache capable devices

This commit introduces pci_ats_required() to allow IOMMU drivers to keep
ATS always on for specific devices, like CXL.cache devices. It includes
logic for Virtual Functions (VFs) to inherit the ATS requirements of their
Physical Functions (PFs).

> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
[ ... ]
> +bool pci_ats_required(struct pci_dev *pdev)
> +{
> +	if (pci_ats_disabled() || !pci_ats_supported(pdev))
> +		return false;
> +
> +	/* A VF inherits its PF's requirement for ATS function */
> +	if (pdev->is_virtfn)
> +		pdev = pci_physfn(pdev);

[Severity: High]
Will this VF inheritance logic ever be reached?

According to the PCIe SR-IOV specification (section 9.3.3.1), VFs do not
implement the ATS Extended Capability, which means pdev->ats_cap is always
0 for VFs.

Because of this, pci_ats_supported(pdev) will unconditionally return false
for any VF. This causes the function to return false before it can ever
reach the pdev->is_virtfn check.

Could this prevent VFs from correctly enabling the ATS always on feature
and leave them unable to access host memory without triggering IOMMU
faults?

> +
> +	return pci_cxl_ats_required(pdev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1779304390.git.nicolinc@nvidia.com?part=1