[PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC

Babu Moger posted 26 patches 3 weeks, 5 days ago
[PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Babu Moger 3 weeks, 5 days ago
The ABMC feature provides an option to the user to assign a hardware
counter to an RMID, event pair and monitor the bandwidth as long as it is
assigned. The assigned RMID will be tracked by the hardware until the user
unassigns it manually.

Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
specifying the counter id, bandwidth source, and bandwidth types.

Provide the interface to assign the counter ids to RMID.

The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
    Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
    Monitoring (ABMC).

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v9: Removed the code to reset the architectural state. It will done
    in another patch.

v8: Rename resctrl_arch_assign_cntr to resctrl_arch_config_cntr.

v7: Separated arch and fs functions. This patch only has arch implementation.
    Added struct rdt_resource to the interface resctrl_arch_assign_cntr.
    Rename rdtgroup_abmc_cfg() to resctrl_abmc_config_one_amd().

v6: Removed mbm_cntr_alloc() from this patch to keep fs and arch code
    separate.
    Added code to update the counter assignment at domain level.

v5: Few name changes to match cntr_id.
    Changed the function names to
      rdtgroup_assign_cntr
      resctr_arch_assign_cntr
      More comments on commit log.
      Added function summary.

v4: Commit message update.
      User bitmap APIs where applicable.
      Changed the interfaces considering MPAM(arm).
      Added domain specific assignment.

v3: Removed the static from the prototype of rdtgroup_assign_abmc.
      The function is not called directly from user anymore. These
      changes are related to global assignment interface.

v2: Minor text changes in commit message.
---
 arch/x86/kernel/cpu/resctrl/internal.h |  3 ++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 38 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index d1f3f3ca4df9..00f7bf60e16a 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -714,6 +714,9 @@ int mbm_cntr_alloc(struct rdt_resource *r);
 void mbm_cntr_free(struct rdt_resource *r, u32 cntr_id);
 void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
 unsigned int mon_event_config_index_get(u32 evtid);
+int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
+			     enum resctrl_event_id evtid, u32 rmid, u32 closid,
+			     u32 cntr_id, bool assign);
 void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 36845e8e400d..1b5529c212f5 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1886,6 +1886,44 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
 	return ret ?: nbytes;
 }
 
+static void resctrl_abmc_config_one_amd(void *info)
+{
+	u64 *msrval = info;
+
+	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *msrval);
+}
+
+/*
+ * Send an IPI to the domain to assign the counter to RMID, event pair.
+ */
+int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
+			     enum resctrl_event_id evtid, u32 rmid, u32 closid,
+			     u32 cntr_id, bool assign)
+{
+	struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
+	union l3_qos_abmc_cfg abmc_cfg = { 0 };
+	struct arch_mbm_state *arch_mbm;
+
+	abmc_cfg.split.cfg_en = 1;
+	abmc_cfg.split.cntr_en = assign ? 1 : 0;
+	abmc_cfg.split.cntr_id = cntr_id;
+	abmc_cfg.split.bw_src = rmid;
+
+	/* Update the event configuration from the domain */
+	if (evtid == QOS_L3_MBM_TOTAL_EVENT_ID) {
+		abmc_cfg.split.bw_type = hw_dom->mbm_total_cfg;
+		arch_mbm = &hw_dom->arch_mbm_total[rmid];
+	} else {
+		abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
+		arch_mbm = &hw_dom->arch_mbm_local[rmid];
+	}
+
+	smp_call_function_any(&d->hdr.cpu_mask, resctrl_abmc_config_one_amd,
+			      &abmc_cfg, 1);
+
+	return 0;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
-- 
2.34.1
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Reinette Chatre 1 week, 2 days ago
Hi Babu,

On 10/29/24 4:21 PM, Babu Moger wrote:
> The ABMC feature provides an option to the user to assign a hardware
> counter to an RMID, event pair and monitor the bandwidth as long as it is
> assigned. The assigned RMID will be tracked by the hardware until the user
> unassigns it manually.
> 
> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
> specifying the counter id, bandwidth source, and bandwidth types.

needs imperative tone

> 
> Provide the interface to assign the counter ids to RMID.
> 
> The feature details are documented in the APM listed below [1].
> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>     Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
>     Monitoring (ABMC).
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> Signed-off-by: Babu Moger <babu.moger@amd.com>

Reinette
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Moger, Babu 5 days, 8 hours ago
Hi Reinette,

On 11/15/24 18:44, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/29/24 4:21 PM, Babu Moger wrote:
>> The ABMC feature provides an option to the user to assign a hardware
>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>> assigned. The assigned RMID will be tracked by the hardware until the user
>> unassigns it manually.
>>
>> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
>> specifying the counter id, bandwidth source, and bandwidth types.
> 
> needs imperative tone

How about this?

Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and
specifying the counter ID, bandwidth source, and bandwidth types.

> 
>>
>> Provide the interface to assign the counter ids to RMID.
>>
>> The feature details are documented in the APM listed below [1].
>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>>     Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
>>     Monitoring (ABMC).
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
> 
> Reinette
> 

-- 
Thanks
Babu Moger
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Reinette Chatre 3 days, 8 hours ago
Hi Babu,

On 11/19/24 12:12 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 11/15/24 18:44, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 10/29/24 4:21 PM, Babu Moger wrote:
>>> The ABMC feature provides an option to the user to assign a hardware
>>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>>> assigned. The assigned RMID will be tracked by the hardware until the user
>>> unassigns it manually.
>>>
>>> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
>>> specifying the counter id, bandwidth source, and bandwidth types.
>>
>> needs imperative tone
> 
> How about this?
> 
> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and
> specifying the counter ID, bandwidth source, and bandwidth types.
> 

ok with me. Exactly what ChatGPT suggests.

Please do note that that first paragraph informs reader that
a counter is assigned by user to "an RMID, event pair" while the hardware is configured with
"the counter ID, bandwidth source, and bandwidth types". There thus does not seem
to be a clear connection between what user assigns and what is programmed to hardware.

Reinette
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Moger, Babu 2 days, 10 hours ago
Hi Reinette,

On 11/21/2024 2:18 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 11/19/24 12:12 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 11/15/24 18:44, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 10/29/24 4:21 PM, Babu Moger wrote:
>>>> The ABMC feature provides an option to the user to assign a hardware
>>>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>>>> assigned. The assigned RMID will be tracked by the hardware until the user
>>>> unassigns it manually.
>>>>
>>>> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
>>>> specifying the counter id, bandwidth source, and bandwidth types.
>>>
>>> needs imperative tone
>>
>> How about this?
>>
>> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and
>> specifying the counter ID, bandwidth source, and bandwidth types.
>>
> 
> ok with me. Exactly what ChatGPT suggests.

Hmm. ):

> 
> Please do note that that first paragraph informs reader that
> a counter is assigned by user to "an RMID, event pair" while the hardware is configured with
> "the counter ID, bandwidth source, and bandwidth types". There thus does not seem
> to be a clear connection between what user assigns and what is programmed to hardware.
> 

Adding RMID in the text might help.

Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and 
specifying the counter ID, RMID, bandwidth source, and bandwidth types.


-- 
- Babu Moger
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Reinette Chatre 2 days, 7 hours ago
Hi Babu,

On 11/22/24 10:54 AM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 11/21/2024 2:18 PM, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 11/19/24 12:12 PM, Moger, Babu wrote:
>>> Hi Reinette,
>>>
>>> On 11/15/24 18:44, Reinette Chatre wrote:
>>>> Hi Babu,
>>>>
>>>> On 10/29/24 4:21 PM, Babu Moger wrote:
>>>>> The ABMC feature provides an option to the user to assign a hardware
>>>>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>>>>> assigned. The assigned RMID will be tracked by the hardware until the user
>>>>> unassigns it manually.
>>>>>
>>>>> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
>>>>> specifying the counter id, bandwidth source, and bandwidth types.
>>>>
>>>> needs imperative tone
>>>
>>> How about this?
>>>
>>> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and
>>> specifying the counter ID, bandwidth source, and bandwidth types.
>>>
>>
>> ok with me. Exactly what ChatGPT suggests.
> 
> Hmm. ):
> 
>>
>> Please do note that that first paragraph informs reader that
>> a counter is assigned by user to "an RMID, event pair" while the hardware is configured with
>> "the counter ID, bandwidth source, and bandwidth types". There thus does not seem
>> to be a clear connection between what user assigns and what is programmed to hardware.
>>
> 
> Adding RMID in the text might help.
> 
> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and specifying the counter ID, RMID, bandwidth source, and bandwidth types.
> 

Isn't the bandwidth source and the RMID the same thing? How about something like:
"Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and specifying
 the counter ID, bandwidth source (RMID), and bandwidth event configuration."

Please feel free to improve.

Reinette
Re: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Moger, Babu 2 days, 4 hours ago
Hi Reinette,

On 11/22/2024 3:52 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 11/22/24 10:54 AM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 11/21/2024 2:18 PM, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 11/19/24 12:12 PM, Moger, Babu wrote:
>>>> Hi Reinette,
>>>>
>>>> On 11/15/24 18:44, Reinette Chatre wrote:
>>>>> Hi Babu,
>>>>>
>>>>> On 10/29/24 4:21 PM, Babu Moger wrote:
>>>>>> The ABMC feature provides an option to the user to assign a hardware
>>>>>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>>>>>> assigned. The assigned RMID will be tracked by the hardware until the user
>>>>>> unassigns it manually.
>>>>>>
>>>>>> Counters are configured by writing to L3_QOS_ABMC_CFG MSR and
>>>>>> specifying the counter id, bandwidth source, and bandwidth types.
>>>>>
>>>>> needs imperative tone
>>>>
>>>> How about this?
>>>>
>>>> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and
>>>> specifying the counter ID, bandwidth source, and bandwidth types.
>>>>
>>>
>>> ok with me. Exactly what ChatGPT suggests.
>>
>> Hmm. ):
>>
>>>
>>> Please do note that that first paragraph informs reader that
>>> a counter is assigned by user to "an RMID, event pair" while the hardware is configured with
>>> "the counter ID, bandwidth source, and bandwidth types". There thus does not seem
>>> to be a clear connection between what user assigns and what is programmed to hardware.
>>>
>>
>> Adding RMID in the text might help.
>>
>> Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and specifying the counter ID, RMID, bandwidth source, and bandwidth types.
>>
> 
> Isn't the bandwidth source and the RMID the same thing? How about something like:
> "Configure the counters by writing to the L3_QOS_ABMC_CFG MSR and specifying
>   the counter ID, bandwidth source (RMID), and bandwidth event configuration."
> 
> Please feel free to improve.

Looks good. Thanks
-- 
- Babu Moger
RE: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Luck, Tony 3 weeks, 5 days ago
> +int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
> +                          enum resctrl_event_id evtid, u32 rmid, u32 closid,
> +                          u32 cntr_id, bool assign)
> +{
> +     struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
> +     union l3_qos_abmc_cfg abmc_cfg = { 0 };
> +     struct arch_mbm_state *arch_mbm;
> +
> +     abmc_cfg.split.cfg_en = 1;
> +     abmc_cfg.split.cntr_en = assign ? 1 : 0;
> +     abmc_cfg.split.cntr_id = cntr_id;
> +     abmc_cfg.split.bw_src = rmid;
> +
> +     /* Update the event configuration from the domain */
> +     if (evtid == QOS_L3_MBM_TOTAL_EVENT_ID) {
> +             abmc_cfg.split.bw_type = hw_dom->mbm_total_cfg;
> +             arch_mbm = &hw_dom->arch_mbm_total[rmid];
> +     } else {
> +             abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
> +             arch_mbm = &hw_dom->arch_mbm_local[rmid];
> +     }
> +
> +     smp_call_function_any(&d->hdr.cpu_mask, resctrl_abmc_config_one_amd,
> +                           &abmc_cfg, 1);
> +
> +     return 0;
> +}

Compiling with W=1:

warning: variable 'arch_mbm' set but not used [-Wunused-but-set-variable]

[still not used by patch 26]

-Tony
Re: RE: [PATCH v9 17/26] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC
Posted by Moger, Babu 3 weeks, 4 days ago
Hi Tony,


On 10/29/24 18:54, Luck, Tony wrote:
>> +int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
>> +                          enum resctrl_event_id evtid, u32 rmid, u32 closid,
>> +                          u32 cntr_id, bool assign)
>> +{
>> +     struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
>> +     union l3_qos_abmc_cfg abmc_cfg = { 0 };
>> +     struct arch_mbm_state *arch_mbm;
>> +
>> +     abmc_cfg.split.cfg_en = 1;
>> +     abmc_cfg.split.cntr_en = assign ? 1 : 0;
>> +     abmc_cfg.split.cntr_id = cntr_id;
>> +     abmc_cfg.split.bw_src = rmid;
>> +
>> +     /* Update the event configuration from the domain */
>> +     if (evtid == QOS_L3_MBM_TOTAL_EVENT_ID) {
>> +             abmc_cfg.split.bw_type = hw_dom->mbm_total_cfg;
>> +             arch_mbm = &hw_dom->arch_mbm_total[rmid];
>> +     } else {
>> +             abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
>> +             arch_mbm = &hw_dom->arch_mbm_local[rmid];
>> +     }
>> +
>> +     smp_call_function_any(&d->hdr.cpu_mask, resctrl_abmc_config_one_amd,
>> +                           &abmc_cfg, 1);
>> +
>> +     return 0;
>> +}
> 
> Compiling with W=1:
> 
> warning: variable 'arch_mbm' set but not used [-Wunused-but-set-variable]
> 
> [still not used by patch 26]

I knew I am going to miss something like this.   Thanks. Will fix it.

-- 
Thanks
Babu Moger