linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] perf/smmuv3: Don't reserve the PMCG register spaces
@ 2021-01-30  7:14 Zhen Lei
  2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
  2021-01-30  7:14 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Reserving the entire SMMU register space Zhen Lei
  0 siblings, 2 replies; 7+ messages in thread
From: Zhen Lei @ 2021-01-30  7:14 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Mark Rutland, Joerg Roedel,
	linux-arm-kernel, iommu, linux-kernel
  Cc: Zhen Lei, Jean-Philippe Brucker, Shameer Kolothum

v3 --> v4:
1. Delete the unnecessary encapsulation function smmu_pmu_get_and_ioremap_resource().
2. Discard adding MODULE_SOFTDEP.

v2 --> v3:
Patch 3 is updated because https://lkml.org/lkml/2021/1/22/532 has been queued in advance.

v1 --> v2:
According to Robin Murphy's suggestion: https://lkml.org/lkml/2021/1/20/470
Don't reserve the PMCG register spaces, and reserve the entire SMMU register space.

v1:
Since the PMCG may implement its resigters space(4KB Page0 and 4KB Page1)
within the SMMUv3 64KB Page0. In this case, when the SMMUv3 driver reserves the
64KB Page0 resource in advance, the PMCG driver try to reserve its Page0 and
Page1 resources, a resource conflict occurs.

commit 52f3fab0067d6fa ("iommu/arm-smmu-v3: Don't reserve implementation
defined register space") reduce the resource reservation range of the SMMUv3
driver, it only reserves the first 0xe00 bytes in the 64KB Page0, to avoid
the above-mentioned resource conflicts.

But the SMMUv3.3 add support for ECMDQ, its registers space is also implemented
in the SMMUv3 64KB Page0. This means we need to build two separate mappings.
New features may be added in the future, and more independent mappings may be
required. The simple problem is complicated because the user expects to map the
entire SMMUv3 64KB Page0.

Therefore, the proper solution is: If the PMCG register resources are located in
the 64KB Page0 of the SMMU, the PMCG driver does not reserve the conflict resources
when the SMMUv3 driver has reserved the conflict resources before. Instead, the PMCG
driver only performs devm_ioremap() to ensure that it can work properly.

Zhen Lei (2):
  perf/smmuv3: Don't reserve the PMCG register spaces
  iommu/arm-smmu-v3: Reserving the entire SMMU register space

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 24 ++++--------------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 --
 drivers/perf/arm_smmuv3_pmu.c               | 25 +++++++++++++++++++------
 3 files changed, 23 insertions(+), 28 deletions(-)

-- 
1.8.3



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] perf/smmuv3: Don't reserve the PMCG register spaces
  2021-01-30  7:14 [PATCH v4 0/2] perf/smmuv3: Don't reserve the PMCG register spaces Zhen Lei
@ 2021-01-30  7:14 ` Zhen Lei
  2021-01-30  7:33   ` Leizhen (ThunderTown)
                     ` (2 more replies)
  2021-01-30  7:14 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Reserving the entire SMMU register space Zhen Lei
  1 sibling, 3 replies; 7+ messages in thread
From: Zhen Lei @ 2021-01-30  7:14 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Mark Rutland, Joerg Roedel,
	linux-arm-kernel, iommu, linux-kernel
  Cc: Zhen Lei, Jean-Philippe Brucker, Shameer Kolothum

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.murphy@arm.com>
Signed-off-by: Zhen Lei <thunder.leizhen@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;
 	}
-- 
1.8.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] iommu/arm-smmu-v3: Reserving the entire SMMU register space
  2021-01-30  7:14 [PATCH v4 0/2] perf/smmuv3: Don't reserve the PMCG register spaces Zhen Lei
  2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
@ 2021-01-30  7:14 ` Zhen Lei
  1 sibling, 0 replies; 7+ messages in thread
From: Zhen Lei @ 2021-01-30  7:14 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Mark Rutland, Joerg Roedel,
	linux-arm-kernel, iommu, linux-kernel
  Cc: Zhen Lei, Jean-Philippe Brucker, Shameer Kolothum

commit 52f3fab0067d ("iommu/arm-smmu-v3: Don't reserve implementation
defined register space") only reserves the basic SMMU register space. So
the ECMDQ register space is not covered, it should be mapped again. Due
to the size of this ECMDQ resource is not fixed, depending on
SMMU_IDR6.CMDQ_CONTROL_PAGE_LOG2NUMQ. Processing its resource reservation
to avoid resource conflict with PMCG is a bit more complicated.

Therefore, the resources of the PMCG are not reserved, and the entire SMMU
resources are reserved.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 24 ++++--------------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 --
 2 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index f04c55a7503c790..fde24403b06a9e3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3476,14 +3476,6 @@ static int arm_smmu_set_bus_ops(struct iommu_ops *ops)
 	return err;
 }
 
-static void __iomem *arm_smmu_ioremap(struct device *dev, resource_size_t start,
-				      resource_size_t size)
-{
-	struct resource res = DEFINE_RES_MEM(start, size);
-
-	return devm_ioremap_resource(dev, &res);
-}
-
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -3519,22 +3511,14 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 	ioaddr = res->start;
 
-	/*
-	 * Don't map the IMPLEMENTATION DEFINED regions, since they may contain
-	 * the PMCG registers which are reserved by the PMU driver.
-	 */
-	smmu->base = arm_smmu_ioremap(dev, ioaddr, ARM_SMMU_REG_SZ);
+	smmu->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(smmu->base))
 		return PTR_ERR(smmu->base);
 
-	if (arm_smmu_resource_size(smmu) > SZ_64K) {
-		smmu->page1 = arm_smmu_ioremap(dev, ioaddr + SZ_64K,
-					       ARM_SMMU_REG_SZ);
-		if (IS_ERR(smmu->page1))
-			return PTR_ERR(smmu->page1);
-	} else {
+	if (arm_smmu_resource_size(smmu) > SZ_64K)
+		smmu->page1 = smmu->base + SZ_64K;
+	else
 		smmu->page1 = smmu->base;
-	}
 
 	/* Interrupt lines */
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index da525f46dab4fc1..63f2b476987d6ae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -152,8 +152,6 @@
 #define ARM_SMMU_PRIQ_IRQ_CFG1		0xd8
 #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
 
-#define ARM_SMMU_REG_SZ			0xe00
-
 /* Common MSI config fields */
 #define MSI_CFG0_ADDR_MASK		GENMASK_ULL(51, 2)
 #define MSI_CFG2_SH			GENMASK(5, 4)
-- 
1.8.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] perf/smmuv3: Don't reserve the PMCG register spaces
  2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
@ 2021-01-30  7:33   ` Leizhen (ThunderTown)
  2021-02-01 12:08   ` Robin Murphy
  2021-02-01 12:54   ` Will Deacon
  2 siblings, 0 replies; 7+ messages in thread
From: Leizhen (ThunderTown) @ 2021-01-30  7:33 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Mark Rutland, Joerg Roedel,
	linux-arm-kernel, iommu, linux-kernel
  Cc: Jean-Philippe Brucker, Shameer Kolothum

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.murphy@arm.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@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;
>  	}
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] perf/smmuv3: Don't reserve the PMCG register spaces
  2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
  2021-01-30  7:33   ` Leizhen (ThunderTown)
@ 2021-02-01 12:08   ` Robin Murphy
  2021-02-01 12:54   ` Will Deacon
  2 siblings, 0 replies; 7+ messages in thread
From: Robin Murphy @ 2021-02-01 12:08 UTC (permalink / raw)
  To: Zhen Lei, Will Deacon, Mark Rutland, Joerg Roedel,
	linux-arm-kernel, iommu, linux-kernel
  Cc: Jean-Philippe Brucker, Shameer Kolothum

On 2021-01-30 07: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.

I said my review on v3 stood either way, but for the avoidance of doubt,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

I hadn't considered that a comment is a very good idea, in case the 
cleanup-script crew find this in future and try to "simplify" it :)

Thanks,
Robin.

> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@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;
>   	}
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] perf/smmuv3: Don't reserve the PMCG register spaces
  2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
  2021-01-30  7:33   ` Leizhen (ThunderTown)
  2021-02-01 12:08   ` Robin Murphy
@ 2021-02-01 12:54   ` Will Deacon
  2021-02-01 13:10     ` Leizhen (ThunderTown)
  2 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2021-02-01 12:54 UTC (permalink / raw)
  To: Zhen Lei
  Cc: Robin Murphy, Mark Rutland, Joerg Roedel, linux-arm-kernel,
	iommu, linux-kernel, Jean-Philippe Brucker, Shameer Kolothum

On Sat, Jan 30, 2021 at 03:14:13PM +0800, 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.murphy@arm.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@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);

I tried to apply this, but you've got your return type in a muddle:

@@ @@
+drivers/perf/arm_smmuv3_pmu.c: In function ‘smmu_pmu_probe’:
+drivers/perf/arm_smmuv3_pmu.c:803:10: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
+  803 |   return ERR_PTR(-EINVAL);
+      |          ^~~~~~~~~~~~~~~~
+drivers/perf/arm_smmuv3_pmu.c:803:31: warning: incorrect type in return expression (different base types) [sparse]
+drivers/perf/arm_smmuv3_pmu.c:803:31:    expected int [sparse]
+drivers/perf/arm_smmuv3_pmu.c:803:31:    got void * [sparse]
+drivers/perf/arm_smmuv3_pmu.c:806:10: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
+  806 |   return ERR_PTR(-ENOMEM);
+      |          ^~~~~~~~~~~~~~~~
+drivers/perf/arm_smmuv3_pmu.c:806:31: warning: incorrect type in return expression (different base types) [sparse]
+drivers/perf/arm_smmuv3_pmu.c:806:31:    expected int [sparse]
+drivers/perf/arm_smmuv3_pmu.c:806:31:    got void * [sparse]
+drivers/perf/arm_smmuv3_pmu.c:816:11: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
+  816 |    return ERR_PTR(-EINVAL);
+      |           ^~~~~~~~~~~~~~~~
+drivers/perf/arm_smmuv3_pmu.c:816:39: warning: incorrect type in return expression (different base types) [sparse]
+drivers/perf/arm_smmuv3_pmu.c:816:39:    expected int [sparse]
+drivers/perf/arm_smmuv3_pmu.c:816:39:    got void * [sparse]
+drivers/perf/arm_smmuv3_pmu.c:819:11: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
+  819 |    return ERR_PTR(-ENOMEM);
+      |           ^~~~~~~~~~~~~~~~
+drivers/perf/arm_smmuv3_pmu.c:819:39: warning: incorrect type in return expression (different base types) [sparse]
+drivers/perf/arm_smmuv3_pmu.c:819:39:    expected int [sparse]
+drivers/perf/arm_smmuv3_pmu.c:819:39:    got void * [sparse]

Will

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] perf/smmuv3: Don't reserve the PMCG register spaces
  2021-02-01 12:54   ` Will Deacon
@ 2021-02-01 13:10     ` Leizhen (ThunderTown)
  0 siblings, 0 replies; 7+ messages in thread
From: Leizhen (ThunderTown) @ 2021-02-01 13:10 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Mark Rutland, Joerg Roedel, linux-arm-kernel,
	iommu, linux-kernel, Jean-Philippe Brucker, Shameer Kolothum



On 2021/2/1 20:54, Will Deacon wrote:
> On Sat, Jan 30, 2021 at 03:14:13PM +0800, 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.murphy@arm.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@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);
> 
> I tried to apply this, but you've got your return type in a muddle:

I'm dizzy. I don't know how this bug patch came out. I just pinched my leg, like I'm still in the real world.

The "ERR_PTR()" of the four ERR_PTR(xxx) should be removed. Can you help me? Or I send a new one.

> 
> @@ @@
> +drivers/perf/arm_smmuv3_pmu.c: In function ‘smmu_pmu_probe’:
> +drivers/perf/arm_smmuv3_pmu.c:803:10: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
> +  803 |   return ERR_PTR(-EINVAL);
> +      |          ^~~~~~~~~~~~~~~~
> +drivers/perf/arm_smmuv3_pmu.c:803:31: warning: incorrect type in return expression (different base types) [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:803:31:    expected int [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:803:31:    got void * [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:806:10: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
> +  806 |   return ERR_PTR(-ENOMEM);
> +      |          ^~~~~~~~~~~~~~~~
> +drivers/perf/arm_smmuv3_pmu.c:806:31: warning: incorrect type in return expression (different base types) [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:806:31:    expected int [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:806:31:    got void * [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:816:11: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
> +  816 |    return ERR_PTR(-EINVAL);
> +      |           ^~~~~~~~~~~~~~~~
> +drivers/perf/arm_smmuv3_pmu.c:816:39: warning: incorrect type in return expression (different base types) [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:816:39:    expected int [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:816:39:    got void * [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:819:11: warning: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast [-Wint-conversion]
> +  819 |    return ERR_PTR(-ENOMEM);
> +      |           ^~~~~~~~~~~~~~~~
> +drivers/perf/arm_smmuv3_pmu.c:819:39: warning: incorrect type in return expression (different base types) [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:819:39:    expected int [sparse]
> +drivers/perf/arm_smmuv3_pmu.c:819:39:    got void * [sparse]
> 
> Will
> 
> .
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-02-01 13:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-30  7:14 [PATCH v4 0/2] perf/smmuv3: Don't reserve the PMCG register spaces Zhen Lei
2021-01-30  7:14 ` [PATCH v4 1/2] " Zhen Lei
2021-01-30  7:33   ` Leizhen (ThunderTown)
2021-02-01 12:08   ` Robin Murphy
2021-02-01 12:54   ` Will Deacon
2021-02-01 13:10     ` Leizhen (ThunderTown)
2021-01-30  7:14 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Reserving the entire SMMU register space Zhen Lei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).