On 2021/1/29 23:06, Robin Murphy wrote: > On 2021-01-27 11:32, Zhen Lei wrote: >> According to the SMMUv3 specification: >> Each PMCG counter group is represented by one 4KB page (Page 0) with one >> optional additional 4KB page (Page 1), both of which are at IMPLEMENTATION >> DEFINED base addresses. >> >> This means that the PMCG register spaces may be within the 64KB pages of >> the SMMUv3 register space. When both the SMMU and PMCG drivers reserve >> their own resources, a resource conflict occurs. >> >> To avoid this conflict, don't reserve the PMCG regions. > > I'm still not a fan of this get_and_ioremap notion in general, especially > when the "helper" function ends up over twice the size of all the code it > replaces[1], but for the actual functional change here,
OK,I'll change it to [1] and add some comments. > > Reviewed-by: Robin Murphy <robin.mur...@arm.com> > >> Suggested-by: Robin Murphy <robin.mur...@arm.com> >> Signed-off-by: Zhen Lei <thunder.leiz...@huawei.com> >> --- >> drivers/perf/arm_smmuv3_pmu.c | 27 +++++++++++++++++++++++++-- >> 1 file changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c >> index 74474bb322c3f26..e5e505a0804fe53 100644 >> --- a/drivers/perf/arm_smmuv3_pmu.c >> +++ b/drivers/perf/arm_smmuv3_pmu.c >> @@ -761,6 +761,29 @@ 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 **res) >> +{ >> + void __iomem *base; >> + struct resource *r; >> + >> + r = platform_get_resource(pdev, IORESOURCE_MEM, index); >> + if (!r) { >> + dev_err(&pdev->dev, "invalid resource\n"); >> + return ERR_PTR(-EINVAL); >> + } >> + if (res) >> + *res = r; >> + >> + base = devm_ioremap(&pdev->dev, r->start, resource_size(r)); >> + if (!base) >> + return ERR_PTR(-ENOMEM); >> + >> + return base; >> +} >> + >> static int smmu_pmu_probe(struct platform_device *pdev) >> { >> struct smmu_pmu *smmu_pmu; >> @@ -793,7 +816,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 +824,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 { >> > > [1] > ----->8----- > drivers/perf/arm_smmuv3_pmu.c | 35 +++++++++-------------------------- > 1 file changed, 9 insertions(+), 26 deletions(-) > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index e5e505a0804f..c9adbc7b55a1 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -761,33 +761,10 @@ 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 **res) > -{ > - void __iomem *base; > - struct resource *r; > - > - r = platform_get_resource(pdev, IORESOURCE_MEM, index); > - if (!r) { > - dev_err(&pdev->dev, "invalid resource\n"); > - return ERR_PTR(-EINVAL); > - } > - if (res) > - *res = r; > - > - base = devm_ioremap(&pdev->dev, r->start, resource_size(r)); > - if (!base) > - return ERR_PTR(-ENOMEM); > - > - return base; > -} > - > static int smmu_pmu_probe(struct platform_device *pdev) > { > struct smmu_pmu *smmu_pmu; > - struct resource *res_0; > + struct resource *res_0, *res_1; > u32 cfgr, reg_size; > u64 ceid_64[2]; > int irq, err; > @@ -816,7 +793,10 @@ static int smmu_pmu_probe(struct platform_device *pdev) > .capabilities = PERF_PMU_CAP_NO_EXCLUDE, > }; > > - smmu_pmu->reg_base = smmu_pmu_get_and_ioremap_resource(pdev, 0, &res_0); > + res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res_0) > + return ERR_PTR(-EINVAL); > + smmu_pmu->reg_base = devm_ioremap(dev, res_0->start, > resource_size(res_0)); > if (IS_ERR(smmu_pmu->reg_base)) > return PTR_ERR(smmu_pmu->reg_base); > > @@ -824,7 +804,10 @@ 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 = smmu_pmu_get_and_ioremap_resource(pdev, 1, > NULL); > + res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res_1) > + return -EINVAL; > + smmu_pmu->reloc_base = devm_ioremap(dev, res_1->start, > resource_size(res_1)); > if (IS_ERR(smmu_pmu->reloc_base)) > return PTR_ERR(smmu_pmu->reloc_base); > } else { > > . > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu