| 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
| ||
|
Message-ID: <446cf50b-bae0-4e5d-887d-3769eae4b824@intel.com>
Date: Thu, 8 May 2025 08:59:26 -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>, Anil Keshavamurthy <anil.s.keshavamurthy@...el.com>,
Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<patches@...ts.linux.dev>
Subject: Re: [PATCH v4 27/31] x86/resctrl: Handle number of RMIDs supported by
telemetry resources
Hi Tony,
On 4/28/25 5:33 PM, Tony Luck wrote:
> There are now three meanings for "number of RMIDs":
>
> 1) The number for legacy features enumerated by CPUID leaf 0xF. This
> is the maximum number of distinct values that can be loaded into the
> IA32_PQR_ASSOC MSR. Note that systems with Sub-NUMA Cluster mode enabled
> will force scaling down the CPUID enumerated value by the number of SNC
> nodes per L3-cache.
>
> 2) The number of registers in MMIO space for each event. This
> is enumerated in the XML files and is the value placed into
> event_group::num_rmids.
This is unexpected and not true at this point. Instead this is something
this patch introduces ... kindof since the value is obtained from XML file
and then "adjusted".
>
> 3) The number of "h/w counters" (this isn't a strictly accurate
> description of how things work, but serves as a useful analogy that
> does describe the limitations) feeding to those MMIO registers. This
> is enumerated in telemetry_region::num_rmids returned from the call to
> intel_pmt_get_regions_by_feature()
>
> Event groups with insufficient "h/w counter" to track all RMIDs are
> difficult for users to use, since the system may reassign "h/w counters"
> as any time. This means that users cannot reliably collect two consecutive
> event counts to compute the rate at which events are occurring.
>
> Ignore such under-resourced event groups unless the user explicitly
> requests to enable them using the "rdt=" Linux boot argument.
>
> Scan all enabled event groups and assign the RDT_RESOURCE_PERF_PKG
> resource "num_rmids" value to the smallest of these values to ensure
> that all resctrl groups have equal monitor capabilities.
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 2 ++
> arch/x86/kernel/cpu/resctrl/intel_aet.c | 25 +++++++++++++++++++++++++
> arch/x86/kernel/cpu/resctrl/monitor.c | 2 ++
> 3 files changed, 29 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 92cbba9d82a8..31499bcd2065 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -18,6 +18,8 @@
>
> #define RMID_VAL_UNAVAIL BIT_ULL(62)
>
> +extern int rdt_num_system_rmids;
> +
> /*
> * With the above fields in use 62 bits remain in MSR_IA32_QM_CTR for
> * data to be returned. The counter width is discovered from the hardware
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index aacaedcc7b74..eec5eb625f13 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/io.h>
> +#include <linux/minmax.h>
> #include <linux/resctrl.h>
>
> /* Temporary - delete from final version */
> @@ -51,6 +52,7 @@ struct pmt_event {
> * @pfg: The pmt_feature_group for this event group
> * @name: Name for this group
> * @guid: Unique number per XML description file
> + * @num_rmids: Number of RMIDS supported by this group
Can append (from XML file, then adjusted ... ?)
> * @mmio_size: Number of bytes of mmio registers for this group
> * @pkginfo: Per-package MMIO addresses
> * @num_events: Number of events in this group
> @@ -60,6 +62,7 @@ struct event_group {
> struct pmt_feature_group *pfg;
> char *name;
> int guid;
> + int num_rmids;
> int mmio_size;
> struct mmio_info **pkginfo;
> int num_events;
> @@ -70,6 +73,7 @@ struct event_group {
> static struct event_group energy_0x26696143 = {
> .name = "energy",
> .guid = 0x26696143,
> + .num_rmids = 576,
> .mmio_size = (576 * 2 + 3) * 8,
> .num_events = 2,
> .evts = {
> @@ -82,6 +86,7 @@ static struct event_group energy_0x26696143 = {
> static struct event_group perf_0x26557651 = {
> .name = "perf",
> .guid = 0x26557651,
> + .num_rmids = 576,
> .mmio_size = (576 * 7 + 3) * 8,
> .num_events = 7,
> .evts = {
> @@ -214,6 +219,15 @@ static bool get_pmt_feature(enum pmt_feature_id feature)
> if ((*peg)->guid == p->regions[i].guid) {
> if (rdt_check_option((*peg)->name, false, true))
> return false;
> + /*
> + * Ignore event group with insufficient RMIDs unless the
> + * user used the rdt= boot option to specifically ask
> + * for it to be enabled.
> + */
> + if (p->regions[i].num_rmids < rdt_num_system_rmids &&
> + !rdt_check_option((*peg)->name, true, false))
> + return false;
> + (*peg)->num_rmids = p->regions[i].num_rmids;
Does this need a min()? Since this cycles through multiple regions it seems possible that
if the regions support different numbers of RMIDs then the event group's adjustment to
deal with a region with few RMIDs will be undone by a following region with more RMIDs.
> ret = configure_events((*peg), p);
> if (ret) {
> (*peg)->pfg = no_free_ptr(p);
Reinette
Powered by blists - more mailing lists