linux-kernel - Re: [PATCH v7 22/31] x86/resctrl: Read telemetry events

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: <bf760139-4401-4fe6-8926-94ab72aeaab6@intel.com>
Date: Fri, 25 Jul 2025 16:45:04 -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 v7 22/31] x86/resctrl: Read telemetry events

Hi Tony,

On 7/11/25 4:53 PM, Tony Luck wrote:
> The resctrl file system passes requests to read event monitor files to
> the architecture resctrl_arch_rmid_read() to collect values
> from hardware counters.
> 
> Use the resctrl resource to differentiate between calls to read legacy
> L3 events from the new telemetry events (which are attached to
> RDT_RESOURCE_PERF_PKG).
> 
> There may be multiple aggregators tracking each package, so scan all of
> them and add up all counters.
> 
> Enable the events marked as readable from any CPU providing an
> mon_evt::arch_priv pointer to the struct pmt_event for each
> event.
> 
> At run time when a user reads an event file the file system code
> provides the enum resctrl_event_id for the event and the arch_priv
> pointer that was supplied when the event was enabled.

The changelog ordering seems random. It starts by describing how reading of events are
handled and how counters are added when an event is read, then describes enabling the
events (this should happen before an event can be read?), then how data is passed when
reading an event (that should be followed by adding up the counters?).

I think it may help to clearly describe the phases involved. For example, start
with how events are enabled during enumeration/discovery, then how data is
passed during runtime when a user reads an event file, then how the
data is collected.

> 
> Resctrl now uses readq() so depends on X86_64. Update Kconfig.
> 
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
...

> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index f4bf0f2ccf26..bd6011a95d12 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -14,6 +14,7 @@
>  #include <linux/cleanup.h>
>  #include <linux/cpu.h>
>  #include <linux/intel_vsec.h>
> +#include <linux/io.h>
>  #include <linux/resctrl.h>
>  #include <linux/slab.h>
>  
> @@ -213,6 +214,13 @@ static int discover_events(struct event_group *e, struct pmt_feature_group *p)
>  
>  	list_add(&e->list, &active_event_groups);
>  

Should this addition be documented as "step 3"?

> +	for (int i = 0; i < e->num_events; i++) {
> +		enum resctrl_event_id eventid;
> +
> +		eventid = e->evts[i].id;
> +		resctrl_enable_mon_event(eventid, true, e->evts[i].bin_bits, &e->evts[i]);

Why is eventid needed? I think using e->evts[i].id makes it more obvious how
the parameters relate.

> +	}
> +
>  	return 0;
>  }
>  

Reinette