On 15/04/2019 14:21, Will Deacon wrote: > On Tue, Apr 09, 2019 at 05:52:39PM +0100, Jean-Philippe Brucker wrote: >> Root complex node in IORT has a bit telling whether it supports ATS or >> not. Store this bit in the IOMMU fwspec when setting up a device, so it >> can be accessed later by an IOMMU driver. >> >> Use the negative version (NO_ATS) at the moment because it's not clear >> if/how the bit needs to be integrated in other firmware descriptions. The >> SMMU has a feature bit telling if it supports ATS, which might be >> sufficient in most systems for deciding whether or not we should enable >> the ATS capability in endpoints. > > Hmm, the SMMUv3 architecture manual is pretty explicit about this: > > | It [SMMU_IDR0.ATS] does not guarantee that client devices and intermediate > | components also support ATS and this must be determined separately. > > so we may need to extend the PCI bindings to describe this. I think the > negative logic is likely to get in the way if that's the case. Right. For devicetree I can resurrect the patch I proposed a while ago [1]. Rob wasn't keen on adding an "ats-supported" property to PCI host bridge nodes. Instead the host controller drivers should deduce whether ATS is supported from the compatible string. But I'll resend that patch adding the property only to pci-host-ecam-generic. [1] https://lists.linuxfoundation.org/pipermail/iommu/2017-May/022043.html >> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> >> --- >> drivers/acpi/arm64/iort.c | 11 +++++++++++ >> include/linux/iommu.h | 4 ++++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >> index e48894e002ba..7f2c1c9c6b38 100644 >> --- a/drivers/acpi/arm64/iort.c >> +++ b/drivers/acpi/arm64/iort.c >> @@ -1028,6 +1028,14 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) >> dev_dbg(dev, "dma_pfn_offset(%#08llx)\n", offset); >> } >> >> +static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node) >> +{ >> + struct acpi_iort_root_complex *pci_rc; >> + >> + pci_rc = (struct acpi_iort_root_complex *)node->node_data; >> + return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED; >> +} > > Do we need to worry about the "noats" command-line option here? It feels > like we should be checking with the PCI subsystem before telling the SMMU > we're good to go. I'm checking the noats option in arm_smmu_enable_ats() at the moment, using the pci_ats_disabled() helper. Thanks, Jean
