This series enables detection of ACPI based TRBE devices via a stand alone purpose built representative platform device. But as a pre-requisite this changes coresight_platform_data structure assignment for the TRBE device.
This series is based on v6.5-rc4 kernel, is also dependent on the following EDK2 changes posted earlier by Sami.
https://edk2.groups.io/g/devel/message/107239 https://edk2.groups.io/g/devel/message/107241
Changes in V4:
- Added in-code comment for arm_trbe_device_probe() - Reverted back using IS_ENABLED() for SPE PMU platform device - Replaced #ifdef with IS_ENABLED() for TRBE platform device - Protected arm_trbe_acpi_match with ACPI_PTR() - preventing a build failure when CONFIG_ACPI is not enabled - Added __maybe_unused for arm_acpi_register_pmu_device() and dropped config checks with IS_ENABLED()
Changes in V3:
https://lore.kernel.org/all/20230803055652.1322801-1-anshuman.khandual@arm.c...
- Changed ARMV8_TRBE_PDEV_NAME from "arm-trbe-acpi" into "arm,trbe" - Dropped local variable 'matched' - Replaced 'matched' with 'valid gsi' as being already matched once - Moved find_acpi_cpu_topology_hetero_id() outside conditional check
Changes in V2:
https://lore.kernel.org/all/20230801094052.750416-1-anshuman.khandual@arm.co...
- Refactored arm_spe_acpi_register_device() in a separate patch - Renamed trbe_acpi_resources as trbe_resources - Renamed trbe_acpi_dev as trbe_dev
Changes in V1:
https://lore.kernel.org/all/20230728112733.359620-1-anshuman.khandual@arm.co...
Cc: Sami Mujawar sami.mujawar@arm.com Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org
Anshuman Khandual (4): arm_pmu: acpi: Refactor arm_spe_acpi_register_device() arm_pmu: acpi: Add a representative platform device for TRBE coresight: trbe: Add a representative coresight_platform_data for TRBE coresight: trbe: Enable ACPI based TRBE devices
arch/arm64/include/asm/acpi.h | 3 + drivers/hwtracing/coresight/coresight-trbe.c | 26 +++- drivers/hwtracing/coresight/coresight-trbe.h | 2 + drivers/perf/arm_pmu_acpi.c | 138 +++++++++++++------ include/linux/perf/arm_pmu.h | 1 + 5 files changed, 128 insertions(+), 42 deletions(-)
Sanity checking all the GICC tables for same interrupt number, and ensuring a homogeneous ACPI based machine, could be used for other platform devices as well. Hence this refactors arm_spe_acpi_register_device() into a common helper arm_acpi_register_pmu_device().
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Co-developed-by: Will Deacon will@kernel.org Signed-off-by: Will Deacon will@kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/perf/arm_pmu_acpi.c | 105 ++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 90815ad762eb..72454bef2a70 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -69,6 +69,63 @@ static void arm_pmu_acpi_unregister_irq(int cpu) acpi_unregister_gsi(gsi); }
+static int __maybe_unused +arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len, + u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *)) +{ + int cpu, this_hetid, hetid, irq, ret; + u16 this_gsi, gsi = 0; + + /* + * Ensure that platform device must have IORESOURCE_IRQ + * resource to hold gsi interrupt. + */ + if (pdev->num_resources != 1) + return -ENXIO; + + if (pdev->resource[0].flags != IORESOURCE_IRQ) + return -ENXIO; + + /* + * Sanity check all the GICC tables for the same interrupt + * number. For now, only support homogeneous ACPI machines. + */ + for_each_possible_cpu(cpu) { + struct acpi_madt_generic_interrupt *gicc; + + gicc = acpi_cpu_get_madt_gicc(cpu); + if (gicc->header.length < len) + return gsi ? -ENXIO : 0; + + this_gsi = parse_gsi(gicc); + if (!this_gsi) + return gsi ? -ENXIO : 0; + + this_hetid = find_acpi_cpu_topology_hetero_id(cpu); + if (!gsi) { + hetid = this_hetid; + gsi = this_gsi; + } else if (hetid != this_hetid || gsi != this_gsi) { + pr_warn("ACPI: %s: must be homogeneous\n", pdev->name); + return -ENXIO; + } + } + + irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH); + if (irq < 0) { + pr_warn("ACPI: %s Unable to register interrupt: %d\n", pdev->name, gsi); + return -ENXIO; + } + + pdev->resource[0].start = irq; + ret = platform_device_register(pdev); + if (ret < 0) { + pr_warn("ACPI: %s: Unable to register device\n", pdev->name); + acpi_unregister_gsi(gsi); + } + return ret; +} + #if IS_ENABLED(CONFIG_ARM_SPE_PMU) static struct resource spe_resources[] = { { @@ -84,6 +141,11 @@ static struct platform_device spe_dev = { .num_resources = ARRAY_SIZE(spe_resources) };
+static u16 arm_spe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{ + return gicc->spe_interrupt; +} + /* * For lack of a better place, hook the normal PMU MADT walk * and create a SPE device if we detect a recent MADT with @@ -91,47 +153,10 @@ static struct platform_device spe_dev = { */ static void arm_spe_acpi_register_device(void) { - int cpu, hetid, irq, ret; - bool first = true; - u16 gsi = 0; - - /* - * Sanity check all the GICC tables for the same interrupt number. - * For now, we only support homogeneous ACPI/SPE machines. - */ - for_each_possible_cpu(cpu) { - struct acpi_madt_generic_interrupt *gicc; - - gicc = acpi_cpu_get_madt_gicc(cpu); - if (gicc->header.length < ACPI_MADT_GICC_SPE) - return; - - if (first) { - gsi = gicc->spe_interrupt; - if (!gsi) - return; - hetid = find_acpi_cpu_topology_hetero_id(cpu); - first = false; - } else if ((gsi != gicc->spe_interrupt) || - (hetid != find_acpi_cpu_topology_hetero_id(cpu))) { - pr_warn("ACPI: SPE must be homogeneous\n"); - return; - } - } - - irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, - ACPI_ACTIVE_HIGH); - if (irq < 0) { - pr_warn("ACPI: SPE Unable to register interrupt: %d\n", gsi); - return; - } - - spe_resources[0].start = irq; - ret = platform_device_register(&spe_dev); - if (ret < 0) { + int ret = arm_acpi_register_pmu_device(&spe_dev, ACPI_MADT_GICC_SPE, + arm_spe_parse_gsi); + if (ret) pr_warn("ACPI: SPE: Unable to register device\n"); - acpi_unregister_gsi(gsi); - } } #else static inline void arm_spe_acpi_register_device(void)
On 08/08/2023 09:22, Anshuman Khandual wrote:
Sanity checking all the GICC tables for same interrupt number, and ensuring a homogeneous ACPI based machine, could be used for other platform devices as well. Hence this refactors arm_spe_acpi_register_device() into a common helper arm_acpi_register_pmu_device().
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Co-developed-by: Will Deacon will@kernel.org Signed-off-by: Will Deacon will@kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/perf/arm_pmu_acpi.c | 105 ++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 90815ad762eb..72454bef2a70 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -69,6 +69,63 @@ static void arm_pmu_acpi_unregister_irq(int cpu) acpi_unregister_gsi(gsi); } +static int __maybe_unused +arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
+{
- int cpu, this_hetid, hetid, irq, ret;
- u16 this_gsi, gsi = 0;
- /*
* Ensure that platform device must have IORESOURCE_IRQ
* resource to hold gsi interrupt.
*/
- if (pdev->num_resources != 1)
return -ENXIO;
- if (pdev->resource[0].flags != IORESOURCE_IRQ)
return -ENXIO;
- /*
* Sanity check all the GICC tables for the same interrupt
* number. For now, only support homogeneous ACPI machines.
*/
- for_each_possible_cpu(cpu) {
struct acpi_madt_generic_interrupt *gicc;
gicc = acpi_cpu_get_madt_gicc(cpu);
if (gicc->header.length < len)
return gsi ? -ENXIO : 0;
this_gsi = parse_gsi(gicc);
if (!this_gsi)
return gsi ? -ENXIO : 0;
this_hetid = find_acpi_cpu_topology_hetero_id(cpu);
if (!gsi) {
hetid = this_hetid;
gsi = this_gsi;
} else if (hetid != this_hetid || gsi != this_gsi) {
pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
return -ENXIO;
}
- }
- irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
- if (irq < 0) {
pr_warn("ACPI: %s Unable to register interrupt: %d\n", pdev->name, gsi);
return -ENXIO;
- }
- pdev->resource[0].start = irq;
- ret = platform_device_register(pdev);
- if (ret < 0) {
pr_warn("ACPI: %s: Unable to register device\n", pdev->name);
acpi_unregister_gsi(gsi);
- }
- return ret;
A postivie return value here could confuse the caller. Also, with my comment below, we don't really need to return something from here.
+}
- #if IS_ENABLED(CONFIG_ARM_SPE_PMU) static struct resource spe_resources[] = { {
@@ -84,6 +141,11 @@ static struct platform_device spe_dev = { .num_resources = ARRAY_SIZE(spe_resources) }; +static u16 arm_spe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{
- return gicc->spe_interrupt;
+}
- /*
- For lack of a better place, hook the normal PMU MADT walk
- and create a SPE device if we detect a recent MADT with
@@ -91,47 +153,10 @@ static struct platform_device spe_dev = { */ static void arm_spe_acpi_register_device(void) {
- int cpu, hetid, irq, ret;
- bool first = true;
- u16 gsi = 0;
- /*
* Sanity check all the GICC tables for the same interrupt number.
* For now, we only support homogeneous ACPI/SPE machines.
*/
- for_each_possible_cpu(cpu) {
struct acpi_madt_generic_interrupt *gicc;
gicc = acpi_cpu_get_madt_gicc(cpu);
if (gicc->header.length < ACPI_MADT_GICC_SPE)
return;
if (first) {
gsi = gicc->spe_interrupt;
if (!gsi)
return;
hetid = find_acpi_cpu_topology_hetero_id(cpu);
first = false;
} else if ((gsi != gicc->spe_interrupt) ||
(hetid != find_acpi_cpu_topology_hetero_id(cpu))) {
pr_warn("ACPI: SPE must be homogeneous\n");
return;
}
- }
- irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE,
ACPI_ACTIVE_HIGH);
- if (irq < 0) {
pr_warn("ACPI: SPE Unable to register interrupt: %d\n", gsi);
return;
- }
- spe_resources[0].start = irq;
- ret = platform_device_register(&spe_dev);
- if (ret < 0) {
- int ret = arm_acpi_register_pmu_device(&spe_dev, ACPI_MADT_GICC_SPE,
arm_spe_parse_gsi);
- if (ret) pr_warn("ACPI: SPE: Unable to register device\n");
With this change, a system without SPE interrupt description always generates the above message. Is this intended ? Could we not drop the above message as all the other possible error scenarios are reported. We could simply make the above helper void, see my comment above.
Suzuki
acpi_unregister_gsi(gsi);
- } } #else static inline void arm_spe_acpi_register_device(void)
On 8/8/23 13:52, Anshuman Khandual wrote:
- /*
* Sanity check all the GICC tables for the same interrupt
* number. For now, only support homogeneous ACPI machines.
*/
- for_each_possible_cpu(cpu) {
struct acpi_madt_generic_interrupt *gicc;
gicc = acpi_cpu_get_madt_gicc(cpu);
if (gicc->header.length < len)
return gsi ? -ENXIO : 0;
this_gsi = parse_gsi(gicc);
if (!this_gsi)
return gsi ? -ENXIO : 0;
this_hetid = find_acpi_cpu_topology_hetero_id(cpu);
if (!gsi) {
hetid = this_hetid;
gsi = this_gsi;
} else if (hetid != this_hetid || gsi != this_gsi) {
pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
return -ENXIO;
}
- }
As discussed on the previous version i.e V3 thread, will move the 'this_gsi' check after parse_gsi(), inside if (!gsi) conditional block. This will treat subsequent cpu parse_gsi()'s failure as a mismatch thus triggering the pr_warn() message.
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 845683ca7c64..6eae772d6298 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -98,11 +98,11 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len, return gsi ? -ENXIO : 0;
this_gsi = parse_gsi(gicc); - if (!this_gsi) - return gsi ? -ENXIO : 0; - this_hetid = find_acpi_cpu_topology_hetero_id(cpu); if (!gsi) { + if (!this_gsi) + return 0; + hetid = this_hetid; gsi = this_gsi; } else if (hetid != this_hetid || gsi != this_gsi) {
ACPI TRBE does not have a HID for identification which could create and add a platform device into the platform bus. Also without a platform device, it cannot be probed and bound to a platform driver.
This creates a dummy platform device for TRBE after ascertaining that ACPI provides required interrupts uniformly across all cpus on the system. This device gets created inside drivers/perf/arm_pmu_acpi.c to accommodate TRBE being built as a module.
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- arch/arm64/include/asm/acpi.h | 3 +++ drivers/perf/arm_pmu_acpi.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/perf/arm_pmu.h | 1 + 3 files changed, 39 insertions(+)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index bd68e1b7f29f..4d537d56eb84 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -42,6 +42,9 @@ #define ACPI_MADT_GICC_SPE (offsetof(struct acpi_madt_generic_interrupt, \ spe_interrupt) + sizeof(u16))
+#define ACPI_MADT_GICC_TRBE (offsetof(struct acpi_madt_generic_interrupt, \ + trbe_interrupt) + sizeof(u16)) + /* Basic configuration for ACPI */ #ifdef CONFIG_ACPI pgprot_t __acpi_get_mem_attribute(phys_addr_t addr); diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 72454bef2a70..845683ca7c64 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -164,6 +164,40 @@ static inline void arm_spe_acpi_register_device(void) } #endif /* CONFIG_ARM_SPE_PMU */
+#if IS_ENABLED(CONFIG_CORESIGHT_TRBE) +static struct resource trbe_resources[] = { + { + /* irq */ + .flags = IORESOURCE_IRQ, + } +}; + +static struct platform_device trbe_dev = { + .name = ARMV8_TRBE_PDEV_NAME, + .id = -1, + .resource = trbe_resources, + .num_resources = ARRAY_SIZE(trbe_resources) +}; + +static u16 arm_trbe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{ + return gicc->trbe_interrupt; +} + +static void arm_trbe_acpi_register_device(void) +{ + int ret = arm_acpi_register_pmu_device(&trbe_dev, ACPI_MADT_GICC_TRBE, + arm_trbe_parse_gsi); + if (ret) + pr_warn("ACPI: TRBE: Unable to register device\n"); +} +#else +static inline void arm_trbe_acpi_register_device(void) +{ + +} +#endif /* CONFIG_CORESIGHT_TRBE */ + static int arm_pmu_acpi_parse_irqs(void) { int irq, cpu, irq_cpu, err; @@ -399,6 +433,7 @@ static int arm_pmu_acpi_init(void) return 0;
arm_spe_acpi_register_device(); + arm_trbe_acpi_register_device();
return 0; } diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h index a0801f68762b..143fbc10ecfe 100644 --- a/include/linux/perf/arm_pmu.h +++ b/include/linux/perf/arm_pmu.h @@ -187,5 +187,6 @@ void armpmu_free_irq(int irq, int cpu); #endif /* CONFIG_ARM_PMU */
#define ARMV8_SPE_PDEV_NAME "arm,spe-v1" +#define ARMV8_TRBE_PDEV_NAME "arm,trbe"
#endif /* __ARM_PMU_H__ */
TRBE coresight devices do not need regular connections information, as the paths get built between all percpu source and their respective percpu sink devices. Please refer 'commit 2cd87a7b293d ("coresight: core: Add support for dedicated percpu sinks")' which added support for percpu sink devices.
coresight_register() expect device connections via the platform_data. TRBE devices do not have any graph connections and thus is empty. With upcoming ACPI support for TRBE, we do not get a real acpi_device and thus coresight_get_platform_dat() will end up in failures. Hence this allocates a zeroed coresight_platform_data structure and assigns that back into the device.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7720619909d6..9455d62ca620 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1494,9 +1494,20 @@ static int arm_trbe_device_probe(struct platform_device *pdev) if (!drvdata) return -ENOMEM;
- pdata = coresight_get_platform_data(dev); - if (IS_ERR(pdata)) - return PTR_ERR(pdata); + /* + * TRBE coresight devices do not need regular connections + * information, as the paths get built between all percpu + * source and their respective percpu sink devices. Though + * coresight_register() expect device connections via the + * platform_data, which TRBE devices do not have. As they + * are not real ACPI devices, coresight_get_platform_data() + * ends up failing. Instead let's allocate a dummy zeroed + * coresight_platform_data structure and assign that back + * into the device for that purpose. + */ + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM;
dev_set_drvdata(dev, drvdata); dev->platform_data = pdata;
This detects and enables ACPI based TRBE devices via the dummy platform device created earlier for this purpose.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ drivers/hwtracing/coresight/coresight-trbe.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 9455d62ca620..9c59e2652b20 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1548,7 +1548,16 @@ static const struct of_device_id arm_trbe_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_trbe_of_match);
+#ifdef CONFIG_ACPI +static const struct platform_device_id arm_trbe_acpi_match[] = { + { ARMV8_TRBE_PDEV_NAME, 0 }, + { } +}; +MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); +#endif + static struct platform_driver arm_trbe_driver = { + .id_table = ACPI_PTR(arm_trbe_acpi_match), .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match), diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 77cbb5c63878..fce1735d5c58 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -7,11 +7,13 @@ * * Author: Anshuman Khandual anshuman.khandual@arm.com */ +#include <linux/acpi.h> #include <linux/coresight.h> #include <linux/device.h> #include <linux/irq.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/perf/arm_pmu.h> #include <linux/platform_device.h> #include <linux/smp.h>