linux-kernel - Re: [PATCH 1/2] perf/smmuv3: Don't reserve the register space that overlaps with the SMMUv3

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Jan 2021 12:32:21 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Zhen Lei <thunder.leizhen@...wei.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>,
        Neil Leeder <nleeder@...eaurora.org>,
        Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>
Subject: Re: [PATCH 1/2] perf/smmuv3: Don't reserve the register space that
 overlaps with the SMMUv3

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@...wei.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?

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