lkml.org 
[lkml]   [2021]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/2] perf/smmuv3: Don't reserve the register space that overlaps with the SMMUv3
    From
    Date


    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 {
    >>
    >
    > .
    >

    \
     
     \ /
      Last update: 2021-01-20 04:41    [W:2.680 / U:0.516 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site