Messages in this thread | | | Subject | Re: [PATCH 1/2] perf/smmuv3: Don't reserve the register space that overlaps with the SMMUv3 | From | "Leizhen (ThunderTown)" <> | Date | Wed, 20 Jan 2021 11:37:05 +0800 |
| |
On 2021/1/19 20:32, Robin Murphy wrote: > On 2021-01-19 01:59, Zhen Lei wrote: >> Some SMMUv3 implementation embed the Perf Monitor Group Registers (PMCG) >> inside the first 64kB region of the SMMU. Since SMMU and PMCG are managed >> by two separate drivers, and this driver depends on ARM_SMMU_V3, so the >> SMMU driver reserves the corresponding resource first, this driver should >> not reserve the corresponding resource again. Otherwise, a resource >> reservation conflict is reported during boot. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> drivers/perf/arm_smmuv3_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c >> index 74474bb322c3f26..dcce085431c6ce8 100644 >> --- a/drivers/perf/arm_smmuv3_pmu.c >> +++ b/drivers/perf/arm_smmuv3_pmu.c >> @@ -761,6 +761,44 @@ static void smmu_pmu_get_acpi_options(struct smmu_pmu *smmu_pmu) >> dev_notice(smmu_pmu->dev, "option mask 0x%x\n", smmu_pmu->options); >> } >> +static void __iomem * >> +smmu_pmu_get_and_ioremap_resource(struct platform_device *pdev, >> + unsigned int index, >> + struct resource **out_res) >> +{ >> + int ret; >> + void __iomem *base; >> + struct resource *res; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, index); >> + if (!res) { >> + dev_err(&pdev->dev, "invalid resource\n"); >> + return IOMEM_ERR_PTR(-EINVAL); >> + } >> + if (out_res) >> + *out_res = res; >> + >> + ret = region_intersects(res->start, resource_size(res), >> + IORESOURCE_MEM, IORES_DESC_NONE); >> + if (ret == REGION_INTERSECTS) { >> + /* >> + * The resource has already been reserved by the SMMUv3 driver. >> + * Don't reserve it again, just do devm_ioremap(). >> + */ >> + base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); >> + } else { >> + /* >> + * The resource may have not been reserved by any driver, or >> + * has been reserved but not type IORESOURCE_MEM. In the latter >> + * case, devm_ioremap_resource() reports a conflict and returns >> + * IOMEM_ERR_PTR(-EBUSY). >> + */ >> + base = devm_ioremap_resource(&pdev->dev, res); >> + } > > What if the PMCG driver simply happens to probe first?
There are 4 cases: 1) ARM_SMMU_V3=m, ARM_SMMU_V3_PMU=y It's not allowed. Becase: ARM_SMMU_V3_PMU depends on ARM_SMMU_V3 config ARM_SMMU_V3_PMU tristate "ARM SMMUv3 Performance Monitors Extension" depends on ARM64 && ACPI && ARM_SMMU_V3
2) ARM_SMMU_V3=y, ARM_SMMU_V3_PMU=m No problem, SMMUv3 will be initialized first.
3) ARM_SMMU_V3=y, ARM_SMMU_V3_PMU=y vi drivers/Makefile 60 obj-y += iommu/ 172 obj-$(CONFIG_PERF_EVENTS) += perf/
This link sequence ensure that SMMUv3 driver will be initialized first. They are currently at the same initialization level.
4) ARM_SMMU_V3=m, ARM_SMMU_V3_PMU=m Sorry, I thought module dependencies were generated based on "depends on". But I tried it today,module dependencies are generated only when symbol dependencies exist. I should use MODULE_SOFTDEP() to explicitly mark the dependency. I will send V2 later.
> > Robin. > >> + >> + return base; >> +} >> + >> static int smmu_pmu_probe(struct platform_device *pdev) >> { >> struct smmu_pmu *smmu_pmu; >> @@ -793,7 +831,7 @@ static int smmu_pmu_probe(struct platform_device *pdev) >> .capabilities = PERF_PMU_CAP_NO_EXCLUDE, >> }; >> - smmu_pmu->reg_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res_0); >> + smmu_pmu->reg_base = smmu_pmu_get_and_ioremap_resource(pdev, 0, &res_0); >> if (IS_ERR(smmu_pmu->reg_base)) >> return PTR_ERR(smmu_pmu->reg_base); >> @@ -801,7 +839,7 @@ static int smmu_pmu_probe(struct platform_device *pdev) >> /* Determine if page 1 is present */ >> if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) { >> - smmu_pmu->reloc_base = devm_platform_ioremap_resource(pdev, 1); >> + smmu_pmu->reloc_base = smmu_pmu_get_and_ioremap_resource(pdev, 1, NULL); >> if (IS_ERR(smmu_pmu->reloc_base)) >> return PTR_ERR(smmu_pmu->reloc_base); >> } else { >> > > . >
| |