Re: [PATCH v2 00/17] Arm SMMU refactoring
On 8/16/2019 12:07 AM, Robin Murphy wrote: Hi all, v1 for context: https://patchwork.kernel.org/cover/11087347/ Here's a quick v2 attempting to address all the minor comments; I've tweaked a whole bunch of names, added some verbosity in macros and comments for clarity, and rejigged arm_smmu_impl_init() for a bit more structure. The (new) patches #1 and #2 are up front as conceptual fixes, although they're not actually critical - it turns out to be more of an embarrassment than a real problem in practice. For ease of reference, the overall diff against v1 is attached below. Robin. Hi, I have given this series a shot with 5.3-rc5 kernel on MTP sdm845 device, and smmu works as expected. Tested-by: Vivek Gautam Best regards Vivek Robin Murphy (17): iommu/arm-smmu: Mask TLBI address correctly iommu/qcom: Mask TLBI addresses correctly iommu/arm-smmu: Convert GR0 registers to bitfields iommu/arm-smmu: Convert GR1 registers to bitfields iommu/arm-smmu: Convert context bank registers to bitfields iommu/arm-smmu: Rework cb_base handling iommu/arm-smmu: Split arm_smmu_tlb_inv_range_nosync() iommu/arm-smmu: Get rid of weird "atomic" write iommu/arm-smmu: Abstract GR1 accesses iommu/arm-smmu: Abstract context bank accesses iommu/arm-smmu: Abstract GR0 accesses iommu/arm-smmu: Rename arm-smmu-regs.h iommu/arm-smmu: Add implementation infrastructure iommu/arm-smmu: Move Secure access quirk to implementation iommu/arm-smmu: Add configuration implementation hook iommu/arm-smmu: Add reset implementation hook iommu/arm-smmu: Add context init implementation hook MAINTAINERS | 3 +- drivers/iommu/Makefile| 2 +- drivers/iommu/arm-smmu-impl.c | 174 +++ drivers/iommu/arm-smmu-regs.h | 210 - drivers/iommu/arm-smmu.c | 573 +++--- drivers/iommu/arm-smmu.h | 394 +++ drivers/iommu/qcom_iommu.c| 17 +- 7 files changed, 764 insertions(+), 609 deletions(-) create mode 100644 drivers/iommu/arm-smmu-impl.c delete mode 100644 drivers/iommu/arm-smmu-regs.h create mode 100644 drivers/iommu/arm-smmu.h ->8- diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c index 3c731e087854..e22e9004f449 100644 --- a/drivers/iommu/arm-smmu-impl.c +++ b/drivers/iommu/arm-smmu-impl.c @@ -28,7 +28,7 @@ static int arm_smmu_gr0_ns(int offset) static u32 arm_smmu_read_ns(struct arm_smmu_device *smmu, int page, int offset) { - if (page == 0) + if (page == ARM_SMMU_GR0) offset = arm_smmu_gr0_ns(offset); return readl_relaxed(arm_smmu_page(smmu, page) + offset); } @@ -36,7 +36,7 @@ static u32 arm_smmu_read_ns(struct arm_smmu_device *smmu, int page, static void arm_smmu_write_ns(struct arm_smmu_device *smmu, int page, int offset, u32 val) { - if (page == 0) + if (page == ARM_SMMU_GR0) offset = arm_smmu_gr0_ns(offset); writel_relaxed(val, arm_smmu_page(smmu, page) + offset); } @@ -52,18 +52,17 @@ struct cavium_smmu { struct arm_smmu_device smmu; u32 id_base; }; -#define to_csmmu(s)container_of(s, struct cavium_smmu, smmu) static int cavium_cfg_probe(struct arm_smmu_device *smmu) { static atomic_t context_count = ATOMIC_INIT(0); + struct cavium_smmu *cs = container_of(smmu, struct cavium_smmu, smmu); /* * Cavium CN88xx erratum #27704. * Ensure ASID and VMID allocation is unique across all SMMUs in * the system. */ - to_csmmu(smmu)->id_base = atomic_fetch_add(smmu->num_context_banks, - _count); + cs->id_base = atomic_fetch_add(smmu->num_context_banks, _count); dev_notice(smmu->dev, "\tenabling workaround for Cavium erratum 27704\n"); return 0; @@ -71,12 +70,13 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu) int cavium_init_context(struct arm_smmu_domain *smmu_domain) { - u32 id_base = to_csmmu(smmu_domain->smmu)->id_base; + struct cavium_smmu *cs = container_of(smmu_domain->smmu, + struct cavium_smmu, smmu); if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2) - smmu_domain->cfg.vmid += id_base; + smmu_domain->cfg.vmid += cs->id_base; else - smmu_domain->cfg.asid += id_base; + smmu_domain->cfg.asid += cs->id_base; return 0; } @@ -88,18 +88,18 @@ const struct arm_smmu_impl cavium_impl = { struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smmu) { - struct cavium_smmu *csmmu; + struct cavium_smmu *cs; - csmmu = devm_kzalloc(smmu->dev, sizeof(*csmmu), GFP_KERNEL); - if (!csmmu) + cs = devm_kzalloc(smmu->dev, sizeof(*cs), GFP_KERNEL); +
Re: [PATCH v2 00/17] Arm SMMU refactoring
On Mon, Aug 19, 2019 at 07:10:45PM +0100, Robin Murphy wrote: > On 19/08/2019 16:56, Will Deacon wrote: > > On Thu, Aug 15, 2019 at 07:37:20PM +0100, Robin Murphy wrote: > > > v1 for context: https://patchwork.kernel.org/cover/11087347/ > > > > > > Here's a quick v2 attempting to address all the minor comments; I've > > > tweaked a whole bunch of names, added some verbosity in macros and > > > comments for clarity, and rejigged arm_smmu_impl_init() for a bit more > > > structure. The (new) patches #1 and #2 are up front as conceptual fixes, > > > although they're not actually critical - it turns out to be more of an > > > embarrassment than a real problem in practice. > > > > Thanks, I'll pick this up and send to Joerg later this week. > > Oops, I've just noticed that the io-64-nonatomic-hi-lo.h include also needs > to move to arm-smmu.h in #14 to avoid breaking 32-bit builds. I've pushed > out an updated branch (along with the static fixes for good measure) - let > me know if you'd like a resend of the patches. Can you just send a patch on top instead, please? I'd prefer not to rebase things unless we really need to, and I've already pushed this stuff to for-joerg/arm-smmu/refactoring. Thanks, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/17] Arm SMMU refactoring
On 19/08/2019 16:56, Will Deacon wrote: On Thu, Aug 15, 2019 at 07:37:20PM +0100, Robin Murphy wrote: v1 for context: https://patchwork.kernel.org/cover/11087347/ Here's a quick v2 attempting to address all the minor comments; I've tweaked a whole bunch of names, added some verbosity in macros and comments for clarity, and rejigged arm_smmu_impl_init() for a bit more structure. The (new) patches #1 and #2 are up front as conceptual fixes, although they're not actually critical - it turns out to be more of an embarrassment than a real problem in practice. Thanks, I'll pick this up and send to Joerg later this week. Oops, I've just noticed that the io-64-nonatomic-hi-lo.h include also needs to move to arm-smmu.h in #14 to avoid breaking 32-bit builds. I've pushed out an updated branch (along with the static fixes for good measure) - let me know if you'd like a resend of the patches. Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/17] Arm SMMU refactoring
On Thu, Aug 15, 2019 at 07:37:20PM +0100, Robin Murphy wrote: > v1 for context: https://patchwork.kernel.org/cover/11087347/ > > Here's a quick v2 attempting to address all the minor comments; I've > tweaked a whole bunch of names, added some verbosity in macros and > comments for clarity, and rejigged arm_smmu_impl_init() for a bit more > structure. The (new) patches #1 and #2 are up front as conceptual fixes, > although they're not actually critical - it turns out to be more of an > embarrassment than a real problem in practice. Thanks, I'll pick this up and send to Joerg later this week. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/17] Arm SMMU refactoring
Hi all, v1 for context: https://patchwork.kernel.org/cover/11087347/ Here's a quick v2 attempting to address all the minor comments; I've tweaked a whole bunch of names, added some verbosity in macros and comments for clarity, and rejigged arm_smmu_impl_init() for a bit more structure. The (new) patches #1 and #2 are up front as conceptual fixes, although they're not actually critical - it turns out to be more of an embarrassment than a real problem in practice. For ease of reference, the overall diff against v1 is attached below. Robin. Robin Murphy (17): iommu/arm-smmu: Mask TLBI address correctly iommu/qcom: Mask TLBI addresses correctly iommu/arm-smmu: Convert GR0 registers to bitfields iommu/arm-smmu: Convert GR1 registers to bitfields iommu/arm-smmu: Convert context bank registers to bitfields iommu/arm-smmu: Rework cb_base handling iommu/arm-smmu: Split arm_smmu_tlb_inv_range_nosync() iommu/arm-smmu: Get rid of weird "atomic" write iommu/arm-smmu: Abstract GR1 accesses iommu/arm-smmu: Abstract context bank accesses iommu/arm-smmu: Abstract GR0 accesses iommu/arm-smmu: Rename arm-smmu-regs.h iommu/arm-smmu: Add implementation infrastructure iommu/arm-smmu: Move Secure access quirk to implementation iommu/arm-smmu: Add configuration implementation hook iommu/arm-smmu: Add reset implementation hook iommu/arm-smmu: Add context init implementation hook MAINTAINERS | 3 +- drivers/iommu/Makefile| 2 +- drivers/iommu/arm-smmu-impl.c | 174 +++ drivers/iommu/arm-smmu-regs.h | 210 - drivers/iommu/arm-smmu.c | 573 +++--- drivers/iommu/arm-smmu.h | 394 +++ drivers/iommu/qcom_iommu.c| 17 +- 7 files changed, 764 insertions(+), 609 deletions(-) create mode 100644 drivers/iommu/arm-smmu-impl.c delete mode 100644 drivers/iommu/arm-smmu-regs.h create mode 100644 drivers/iommu/arm-smmu.h ->8- diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c index 3c731e087854..e22e9004f449 100644 --- a/drivers/iommu/arm-smmu-impl.c +++ b/drivers/iommu/arm-smmu-impl.c @@ -28,7 +28,7 @@ static int arm_smmu_gr0_ns(int offset) static u32 arm_smmu_read_ns(struct arm_smmu_device *smmu, int page, int offset) { - if (page == 0) + if (page == ARM_SMMU_GR0) offset = arm_smmu_gr0_ns(offset); return readl_relaxed(arm_smmu_page(smmu, page) + offset); } @@ -36,7 +36,7 @@ static u32 arm_smmu_read_ns(struct arm_smmu_device *smmu, int page, static void arm_smmu_write_ns(struct arm_smmu_device *smmu, int page, int offset, u32 val) { - if (page == 0) + if (page == ARM_SMMU_GR0) offset = arm_smmu_gr0_ns(offset); writel_relaxed(val, arm_smmu_page(smmu, page) + offset); } @@ -52,18 +52,17 @@ struct cavium_smmu { struct arm_smmu_device smmu; u32 id_base; }; -#define to_csmmu(s)container_of(s, struct cavium_smmu, smmu) static int cavium_cfg_probe(struct arm_smmu_device *smmu) { static atomic_t context_count = ATOMIC_INIT(0); + struct cavium_smmu *cs = container_of(smmu, struct cavium_smmu, smmu); /* * Cavium CN88xx erratum #27704. * Ensure ASID and VMID allocation is unique across all SMMUs in * the system. */ - to_csmmu(smmu)->id_base = atomic_fetch_add(smmu->num_context_banks, - _count); + cs->id_base = atomic_fetch_add(smmu->num_context_banks, _count); dev_notice(smmu->dev, "\tenabling workaround for Cavium erratum 27704\n"); return 0; @@ -71,12 +70,13 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu) int cavium_init_context(struct arm_smmu_domain *smmu_domain) { - u32 id_base = to_csmmu(smmu_domain->smmu)->id_base; + struct cavium_smmu *cs = container_of(smmu_domain->smmu, + struct cavium_smmu, smmu); if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2) - smmu_domain->cfg.vmid += id_base; + smmu_domain->cfg.vmid += cs->id_base; else - smmu_domain->cfg.asid += id_base; + smmu_domain->cfg.asid += cs->id_base; return 0; } @@ -88,18 +88,18 @@ const struct arm_smmu_impl cavium_impl = { struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smmu) { - struct cavium_smmu *csmmu; + struct cavium_smmu *cs; - csmmu = devm_kzalloc(smmu->dev, sizeof(*csmmu), GFP_KERNEL); - if (!csmmu) + cs = devm_kzalloc(smmu->dev, sizeof(*cs), GFP_KERNEL); + if (!cs) return ERR_PTR(-ENOMEM); - csmmu->smmu = *smmu; - csmmu->smmu.impl = _impl; + cs->smmu = *smmu; + cs->smmu.impl = _impl; devm_kfree(smmu->dev, smmu); - return >smmu; +