Re: [PATCH] iommu: fix ARM_SMMU vs QCOM_SCM compilation
On 10/9/21 9:33 PM, Dmitry Baryshkov wrote: After commit 424953cf3c66 ("qcom_scm: hide Kconfig symbol") arm-smmu got qcom_smmu_impl_init() call guarded by IS_ENABLED(CONFIG_ARM_SMMU_QCOM). However the CONFIG_ARM_SMMU_QCOM Kconfig entry does not exist, so the qcom_smmu_impl_init() is never called. So, let's fix this by always calling qcom_smmu_impl_init(). It does not touch the smmu passed unless the device is a non-Qualcomm one. Make ARM_SMMU select QCOM_SCM for ARCH_QCOM. Fixes: 424953cf3c66 ("qcom_scm: hide Kconfig symbol") Cc: Arnd Bergmann Reported-by: Daniel Lezcano Signed-off-by: Dmitry Baryshkov --- drivers/iommu/Kconfig | 1 + drivers/iommu/arm/arm-smmu/Makefile| 3 +-- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 9 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index c5c71b7ab7e8..a4593e53fe7d 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -309,6 +309,7 @@ config ARM_SMMU tristate "ARM Ltd. System MMU (SMMU) Support" depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64) select IOMMU_API + select QCOM_SCM select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU if ARM help diff --git a/drivers/iommu/arm/arm-smmu/Makefile b/drivers/iommu/arm/arm-smmu/Makefile index b0cc01aa20c9..e240a7bcf310 100644 --- a/drivers/iommu/arm/arm-smmu/Makefile +++ b/drivers/iommu/arm/arm-smmu/Makefile @@ -1,5 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o obj-$(CONFIG_ARM_SMMU) += arm_smmu.o -arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o -arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o +arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 2c25cce38060..8199185dd262 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -215,8 +215,13 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); - if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM)) - smmu = qcom_smmu_impl_init(smmu); + /* +* qcom_smmu_impl_init() will not touch smmu if the device is not +* a Qualcomm one. +*/ + smmu = qcom_smmu_impl_init(smmu); + if (IS_ERR(smmu)) + return smmu; if (of_device_is_compatible(np, "marvell,ap806-smmu-500")) smmu->impl = _mmu500_impl; Without this applied, my Lenovo Yoga C630 just boot loops with 424953cf3c66 applied when it gets to the smmu. This fixes it. Tested-By: Steev Klimaszewski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: fix ARM_SMMU vs QCOM_SCM compilation
On Sat 09 Oct 21:33 CDT 2021, Dmitry Baryshkov wrote: > After commit 424953cf3c66 ("qcom_scm: hide Kconfig symbol") arm-smmu got > qcom_smmu_impl_init() call guarded by IS_ENABLED(CONFIG_ARM_SMMU_QCOM). > However the CONFIG_ARM_SMMU_QCOM Kconfig entry does not exist, so the > qcom_smmu_impl_init() is never called. > > So, let's fix this by always calling qcom_smmu_impl_init(). It does not > touch the smmu passed unless the device is a non-Qualcomm one. Make > ARM_SMMU select QCOM_SCM for ARCH_QCOM. > Arnd's intention was to not force QCOM_SCM to be built on non-Qualcomm devices. But as Daniel experienced, attempting to boot most Qualcomm boards without this results in a instant reboot. I think it's okay if we tinker with CONFIG_ARM_SMMU_QCOM for v5.16, but we're getting late in v5.15 so I would prefer if we make sure this works out of the box. Reviewed-by: Bjorn Andersson Regards, Bjorn > Fixes: 424953cf3c66 ("qcom_scm: hide Kconfig symbol") > Cc: Arnd Bergmann > Reported-by: Daniel Lezcano > Signed-off-by: Dmitry Baryshkov > --- > drivers/iommu/Kconfig | 1 + > drivers/iommu/arm/arm-smmu/Makefile| 3 +-- > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 9 +++-- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index c5c71b7ab7e8..a4593e53fe7d 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -309,6 +309,7 @@ config ARM_SMMU > tristate "ARM Ltd. System MMU (SMMU) Support" > depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64) > select IOMMU_API > + select QCOM_SCM > select IOMMU_IO_PGTABLE_LPAE > select ARM_DMA_USE_IOMMU if ARM > help > diff --git a/drivers/iommu/arm/arm-smmu/Makefile > b/drivers/iommu/arm/arm-smmu/Makefile > index b0cc01aa20c9..e240a7bcf310 100644 > --- a/drivers/iommu/arm/arm-smmu/Makefile > +++ b/drivers/iommu/arm/arm-smmu/Makefile > @@ -1,5 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o > obj-$(CONFIG_ARM_SMMU) += arm_smmu.o > -arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o > -arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o > +arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > index 2c25cce38060..8199185dd262 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > @@ -215,8 +215,13 @@ struct arm_smmu_device *arm_smmu_impl_init(struct > arm_smmu_device *smmu) > of_device_is_compatible(np, "nvidia,tegra186-smmu")) > return nvidia_smmu_impl_init(smmu); > > - if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM)) > - smmu = qcom_smmu_impl_init(smmu); > + /* > + * qcom_smmu_impl_init() will not touch smmu if the device is not > + * a Qualcomm one. > + */ > + smmu = qcom_smmu_impl_init(smmu); > + if (IS_ERR(smmu)) > + return smmu; > > if (of_device_is_compatible(np, "marvell,ap806-smmu-500")) > smmu->impl = _mmu500_impl; > -- > 2.30.2 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: fix ARM_SMMU vs QCOM_SCM compilation
After commit 424953cf3c66 ("qcom_scm: hide Kconfig symbol") arm-smmu got qcom_smmu_impl_init() call guarded by IS_ENABLED(CONFIG_ARM_SMMU_QCOM). However the CONFIG_ARM_SMMU_QCOM Kconfig entry does not exist, so the qcom_smmu_impl_init() is never called. So, let's fix this by always calling qcom_smmu_impl_init(). It does not touch the smmu passed unless the device is a non-Qualcomm one. Make ARM_SMMU select QCOM_SCM for ARCH_QCOM. Fixes: 424953cf3c66 ("qcom_scm: hide Kconfig symbol") Cc: Arnd Bergmann Reported-by: Daniel Lezcano Signed-off-by: Dmitry Baryshkov --- drivers/iommu/Kconfig | 1 + drivers/iommu/arm/arm-smmu/Makefile| 3 +-- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 9 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index c5c71b7ab7e8..a4593e53fe7d 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -309,6 +309,7 @@ config ARM_SMMU tristate "ARM Ltd. System MMU (SMMU) Support" depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64) select IOMMU_API + select QCOM_SCM select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU if ARM help diff --git a/drivers/iommu/arm/arm-smmu/Makefile b/drivers/iommu/arm/arm-smmu/Makefile index b0cc01aa20c9..e240a7bcf310 100644 --- a/drivers/iommu/arm/arm-smmu/Makefile +++ b/drivers/iommu/arm/arm-smmu/Makefile @@ -1,5 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o obj-$(CONFIG_ARM_SMMU) += arm_smmu.o -arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o -arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o +arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 2c25cce38060..8199185dd262 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -215,8 +215,13 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); - if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM)) - smmu = qcom_smmu_impl_init(smmu); + /* +* qcom_smmu_impl_init() will not touch smmu if the device is not +* a Qualcomm one. +*/ + smmu = qcom_smmu_impl_init(smmu); + if (IS_ERR(smmu)) + return smmu; if (of_device_is_compatible(np, "marvell,ap806-smmu-500")) smmu->impl = _mmu500_impl; -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 9/9] iommu/dma: Reserve any RMR regions associated with a dev
On Fri, Oct 8, 2021 at 3:10 PM Robin Murphy wrote: > > On 2021-08-05 09:07, Shameer Kolothum wrote: > > Get ACPI IORT RMR regions associated with a dev reserved > > so that there is a unity mapping for them in SMMU. > > This feels like most of it belongs in the IORT code rather than > iommu-dma (which should save the temporary list copy as well). See previous comment. The original intent was for device-tree to also be able to use these mechanisms to create RMR's and support them in the SMMU. -Jon > > Robin. > > > Signed-off-by: Shameer Kolothum > > --- > > drivers/iommu/dma-iommu.c | 56 +++ > > 1 file changed, 51 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index 1b6e27475279..c1ae0c3d4b33 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -207,22 +207,68 @@ void iommu_dma_put_rmrs(struct fwnode_handle > > *iommu_fwnode, > > } > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, > > + struct iommu_resv_region *e) > > +{ > > + int i; > > + > > + for (i = 0; i < fwspec->num_ids; i++) { > > + if (e->fw_data.rmr.sid == fwspec->ids[i]) > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static void iommu_dma_get_rmr_resv_regions(struct device *dev, > > +struct list_head *list) > > +{ > > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > + struct list_head rmr_list; > > + struct iommu_resv_region *rmr, *tmp; > > + > > + INIT_LIST_HEAD(_list); > > + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, _list)) > > + return; > > + > > + if (dev_is_pci(dev)) { > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct pci_host_bridge *host = > > pci_find_host_bridge(pdev->bus); > > + > > + if (!host->preserve_config) > > + return; > > + } > > + > > + list_for_each_entry_safe(rmr, tmp, _list, list) { > > + if (!iommu_dma_dev_has_rmr(fwspec, rmr)) > > + continue; > > + > > + /* Remove from iommu RMR list and add to dev resv_regions */ > > + list_del_init(>list); > > + list_add_tail(>list, list); > > + } > > + > > + iommu_dma_put_rmrs(fwspec->iommu_fwnode, _list); > > +} > > + > > /** > >* iommu_dma_get_resv_regions - Reserved region driver helper > >* @dev: Device from iommu_get_resv_regions() > >* @list: Reserved region list from iommu_get_resv_regions() > >* > >* IOMMU drivers can use this to implement their .get_resv_regions > > callback > > - * for general non-IOMMU-specific reservations. Currently, this covers > > GICv3 > > - * ITS region reservation on ACPI based ARM platforms that may require HW > > MSI > > - * reservation. > > + * for general non-IOMMU-specific reservations. Currently this covers, > > + * -GICv3 ITS region reservation on ACPI based ARM platforms that may > > + * require HW MSI reservation. > > + * -Any ACPI IORT RMR memory range reservations (IORT spec rev E.b) > >*/ > > void iommu_dma_get_resv_regions(struct device *dev, struct list_head > > *list) > > { > > > > - if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) > > + if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) { > > iort_iommu_msi_get_resv_regions(dev, list); > > - > > + iommu_dma_get_rmr_resv_regions(dev, list); > > + } > > } > > EXPORT_SYMBOL(iommu_dma_get_resv_regions); > > > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing
On Fri, Oct 8, 2021 at 2:49 PM Robin Murphy wrote: > > On 2021-08-05 09:07, Shameer Kolothum wrote: > > Add support for parsing RMR node information from ACPI. > > > > Find the associated streamid and smmu node info from the > > RMR node and populate a linked list with RMR memory > > descriptors. > > > > Signed-off-by: Shameer Kolothum > > --- > > drivers/acpi/arm64/iort.c | 134 +- > > 1 file changed, 133 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > > index 3b23fb775ac4..d76ba46ebe67 100644 > > --- a/drivers/acpi/arm64/iort.c > > +++ b/drivers/acpi/arm64/iort.c > > @@ -40,6 +40,8 @@ struct iort_fwnode { > > static LIST_HEAD(iort_fwnode_list); > > static DEFINE_SPINLOCK(iort_fwnode_lock); > > > > +static LIST_HEAD(iort_rmr_list); /* list of RMR regions from ACPI */ > > + > > /** > >* iort_set_fwnode() - Create iort_fwnode and use it to register > >* iommu data in the iort_fwnode_list > > @@ -393,7 +395,8 @@ static struct acpi_iort_node *iort_node_get_id(struct > > acpi_iort_node *node, > > if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT || > > node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX || > > node->type == ACPI_IORT_NODE_SMMU_V3 || > > - node->type == ACPI_IORT_NODE_PMCG) { > > + node->type == ACPI_IORT_NODE_PMCG || > > + node->type == ACPI_IORT_NODE_RMR) { > > *id_out = map->output_base; > > return parent; > > } > > @@ -1566,6 +1569,134 @@ static void __init iort_enable_acs(struct > > acpi_iort_node *iort_node) > > #else > > static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { } > > #endif > > +static void iort_rmr_desc_check_overlap(struct acpi_iort_rmr_desc *desc, > > u32 count) > > +{ > > + int i, j; > > + > > + for (i = 0; i < count; i++) { > > + u64 end, start = desc[i].base_address, length = > > desc[i].length; > > + > > + end = start + length - 1; > > + > > + /* Check for address overlap */ > > + for (j = i + 1; j < count; j++) { > > + u64 e_start = desc[j].base_address; > > + u64 e_end = e_start + desc[j].length - 1; > > + > > + if (start <= e_end && end >= e_start) > > + pr_err(FW_BUG "RMR descriptor[0x%llx - > > 0x%llx] overlaps, continue anyway\n", > > +start, end); > > + } > > + } > > +} > > + > > +static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node) > > +{ > > + struct acpi_iort_node *smmu; > > + struct acpi_iort_rmr *rmr; > > + struct acpi_iort_rmr_desc *rmr_desc; > > + u32 map_count = iort_node->mapping_count; > > + u32 sid; > > + int i; > > + > > + if (!iort_node->mapping_offset || map_count != 1) { > > + pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n", > > +iort_node); > > + return; > > + } > > + > > + /* Retrieve associated smmu and stream id */ > > + smmu = iort_node_get_id(iort_node, , 0); > > + if (!smmu) { > > + pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node > > %p\n", > > +iort_node); > > + return; > > + } > > + > > + /* Retrieve RMR data */ > > + rmr = (struct acpi_iort_rmr *)iort_node->node_data; > > + if (!rmr->rmr_offset || !rmr->rmr_count) { > > + pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR > > node %p\n", > > +iort_node); > > + return; > > + } > > + > > + rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node, > > + rmr->rmr_offset); > > + > > + iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count); > > + > > + for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) { > > + struct iommu_resv_region *region; > > + enum iommu_resv_type type; > > + int prot = IOMMU_READ | IOMMU_WRITE; > > + u64 addr = rmr_desc->base_address, size = rmr_desc->length; > > + > > + if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) { > > + /* PAGE align base addr and size */ > > + addr &= PAGE_MASK; > > + size = PAGE_ALIGN(size + > > offset_in_page(rmr_desc->base_address)); > > + > > + pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not > > aligned to 64K, continue with [0x%llx - 0x%llx]\n", > > +rmr_desc->base_address, > > +rmr_desc->base_address + rmr_desc->length - 1, > > +addr, addr + size - 1); > > + } > > + if (rmr->flags &
Re: [PATCH v7 1/9] iommu: Introduce a union to struct iommu_resv_region
On Fri, Oct 8, 2021 at 2:14 PM Robin Murphy wrote: > > On 2021-08-05 09:07, Shameer Kolothum wrote: > > A union is introduced to struct iommu_resv_region to hold > > any firmware specific data. This is in preparation to add > > support for IORT RMR reserve regions and the union now holds > > the RMR specific information. > > > > Signed-off-by: Shameer Kolothum > > --- > > include/linux/iommu.h | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > > index 32d448050bf7..bd0e4641c569 100644 > > --- a/include/linux/iommu.h > > +++ b/include/linux/iommu.h > > @@ -114,6 +114,13 @@ enum iommu_resv_type { > > IOMMU_RESV_SW_MSI, > > }; > > > > +struct iommu_iort_rmr_data { > > +#define IOMMU_RMR_REMAP_PERMITTED(1 << 0) > > + u32 flags; > > + u32 sid;/* Stream Id associated with RMR entry */ > > + void *smmu; /* Associated IORT SMMU node pointer */ > > +}; > > Do we really need to duplicate all this data? AFAICS we could just save > the acpi_iort_rmr pointer in the iommu_resv_region (with a forward > declaration here if necessary) and defer parsing its actual mappings > until the point where we can directly consume the results. >From earlier discussions on this patchset, the original goal was also for device-tree mechanisms to be able to hook into this code to support similar RMR's and SMMU initialization, just not through the ACPI / IORT path. > > Robin. > > > + > > /** > >* struct iommu_resv_region - descriptor for a reserved memory region > >* @list: Linked list pointers > > @@ -121,6 +128,7 @@ enum iommu_resv_type { > >* @length: Length of the region in bytes > >* @prot: IOMMU Protection flags (READ/WRITE/...) > >* @type: Type of the reserved region > > + * @rmr: ACPI IORT RMR specific data > >*/ > > struct iommu_resv_region { > > struct list_headlist; > > @@ -128,6 +136,9 @@ struct iommu_resv_region { > > size_t length; > > int prot; > > enum iommu_resv_typetype; > > + union { > > + struct iommu_iort_rmr_data rmr; > > + } fw_data; > > }; > > > > /** > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu