linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct
@ 2020-09-15 20:41 Mark Salter
  2020-09-15 20:41 ` [PATCH v3 2/2] drivers/perf: thunderx2_pmu: Fix memory resource error handling Mark Salter
  2020-09-18 16:17 ` [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Will Deacon
  0 siblings, 2 replies; 3+ messages in thread
From: Mark Salter @ 2020-09-15 20:41 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland; +Cc: Khuong Dinh, linux-kernel, linux-arm-kernel

This splat was reported on newer Fedora kernels booting on certain
X-gene based machines:

 xgene-pmu APMC0D83:00: X-Gene PMU version 3
 Unable to handle kernel read from unreadable memory at virtual \
 address 0000000000004006
 ...
 Call trace:
  string+0x50/0x100
  vsnprintf+0x160/0x750
  devm_kvasprintf+0x5c/0xb4
  devm_kasprintf+0x54/0x60
  __devm_ioremap_resource+0xdc/0x1a0
  devm_ioremap_resource+0x14/0x20
  acpi_get_pmu_hw_inf.isra.0+0x84/0x15c
  acpi_pmu_dev_add+0xbc/0x21c
  acpi_ns_walk_namespace+0x16c/0x1e4
  acpi_walk_namespace+0xb4/0xfc
  xgene_pmu_probe_pmu_dev+0x7c/0xe0
  xgene_pmu_probe.part.0+0x2c0/0x310
  xgene_pmu_probe+0x54/0x64
  platform_drv_probe+0x60/0xb4
  really_probe+0xe8/0x4a0
  driver_probe_device+0xe4/0x100
  device_driver_attach+0xcc/0xd4
  __driver_attach+0xb0/0x17c
  bus_for_each_dev+0x6c/0xb0
  driver_attach+0x30/0x40
  bus_add_driver+0x154/0x250
  driver_register+0x84/0x140
  __platform_driver_register+0x54/0x60
  xgene_pmu_driver_init+0x28/0x34
  do_one_initcall+0x40/0x204
  do_initcalls+0x104/0x144
  kernel_init_freeable+0x198/0x210
  kernel_init+0x20/0x12c
  ret_from_fork+0x10/0x18
 Code: 91000400 110004e1 eb08009f 540000c0 (38646846)
 ---[ end trace f08c10566496a703 ]---

This is due to use of an uninitialized local resource struct in the xgene
pmu driver. The thunderx2_pmu driver avoids this by using the resource list
constructed by acpi_dev_get_resources() rather than using a callback from
that function. The callback in the xgene driver didn't fully initialize
the resource. So get rid of the callback and search the resource list as
done by thunderx2.

Fixes: 832c927d119b ("perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver")
Signed-off-by: Mark Salter <msalter@redhat.com>
---
 drivers/perf/xgene_pmu.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index edac28cd25dd..633cf07ba672 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1453,17 +1453,6 @@ static char *xgene_pmu_dev_name(struct device *dev, u32 type, int id)
 }
 
 #if defined(CONFIG_ACPI)
-static int acpi_pmu_dev_add_resource(struct acpi_resource *ares, void *data)
-{
-	struct resource *res = data;
-
-	if (ares->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32)
-		acpi_dev_resource_memory(ares, res);
-
-	/* Always tell the ACPI core to skip this resource */
-	return 1;
-}
-
 static struct
 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
 				       struct acpi_device *adev, u32 type)
@@ -1475,6 +1464,7 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
 	struct hw_pmu_info *inf;
 	void __iomem *dev_csr;
 	struct resource res;
+	struct resource_entry *rentry;
 	int enable_bit;
 	int rc;
 
@@ -1483,11 +1473,23 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
 		return NULL;
 
 	INIT_LIST_HEAD(&resource_list);
-	rc = acpi_dev_get_resources(adev, &resource_list,
-				    acpi_pmu_dev_add_resource, &res);
+	rc = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
+	if (rc <= 0) {
+		dev_err(dev, "PMU type %d: No resources found\n", type);
+		return NULL;
+	}
+
+	list_for_each_entry(rentry, &resource_list, node) {
+		if (resource_type(rentry->res) == IORESOURCE_MEM) {
+			res = *rentry->res;
+			rentry = NULL;
+			break;
+		}
+	}
 	acpi_dev_free_resource_list(&resource_list);
-	if (rc < 0) {
-		dev_err(dev, "PMU type %d: No resource address found\n", type);
+
+	if (rentry) {
+		dev_err(dev, "PMU type %d: No memory resource found\n", type);
 		return NULL;
 	}
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 2/2] drivers/perf: thunderx2_pmu: Fix memory resource error handling
  2020-09-15 20:41 [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Mark Salter
@ 2020-09-15 20:41 ` Mark Salter
  2020-09-18 16:17 ` [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Will Deacon
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Salter @ 2020-09-15 20:41 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland; +Cc: Khuong Dinh, linux-kernel, linux-arm-kernel

In tx2_uncore_pmu_init_dev(), a call to acpi_dev_get_resources() is used
to create a list _CRS resources which is searched for the device base
address. There is an error check following this:

   if (!rentry->res)
           return NULL

In no case, will rentry->res be NULL, so the test is useless. Even
if the test worked, it comes before the resource list memory is
freed. None of this really matters as long as the ACPI table has
the memory resource. Let's clean it up so that it makes sense and
will give a meaningful error should firmware leave out the memory
resource.

Fixes: 69c32972d593 ("drivers/perf: Add Cavium ThunderX2 SoC UNCORE PMU driver")
Signed-off-by: Mark Salter <msalter@redhat.com>
---
 drivers/perf/thunderx2_pmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
index aac9823b0c6b..e116815fa809 100644
--- a/drivers/perf/thunderx2_pmu.c
+++ b/drivers/perf/thunderx2_pmu.c
@@ -805,14 +805,17 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 	list_for_each_entry(rentry, &list, node) {
 		if (resource_type(rentry->res) == IORESOURCE_MEM) {
 			res = *rentry->res;
+			rentry = NULL;
 			break;
 		}
 	}
+	acpi_dev_free_resource_list(&list);
 
-	if (!rentry->res)
+	if (rentry) {
+		dev_err(dev, "PMU type %d: Fail to find resource\n", type);
 		return NULL;
+	}
 
-	acpi_dev_free_resource_list(&list);
 	base = devm_ioremap_resource(dev, &res);
 	if (IS_ERR(base)) {
 		dev_err(dev, "PMU type %d: Fail to map resource\n", type);
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct
  2020-09-15 20:41 [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Mark Salter
  2020-09-15 20:41 ` [PATCH v3 2/2] drivers/perf: thunderx2_pmu: Fix memory resource error handling Mark Salter
@ 2020-09-18 16:17 ` Will Deacon
  1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2020-09-18 16:17 UTC (permalink / raw)
  To: Mark Salter, Mark Rutland
  Cc: Khuong Dinh, Will Deacon, catalin.marinas, linux-kernel,
	kernel-team, linux-arm-kernel

On Tue, 15 Sep 2020 16:41:09 -0400, Mark Salter wrote:
> This splat was reported on newer Fedora kernels booting on certain
> X-gene based machines:
> 
>  xgene-pmu APMC0D83:00: X-Gene PMU version 3
>  Unable to handle kernel read from unreadable memory at virtual \
>  address 0000000000004006
>  ...
>  Call trace:
>   string+0x50/0x100
>   vsnprintf+0x160/0x750
>   devm_kvasprintf+0x5c/0xb4
>   devm_kasprintf+0x54/0x60
>   __devm_ioremap_resource+0xdc/0x1a0
>   devm_ioremap_resource+0x14/0x20
>   acpi_get_pmu_hw_inf.isra.0+0x84/0x15c
>   acpi_pmu_dev_add+0xbc/0x21c
>   acpi_ns_walk_namespace+0x16c/0x1e4
>   acpi_walk_namespace+0xb4/0xfc
>   xgene_pmu_probe_pmu_dev+0x7c/0xe0
>   xgene_pmu_probe.part.0+0x2c0/0x310
>   xgene_pmu_probe+0x54/0x64
>   platform_drv_probe+0x60/0xb4
>   really_probe+0xe8/0x4a0
>   driver_probe_device+0xe4/0x100
>   device_driver_attach+0xcc/0xd4
>   __driver_attach+0xb0/0x17c
>   bus_for_each_dev+0x6c/0xb0
>   driver_attach+0x30/0x40
>   bus_add_driver+0x154/0x250
>   driver_register+0x84/0x140
>   __platform_driver_register+0x54/0x60
>   xgene_pmu_driver_init+0x28/0x34
>   do_one_initcall+0x40/0x204
>   do_initcalls+0x104/0x144
>   kernel_init_freeable+0x198/0x210
>   kernel_init+0x20/0x12c
>   ret_from_fork+0x10/0x18
>  Code: 91000400 110004e1 eb08009f 540000c0 (38646846)
>  ---[ end trace f08c10566496a703 ]---
> 
> [...]

Applied to will (for-next/perf), thanks!

[1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct
      https://git.kernel.org/will/c/a76b8236edcf
[2/2] drivers/perf: thunderx2_pmu: Fix memory resource error handling
      https://git.kernel.org/will/c/688494a407d1

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-09-18 16:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 20:41 [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Mark Salter
2020-09-15 20:41 ` [PATCH v3 2/2] drivers/perf: thunderx2_pmu: Fix memory resource error handling Mark Salter
2020-09-18 16:17 ` [PATCH v3 1/2] drivers/perf: xgene_pmu: Fix uninitialized resource struct Will Deacon

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