| Message ID | 3686dae7-e005-47b5-9235-14208a68eec5@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VT-d: SATC handling; ATS: tidying | expand |
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 <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@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.
--- 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) ) return 0; ats_drhd = find_ats_dev_drhd(drhd->iommu);
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 <jbeulich@suse.com> --- v2: New.