[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a6f5016-0997-47ef-b7d9-2dc4c578e345@intel.com>
Date: Mon, 6 Oct 2025 14:33:49 -0700
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 v11 17/31] x86/resctrl: Find and enable usable telemetry
events
Hi Tony,
On 10/6/25 12:58 PM, Luck, Tony wrote:
> On Fri, Oct 03, 2025 at 04:52:01PM -0700, Reinette Chatre wrote:
>> On 9/25/25 1:03 PM, Tony Luck wrote:
>>> @@ -114,12 +116,44 @@ static struct event_group *known_perf_event_groups[] = {
>>> for (_peg = (_grp); _peg < &_grp[ARRAY_SIZE(_grp)]; _peg++) \
>>> if ((*_peg)->pfg)
>>>
>>> -/* Stub for now */
>>> -static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
>>> +static bool skip_telem_region(struct telemetry_region *tr, struct event_group *e)
>>> {
>>> + if (tr->guid != e->guid)
>>> + return true;
>>> + if (tr->plat_info.package_id >= topology_max_packages()) {
>>> + pr_warn("Bad package %u in guid 0x%x\n", tr->plat_info.package_id,
>>> + tr->guid);
>>> + return true;
>>> + }
>>
>> I have not encountered any mention of the possibility that packages may differ
>> in which telemetry region types they support. For example, could it be possible for package
>> A to have usable regions of the PERF type but package B doesn't? From what I can tell
>> INTEL_PMT_TELEMETRY supports layouts where this can be possible. If I understand correctly
>> this implementation will create event files for these domains but when the user attempts to
>> read the data it will fail. Can this work add some snippet about possibility of this
>> scenario and if/how it is supported?
>
> Yes, this is architecturally possible. But I do not expect that systems will
> be built that do this. You are right that such a system will create files that
> always return "Unavailable" when read.
>
> Is it sufficient to document this in the commit message?
>
> I don't feel that it would be worthwhile to suppress creation of these files for
> a "can't happen" situation. I'm not sure that doing so would be significantly
> better from a user interface perspective. Users would get slightly more notice
> (-ENOENT when trying to open the file). But the code would require
> architecture calls from file system code to check which files need to be created
> separately for each domain.
I think it is sufficient to document this in the commit message to help create
confidence in robustness in support of different scenarios. I have not encountered such
a system but could this scenario be similar to one where a two socket system supports MBM
but only one socket has memory populated? I do not know what reading the counter MSR will
return in this case though.
>
>>
>>> + if (tr->size != e->mmio_size) {
>>> + pr_warn("MMIO space wrong size (%zu bytes) for guid 0x%x. Expected %zu bytes.\n",
>>> + tr->size, e->guid, e->mmio_size);
>>> + return true;
>>> + }
>>> +
>>> return false;
>>> }
>>>
>>> +static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
>>> +{
>>> + bool usable_events = false;
>>> +
>>> + for (int i = 0; i < p->count; i++) {
>>> + if (skip_telem_region(&p->regions[i], e))
>>> + continue;
>>> + usable_events = true;
>>
>> A previous concern [1] was why this loop does not break out at this point. I think it will
>> help to make this clear if marking a telemetry region as unusable (mark_telem_region_unusable())
>> is done in this patch. Doing so makes the "usable" and "unusable" distinction in one
>> patch while making clear that the loop needs to complete.
>
> Ok. I'll pull mark_telem_region_unusable() into this patch.
Thank you.
Reinette
Powered by blists - more mailing lists