Re: [PATCH v5 2/3] PCI: Allow ATS to be always on for pre-CXL devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The matching logic in `pci_dev_specific_ats_required()` is flawed: it ignores the `device` field when a callback is provided and fails to handle `PCI_ANY_ID` wildcards when a callback is absent.
--

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

PCI: Allow ATS to be always on for pre-CXL devices

This commit introduces a pci_dev_specific_ats_required() quirk function to
allow certain pre-CXL devices to keep ATS always on. It adds a list of IDs
for these devices and integrates the quirk into pci_ats_required().

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index caaed1a01dc02..c0242f3e9f063 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
[ ... ]
> +bool pci_dev_specific_ats_required(struct pci_dev *pdev)
> +{
> +	const struct pci_dev_ats_required *i;
> +
> +	for (i = pci_dev_ats_required; i->vendor; i++) {
> +		if (i->vendor != pdev->vendor)
> +			continue;
> +		if (i->ats_required && i->ats_required(pdev))
> +			return true;
> +		if (!i->ats_required && i->device == pdev->device)
> +			return true;
> +	}
> +
> +	return false;
> +}

[Severity: Medium]
Does this matching logic inadvertently ignore the device field when a
callback is provided, and miss handling PCI_ANY_ID when a callback is absent?

If a future entry specifies a precise device ID along with a callback (e.g.,
{ VENDOR, 0x1234, my_quirk }), the specific device ID will be ignored, and
the callback will execute for all devices from that vendor.

Alternatively, if a callback is absent, a catch-all definition like
{ VENDOR, PCI_ANY_ID, NULL } would literally compare pdev->device against
0xFFFF and silently fail to match because it lacks a PCI_ANY_ID check.

While this functionally works for the two currently defined table entries in
the patch, could this be updated to mirror standard PCI quirk parsing
behavior (like pci_dev_specific_reset()) to prevent issues for future
additions?

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