Hi, Robin:
  Can you review this patch again?

On 2021/1/30 15:14, 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.
> 
> Suggested-by: Robin Murphy <robin.mur...@arm.com>
> Signed-off-by: Zhen Lei <thunder.leiz...@huawei.com>
> ---
>  drivers/perf/arm_smmuv3_pmu.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index 74474bb322c3f26..5e894f957c7b935 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -793,17 +793,30 @@ 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);
> -     if (IS_ERR(smmu_pmu->reg_base))
> -             return PTR_ERR(smmu_pmu->reg_base);
> +     /*
> +      * The register spaces of the PMCG may be in the register space of
> +      * other devices. For example, SMMU. Therefore, the PMCG resources are
> +      * not reserved to avoid resource conflicts with other drivers.
> +      */
> +     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 (!smmu_pmu->reg_base)
> +             return ERR_PTR(-ENOMEM);
>  
>       cfgr = readl_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CFGR);
>  
>       /* Determine if page 1 is present */
>       if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
> -             smmu_pmu->reloc_base = devm_platform_ioremap_resource(pdev, 1);
> -             if (IS_ERR(smmu_pmu->reloc_base))
> -                     return PTR_ERR(smmu_pmu->reloc_base);
> +             struct resource *res_1;
> +
> +             res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +             if (!res_1)
> +                     return ERR_PTR(-EINVAL);
> +             smmu_pmu->reloc_base = devm_ioremap(dev, res_1->start, 
> resource_size(res_1));
> +             if (!smmu_pmu->reloc_base)
> +                     return ERR_PTR(-ENOMEM);
>       } else {
>               smmu_pmu->reloc_base = smmu_pmu->reg_base;
>       }
> 

Reply via email to