[PATCH] drivers: iommu: Directly use ida_alloc()/free()
Use ida_alloc()/ida_free() instead of deprecated ida_simple_get()/ida_simple_remove() . Signed-off-by: keliu --- drivers/iommu/iommu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 857d4c2fd1a2..8d56146a0b44 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -595,7 +595,7 @@ static void iommu_group_release(struct kobject *kobj) if (group->iommu_data_release) group->iommu_data_release(group->iommu_data); - ida_simple_remove(&iommu_group_ida, group->id); + ida_free(&iommu_group_ida, group->id); if (group->default_domain) iommu_domain_free(group->default_domain); @@ -635,7 +635,7 @@ struct iommu_group *iommu_group_alloc(void) INIT_LIST_HEAD(&group->entry); BLOCKING_INIT_NOTIFIER_HEAD(&group->notifier); - ret = ida_simple_get(&iommu_group_ida, 0, 0, GFP_KERNEL); + ret = ida_alloc(&iommu_group_ida, GFP_KERNEL); if (ret < 0) { kfree(group); return ERR_PTR(ret); @@ -645,7 +645,7 @@ struct iommu_group *iommu_group_alloc(void) ret = kobject_init_and_add(&group->kobj, &iommu_group_ktype, NULL, "%d", group->id); if (ret) { - ida_simple_remove(&iommu_group_ida, group->id); + ida_free(&iommu_group_ida, group->id); kobject_put(&group->kobj); return ERR_PTR(ret); } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH V3 2/2] net: netvsc: Allocate per-device swiotlb bounce buffer for netvsc
On 5/27/2022 2:43 AM, Dexuan Cui wrote: From: Tianyu Lan Sent: Thursday, May 26, 2022 5:01 AM ... @@ -119,6 +124,10 @@ static void netvsc_subchan_work(struct work_struct *w) nvdev->max_chn = 1; nvdev->num_chn = 1; } + + /* Allocate boucne buffer.*/ + swiotlb_device_allocate(&hdev->device, nvdev->num_chn, + 10 * IO_TLB_BLOCK_UNIT); } Looks like swiotlb_device_allocate() is not called if the netvsc device has only 1 primary channel and no sub-schannel, e.g. in the case of single-vCPU VM? When there is only sinlge,there seems not to be much performance penalty. But you are right, we should keep the same behavior when single CPU and multi CPU. Will update in the next version. Thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 20/21] PCI/P2PDMA: Introduce pci_mmap_p2pmem()
On Thu, Apr 07, 2022 at 09:47:16AM -0600, Logan Gunthorpe wrote: > +static void pci_p2pdma_unmap_mappings(void *data) > +{ > + struct pci_dev *pdev = data; > + struct pci_p2pdma *p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); > + > + /* Ensure no new pages can be allocated in mappings */ > + p2pdma->active = false; > + synchronize_rcu(); > + > + unmap_mapping_range(p2pdma->inode->i_mapping, 0, 0, 1); > + > + /* > + * On some architectures, TLB flushes are done with call_rcu() > + * so to ensure GUP fast is done with the pages, call synchronize_rcu() > + * before freeing them. > + */ > + synchronize_rcu(); > + pci_p2pdma_free_mappings(p2pdma->inode->i_mapping); With the series from Felix getting close this should get updated to not set pte_devmap and use proper natural refcounting without any of this stuff. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] drivers: iommu: Directly use ida_alloc()/free()
On Fri, May 27, 2022 at 07:03:07AM +, keliu wrote: > Use ida_alloc()/ida_free() instead of deprecated > ida_simple_get()/ida_simple_remove() . > > Signed-off-by: keliu > --- > drivers/iommu/iommu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/12] iommu/vt-d: Use iommu_get_domain_for_dev() in debugfs
On Fri, May 27, 2022 at 02:30:08PM +0800, Lu Baolu wrote: > Retrieve the attached domain for a device through the generic interface > exposed by the iommu core. This also makes device_domain_lock static. > > Signed-off-by: Lu Baolu > drivers/iommu/intel/iommu.h | 1 - > drivers/iommu/intel/debugfs.c | 20 > drivers/iommu/intel/iommu.c | 2 +- > 3 files changed, 9 insertions(+), 14 deletions(-) > > diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h > index a22adfbdf870..8a6d64d726c0 100644 > +++ b/drivers/iommu/intel/iommu.h > @@ -480,7 +480,6 @@ enum { > #define VTD_FLAG_SVM_CAPABLE (1 << 2) > > extern int intel_iommu_sm; > -extern spinlock_t device_domain_lock; > > #define sm_supported(iommu) (intel_iommu_sm && ecap_smts((iommu)->ecap)) > #define pasid_supported(iommu) (sm_supported(iommu) && > \ > diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c > index d927ef10641b..eea8727aa7bc 100644 > +++ b/drivers/iommu/intel/debugfs.c > @@ -344,19 +344,21 @@ static void pgtable_walk_level(struct seq_file *m, > struct dma_pte *pde, > > static int show_device_domain_translation(struct device *dev, void *data) > { > - struct device_domain_info *info = dev_iommu_priv_get(dev); > - struct dmar_domain *domain = info->domain; > + struct dmar_domain *dmar_domain; > + struct iommu_domain *domain; > struct seq_file *m = data; > u64 path[6] = { 0 }; > > + domain = iommu_get_domain_for_dev(dev); > if (!domain) > return 0; The iommu_get_domain_for_dev() API should be called something like 'iommu_get_dma_api_domain()' and clearly documented that it is safe to call only so long as a DMA API using driver is attached to the device, which is most of the current callers. This use in random sysfs inside the iommu driver is not OK because it doesn't have any locking protecting domain from concurrent free. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/12] iommu/vt-d: Remove for_each_device_domain()
On Fri, May 27, 2022 at 02:30:09PM +0800, Lu Baolu wrote: > The per-device device_domain_info data could be retrieved from the > device itself. There's no need to search a global list. > > Signed-off-by: Lu Baolu > --- > drivers/iommu/intel/iommu.h | 2 -- > drivers/iommu/intel/iommu.c | 25 - > drivers/iommu/intel/pasid.c | 37 +++-- > 3 files changed, 11 insertions(+), 53 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
On Fri, May 27, 2022 at 02:30:10PM +0800, Lu Baolu wrote: > The disable_dmar_iommu() is called when IOMMU initialzation fails or > the IOMMU is hot-removed from the system. In both cases, there is no > need to clear the IOMMU translation data structures for devices. > > On the initialization path, the device probing only happens after the > IOMMU is initialized successfully, hence there're no translation data > structures. > > On the hot-remove path, there is no real use case where the IOMMU is > hot-removed, but the devices that it manages are still alive in the > system. The translation data structures were torn down during device > release, hence there's no need to repeat it in IOMMU hot-remove path > either. Can you leave behind a 1 statement WARN_ON of some kind to check this? Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 04/12] iommu/vt-d: Use pci_get_domain_bus_and_slot() in pgtable_walk()
On Fri, May 27, 2022 at 02:30:11PM +0800, Lu Baolu wrote: > Use pci_get_domain_bus_and_slot() instead of searching the global list > to retrieve the pci device pointer. This removes device_domain_list > global list as there are no consumers anymore. > > Signed-off-by: Lu Baolu > --- > drivers/iommu/intel/iommu.h | 1 - > drivers/iommu/intel/iommu.c | 33 ++--- > 2 files changed, 6 insertions(+), 28 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/12] iommu/vt-d: Check device list of domain in domain free path
On Fri, May 27, 2022 at 02:30:16PM +0800, Lu Baolu wrote: > When the IOMMU domain is about to be freed, it should not be set on any > device. Instead of silently dealing with some bug cases, it's better to > trigger a warning to report and fix any potential bugs at the first time. > > Signed-off-by: Lu Baolu > --- > drivers/iommu/intel/iommu.c | 17 ++--- > 1 file changed, 2 insertions(+), 15 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 20/21] PCI/P2PDMA: Introduce pci_mmap_p2pmem()
On 2022-05-27 06:55, Jason Gunthorpe wrote: > On Thu, Apr 07, 2022 at 09:47:16AM -0600, Logan Gunthorpe wrote: >> +static void pci_p2pdma_unmap_mappings(void *data) >> +{ >> +struct pci_dev *pdev = data; >> +struct pci_p2pdma *p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); >> + >> +/* Ensure no new pages can be allocated in mappings */ >> +p2pdma->active = false; >> +synchronize_rcu(); >> + >> +unmap_mapping_range(p2pdma->inode->i_mapping, 0, 0, 1); >> + >> +/* >> + * On some architectures, TLB flushes are done with call_rcu() >> + * so to ensure GUP fast is done with the pages, call synchronize_rcu() >> + * before freeing them. >> + */ >> +synchronize_rcu(); >> +pci_p2pdma_free_mappings(p2pdma->inode->i_mapping); > > With the series from Felix getting close this should get updated to > not set pte_devmap and use proper natural refcounting without any of > this stuff. Can you send a link? I'm not sure what you are referring to. Thanks, Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 20/21] PCI/P2PDMA: Introduce pci_mmap_p2pmem()
On Fri, May 27, 2022 at 09:35:07AM -0600, Logan Gunthorpe wrote: > > > On 2022-05-27 06:55, Jason Gunthorpe wrote: > > On Thu, Apr 07, 2022 at 09:47:16AM -0600, Logan Gunthorpe wrote: > >> +static void pci_p2pdma_unmap_mappings(void *data) > >> +{ > >> + struct pci_dev *pdev = data; > >> + struct pci_p2pdma *p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); > >> + > >> + /* Ensure no new pages can be allocated in mappings */ > >> + p2pdma->active = false; > >> + synchronize_rcu(); > >> + > >> + unmap_mapping_range(p2pdma->inode->i_mapping, 0, 0, 1); > >> + > >> + /* > >> + * On some architectures, TLB flushes are done with call_rcu() > >> + * so to ensure GUP fast is done with the pages, call synchronize_rcu() > >> + * before freeing them. > >> + */ > >> + synchronize_rcu(); > >> + pci_p2pdma_free_mappings(p2pdma->inode->i_mapping); > > > > With the series from Felix getting close this should get updated to > > not set pte_devmap and use proper natural refcounting without any of > > this stuff. > > Can you send a link? I'm not sure what you are referring to. IIRC this is the last part: https://lore.kernel.org/linux-mm/20220524190632.3304-1-alex.sie...@amd.com/ And the earlier bit with Christoph's pieces looks like it might get merged to v5.19.. The general idea is once pte_devmap is not set then all the refcounting works the way it should. This is what all new ZONE_DEVICE users should do.. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/6] iommu/qcom: Index contexts by asid number to allow asid 0
From: AngeloGioacchino Del Regno This driver was indexing the contexts by asid-1, which is probably done under the assumption that the first ASID is always 1. Unfortunately this is not entirely true: at least in the MSM8956 and MSM8976 GPU IOMMU, the gpu_user context's ASID number is zero. To allow using an asid number of zero, stop indexing the contexts by asid-1 and rather index them by asid. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 530aa92bf6a1..4fefbab15b71 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -53,7 +53,7 @@ struct qcom_iommu_dev { u32 sec_id; u8 num_ctxs; bool use_aarch64_pt; - struct qcom_iommu_ctx *ctxs[]; /* indexed by asid-1 */ + struct qcom_iommu_ctx *ctxs[]; /* indexed by asid */ }; struct qcom_iommu_ctx { @@ -95,7 +95,7 @@ static struct qcom_iommu_ctx * to_ctx(struct qcom_iommu_domain *d, unsigned asid struct qcom_iommu_dev *qcom_iommu = d->iommu; if (!qcom_iommu) return NULL; - return qcom_iommu->ctxs[asid - 1]; + return qcom_iommu->ctxs[asid]; } static inline void @@ -614,12 +614,10 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) qcom_iommu = platform_get_drvdata(iommu_pdev); /* make sure the asid specified in dt is valid, so we don't have -* to sanity check this elsewhere, since 'asid - 1' is used to -* index into qcom_iommu->ctxs: +* to sanity check this elsewhere: */ - if (WARN_ON(asid < 1) || - WARN_ON(asid > qcom_iommu->num_ctxs) || - WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { + if (WARN_ON(asid >= qcom_iommu->num_ctxs) || + WARN_ON(qcom_iommu->ctxs[asid] == NULL)) { put_device(&iommu_pdev->dev); return -EINVAL; } @@ -778,7 +776,7 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev) dev_dbg(dev, "found asid %u\n", ctx->asid); - qcom_iommu->ctxs[ctx->asid - 1] = ctx; + qcom_iommu->ctxs[ctx->asid] = ctx; return 0; } @@ -790,7 +788,7 @@ static int qcom_iommu_ctx_remove(struct platform_device *pdev) platform_set_drvdata(pdev, NULL); - qcom_iommu->ctxs[ctx->asid - 1] = NULL; + qcom_iommu->ctxs[ctx->asid] = NULL; return 0; } @@ -828,7 +826,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct resource *res; struct clk *clk; - int ret, max_asid = 0; + int ret, num_ctxs, max_asid = 0; /* find the max asid (which is 1:1 to ctx bank idx), so we know how * many child ctx devices we have: @@ -836,11 +834,13 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) for_each_child_of_node(dev->of_node, child) max_asid = max(max_asid, get_asid(child)); - qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, max_asid), + num_ctxs = max_asid + 1; + + qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, num_ctxs), GFP_KERNEL); if (!qcom_iommu) return -ENOMEM; - qcom_iommu->num_ctxs = max_asid; + qcom_iommu->num_ctxs = num_ctxs; qcom_iommu->dev = dev; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/6] iommu/qcom: Add support for AArch64 IOMMU pagetables
From: AngeloGioacchino Del Regno Some IOMMUs associated with some TZ firmwares may support switching to the AArch64 pagetable format by sending a "set pagetable format" scm command indicating the IOMMU secure ID and the context number to switch. Add a DT property "qcom,use-aarch64-pagetables" for this driver to send this command to the secure world and to switch the pagetable format to benefit of the ARM64 IOMMU pagetables, where possible. Note that, even though the command should be valid to switch each context, the property is made global because: 1. It doesn't make too much sense to switch only one or two context(s) to AA64 instead of just the entire thing 2. Some IOMMUs will go crazy and produce spectacular results when trying to mix up the pagetables on a per-context basis. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- .../devicetree/bindings/iommu/qcom,iommu.txt | 2 + drivers/iommu/arm/arm-smmu/qcom_iommu.c | 54 +++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt index ba0b77889f02..72ae0595efff 100644 --- a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt @@ -47,6 +47,8 @@ to non-secure vs secure interrupt line. secure lines. (Ie. if the iommu contains secure context banks) - qcom,ctx-num : The number associated to the context bank +- qcom,use-aarch64-pagetables : Switch to AArch64 pagetable format on all +contexts declared in this IOMMU ** Examples: diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 129e322f56a6..530aa92bf6a1 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -52,6 +52,7 @@ struct qcom_iommu_dev { void __iomem*local_base; u32 sec_id; u8 num_ctxs; + bool use_aarch64_pt; struct qcom_iommu_ctx *ctxs[]; /* indexed by asid-1 */ }; @@ -164,11 +165,17 @@ static void qcom_iommu_tlb_inv_range_nosync(unsigned long iova, size_t size, reg = leaf ? ARM_SMMU_CB_S1_TLBIVAL : ARM_SMMU_CB_S1_TLBIVA; for (i = 0; i < fwspec->num_ids; i++) { + struct qcom_iommu_dev *qcom_iommu = qcom_domain->iommu; struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); size_t s = size; - iova = (iova >> 12) << 12; - iova |= ctx->asid; + if (qcom_iommu->use_aarch64_pt) { + iova >>= 12; + iova |= (unsigned long)ctx->asid << 48; + } else { + iova &= (1UL << 12) - 1UL; + iova |= ctx->asid; + } do { iommu_writel(ctx, reg, iova); iova += granule; @@ -248,6 +255,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct io_pgtable_ops *pgtbl_ops; struct io_pgtable_cfg pgtbl_cfg; + enum io_pgtable_fmt pgtbl_fmt; + unsigned long ias, oas; int i, ret = 0; u32 reg; @@ -255,10 +264,19 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, if (qcom_domain->iommu) goto out_unlock; + if (qcom_iommu->use_aarch64_pt) { + pgtbl_fmt = ARM_64_LPAE_S1; + ias = oas = 48; + } else { + pgtbl_fmt = ARM_32_LPAE_S1; + ias = 32; + oas = 40; + } + pgtbl_cfg = (struct io_pgtable_cfg) { .pgsize_bitmap = qcom_iommu_ops.pgsize_bitmap, - .ias= 32, - .oas= 40, + .ias= ias, + .oas= oas, .tlb= &qcom_flush_ops, .iommu_dev = qcom_iommu->dev, }; @@ -266,7 +284,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, qcom_domain->iommu = qcom_iommu; qcom_domain->fwspec = fwspec; - pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, &pgtbl_cfg, qcom_domain); + pgtbl_ops = alloc_io_pgtable_ops(pgtbl_fmt, &pgtbl_cfg, qcom_domain); if (!pgtbl_ops) { dev_err(qcom_iommu->dev, "failed to allocate pagetable ops\n"); ret = -ENOMEM; @@ -280,6 +298,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, for (i = 0; i < fwspec->num_ids; i++) { struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); + u32 tcr
[PATCH 6/6] iommu/qcom: Add support for QCIOMMUv2 and QCIOMMU-500 secured contexts
From: AngeloGioacchino Del Regno This IOMMU is yet another Qualcomm variant of known IOMMUs, found in Family-B SoCs, such as MSM8956, MSM8976, MSM8953, MSM8917 and others, and that firmware perfectly adheres to this driver logic. This time, though, the catch is that the secure contexts are also secured, meaning that these are programmed by the bootloader or TZ and their "interesting" registers are locked out, so the hypervisor disallows touching them from the non-secure world: in this case the OS is supposed to blindly trust the secure configuration of these contexts and just use them "as they are". For this reason, it is necessary to distinguish between the v1 and 500/v2 secure contexts in this driver in order to adhere to this specification. To do this, add a new DT compatible, named "qcom,msm-iommu-v2-sec" that will trigger the new behavior. For the sake of completeness, also add a "qcom,msm-iommu-v2-ns" so that the human eye gets pleased with it when reading the contexts in the final SoC DT. Of course, the latter is just cosmetic. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- .../devicetree/bindings/iommu/qcom,iommu.txt | 2 ++ drivers/iommu/arm/arm-smmu/qcom_iommu.c| 18 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt index 72ae0595efff..861c0cd9c512 100644 --- a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt @@ -36,6 +36,8 @@ to non-secure vs secure interrupt line. - compatible : Should be one of: - "qcom,msm-iommu-v1-ns" : non-secure context bank - "qcom,msm-iommu-v1-sec" : secure context bank +- "qcom,msm-iommu-v2-ns" : non-secure QSMMUv2/QSMMU500 context bank +- "qcom,msm-iommu-v2-sec" : secure QSMMUv2/QSMMU500 context bank - reg: Base address and size of context bank within the iommu - interrupts : The context fault irq. diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 4fefbab15b71..aa7359ae34a9 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -60,6 +60,7 @@ struct qcom_iommu_ctx { struct device *dev; void __iomem*base; bool secure_init; + bool secured_ctx; u8 asid; /* asid and ctx bank # are 1:1 */ struct iommu_domain *domain; }; @@ -309,6 +310,12 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, ctx->secure_init = true; } + /* Secured QSMMU-500/QSMMU-v2 contexts cannot be programmed */ + if (ctx->secured_ctx) { + ctx->domain = domain; + break; + } + qcom_iommu_reset_ctx(ctx); @@ -751,10 +758,14 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev) if (irq < 0) return -ENODEV; + if (of_device_is_compatible(dev->of_node, "qcom,msm-iommu-v2-sec")) + ctx->secured_ctx = true; + /* clear IRQs before registering fault handler, just in case the * boot-loader left us a surprise: */ - iommu_writel(ctx, ARM_SMMU_CB_FSR, iommu_readl(ctx, ARM_SMMU_CB_FSR)); + if (!ctx->secured_ctx) + iommu_writel(ctx, ARM_SMMU_CB_FSR, iommu_readl(ctx, ARM_SMMU_CB_FSR)); ret = devm_request_irq(dev, irq, qcom_iommu_fault, @@ -796,6 +807,8 @@ static int qcom_iommu_ctx_remove(struct platform_device *pdev) static const struct of_device_id ctx_of_match[] = { { .compatible = "qcom,msm-iommu-v1-ns" }, { .compatible = "qcom,msm-iommu-v1-sec" }, + { .compatible = "qcom,msm-iommu-v2-ns" }, + { .compatible = "qcom,msm-iommu-v2-sec" }, { /* sentinel */ } }; @@ -813,7 +826,8 @@ static bool qcom_iommu_has_secure_context(struct qcom_iommu_dev *qcom_iommu) struct device_node *child; for_each_child_of_node(qcom_iommu->dev->of_node, child) - if (of_device_is_compatible(child, "qcom,msm-iommu-v1-sec")) + if (of_device_is_compatible(child, "qcom,msm-iommu-v1-sec") || + of_device_is_compatible(child, "qcom,msm-iommu-v2-sec")) return true; return false; -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/6] iommu/qcom: Write TCR before TTBRs to fix ASID access behavior
From: AngeloGioacchino Del Regno As also stated in the arm-smmu driver, we must write the TCR before writing the TTBRs, since the TCR determines the access behavior of some fields. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 1728d4d7fe25..75f353866c40 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -273,18 +273,18 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, ctx->secure_init = true; } - /* TTBRs */ - iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, - pgtbl_cfg.arm_lpae_s1_cfg.ttbr | - FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid)); - iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0); - /* TCR */ iommu_writel(ctx, ARM_SMMU_CB_TCR2, arm_smmu_lpae_tcr2(&pgtbl_cfg)); iommu_writel(ctx, ARM_SMMU_CB_TCR, arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE); + /* TTBRs */ + iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, + pgtbl_cfg.arm_lpae_s1_cfg.ttbr | + FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid)); + iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0); + /* MAIRs (stage-1 only) */ iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0, pgtbl_cfg.arm_lpae_s1_cfg.mair); -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/6] Fix and extend Qualcomm IOMMU support
Due to Qualcomm's software solutions, genius or otherwise, the IOMMUs on pre-msm8998-and-sdm630 SoCs are flawed beyond reason, and conviniently it is all due to an (almost in all cases) irreplaceable hypervisor and TZ stack. As end users and developers, we have no choice but to adapt to that and follow whatever the IOMMUs expect. This series fixes and improves the existing support (adjusts the code flow to make sure things are written in correct order and adds a way to properly (i.e. in compliance with the firmware's expectations) reset the IOMMUs) and extends it with features for the SoCs that came near the end of an era of what we call "qcom_iommu" upstream, namely 8952 family (8917, 8937, 8952, 8956/76, 8953 and possibly more) (Aarch64 pagetables and secured QCIOMMUv2/QCIOMMU_500 ctxs) and at the same time builds another milestone in getting msm8974/94 IOMMU support that has been in the works for something like 7 years, and never got upstreamed in the end (we'll get it one day, eventually...). The dt-bindings are NOT converted to YAML as a part of this series, that will come in a later patchset. AngeloGioacchino Del Regno (6): iommu/qcom: Use the asid read from device-tree if specified iommu/qcom: Write TCR before TTBRs to fix ASID access behavior iommu/qcom: Properly reset the IOMMU context iommu/qcom: Add support for AArch64 IOMMU pagetables iommu/qcom: Index contexts by asid number to allow asid 0 iommu/qcom: Add support for QCIOMMUv2 and QCIOMMU-500 secured contexts .../devicetree/bindings/iommu/qcom,iommu.txt | 5 + drivers/iommu/arm/arm-smmu/qcom_iommu.c | 137 ++ 2 files changed, 114 insertions(+), 28 deletions(-) -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/6] iommu/qcom: Properly reset the IOMMU context
From: AngeloGioacchino Del Regno To avoid context faults reset the context entirely on detach and to ensure a fresh clean start also do a complete reset before programming the context for domain initialization. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 75f353866c40..129e322f56a6 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -223,6 +223,23 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev) return IRQ_HANDLED; } +static void qcom_iommu_reset_ctx(struct qcom_iommu_ctx *ctx) +{ + iommu_writel(ctx, ARM_SMMU_CB_FAR, 0); + iommu_writel(ctx, ARM_SMMU_CB_FSR, 0); + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1, 0); + iommu_writel(ctx, ARM_SMMU_CB_PAR, 0); + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0, 0); + iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); + iommu_writel(ctx, ARM_SMMU_CB_TCR2, 0); + iommu_writel(ctx, ARM_SMMU_CB_TCR, 0); + iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, 0); + iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0); + + /* Should we issue a TLBSYNC there instead? */ + wmb(); +} + static int qcom_iommu_init_domain(struct iommu_domain *domain, struct qcom_iommu_dev *qcom_iommu, struct device *dev) @@ -273,6 +290,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, ctx->secure_init = true; } + qcom_iommu_reset_ctx(ctx); + /* TCR */ iommu_writel(ctx, ARM_SMMU_CB_TCR2, arm_smmu_lpae_tcr2(&pgtbl_cfg)); @@ -406,8 +425,8 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de for (i = 0; i < fwspec->num_ids; i++) { struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); - /* Disable the context bank: */ - iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); + /* Disable and reset the context bank */ + qcom_iommu_reset_ctx(ctx); ctx->domain = NULL; } -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/6] iommu/qcom: Use the asid read from device-tree if specified
From: AngeloGioacchino Del Regno As specified in this driver, the context banks are 0x1000 apart. Problem is that sometimes the context number (our asid) does not match this logic and we end up using the wrong one: this starts being a problem in the case that we need to send TZ commands to do anything on a specific context. For this reason, read the ASID from the DT if the property "qcom,ctx-num" is present on the IOMMU context node. Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Marijn Suijten Signed-off-by: Konrad Dybcio --- .../devicetree/bindings/iommu/qcom,iommu.txt | 1 + drivers/iommu/arm/arm-smmu/qcom_iommu.c| 18 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt index 059139abce35..ba0b77889f02 100644 --- a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt @@ -46,6 +46,7 @@ to non-secure vs secure interrupt line. for routing of context bank irq's to secure vs non- secure lines. (Ie. if the iommu contains secure context banks) +- qcom,ctx-num : The number associated to the context bank ** Examples: diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 4c077c38fbd6..1728d4d7fe25 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -566,7 +566,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) * index into qcom_iommu->ctxs: */ if (WARN_ON(asid < 1) || - WARN_ON(asid > qcom_iommu->num_ctxs)) { + WARN_ON(asid > qcom_iommu->num_ctxs) || + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { put_device(&iommu_pdev->dev); return -EINVAL; } @@ -654,7 +655,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev) static int get_asid(const struct device_node *np) { - u32 reg; + u32 reg, val; + int asid; /* read the "reg" property directly to get the relative address * of the context bank, and calculate the asid from that: @@ -662,7 +664,17 @@ static int get_asid(const struct device_node *np) if (of_property_read_u32_index(np, "reg", 0, ®)) return -ENODEV; - return reg / 0x1000; /* context banks are 0x1000 apart */ + /* +* Context banks are 0x1000 apart but, in some cases, the ASID +* number doesn't match to this logic and needs to be passed +* from the DT configuration explicitly. +*/ + if (of_property_read_u32(np, "qcom,ctx-num", &val)) + asid = reg / 0x1000; + else + asid = val; + + return asid; } static int qcom_iommu_ctx_probe(struct platform_device *pdev) -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 20/21] PCI/P2PDMA: Introduce pci_mmap_p2pmem()
On 2022-05-27 13:03, Jason Gunthorpe wrote: > On Fri, May 27, 2022 at 09:35:07AM -0600, Logan Gunthorpe wrote: >> >> >> On 2022-05-27 06:55, Jason Gunthorpe wrote: >>> On Thu, Apr 07, 2022 at 09:47:16AM -0600, Logan Gunthorpe wrote: +static void pci_p2pdma_unmap_mappings(void *data) +{ + struct pci_dev *pdev = data; + struct pci_p2pdma *p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); + + /* Ensure no new pages can be allocated in mappings */ + p2pdma->active = false; + synchronize_rcu(); + + unmap_mapping_range(p2pdma->inode->i_mapping, 0, 0, 1); + + /* + * On some architectures, TLB flushes are done with call_rcu() + * so to ensure GUP fast is done with the pages, call synchronize_rcu() + * before freeing them. + */ + synchronize_rcu(); + pci_p2pdma_free_mappings(p2pdma->inode->i_mapping); >>> >>> With the series from Felix getting close this should get updated to >>> not set pte_devmap and use proper natural refcounting without any of >>> this stuff. >> >> Can you send a link? I'm not sure what you are referring to. > > IIRC this is the last part: > > https://lore.kernel.org/linux-mm/20220524190632.3304-1-alex.sie...@amd.com/ > > And the earlier bit with Christoph's pieces looks like it might get > merged to v5.19.. > > The general idea is once pte_devmap is not set then all the > refcounting works the way it should. This is what all new ZONE_DEVICE > users should do.. Ok, I don't actually follow how those patches relate to this. Based on your description I guess I don't need to set PFN_DEV and perhaps not use vmf_insert_mixed()? And then just use vm_normal_page()? But the refcounting of the pages seemed like it was already sane to me, unless you mean that the code no longer has to synchronize_rcu() before returning the pages... that would be spectacular and clean things up a lot (plus fix an annoying issue where if you use then free all the memory you can't allocate new memory for an indeterminate amount of time). Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/6] iommu/qcom: Add support for AArch64 IOMMU pagetables
Hi Konrad, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on joro-iommu/next] [also build test WARNING on v5.18 next-20220527] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/iommu-qcom-Use-the-asid-read-from-device-tree-if-specified/20220528-062952 base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220528/202205280904.vsncfpph-...@intel.com/config) compiler: arceb-elf-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/0744f7d6ebfff8d6854a24d0f95f8e58885b5212 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Konrad-Dybcio/iommu-qcom-Use-the-asid-read-from-device-tree-if-specified/20220528-062952 git checkout 0744f7d6ebfff8d6854a24d0f95f8e58885b5212 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/iommu/arm/arm-smmu/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/iommu/arm/arm-smmu/qcom_iommu.c: In function 'qcom_iommu_tlb_inv_range_nosync': >> drivers/iommu/arm/arm-smmu/qcom_iommu.c:174:58: warning: left shift count >= >> width of type [-Wshift-count-overflow] 174 | iova |= (unsigned long)ctx->asid << 48; | ^~ vim +174 drivers/iommu/arm/arm-smmu/qcom_iommu.c 157 158 static void qcom_iommu_tlb_inv_range_nosync(unsigned long iova, size_t size, 159 size_t granule, bool leaf, void *cookie) 160 { 161 struct qcom_iommu_domain *qcom_domain = cookie; 162 struct iommu_fwspec *fwspec = qcom_domain->fwspec; 163 unsigned i, reg; 164 165 reg = leaf ? ARM_SMMU_CB_S1_TLBIVAL : ARM_SMMU_CB_S1_TLBIVA; 166 167 for (i = 0; i < fwspec->num_ids; i++) { 168 struct qcom_iommu_dev *qcom_iommu = qcom_domain->iommu; 169 struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); 170 size_t s = size; 171 172 if (qcom_iommu->use_aarch64_pt) { 173 iova >>= 12; > 174 iova |= (unsigned long)ctx->asid << 48; 175 } else { 176 iova &= (1UL << 12) - 1UL; 177 iova |= ctx->asid; 178 } 179 do { 180 iommu_writel(ctx, reg, iova); 181 iova += granule; 182 } while (s -= granule); 183 } 184 } 185 -- 0-DAY CI Kernel Test Service https://01.org/lkp ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu