| Message ID | 20190320173634.21895-2-jean-philippe.brucker@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add PCI ATS support to Arm SMMUv3 | expand |
On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: > err = pci_for_each_dma_alias(to_pci_dev(dev), > iort_pci_iommu_init, &info); > + > + if (!err && !iort_pci_rc_supports_ats(node)) > + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS; Is it possible to remove !err check here. ATS supported is just a flag in IORT table. Does this decision have to rely on having a correct dma alias?
On 21/03/2019 16:00, Sinan Kaya wrote: > On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: >> err = pci_for_each_dma_alias(to_pci_dev(dev), >> iort_pci_iommu_init, &info); >> + >> + if (!err && !iort_pci_rc_supports_ats(node)) >> + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS; > > Is it possible to remove !err check here. ATS supported is just a flag > in IORT table. Does this decision have to rely on having a correct dma > alias? iort_pci_iommu_init() allocates and initializes dev->iommu_fwspec, so checking !err ensures that dev->iommu_fwspec->flags is valid. Thanks, Jean
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; +} + /** * iort_iommu_configure - Set-up IOMMU configuration for a device. * @@ -1063,6 +1071,9 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) info.node = node; err = pci_for_each_dma_alias(to_pci_dev(dev), iort_pci_iommu_init, &info); + + if (!err && !iort_pci_rc_supports_ats(node)) + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS; } else { int i = 0; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 3dbeb457fb16..ed6738c358ca 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -509,10 +509,14 @@ struct iommu_fwspec { const struct iommu_ops *ops; struct fwnode_handle *iommu_fwnode; void *iommu_priv; + u32 flags; unsigned int num_ids; u32 ids[1]; }; +/* Firmware disabled ATS in the root complex */ +#define IOMMU_FWSPEC_PCI_NO_ATS (1 << 0) + int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode, const struct iommu_ops *ops); void iommu_fwspec_free(struct device *dev);
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. Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> --- drivers/acpi/arm64/iort.c | 11 +++++++++++ include/linux/iommu.h | 4 ++++ 2 files changed, 15 insertions(+)