On Thu, Feb 15, 2024 at 11:13:24AM +0100, Jan Beulich wrote:
> Spec version 4.1 says
>
> "The ATSR structures identifies PCI Express Root-Ports supporting
> Address Translation Services (ATS) transactions. Software must enable
> ATS on endpoint devices behind a Root Port only if the Root Port is
> reported as supporting ATS transactions."
>
> Clearly root complex integrated devices aren't "behind root ports",
> matching my observation on a SapphireRapids system having an ATS-
> capable root complex integrated device. Hence for such devices we
> shouldn't try to locate a corresponding ATSR.
>
> Since both pci_find_ext_capability() and pci_find_cap_offset() return
> "unsigned int", change "pos" to that type at the same time.
>
> Fixes: 903b93211f56 ("[VTD] laying the ground work for ATS")
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Acked-by: Roger Pau Monné <roger....@citrix.com>
> ---
> v2: New.
>
> --- a/xen/drivers/passthrough/vtd/x86/ats.c
> +++ b/xen/drivers/passthrough/vtd/x86/ats.c
> @@ -44,7 +44,7 @@ struct acpi_drhd_unit *find_ats_dev_drhd
> int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd)
> {
> struct acpi_drhd_unit *ats_drhd;
> - int pos;
> + unsigned int pos, expfl = 0;
>
> if ( !ats_enabled || !iommu_qinval )
> return 0;
> @@ -53,7 +53,12 @@ int ats_device(const struct pci_dev *pde
> !ecap_dev_iotlb(drhd->iommu->ecap) )
> return 0;
>
> - if ( !acpi_find_matched_atsr_unit(pdev) )
> + pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP);
> + if ( pos )
> + expfl = pci_conf_read16(pdev->sbdf, pos + PCI_EXP_FLAGS);
> +
> + if ( MASK_EXTR(expfl, PCI_EXP_FLAGS_TYPE) != PCI_EXP_TYPE_RC_END &&
> + !acpi_find_matched_atsr_unit(pdev) )
Given the spec quote above, it might also be helpful to check that the
type is PCI_EXP_TYPE_ENDPOINT before attempting the ATSR check?
I would assume a well formed ATSR won't list non-endpoint devices.
Thanks, Roger.
