linux-kernel - Re: [PATCH v13 25/32] x86/resctrl: Handle number of RMIDs supported by RDT_RESOURCE_PERF

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: <8ca676bf-7b50-4898-baf1-92241712f871@intel.com>
Date: Fri, 14 Nov 2025 15:26:42 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: 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>, <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v13 25/32] x86/resctrl: Handle number of RMIDs supported
 by RDT_RESOURCE_PERF_PKG

Hi Tony,

On 11/14/25 1:55 PM, Luck, Tony wrote:
> 
> resctrl: Feature energy guid=0x26696143 not enabled due to insufficient RMIDs
> 
> 
> static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
> {
> 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> 	bool warn_disable = false;
> 
> 	if (!group_has_usable_regions(e, p))
> 		return false;
> 
> 	/* Disable feature if insufficient RMIDs */
> 	if (!all_regions_have_sufficient_rmid(e, p)) {
> 		warn_disable = true;
> 		rdt_set_feature_disabled(e->name);
> 	}
> 
> 	/* User can override above disable from kernel command line */
> 	if (!rdt_is_feature_enabled(e->name)) {
> 		if (warn_disable)
> 			pr_info("Feature %s guid=0x%x not enabled due to insufficient RMIDs\n",
> 				e->name, e->guid);
> 		return false;
> 	}
> 	...
> }

Thank you for considering. This looks good to me.

I now realize that if a system supports, for example, two energy guid and only one has insufficient
RMID then one or both may be disabled by default depending on which resctrl attempts to enable
first. This is arbitrary based on where the event group appears in the array.

How a system with two guid of the same feature type would work is not clear to me though. Looks
like they cannot share events at all since an event is uniquely associated with a struct pmt_event
that can belong to only one event group. If they may share events then enable_events()->resctrl_enable_mon_event()
will complain loudly but still proceed and allow the event group to be enabled.
I think the resctrl_enable_mon_event() warnings were added to support enabling of new features
so that the WARNs can catch issues during development ... now it may encounter issues when a
kernel with this implementation is run on a system that supports a single feature with
multiple guid. Do you have more insight in how the "single feature with multiple guid" may look to
better prepare resctrl to handle them?

Should "enable_events" be split so that a feature can be disabled for all its event groups if
any of them cannot be enabled due to insufficient RMIDs?
Perhaps resctrl_enable_mon_event() should also now return success/fail so that an event group
cannot be enabled if its events cannot be enabled?
Finally, a system with two guid of the same feature type will end up printing duplicate
"<feature type> monitoring detected" that could be more descriptive?

Reinette