linux-kernel - Re: [PATCH 2/2] perf: arm-ni: add topology debug info for the clock domain

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] [day] [month] [year] [list]
Message-ID: <4813dc26-0f5c-47f8-b4e4-333f3a26c042@arm.com>
Date: Mon, 24 Nov 2025 18:28:48 +0000
From: Robin Murphy <robin.murphy@....com>
To: Shouping Wang <allen.wang@...micro.com>, will@...nel.org
Cc: mark.rutland@....com, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, andy.xu@...micro.com, peter.du@...micro.com
Subject: Re: [PATCH 2/2] perf: arm-ni: add topology debug info for the clock
 domain

On 2025-11-07 8:43 am, Shouping Wang wrote:
> When capturing PMU info with perf, it is necessary to know the nodeid of
> the interface. Here, we add topology debug info for the clock domain to
> display the nodeid of the interfaces.
> 
> $ cat /sys/kernel/debug/arm-ni/map_1f3c06
> as shown below:
> | HSNI #5 || PMNI #6 || PMU  #14 |

This doesn't really provide any useful driver debugging facility though, 
it's just repeating the exact same information that's already perfectly 
visible through the perf interface.

Debugfs is not meant for presenting information to end users. If they 
really don't know the NI topology, it's trivial to query what interfaces 
the driver knows about by simply opening an event for every possible ID 
of the relevant type and seeing which are considered valid - at most 
that's 128*5 perf_event_open syscalls, which can be done in a matter of 
milliseconds. A user only needs to do that once per platform, then 
they're free to use that data to generate numbers that are still largely 
meaningless without knowledge of what those interfaces represent in the 
context of the system outside the NI :)

Thanks,
Robin.

> 
> Signed-off-by: Shouping Wang <allen.wang@...micro.com>
> ---
>   drivers/perf/arm-ni.c | 63 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> index 32eabdbe877a..e0f0e18d0120 100644
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -13,6 +13,7 @@
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> +#include <linux/debugfs.h>
>   
>   /* Common registers */
>   #define NI_NODE_TYPE		0x000
> @@ -110,6 +111,7 @@ struct arm_ni_cd {
>   	struct arm_ni_unit *units;
>   	struct perf_event *evcnt[NI_NUM_COUNTERS];
>   	struct perf_event *ccnt;
> +	struct dentry *debug;
>   };
>   
>   struct arm_ni {
> @@ -476,6 +478,59 @@ static irqreturn_t arm_ni_handle_irq(int irq, void *dev_id)
>   	}
>   }
>   
> +static struct dentry *arm_ni_cd_debugfs;
> +
> +#ifdef CONFIG_DEBUG_FS
> +static const char *arm_ni_node_type(enum ni_node_type type)
> +{
> +	switch (type) {
> +	case NI_ASNI:
> +		return "| ASNI ";
> +	case NI_AMNI:
> +		return "| AMNI ";
> +	case NI_PMU:
> +		return "| PMU  ";
> +	case NI_HSNI:
> +		return "| HSNI ";
> +	case NI_HMNI:
> +		return "| HMNI ";
> +	case NI_PMNI:
> +		return "| PMNI ";
> +	default:
> +		return "| ???? ";
> +	}
> +}
> +
> +static int arm_ni_cd_map_show(struct seq_file *s, void *data)
> +{
> +	struct arm_ni_cd *cd = s->private;
> +
> +	cd_for_each_unit(cd, unit) {
> +		seq_printf(s, "%s#%-2d |", arm_ni_node_type(unit->type), unit->id);
> +	}
> +	seq_puts(s, "\n");
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(arm_ni_cd_map);
> +
> +static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start)
> +{
> +	const char *name = "map";
> +
> +	if (res_start > 0)
> +		name = devm_kasprintf(cd_to_ni(cd)->dev, GFP_KERNEL, "map_%llx",
> +				res_start >> NI_PMU_PA_SHIFT);
> +	if (!name)
> +		return;
> +
> +	cd->debug = debugfs_create_file(name, 0444, arm_ni_cd_debugfs, cd, &arm_ni_cd_map_fops);
> +}
> +#else
> +static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start) {}
> +#endif
> +
>   static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_start)
>   {
>   	struct arm_ni_cd *cd = ni->cds + node->id;
> @@ -563,6 +618,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);
>   	if (!name)
>   		return -ENOMEM;
> +	arm_ni_cd_debugfs_init(cd, res_start);
>   
>   	return perf_pmu_register(&cd->pmu, name, -1);
>   }
> @@ -575,6 +631,7 @@ static void arm_ni_remove(struct platform_device *pdev)
>   		writel_relaxed(0, cd->pmu_base + NI_PMCR);
>   		writel_relaxed(U32_MAX, cd->pmu_base + NI_PMINTENCLR);
>   		perf_pmu_unregister(&cd->pmu);
> +		debugfs_remove(cd->debug);
>   	}
>   	cpuhp_state_remove_instance_nocalls(arm_ni_hp_state, &ni->cpuhp_node);
>   }
> @@ -787,9 +844,12 @@ static int __init arm_ni_init(void)
>   
>   	arm_ni_hp_state = ret;
>   
> +	arm_ni_cd_debugfs = debugfs_create_dir("arm-ni", NULL);
>   	ret = platform_driver_register(&arm_ni_driver);
> -	if (ret)
> +	if (ret) {
>   		cpuhp_remove_multi_state(arm_ni_hp_state);
> +		debugfs_remove(arm_ni_cd_debugfs);
> +	}
>   	return ret;
>   }
>   
> @@ -797,6 +857,7 @@ static void __exit arm_ni_exit(void)
>   {
>   	platform_driver_unregister(&arm_ni_driver);
>   	cpuhp_remove_multi_state(arm_ni_hp_state);
> +	debugfs_remove(arm_ni_cd_debugfs);
>   }
>   
>   module_init(arm_ni_init);