| Message ID | 20191219163033.2608177-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Will, On Thu, Dec 19, 2019 at 05:30:20PM +0100, Jean-Philippe Brucker wrote: > Add support for Substream ID and PASIDs to the SMMUv3 driver. Since v3 > [1], I added review and tested tags where appropriate and applied the > suggested changes, shown in the diff below. Thanks all! > > I'm testing using the zip accelerator on the Hisilicon KunPeng920 and > Zhangfei's uacce module [2]. The full SVA support, which I'll send out > early next year, is available on my branch sva/zip-devel at > https://jpbrucker.net/git/linux/ Is there anything more I should do for the PASID support? Ideally I'd like to get this in v5.6 so I can focus on the rest of the SVA work and on performance improvements. Thanks, Jean > > [1] https://lore.kernel.org/linux-iommu/20191209180514.272727-1-jean-philippe@linaro.org/ > [2] https://lore.kernel.org/linux-iommu/1576465697-27946-1-git-send-email-zhangfei.gao@linaro.org/ > > Jean-Philippe Brucker (13): > iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation > dt-bindings: document PASID property for IOMMU masters > iommu/arm-smmu-v3: Parse PASID devicetree property of platform devices > ACPI/IORT: Parse SSID property of named component node > iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support > iommu/arm-smmu-v3: Add context descriptor tables allocators > iommu/arm-smmu-v3: Add support for Substream IDs > iommu/arm-smmu-v3: Propagate ssid_bits > iommu/arm-smmu-v3: Prepare for handling arm_smmu_write_ctx_desc() > failure > iommu/arm-smmu-v3: Add second level of context descriptor table > iommu/arm-smmu-v3: Improve add_device() error handling > PCI/ATS: Add PASID stubs > iommu/arm-smmu-v3: Add support for PCI PASID > > .../devicetree/bindings/iommu/iommu.txt | 6 + > drivers/acpi/arm64/iort.c | 18 + > drivers/iommu/arm-smmu-v3.c | 467 +++++++++++++++--- > drivers/iommu/of_iommu.c | 6 +- > include/linux/iommu.h | 2 + > include/linux/pci-ats.h | 3 + > 6 files changed, 442 insertions(+), 60 deletions(-) > > -- > Diff since v3: > #diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index cde7af39681c..8e95ecad4c9a 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -589,8 +589,10 @@ struct arm_smmu_cd_table { > }; > > struct arm_smmu_s1_cfg { > + /* Leaf tables or linear table */ > struct arm_smmu_cd_table *tables; > size_t num_tables; > + /* First level tables, when two levels are used */ > __le64 *l1ptr; > dma_addr_t l1ptr_dma; > struct arm_smmu_ctx_desc cd; > @@ -1561,27 +1563,22 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain, > struct arm_smmu_device *smmu = smmu_domain->smmu; > struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg; > > - if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) { > - table = &cfg->tables[0]; > - idx = ssid; > - } else { > - idx = ssid >> CTXDESC_SPLIT; > - if (idx >= cfg->num_tables) > - return NULL; > + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) > + return cfg->tables[0].ptr + ssid * CTXDESC_CD_DWORDS; > > - table = &cfg->tables[idx]; > - if (!table->ptr) { > - if (arm_smmu_alloc_cd_leaf_table(smmu, table, > - CTXDESC_L2_ENTRIES)) > - return NULL; > + idx = ssid >> CTXDESC_SPLIT; > + table = &cfg->tables[idx]; > + if (!table->ptr) { > + if (arm_smmu_alloc_cd_leaf_table(smmu, table, > + CTXDESC_L2_ENTRIES)) > + return NULL; > > - l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS; > - arm_smmu_write_cd_l1_desc(l1ptr, table); > - /* An invalid L1CD can be cached */ > - arm_smmu_sync_cd(smmu_domain, ssid, false); > - } > - idx = ssid & (CTXDESC_L2_ENTRIES - 1); > + l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS; > + arm_smmu_write_cd_l1_desc(l1ptr, table); > + /* An invalid L1CD can be cached */ > + arm_smmu_sync_cd(smmu_domain, ssid, false); > } > + idx = ssid & (CTXDESC_L2_ENTRIES - 1); > return table->ptr + idx * CTXDESC_CD_DWORDS; > } > > @@ -1617,8 +1614,12 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, > u64 val; > bool cd_live; > struct arm_smmu_device *smmu = smmu_domain->smmu; > - __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid); > + __le64 *cdptr; > > + if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax))) > + return -E2BIG; > + > + cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid); > if (!cdptr) > return -ENOMEM; > > @@ -1640,9 +1641,9 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, > cdptr[3] = cpu_to_le64(cd->mair); > > /* > - * STE is live, and the SMMU might fetch this CD at any > - * time. Ensure that it observes the rest of the CD before we > - * enable it. > + * STE is live, and the SMMU might read dwords of this CD in any > + * order. Ensure that it observes valid values before reading > + * V=1. > */ > arm_smmu_sync_cd(smmu_domain, ssid, true); > > @@ -1706,7 +1707,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain) > * Only allocate a leaf table for linear case. With two levels, leaf > * tables are allocated lazily. > */ > - if (!cfg->l1ptr) { > + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) { > ret = arm_smmu_alloc_cd_leaf_table(smmu, &cfg->tables[0], > max_contexts); > if (ret) > @@ -2657,17 +2658,21 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master) > > features = pci_pasid_features(pdev); > if (features < 0) > - return -ENODEV; > + return features; > > num_pasids = pci_max_pasids(pdev); > if (num_pasids <= 0) > - return -ENODEV; > + return num_pasids; > > ret = pci_enable_pasid(pdev, features); > - if (!ret) > - master->ssid_bits = min_t(u8, ilog2(num_pasids), > - master->smmu->ssid_bits); > - return ret; > + if (ret) { > + dev_err(&pdev->dev, "Failed to enable PASID\n"); > + return ret; > + } > + > + master->ssid_bits = min_t(u8, ilog2(num_pasids), > + master->smmu->ssid_bits); > + return 0; > } > > static void arm_smmu_disable_pasid(struct arm_smmu_master *master) > > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
On Thu, Jan 09, 2020 at 03:36:18PM +0100, Jean-Philippe Brucker wrote: > On Thu, Dec 19, 2019 at 05:30:20PM +0100, Jean-Philippe Brucker wrote: > > Add support for Substream ID and PASIDs to the SMMUv3 driver. Since v3 > > [1], I added review and tested tags where appropriate and applied the > > suggested changes, shown in the diff below. Thanks all! > > > > I'm testing using the zip accelerator on the Hisilicon KunPeng920 and > > Zhangfei's uacce module [2]. The full SVA support, which I'll send out > > early next year, is available on my branch sva/zip-devel at > > https://jpbrucker.net/git/linux/ > > Is there anything more I should do for the PASID support? Ideally I'd like > to get this in v5.6 so I can focus on the rest of the SVA work and on > performance improvements. Apologies, I'm just behind with review what with the timing of the new year. You're on the list, but I was hoping to get Robin's TCR stuff dusted off so that Jordan doesn't have to depend on patches languishing on the mailing list and there's also the nvidia stuff to review as well. Going as fast as I can! Will
On Thu, Jan 09, 2020 at 02:41:01PM +0000, Will Deacon wrote: > On Thu, Jan 09, 2020 at 03:36:18PM +0100, Jean-Philippe Brucker wrote: > > On Thu, Dec 19, 2019 at 05:30:20PM +0100, Jean-Philippe Brucker wrote: > > > Add support for Substream ID and PASIDs to the SMMUv3 driver. Since v3 > > > [1], I added review and tested tags where appropriate and applied the > > > suggested changes, shown in the diff below. Thanks all! > > > > > > I'm testing using the zip accelerator on the Hisilicon KunPeng920 and > > > Zhangfei's uacce module [2]. The full SVA support, which I'll send out > > > early next year, is available on my branch sva/zip-devel at > > > https://jpbrucker.net/git/linux/ > > > > Is there anything more I should do for the PASID support? Ideally I'd like > > to get this in v5.6 so I can focus on the rest of the SVA work and on > > performance improvements. > > Apologies, I'm just behind with review what with the timing of the new > year. You're on the list, but I was hoping to get Robin's TCR stuff dusted > off so that Jordan doesn't have to depend on patches languishing on the > mailing list and there's also the nvidia stuff to review as well. > > Going as fast as I can! No worries, I just wanted to check that it didn't slip through the cracks. Thanks, Jean
On Thu, Dec 19, 2019 at 05:30:20PM +0100, Jean-Philippe Brucker wrote: > Add support for Substream ID and PASIDs to the SMMUv3 driver. Since v3 > [1], I added review and tested tags where appropriate and applied the > suggested changes, shown in the diff below. Thanks all! > > I'm testing using the zip accelerator on the Hisilicon KunPeng920 and > Zhangfei's uacce module [2]. The full SVA support, which I'll send out > early next year, is available on my branch sva/zip-devel at > https://jpbrucker.net/git/linux/ > > [1] https://lore.kernel.org/linux-iommu/20191209180514.272727-1-jean-philippe@linaro.org/ > [2] https://lore.kernel.org/linux-iommu/1576465697-27946-1-git-send-email-zhangfei.gao@linaro.org/ > > Jean-Philippe Brucker (13): > iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation > dt-bindings: document PASID property for IOMMU masters > iommu/arm-smmu-v3: Parse PASID devicetree property of platform devices > ACPI/IORT: Parse SSID property of named component node > iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support > iommu/arm-smmu-v3: Add context descriptor tables allocators > iommu/arm-smmu-v3: Add support for Substream IDs > iommu/arm-smmu-v3: Propagate ssid_bits > iommu/arm-smmu-v3: Prepare for handling arm_smmu_write_ctx_desc() > failure > iommu/arm-smmu-v3: Add second level of context descriptor table > iommu/arm-smmu-v3: Improve add_device() error handling > PCI/ATS: Add PASID stubs > iommu/arm-smmu-v3: Add support for PCI PASID > > .../devicetree/bindings/iommu/iommu.txt | 6 + > drivers/acpi/arm64/iort.c | 18 + > drivers/iommu/arm-smmu-v3.c | 467 +++++++++++++++--- > drivers/iommu/of_iommu.c | 6 +- > include/linux/iommu.h | 2 + > include/linux/pci-ats.h | 3 + > 6 files changed, 442 insertions(+), 60 deletions(-) This is close, and I've replied to all of the patches I have comments on. To summarise: 1-5 I could queue these now 6 I can make the small change we discussed 7 I can make the changes if you agree (but I'd prefer you to change to batch submission since I can't test this) 8 Good to go once above is solved 9 Need your opinion 10 Some refactoring needed (sorry) 11 Needs Robin's input 12 Good to go once above is solved 13 Need clarification on PCIe behaviour from you In other words, I could probably take the first 8 or 9 patches for 5.6 if you can resolve those issues asap. Cheers, Will
--- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -589,8 +589,10 @@ struct arm_smmu_cd_table { }; struct arm_smmu_s1_cfg { + /* Leaf tables or linear table */ struct arm_smmu_cd_table *tables; size_t num_tables; + /* First level tables, when two levels are used */ __le64 *l1ptr; dma_addr_t l1ptr_dma; struct arm_smmu_ctx_desc cd; @@ -1561,27 +1563,22 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain, struct arm_smmu_device *smmu = smmu_domain->smmu; struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg; - if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) { - table = &cfg->tables[0]; - idx = ssid; - } else { - idx = ssid >> CTXDESC_SPLIT; - if (idx >= cfg->num_tables) - return NULL; + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) + return cfg->tables[0].ptr + ssid * CTXDESC_CD_DWORDS; - table = &cfg->tables[idx]; - if (!table->ptr) { - if (arm_smmu_alloc_cd_leaf_table(smmu, table, - CTXDESC_L2_ENTRIES)) - return NULL; + idx = ssid >> CTXDESC_SPLIT; + table = &cfg->tables[idx]; + if (!table->ptr) { + if (arm_smmu_alloc_cd_leaf_table(smmu, table, + CTXDESC_L2_ENTRIES)) + return NULL; - l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS; - arm_smmu_write_cd_l1_desc(l1ptr, table); - /* An invalid L1CD can be cached */ - arm_smmu_sync_cd(smmu_domain, ssid, false); - } - idx = ssid & (CTXDESC_L2_ENTRIES - 1); + l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS; + arm_smmu_write_cd_l1_desc(l1ptr, table); + /* An invalid L1CD can be cached */ + arm_smmu_sync_cd(smmu_domain, ssid, false); } + idx = ssid & (CTXDESC_L2_ENTRIES - 1); return table->ptr + idx * CTXDESC_CD_DWORDS; } @@ -1617,8 +1614,12 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, u64 val; bool cd_live; struct arm_smmu_device *smmu = smmu_domain->smmu; - __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid); + __le64 *cdptr; + if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax))) + return -E2BIG; + + cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid); if (!cdptr) return -ENOMEM; @@ -1640,9 +1641,9 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, cdptr[3] = cpu_to_le64(cd->mair); /* - * STE is live, and the SMMU might fetch this CD at any - * time. Ensure that it observes the rest of the CD before we - * enable it. + * STE is live, and the SMMU might read dwords of this CD in any + * order. Ensure that it observes valid values before reading + * V=1. */ arm_smmu_sync_cd(smmu_domain, ssid, true); @@ -1706,7 +1707,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain) * Only allocate a leaf table for linear case. With two levels, leaf * tables are allocated lazily. */ - if (!cfg->l1ptr) { + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) { ret = arm_smmu_alloc_cd_leaf_table(smmu, &cfg->tables[0], max_contexts); if (ret) @@ -2657,17 +2658,21 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master) features = pci_pasid_features(pdev); if (features < 0) - return -ENODEV; + return features; num_pasids = pci_max_pasids(pdev); if (num_pasids <= 0) - return -ENODEV; + return num_pasids; ret = pci_enable_pasid(pdev, features); - if (!ret) - master->ssid_bits = min_t(u8, ilog2(num_pasids), - master->smmu->ssid_bits); - return ret; + if (ret) { + dev_err(&pdev->dev, "Failed to enable PASID\n"); + return ret; + } + + master->ssid_bits = min_t(u8, ilog2(num_pasids), + master->smmu->ssid_bits); + return 0; } static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
Add support for Substream ID and PASIDs to the SMMUv3 driver. Since v3 [1], I added review and tested tags where appropriate and applied the suggested changes, shown in the diff below. Thanks all! I'm testing using the zip accelerator on the Hisilicon KunPeng920 and Zhangfei's uacce module [2]. The full SVA support, which I'll send out early next year, is available on my branch sva/zip-devel at https://jpbrucker.net/git/linux/ [1] https://lore.kernel.org/linux-iommu/20191209180514.272727-1-jean-philippe@linaro.org/ [2] https://lore.kernel.org/linux-iommu/1576465697-27946-1-git-send-email-zhangfei.gao@linaro.org/ Jean-Philippe Brucker (13): iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation dt-bindings: document PASID property for IOMMU masters iommu/arm-smmu-v3: Parse PASID devicetree property of platform devices ACPI/IORT: Parse SSID property of named component node iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support iommu/arm-smmu-v3: Add context descriptor tables allocators iommu/arm-smmu-v3: Add support for Substream IDs iommu/arm-smmu-v3: Propagate ssid_bits iommu/arm-smmu-v3: Prepare for handling arm_smmu_write_ctx_desc() failure iommu/arm-smmu-v3: Add second level of context descriptor table iommu/arm-smmu-v3: Improve add_device() error handling PCI/ATS: Add PASID stubs iommu/arm-smmu-v3: Add support for PCI PASID .../devicetree/bindings/iommu/iommu.txt | 6 + drivers/acpi/arm64/iort.c | 18 + drivers/iommu/arm-smmu-v3.c | 467 +++++++++++++++--- drivers/iommu/of_iommu.c | 6 +- include/linux/iommu.h | 2 + include/linux/pci-ats.h | 3 + 6 files changed, 442 insertions(+), 60 deletions(-)