[PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode

Babu Moger posted 19 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode
Posted by Babu Moger 1 year, 8 months ago
The ABMC feature provides an option to the user to assign a hardware
counter to an RMID and monitor the bandwidth as long as it is assigned.
ABMC mode is enabled by default when supported. System can be one mode
at a time (Legacy monitor mode or ABMC mode).

Provide an interface to display the monitor mode on the system.
    $cat /sys/fs/resctrl/info/L3_MON/mbm_assign
    [abmc]
    legacy

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v4: Fixed the checks for legacy and abmc mode. Default it ABMC.

v3: New patch to display ABMC capability.
---
 Documentation/arch/x86/resctrl.rst     | 10 ++++++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 7ab8172ef208..ab3cde61a124 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -261,6 +261,16 @@ with the following files:
 	Available when ABMC feature is supported. The number of ABMC counters
 	available for configuration.
 
+"mbm_assign":
+	Available when ABMC feature is supported. Reports the list of assignable
+	monitoring features supported. The enclosed brackets indicate which
+	feature is enabled.
+	::
+
+	  cat /sys/fs/resctrl/info/L3_MON/mbm_assign
+	  [abmc]
+	  mbm_legacy
+
 "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 e75a6146068b..b1d002e5e93d 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -1071,6 +1071,7 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 
 		if (rdt_cpu_has(X86_FEATURE_ABMC)) {
 			r->abmc_capable = true;
+			resctrl_file_fflags_init("mbm_assign", RFTYPE_MON_INFO);
 			/*
 			 * Query CPUID_Fn80000020_EBX_x05 for number of
 			 * ABMC counters
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 9148d1234ede..3071bbb7a15e 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -856,6 +856,23 @@ static int rdtgroup_num_cntrs_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static int rdtgroup_mbm_assign_show(struct kernfs_open_file *of,
+				    struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+
+	if (hw_res->abmc_enabled) {
+		seq_puts(s, "[abmc]\n");
+		seq_puts(s, "mbm_legacy\n");
+	} else {
+		seq_puts(s, "abmc\n");
+		seq_puts(s, "[mbm_legacy]\n");
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*
@@ -1913,6 +1930,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= mbm_local_bytes_config_show,
 		.write		= mbm_local_bytes_config_write,
 	},
+	{
+		.name		= "mbm_assign",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_mbm_assign_show,
+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,
-- 
2.34.1
Re: [PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode
Posted by Reinette Chatre 1 year, 8 months ago
Hi Babu,

On 5/24/24 5:23 AM, Babu Moger wrote:
> The ABMC feature provides an option to the user to assign a hardware
> counter to an RMID and monitor the bandwidth as long as it is assigned.
> ABMC mode is enabled by default when supported. System can be one mode
> at a time (Legacy monitor mode or ABMC mode).
> 
> Provide an interface to display the monitor mode on the system.
>      $cat /sys/fs/resctrl/info/L3_MON/mbm_assign
>      [abmc]
>      legacy

Output is different from cover letter and what this patch implements.

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v4: Fixed the checks for legacy and abmc mode. Default it ABMC.
> 
> v3: New patch to display ABMC capability.
> ---
>   Documentation/arch/x86/resctrl.rst     | 10 ++++++++++
>   arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
>   3 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index 7ab8172ef208..ab3cde61a124 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -261,6 +261,16 @@ with the following files:
>   	Available when ABMC feature is supported. The number of ABMC counters
>   	available for configuration.
>   
> +"mbm_assign":

This name is not ideal but I am having trouble finding a better one ... I have
seen you use "monitor mode" a couple of times (even in shortlog), so maybe that
could be the start of a more generic name? "mbm_mode"?

> +	Available when ABMC feature is supported. Reports the list of assignable

Why not always make this file available? At least it will display that
legacy mode is supported and it gives user space an always present file to query to
determine support.

> +	monitoring features supported. The enclosed brackets indicate which
> +	feature is enabled.
> +	::
> +
> +	  cat /sys/fs/resctrl/info/L3_MON/mbm_assign
> +	  [abmc]
> +	  mbm_legacy
> +
>   "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 e75a6146068b..b1d002e5e93d 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -1071,6 +1071,7 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>   
>   		if (rdt_cpu_has(X86_FEATURE_ABMC)) {
>   			r->abmc_capable = true;
> +			resctrl_file_fflags_init("mbm_assign", RFTYPE_MON_INFO);
>   			/*
>   			 * Query CPUID_Fn80000020_EBX_x05 for number of
>   			 * ABMC counters
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 9148d1234ede..3071bbb7a15e 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -856,6 +856,23 @@ static int rdtgroup_num_cntrs_show(struct kernfs_open_file *of,
>   	return 0;
>   }
>   
> +static int rdtgroup_mbm_assign_show(struct kernfs_open_file *of,
> +				    struct seq_file *s, void *v)
> +{
> +	struct rdt_resource *r = of->kn->parent->priv;
> +	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);

Please use arch helper to just get abmc state instead of fs code
digging into arch structures.

> +
> +	if (hw_res->abmc_enabled) {
> +		seq_puts(s, "[abmc]\n");
> +		seq_puts(s, "mbm_legacy\n");
> +	} else {
> +		seq_puts(s, "abmc\n");
> +		seq_puts(s, "[mbm_legacy]\n");
> +	}
> +
> +	return 0;
> +}
> +
>   #ifdef CONFIG_PROC_CPU_RESCTRL
>   
>   /*
> @@ -1913,6 +1930,12 @@ static struct rftype res_common_files[] = {
>   		.seq_show	= mbm_local_bytes_config_show,
>   		.write		= mbm_local_bytes_config_write,
>   	},
> +	{
> +		.name		= "mbm_assign",
> +		.mode		= 0444,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= rdtgroup_mbm_assign_show,
> +	},
>   	{
>   		.name		= "cpus",
>   		.mode		= 0644,

Reinette
Re: [PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 year, 7 months ago
Hi Reinette,

On 6/13/24 20:40, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/24/24 5:23 AM, Babu Moger wrote:
>> The ABMC feature provides an option to the user to assign a hardware
>> counter to an RMID and monitor the bandwidth as long as it is assigned.
>> ABMC mode is enabled by default when supported. System can be one mode
>> at a time (Legacy monitor mode or ABMC mode).
>>
>> Provide an interface to display the monitor mode on the system.
>>      $cat /sys/fs/resctrl/info/L3_MON/mbm_assign
>>      [abmc]
>>      legacy
> 
> Output is different from cover letter and what this patch implements.

My bad. Will fix it,

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v4: Fixed the checks for legacy and abmc mode. Default it ABMC.
>>
>> v3: New patch to display ABMC capability.
>> ---
>>   Documentation/arch/x86/resctrl.rst     | 10 ++++++++++
>>   arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
>>   3 files changed, 34 insertions(+)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst
>> b/Documentation/arch/x86/resctrl.rst
>> index 7ab8172ef208..ab3cde61a124 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -261,6 +261,16 @@ with the following files:
>>       Available when ABMC feature is supported. The number of ABMC counters
>>       available for configuration.
>>   +"mbm_assign":
> 
> This name is not ideal but I am having trouble finding a better one ... I
> have
> seen you use "monitor mode" a couple of times (even in shortlog), so maybe
> that
> could be the start of a more generic name? "mbm_mode"?

mbm_mode sounds good. Like this.

$cat /sys/fs/resctrl/info/L3_MON/mbm_mode
[abmc]
legacy

Keeping just "legacy" vs mbm_legacy.


> 
>> +    Available when ABMC feature is supported. Reports the list of
>> assignable
> 
> Why not always make this file available? At least it will display that
> legacy mode is supported and it gives user space an always present file to
> query to
> determine support.

Ok. Sure.

> 
>> +    monitoring features supported. The enclosed brackets indicate which
>> +    feature is enabled.
>> +    ::
>> +
>> +      cat /sys/fs/resctrl/info/L3_MON/mbm_assign
>> +      [abmc]
>> +      mbm_legacy
>> +
>>   "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 e75a6146068b..b1d002e5e93d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>> @@ -1071,6 +1071,7 @@ int __init rdt_get_mon_l3_config(struct
>> rdt_resource *r)
>>             if (rdt_cpu_has(X86_FEATURE_ABMC)) {
>>               r->abmc_capable = true;
>> +            resctrl_file_fflags_init("mbm_assign", RFTYPE_MON_INFO);
>>               /*
>>                * Query CPUID_Fn80000020_EBX_x05 for number of
>>                * ABMC counters
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 9148d1234ede..3071bbb7a15e 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -856,6 +856,23 @@ static int rdtgroup_num_cntrs_show(struct
>> kernfs_open_file *of,
>>       return 0;
>>   }
>>   +static int rdtgroup_mbm_assign_show(struct kernfs_open_file *of,
>> +                    struct seq_file *s, void *v)
>> +{
>> +    struct rdt_resource *r = of->kn->parent->priv;
>> +    struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> 
> Please use arch helper to just get abmc state instead of fs code
> digging into arch structures.

Ok. I will use "resctrl_arch_get_abmc_enabled()" to get abmc state.

> 
>> +
>> +    if (hw_res->abmc_enabled) {
>> +        seq_puts(s, "[abmc]\n");
>> +        seq_puts(s, "mbm_legacy\n");
>> +    } else {
>> +        seq_puts(s, "abmc\n");
>> +        seq_puts(s, "[mbm_legacy]\n");
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   #ifdef CONFIG_PROC_CPU_RESCTRL
>>     /*
>> @@ -1913,6 +1930,12 @@ static struct rftype res_common_files[] = {
>>           .seq_show    = mbm_local_bytes_config_show,
>>           .write        = mbm_local_bytes_config_write,
>>       },
>> +    {
>> +        .name        = "mbm_assign",
>> +        .mode        = 0444,
>> +        .kf_ops        = &rdtgroup_kf_single_ops,
>> +        .seq_show    = rdtgroup_mbm_assign_show,
>> +    },
>>       {
>>           .name        = "cpus",
>>           .mode        = 0644,
> 
> Reinette
> 

-- 
Thanks
Babu Moger
Re: [PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode
Posted by Reinette Chatre 1 year, 7 months ago
Hi Babu,

On 6/19/24 9:25 AM, Moger, Babu wrote:
> On 6/13/24 20:40, Reinette Chatre wrote:
>> On 5/24/24 5:23 AM, Babu Moger wrote:

>>> --- a/Documentation/arch/x86/resctrl.rst
>>> +++ b/Documentation/arch/x86/resctrl.rst
>>> @@ -261,6 +261,16 @@ with the following files:
>>>        Available when ABMC feature is supported. The number of ABMC counters
>>>        available for configuration.
>>>    +"mbm_assign":
>>
>> This name is not ideal but I am having trouble finding a better one ... I
>> have
>> seen you use "monitor mode" a couple of times (even in shortlog), so maybe
>> that
>> could be the start of a more generic name? "mbm_mode"?
> 
> mbm_mode sounds good. Like this.
> 
> $cat /sys/fs/resctrl/info/L3_MON/mbm_mode
> [abmc]
> legacy
> 
> Keeping just "legacy" vs mbm_legacy.
> 

Looks good to me. This sounds generic enough to build on.

Reinette
Re: [PATCH v4 08/19] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 year, 7 months ago

On 6/20/2024 5:05 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 6/19/24 9:25 AM, Moger, Babu wrote:
>> On 6/13/24 20:40, Reinette Chatre wrote:
>>> On 5/24/24 5:23 AM, Babu Moger wrote:
> 
>>>> --- a/Documentation/arch/x86/resctrl.rst
>>>> +++ b/Documentation/arch/x86/resctrl.rst
>>>> @@ -261,6 +261,16 @@ with the following files:
>>>>        Available when ABMC feature is supported. The number of ABMC 
>>>> counters
>>>>        available for configuration.
>>>>    +"mbm_assign":
>>>
>>> This name is not ideal but I am having trouble finding a better one 
>>> ... I
>>> have
>>> seen you use "monitor mode" a couple of times (even in shortlog), so 
>>> maybe
>>> that
>>> could be the start of a more generic name? "mbm_mode"?
>>
>> mbm_mode sounds good. Like this.
>>
>> $cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>> [abmc]
>> legacy
>>
>> Keeping just "legacy" vs mbm_legacy.
>>
> 
> Looks good to me. This sounds generic enough to build on.

Thanks
-- 
- Babu Moger