The ABMC feature provides an option to the user to assign a hardware
counter to an RMID and monitor the bandwidth as long as the counter is
assigned. Number of assignments depend on number of monitoring counters
available.

Provide the interface to display the number of monitoring counters
supported.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v6: No changes.

v5: Changed the display name from num_cntrs to num_mbm_cntrs.
    Updated the commit message.
    Moved the patch after mbm_mode is introduced.

v4: Changed the counter name to num_cntrs. And few text changes.

v3: Changed the field name to mbm_assign_cntrs.

v2: Changed the field name to mbm_assignable_counters from abmc_counte
---
 Documentation/arch/x86/resctrl.rst     |  3 +++
 arch/x86/kernel/cpu/resctrl/monitor.c  |  2 ++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index d4ec605b200a..fe9f10766c4f 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -291,6 +291,9 @@ with the following files:
 		as long as there are enough RMID counters available to support number
 		of monitoring groups.
 
+"num_mbm_cntrs":
+	The number of monitoring counters available for assignment.
+
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
 		bytes) at which a previously used LLC_occupancy
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 5e8706ab6361..83329cefebf7 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 			r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
 			if (WARN_ON(r->mon.num_mbm_cntrs > 64))
 				r->mon.num_mbm_cntrs = 64;
+
+			resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
 		}
 	}
 
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d8f85b20ab8f..ab4fab3b7cf1 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -865,6 +865,16 @@ static int rdtgroup_mbm_mode_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static int rdtgroup_num_mbm_cntrs_show(struct kernfs_open_file *of,
+				       struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+
+	seq_printf(s, "%d\n", r->mon.num_mbm_cntrs);
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*
@@ -1936,6 +1946,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= rdtgroup_cpus_show,
 		.fflags		= RFTYPE_BASE,
 	},
+	{
+		.name		= "num_mbm_cntrs",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_num_mbm_cntrs_show,
+	},
 	{
 		.name		= "cpus_list",
 		.mode		= 0644,
-- 
2.34.1

Hi Babu,

On 8/6/24 3:00 PM, Babu Moger wrote:
> The ABMC feature provides an option to the user to assign a hardware

Here and in all patches, when referring to resctrl fs please use the more
generic "mbm_assign_cntr" mode to distinguish it from the hardware/architecture
specific code that involves ABMC. Something like

"The ABMC feature provides" -> ""mbm_cntr_assign" mode provides"

I also think that being explicit with this separation will help us to see
gaps in interface between resctrl fs and arch.

> counter to an RMID and monitor the bandwidth as long as the counter is

Please clarify the scope of this feature. Above mentions that a counter is
assigned to an RMID but later it is mentioned that the counter is assigned
to an event. Perhaps consistently mention that a counter is assigned to
a RMID,event pair?

> assigned. Number of assignments depend on number of monitoring counters
> available.
> 
> Provide the interface to display the number of monitoring counters
> supported.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v6: No changes.
> 
> v5: Changed the display name from num_cntrs to num_mbm_cntrs.
>      Updated the commit message.
>      Moved the patch after mbm_mode is introduced.
> 
> v4: Changed the counter name to num_cntrs. And few text changes.
> 
> v3: Changed the field name to mbm_assign_cntrs.
> 
> v2: Changed the field name to mbm_assignable_counters from abmc_counte
> ---
>   Documentation/arch/x86/resctrl.rst     |  3 +++
>   arch/x86/kernel/cpu/resctrl/monitor.c  |  2 ++
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index d4ec605b200a..fe9f10766c4f 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -291,6 +291,9 @@ with the following files:
>   		as long as there are enough RMID counters available to support number
>   		of monitoring groups.
>   
> +"num_mbm_cntrs":
> +	The number of monitoring counters available for assignment.
> +
>   "max_threshold_occupancy":
>   		Read/write file provides the largest value (in
>   		bytes) at which a previously used LLC_occupancy
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 5e8706ab6361..83329cefebf7 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>   			r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
>   			if (WARN_ON(r->mon.num_mbm_cntrs > 64))
>   				r->mon.num_mbm_cntrs = 64;
> +
> +			resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);

The arch code should not access the resctrl file flags. This should be moved to make
the MPAM support easier. With the arch code setting r->mon.mbm_cntr_assignable the
fs code can use that to set the flags. Something similar to below patch is needed:
https://lore.kernel.org/lkml/20240802172853.22529-27-james.morse@arm.com/

>   		}
>   	}
>   

Reinette

Hi Reinette,

On 8/16/24 16:34, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/6/24 3:00 PM, Babu Moger wrote:
>> The ABMC feature provides an option to the user to assign a hardware
> 
> Here and in all patches, when referring to resctrl fs please use the more
> generic "mbm_assign_cntr" mode to distinguish it from the
> hardware/architecture
> specific code that involves ABMC. Something like
> 
> "The ABMC feature provides" -> ""mbm_cntr_assign" mode provides"

Sure.

> 
> I also think that being explicit with this separation will help us to see
> gaps in interface between resctrl fs and arch.

Yes.
> 
>> counter to an RMID and monitor the bandwidth as long as the counter is
> 
> Please clarify the scope of this feature. Above mentions that a counter is
> assigned to an RMID but later it is mentioned that the counter is assigned
> to an event. Perhaps consistently mention that a counter is assigned to
> a RMID,event pair?

Yes.
"a counter is assigned to an RMID,event pair" gives more clarity.
Will changes it.

> 
>> assigned. Number of assignments depend on number of monitoring counters
>> available.
>>
>> Provide the interface to display the number of monitoring counters
>> supported.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v6: No changes.
>>
>> v5: Changed the display name from num_cntrs to num_mbm_cntrs.
>>      Updated the commit message.
>>      Moved the patch after mbm_mode is introduced.
>>
>> v4: Changed the counter name to num_cntrs. And few text changes.
>>
>> v3: Changed the field name to mbm_assign_cntrs.
>>
>> v2: Changed the field name to mbm_assignable_counters from abmc_counte
>> ---
>>   Documentation/arch/x86/resctrl.rst     |  3 +++
>>   arch/x86/kernel/cpu/resctrl/monitor.c  |  2 ++
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
>>   3 files changed, 21 insertions(+)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst
>> b/Documentation/arch/x86/resctrl.rst
>> index d4ec605b200a..fe9f10766c4f 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -291,6 +291,9 @@ with the following files:
>>           as long as there are enough RMID counters available to support
>> number
>>           of monitoring groups.
>>   +"num_mbm_cntrs":
>> +    The number of monitoring counters available for assignment.
>> +
>>   "max_threshold_occupancy":
>>           Read/write file provides the largest value (in
>>           bytes) at which a previously used LLC_occupancy
>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
>> b/arch/x86/kernel/cpu/resctrl/monitor.c
>> index 5e8706ab6361..83329cefebf7 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>> @@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct
>> rdt_resource *r)
>>               r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
>>               if (WARN_ON(r->mon.num_mbm_cntrs > 64))
>>                   r->mon.num_mbm_cntrs = 64;
>> +
>> +            resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
> 
> The arch code should not access the resctrl file flags. This should be
> moved to make
> the MPAM support easier. With the arch code setting
> r->mon.mbm_cntr_assignable the
> fs code can use that to set the flags. Something similar to below patch is
> needed:
> https://lore.kernel.org/lkml/20240802172853.22529-27-james.morse@arm.com/

It is just moving the calls resctrl_file_fflags_init() to resctrl_init().
The  rdt_resource fields are already setup here. Something like
https://lore.kernel.org/lkml/20240802172853.22529-20-james.morse@arm.com/

I feel it is better done when MBAM fs/arch separation.
-- 
Thanks
Babu Moger

Hi Babu,

On 8/20/24 8:56 AM, Moger, Babu wrote:
> On 8/16/24 16:34, Reinette Chatre wrote:
>> On 8/6/24 3:00 PM, Babu Moger wrote:

>>> diff --git a/Documentation/arch/x86/resctrl.rst
>>> b/Documentation/arch/x86/resctrl.rst
>>> index d4ec605b200a..fe9f10766c4f 100644
>>> --- a/Documentation/arch/x86/resctrl.rst
>>> +++ b/Documentation/arch/x86/resctrl.rst
>>> @@ -291,6 +291,9 @@ with the following files:
>>>            as long as there are enough RMID counters available to support
>>> number
>>>            of monitoring groups.
>>>    +"num_mbm_cntrs":
>>> +    The number of monitoring counters available for assignment.
>>> +
>>>    "max_threshold_occupancy":
>>>            Read/write file provides the largest value (in
>>>            bytes) at which a previously used LLC_occupancy
>>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
>>> b/arch/x86/kernel/cpu/resctrl/monitor.c
>>> index 5e8706ab6361..83329cefebf7 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>>> @@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct
>>> rdt_resource *r)
>>>                r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
>>>                if (WARN_ON(r->mon.num_mbm_cntrs > 64))
>>>                    r->mon.num_mbm_cntrs = 64;
>>> +
>>> +            resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
>>
>> The arch code should not access the resctrl file flags. This should be
>> moved to make
>> the MPAM support easier. With the arch code setting
>> r->mon.mbm_cntr_assignable the
>> fs code can use that to set the flags. Something similar to below patch is
>> needed:
>> https://lore.kernel.org/lkml/20240802172853.22529-27-james.morse@arm.com/
> 
> It is just moving the calls resctrl_file_fflags_init() to resctrl_init().
> The  rdt_resource fields are already setup here. Something like
> https://lore.kernel.org/lkml/20240802172853.22529-20-james.morse@arm.com/
> 
> I feel it is better done when MBAM fs/arch separation.

Indeed, it belongs with the rest of the mon state setup that is organized
as part of the MPAM work.

Reinette