linux-kernel - Re: [PATCH v8 31/32] x86/resctrl: Add debugfs files to show telemetry aggregator status

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]

Message-ID: <52741e7d-6edc-4b36-82aa-65104e2a8ba2@intel.com>
Date: Thu, 14 Aug 2025 14:59:15 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghuay@...dia.com>, "Maciej
 Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
	<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
	<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
	<Dave.Martin@....com>, Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v8 31/32] x86/resctrl: Add debugfs files to show telemetry
 aggregator status

Hi Tony,

On 8/11/25 11:17 AM, Tony Luck wrote:
> Each telemetry aggregator provides three status registers at the top
> end of MMIO space after all the per-RMID per-event counters:
> 
>   agg_data_loss_count: This counts the number of times that this aggregator
>   failed to accumulate a counter value supplied by a CPU core.
> 
>   agg_data_loss_timestamp: This is a "timestamp" from a free running
>   25MHz uncore timer indicating when the most recent data loss occurred.
> 
>   last_update_timestamp: Another 25MHz timestamp indicating when the
>   most recent counter update was successfully applied.

With all three files being "per aggregator" it is not clear to me why
"last_update_timestamp" does not have the "agg_" prefix. Looking at the
debugfs files created all three are indeed created with the "agg" prefix
so this looks inconsistent.

> 
> Create files in /sys/kernel/debug/resctrl/info/PERF_PKG_MON/x86_64/
> to display the value of each of these status registers for each aggregator
> in each enabled event group.

To avoid reviewers needing to dig, this can add that these files are removed 
as part of the debugfs_remove_recursive() called during resctrl exit.

> 
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 53 +++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index 4afc99e74cef..ff70f2b18165 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/cleanup.h>
>  #include <linux/cpu.h>
> +#include <linux/debugfs.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/io.h>
>  #include <linux/minmax.h>
> @@ -299,6 +300,55 @@ static bool get_pmt_feature(enum pmt_feature_id feature, struct event_group **ev
>  	return false;
>  }
>  
> +static int status_read(void *priv, u64 *val)
> +{
> +	void __iomem *info = (void __iomem *)priv;
> +
> +	*val = readq(info);
> +
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(status_fops, status_read, NULL, "%llu\n");
> +
> +static void make_status_files(struct dentry *dir, struct event_group *e, int pkg, int instance)
> +{
> +	void *info = (void __force *)e->pkginfo[pkg]->addrs[instance] + e->mmio_size;
> +	char name[64];
> +
> +	sprintf(name, "%s_pkg%d_agg%d_data_loss_count", e->name, pkg, instance);
> +	debugfs_create_file(name, 0400, dir, info - 24, &status_fops);
> +
> +	sprintf(name, "%s_pkg%d_agg%d_data_loss_timestamp", e->name, pkg, instance);
> +	debugfs_create_file(name, 0400, dir, info - 16, &status_fops);
> +
> +	sprintf(name, "%s_pkg%d_agg%d_last_update_timestamp", e->name, pkg, instance);
> +	debugfs_create_file(name, 0400, dir, info - 8, &status_fops);
> +}
> +
> +static void create_debug_event_status_files(struct dentry *dir, struct event_group *e)
> +{
> +	int num_pkgs = topology_max_packages();
> +
> +	for (int i = 0; i < num_pkgs; i++)
> +		for (int j = 0; j < e->pkginfo[i]->num_regions; j++)
> +			make_status_files(dir, e, i, j);
> +}
> +
> +static void create_debugfs_status_file(void)
> +{
> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> +	struct event_group *evg;
> +	struct dentry *infodir;
> +
> +	infodir = resctrl_debugfs_mon_info_arch_mkdir(r);
> +	if (!infodir)
> +		return;
> +
> +	list_for_each_entry(evg, &active_event_groups, list)
> +		create_debug_event_status_files(infodir, evg);

To avoid using/needing active_event_groups create_debug_event_status_files()
can be called at end of discover_events() and infodir can be a global that
is initialized there if it is NULL.

Reinette