| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d8e2d4ba-e176-5af4-a3a5-7601ef2598d0@huawei.com>
Date: Sat, 30 Jan 2021 10:23:19 +0800
From: "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To: Robin Murphy <robin.murphy@....com>, Will Deacon <will@...nel.org>,
"Mark Rutland" <mark.rutland@....com>,
Joerg Roedel <joro@...tes.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
iommu <iommu@...ts.linux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>
CC: Jean-Philippe Brucker <jean-philippe@...aro.org>
Subject: Re: [PATCH v3 1/3] perf/smmuv3: Don't reserve the PMCG register
spaces
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.murphy@....com>
>
>> Suggested-by: Robin Murphy <robin.murphy@....com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.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 {
>
> .
>
Powered by blists - more mailing lists