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: <aOV8bFiiQxlPDske@agluck-desk3>
Date: Tue, 7 Oct 2025 13:47:40 -0700
From: "Luck, Tony" <tony.luck@...el.com>
To: Reinette Chatre <reinette.chatre@...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 v11 14/31] x86/resctrl: Discover hardware telemetry events

On Mon, Oct 06, 2025 at 02:47:15PM -0700, Luck, Tony wrote:
> On Mon, Oct 06, 2025 at 02:33:00PM -0700, Reinette Chatre wrote:
> > Hi Tony,
> > > static bool get_pmt_feature(enum pmt_feature_id feature, struct event_group **evgs,
> > > 			    unsigned int num_evg)
> > > {
> > > 	struct pmt_feature_group *p = intel_pmt_get_regions_by_feature(feature);
> > > 	struct event_group **peg;
> > > 	bool ret = false;
> > > 
> > > 	if (IS_ERR_OR_NULL(p))
> > > 		return false;
> > > 
> > > 	for (peg = evgs; peg < &evgs[num_evg]; peg++) {
> > > 		if (enable_events(*peg, p)) {
> > > 			kref_get(&p->kref);
> > 
> > This is not clear to me ... would enable_events() still mark all telemetry_regions
> > that do not match the event_group's guid as unusable? It seems to me that if more
> > than one even_group refers to the same pmt_feature_group then the first one to match
> > will "win" and make the other event_group's telemetry regions unusable.
> 
> Extra context needed. Sorry.
> 
> I'm changing enable_events() to only mark telemetry_regions regions as
> unusable if they have a bad package id, or the MMIO size doesn't match.
> I.e. they truly are bad.
> 
> Mis-match on guid will skip then while associating with a specific
> event_gruoup, but leave them as usable.
> 
> This means that intel_aet_read_event() now has to check the guid as
> well as !addr.
> 
> An alternative approach would be to ask the PMT code for separate
> copies of the pmt_feature_group to attach to each event_group. I
> didn't like this, do you think it would be better?

Working through more patches in the series, I've come to the one
that adjusts the number of RMIDs.  The alternative approach of
having a separate copy of the pmt_feature_group is suddently looking
more attractive.

So the code would become:


static bool get_pmt_feature(enum pmt_feature_id feature, struct event_group **evgs,
			    unsigned int num_evg)
{
	struct pmt_feature_group *p;
	struct event_group **peg;
	bool ret = false;

	for (peg = evgs; peg < &evgs[num_evg]; peg++) {
		p = intel_pmt_get_regions_by_feature(feature);
		if (IS_ERR_OR_NULL(p))
			return false;

		if (enable_events(*peg, p)) {
			(*peg)->pfg = p;
			ret = true;
		} else {
			intel_pmt_put_feature_group(p);
		}
	}
	intel_pmt_put_feature_group(p);

	return ret;
}

> 
> > 
> > > 			(*peg)->pfg = no_free_ptr(p);
> > > 			ret = true;
> > > 		}
> > > 	}
> > > 	intel_pmt_put_feature_group(p);
> > > 
> > > 	return ret;
> > > }
> > > 
> > 
> > Reinette
> > 
> 
> -Tony
-Tony

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ