[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08a35a50-480d-48ee-bc07-b7405274a487@intel.com>
Date: Fri, 3 Oct 2025 16:35:11 -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 v11 14/31] x86/resctrl: Discover hardware telemetry events
Hi Tony,
On 9/25/25 1:03 PM, Tony Luck wrote:
> Each CPU collects data for telemetry events that it sends to the nearest
> telemetry event aggregator either when the value of IA32_PQR_ASSOC.RMID
Please note that one of the "Touchups" done during merge of [1] was to
use full names for registers in descriptions. Considering this,
"IA32_PQR_ASSOC.RMID" -> "MSR_IA32_PQR_ASSOC.RMID
(also please make same change in cover letter)
> changes, or when a two millisecond timer expires.
>
...
> +
> +/**
> + * struct event_group - All information about a group of telemetry events.
> + * @pfg: Points to the aggregated telemetry space information
> + * returned by the intel_pmt_get_regions_by_feature()
> + * call to the INTEL_PMT_TELEMETRY driver that contains
> + * data for all telemetry regions of a specific type.
> + * Valid if the system supports the event group.
> + * NULL otherwise.
> + * @guid: Unique number per XML description file.
> + */
> +struct event_group {
> + /* Data fields for additional structures to manage this group. */
> + struct pmt_feature_group *pfg;
> +
> + /* Remaining fields initialized from XML file. */
> + u32 guid;
> +};
...
> +
> +/*
> + * Make a request to the INTEL_PMT_TELEMETRY driver for a copy of the
> + * pmt_feature_group for a specific feature. If there is one, the returned
> + * structure has an array of telemetry_region structures. Each describes
> + * one telemetry aggregator.
> + * Try to use every telemetry aggregator with a known guid.
The guid is associated with struct event_group and every telemetry region has
its own guid. It is not clear to me why the guid is not associated with pmt_feature_group.
To me this implies that a pmt_feature_group my contain telemetry regions that have
different guid.
This is not fully apparent in this patch but as this code evolves I do not think
the scenario where telemetry regions have different supported (by resctrl) guid is handled
by this enumeration.
If I understand correctly, all telemetry regions of a given pmt_feature_group will be
matched against a single supported guid at a time and all telemetry regions with that
guid will be considered usable and any other considered unusable without further processing
of that pmt_feature_group. If there are more than one matching guid supported by resctrl
then only events of the first one will be enumerated?
> + */
> +static bool get_pmt_feature(enum pmt_feature_id feature, struct event_group **evgs,
> + unsigned int num_evg)
> +{
> + struct pmt_feature_group *p __free(intel_pmt_put_feature_group) = NULL;
> + struct event_group **peg;
> + bool ret;
> +
> + p = intel_pmt_get_regions_by_feature(feature);
> +
> + if (IS_ERR_OR_NULL(p))
> + return false;
> +
> + for (peg = evgs; peg < &evgs[num_evg]; peg++) {
> + ret = enable_events(*peg, p);
> + if (ret) {
> + (*peg)->pfg = no_free_ptr(p);
> + return true;
> + }
> + }
> +
> + return false;
> +}
Reinette
[1] https://lore.kernel.org/all/175793566119.709179.8448328033383658699.tip-bot2@tip-bot2/
Powered by blists - more mailing lists