Re: [PATCH v3 25/26] fs-x86/resctrl: Add detailed descriptions for Clearwater Forest events

Reinette Chatre
2025-04-19T05:31:19Z

Hi Tony,

On 4/7/25 4:40 PM, Tony Luck wrote:
> There are two event groups one for energy reporting and another
> for "perf" events.

Please add context.

>
> See the XML description files in https://github.com/intel/Intel-PMT
> in the xml/CWF/OOBMSM/{RMID-ENERGY,RMID-PERF}/ for the detailed
> descriptions that were used to derive these descriptions.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>

The url in text can be a "Link:" here.

> ---
> arch/x86/kernel/cpu/resctrl/intel_aet.c | 57 +++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index 529f6d49e3a3..e1097767009e 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -42,6 +42,8 @@ struct pmt_event {
> enum resctrl_event_type type;
> };
>
> +#define EVT(id, offset, _type) { .evtid = id, .evt_offset = offset, .type = _type }
> +
> /**
> * struct telem_entry - Summarized form from XML telemetry description
> * @name: Name for this group of events
> @@ -82,8 +84,63 @@ static struct evtinfo {
>
> #define EVT_OFFSET(evtid) (evtinfo[evtid].pmt_event->evt_offset)
>
> +/*
> + * https://github.com/intel/Intel-PMT
> + * xml/CWF/OOBMSM/RMID-ENERGY *.xml

This can be one line.

> + */
> +#define NUM_RMIDS_0x26696143 576
> +#define GUID_0x26696143 0x26696143
> +#define NUM_EVENTS_0x26696143 2
> +#define EVT_BYTES_0x26696143 (NUM_RMIDS_0x26696143 * NUM_EVENTS_0x26696143 * sizeof(u64))
> +
> +static struct telem_entry energy_0x26696143 = {
> + .name = "energy",
> + .guid = GUID_0x26696143,
> + .size = EVT_BYTES_0x26696143 + sizeof(u64) * 3,
> + .num_rmids = NUM_RMIDS_0x26696143,
> + .overflow_counter_off = EVT_BYTES_0x26696143 + sizeof(u64) * 0,
> + .last_overflow_tstamp_off = EVT_BYTES_0x26696143 + sizeof(u64) * 1,
> + .last_update_tstamp_off = EVT_BYTES_0x26696143 + sizeof(u64) * 2,
> + .num_events = NUM_EVENTS_0x26696143,
> + .evts = {
> + EVT(PMT_EVENT_ENERGY, 0x0, EVT_TYPE_U46_18),
> + EVT(PMT_EVENT_ACTIVITY, 0x8, EVT_TYPE_U46_18),
> + }
> +};
> +
> +/*
> + * https://github.com/intel/Intel-PMT
> + * xml/CWF/OOBMSM/RMID-PERF *.xml

This can be one line.

> + */
> +#define NUM_RMIDS_0x26557651 576
> +#define GUID_0x26557651 0x26557651
> +#define NUM_EVENTS_0x26557651 7
> +#define EVT_BYTES_0x26557651 (NUM_RMIDS_0x26557651 * NUM_EVENTS_0x26557651 * sizeof(u64))
> +
> +static struct telem_entry perf_0x26557651 = {
> + .name = "perf",
> + .guid = GUID_0x26557651,
> + .size = EVT_BYTES_0x26557651 + sizeof(u64) * 3,
> + .num_rmids = NUM_RMIDS_0x26557651,
> + .overflow_counter_off = EVT_BYTES_0x26557651 + sizeof(u64) * 0,
> + .last_overflow_tstamp_off = EVT_BYTES_0x26557651 + sizeof(u64) * 1,
> + .last_update_tstamp_off = EVT_BYTES_0x26557651 + sizeof(u64) * 2,
> + .num_events = NUM_EVENTS_0x26557651,
> + .evts = {
> + EVT(PMT_EVENT_STALLS_LLC_HIT, 0x0, EVT_TYPE_U64),
> + EVT(PMT_EVENT_C1_RES, 0x8, EVT_TYPE_U64),
> + EVT(PMT_EVENT_UNHALTED_CORE_CYCLES, 0x10, EVT_TYPE_U64),
> + EVT(PMT_EVENT_STALLS_LLC_MISS, 0x18, EVT_TYPE_U64),
> + EVT(PMT_EVENT_AUTO_C6_RES, 0x20, EVT_TYPE_U64),
> + EVT(PMT_EVENT_UNHALTED_REF_CYCLES, 0x28, EVT_TYPE_U64),
> + EVT(PMT_EVENT_UOPS_RETIRED, 0x30, EVT_TYPE_U64),
> + }
> +};
> +
> /* All known telemetry event groups */
> static struct telem_entry *telem_entry[] = {
> + &energy_0x26696143,
> + &perf_0x26557651,
> NULL

Looks like a change from previous design to use telem_entry::num_events instead
of NULL entry. The NULL entry can thus be removed?

> };
>

Reinette