[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <33aff696-21ce-4e84-b7cb-6f05cdef0402@intel.com>
Date: Fri, 23 May 2025 16:38:32 -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 v5 05/29] x86/rectrl: Fake OOBMSM interface
Hi Tony,
shortlog: x86/rectrl -> x86/resctrl
How many reports of a typo does it take for typo to be fixed?
V2: https://lore.kernel.org/all/b69bee17-6a84-4cb2-ab8a-2793c2fe7c49@intel.com/
V3: https://lore.kernel.org/lkml/2897fc2a-8977-4415-ae6d-bd0002874b3a@intel.com/
On 5/21/25 3:50 PM, Tony Luck wrote:
> +
> +/*
> + * Amount of memory for each fake MMIO space
> + * Magic numbers here match values for XML ID 0x26696143 and 0x26557651
> + * 576: Number of RMIDs
> + * 2: Energy events in 0x26557651
> + * 7: Perf events in 0x26696143
> + * 3: Qwords for status counters after the event counters
> + * 8: Bytes for each counter
> + */
> +
> +#define ENERGY_QWORDS ((576 * 2) + 3)
> +#define ENERGY_SIZE (ENERGY_QWORDS * 8)
> +#define PERF_QWORDS ((576 * 7) + 3)
> +#define PERF_SIZE (PERF_QWORDS * 8)
> +
First time asking why energy and perf are both using 576 RMIDs:
V3: https://lore.kernel.org/lkml/2897fc2a-8977-4415-ae6d-bd0002874b3a@intel.com/
Reminded in V4 that V3's question has not been answered:
https://lore.kernel.org/lkml/7fa19421-9093-411b-b8e2-da56156a9971@intel.com/
Question is still not answered in this version (neither was it answered in
response to the emails where I asked the questions).
> +
> +/*
> + * Set up a fake return for call to:
> + * intel_pmt_get_regions_by_feature(FEATURE_PER_RMID_ENERGY_TELEM);
> + * Pretend there are two aggregators on each of the sockets to test
> + * the code that sums over multiple aggregators.
> + * Pretend this group only supports 64 RMIDs to exercise the code
> + * that reconciles support for different RMID counts.
> + */
This version adds this comment. How does it answer the original question?
The comment highlights that energy uses 64 RMIDs and thus highlights that
it is unexpected that above defines uses 576 for energy RMIDs.
Reader is left to decipher the code from multiple patches later to try make
sense of this without it every explained.
Repeating a question and reporting a typo three times makes for an
unproductive and frustrating review. I usually work through a whole series
before I post feedback but when I got to this patch I did not feel like going
further. Why should I bother?
Reinette
Powered by blists - more mailing lists