The resctrl_event_id enum gives names to the counter event numbers on x86.
These are used directly by resctrl.

To allow the MPAM driver to keep an array of these the size of the enum
needs to be known.

Add a 'num_events' define which can be used to size an array. This isn't
a member of the enum to avoid updating switch statements that would
otherwise be missing a case.

Signed-off-by: James Morse <james.morse@arm.com>
Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl_types.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
index 51c51a1aabfb..70226f5ab3e3 100644
--- a/include/linux/resctrl_types.h
+++ b/include/linux/resctrl_types.h
@@ -51,4 +51,6 @@ enum resctrl_event_id {
 	QOS_L3_MBM_LOCAL_EVENT_ID	= 0x03,
 };
 
+#define QOS_NUM_EVENTS		(QOS_L3_MBM_LOCAL_EVENT_ID + 1)
+
 #endif /* __LINUX_RESCTRL_TYPES_H */
-- 
2.39.2

Hi James,

On 2/7/25 12:18, James Morse wrote:
> The resctrl_event_id enum gives names to the counter event numbers on x86.
> These are used directly by resctrl.
> 
> To allow the MPAM driver to keep an array of these the size of the enum
> needs to be known.
> 
> Add a 'num_events' define which can be used to size an array. This isn't
> a member of the enum to avoid updating switch statements that would
> otherwise be missing a case.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/linux/resctrl_types.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index 51c51a1aabfb..70226f5ab3e3 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h
> @@ -51,4 +51,6 @@ enum resctrl_event_id {
>  	QOS_L3_MBM_LOCAL_EVENT_ID	= 0x03,
>  };
>  
> +#define QOS_NUM_EVENTS		(QOS_L3_MBM_LOCAL_EVENT_ID + 1)

Why cant this be part of "enum resctrl_event_id" like we defined
RDT_NUM_RESOURCES?
-- 
Thanks
Babu Moger

Hi Babu,

On 27/02/2025 20:26, Moger, Babu wrote:
> On 2/7/25 12:18, James Morse wrote:
>> The resctrl_event_id enum gives names to the counter event numbers on x86.
>> These are used directly by resctrl.
>>
>> To allow the MPAM driver to keep an array of these the size of the enum
>> needs to be known.
>>
>> Add a 'num_events' define which can be used to size an array. This isn't
>> a member of the enum to avoid updating switch statements that would
>> otherwise be missing a case.

>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>> index 51c51a1aabfb..70226f5ab3e3 100644
>> --- a/include/linux/resctrl_types.h
>> +++ b/include/linux/resctrl_types.h
>> @@ -51,4 +51,6 @@ enum resctrl_event_id {
>>  	QOS_L3_MBM_LOCAL_EVENT_ID	= 0x03,
>>  };
>>  
>> +#define QOS_NUM_EVENTS		(QOS_L3_MBM_LOCAL_EVENT_ID + 1)

> Why cant this be part of "enum resctrl_event_id" like we defined
> RDT_NUM_RESOURCES?

Maybe its a difference that only exists in my head, but the rdt resource array is
completely a resctrl concept, the positions in the enum don't mean anything.
Not so for for resctrl_event_id - those numbers mean something to the X86 CPUs. Resctrl
needs some unique identifier for those, and its simpler just to use these directly. I
didn't want to add anything to this enum.

If there are mpam specific events, (currently there is only the risk of bandwidth counters
on the L2, or scattered at random through the system), I'd prefer to support them via perf
and keep them out of here completely.


Thanks,

James

> >> @@ -51,4 +51,6 @@ enum resctrl_event_id {
> >>    QOS_L3_MBM_LOCAL_EVENT_ID       = 0x03,
> >>  };
> >>
> >> +#define QOS_NUM_EVENTS            (QOS_L3_MBM_LOCAL_EVENT_ID + 1)
>
> > Why cant this be part of "enum resctrl_event_id" like we defined
> > RDT_NUM_RESOURCES?
>
> Maybe its a difference that only exists in my head, but the rdt resource array is
> completely a resctrl concept, the positions in the enum don't mean anything.
> Not so for for resctrl_event_id - those numbers mean something to the X86 CPUs. Resctrl
> needs some unique identifier for those, and its simpler just to use these directly. I
> didn't want to add anything to this enum.
>
> If there are mpam specific events, (currently there is only the risk of bandwidth counters
> on the L2, or scattered at random through the system), I'd prefer to support them via perf
> and keep them out of here completely.

Also note that resctrl code has some "switch (evtid) {" code ... if you make QOS_NUM_EVENTS
a member of the enum, then the compiler will warn if you don't have a "default:" or a
"case QOS_NUM_EVENTS:" to cover all the options.

We don't have any "switch (r->rid)"

-Tony

Hi James,

On 2/7/25 10:18 AM, James Morse wrote:
> The resctrl_event_id enum gives names to the counter event numbers on x86.
> These are used directly by resctrl.
> 
> To allow the MPAM driver to keep an array of these the size of the enum
> needs to be known.
> 
> Add a 'num_events' define which can be used to size an array. This isn't
> a member of the enum to avoid updating switch statements that would
> otherwise be missing a case.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette