[RFC PATCH v3 12/17] x86/resctrl: Add the functionality to assign the RMID

Babu Moger posted 17 patches 1 year, 10 months ago
There is a newer version of this series
[RFC PATCH v3 12/17] x86/resctrl: Add the functionality to assign the RMID
Posted by Babu Moger 1 year, 10 months ago
With the support of ABMC (Assignable Bandwidth Monitoring Counters)
feature, the user has the option to assign or unassign the RMID to
hardware counter and monitor the bandwidth for the longer duration.

Provide the interface to assign the counter to the group.

The ABMC feature implements a pair of MSRs, L3_QOS_ABMC_CFG (MSR
C000_03FDh) and L3_QOS_ABMC_DSC (MSR C000_3FEh). Each logical processor
implements a separate copy of these registers. Attempts to read or write
these MSRs when ABMC is not enabled will result in a #GP(0) exception.

Individual assignable bandwidth counters are configured by writing to
L3_QOS_ABMC_CFG MSR and specifying the Counter ID, Bandwidth Source, and
Bandwidth Types. Reading L3_QOS_ABMC_DSC returns the configuration of the
counter specified by L3_QOS_ABMC_CFG [CtrID].

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).

Signed-off-by: Babu Moger <babu.moger@amd.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
---
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 |  1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 86 ++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 88453c86474b..9d84c80104f9 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -651,6 +651,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
 void __init resctrl_file_fflags_init(const char *config,
 				     unsigned long fflags);
 void arch_domain_mbm_evt_config(struct rdt_hw_domain *hw_dom);
+ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state);
 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 7f54788a58de..cfbdaf8b5f83 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -202,6 +202,18 @@ static void assign_cntrs_init(void)
 	assign_cntrs_free_map_len = r->mbm_assign_cntrs;
 }
 
+static int assign_cntrs_alloc(void)
+{
+	u32 counterid = ffs(assign_cntrs_free_map);
+
+	if (counterid == 0)
+		return -ENOSPC;
+	counterid--;
+	assign_cntrs_free_map &= ~(1 << counterid);
+
+	return counterid;
+}
+
 /**
  * rdtgroup_mode_by_closid - Return mode of resource group with closid
  * @closid: closid if the resource group
@@ -1848,6 +1860,80 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
 	return ret ?: nbytes;
 }
 
+static void rdtgroup_abmc_msrwrite(void *info)
+{
+	u64 *msrval = info;
+
+	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *msrval);
+}
+
+static void rdtgroup_abmc_domain(struct rdt_domain *d, struct rdtgroup *rdtgrp,
+				 u32 evtid, int index, bool assign)
+{
+	struct rdt_hw_domain *hw_dom = resctrl_to_arch_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.ctr_en = assign ? 1 : 0;
+	abmc_cfg.split.ctr_id = rdtgrp->mon.abmc_ctr_id[index];
+	abmc_cfg.split.bw_src = rdtgrp->mon.rmid;
+
+	/*
+	 * Read the event configuration from the domain and pass it as
+	 * bw_type.
+	 */
+	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[rdtgrp->mon.rmid];
+	} else {
+		abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
+		arch_mbm = &hw_dom->arch_mbm_local[rdtgrp->mon.rmid];
+	}
+
+	smp_call_function_any(&d->cpu_mask, rdtgroup_abmc_msrwrite, &abmc_cfg, 1);
+
+	/* Reset the internal counters */
+	if (arch_mbm)
+		memset(arch_mbm, 0, sizeof(struct arch_mbm_state));
+}
+
+ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state)
+{
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+	int counterid = 0, index;
+	struct rdt_domain *d;
+
+	if (rdtgrp->mon.mon_state & mon_state) {
+		rdt_last_cmd_puts("ABMC counter is assigned already\n");
+		return 0;
+	}
+
+	index = mon_event_config_index_get(evtid);
+	if (index == INVALID_CONFIG_INDEX) {
+		pr_warn_once("Invalid event id %d\n", evtid);
+		return -EINVAL;
+	}
+
+	/*
+	 * Allocate a new counter and update domains
+	 */
+	counterid = assign_cntrs_alloc();
+	if (counterid < 0) {
+		rdt_last_cmd_puts("Out of ABMC counters\n");
+		return -ENOSPC;
+	}
+
+	rdtgrp->mon.abmc_ctr_id[index] = counterid;
+
+	list_for_each_entry(d, &r->domains, list)
+		rdtgroup_abmc_domain(d, rdtgrp, evtid, index, 1);
+
+	rdtgrp->mon.mon_state |= mon_state;
+
+	return 0;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
-- 
2.34.1
Re: [RFC PATCH v3 12/17] x86/resctrl: Add the functionality to assign the RMID
Posted by Reinette Chatre 1 year, 9 months ago
Hi Babu,

On 3/28/2024 6:06 PM, Babu Moger wrote:
> With the support of ABMC (Assignable Bandwidth Monitoring Counters)
> feature, the user has the option to assign or unassign the RMID to
> hardware counter and monitor the bandwidth for the longer duration.

What is meant with "the longer duration" (this term is used throughout
this series)? Perhaps "for as long as a hardware counter is assigned"?

> 
> Provide the interface to assign the counter to the group.
> 
> The ABMC feature implements a pair of MSRs, L3_QOS_ABMC_CFG (MSR
> C000_03FDh) and L3_QOS_ABMC_DSC (MSR C000_3FEh). Each logical processor
> implements a separate copy of these registers. Attempts to read or write
> these MSRs when ABMC is not enabled will result in a #GP(0) exception.
> 
> Individual assignable bandwidth counters are configured by writing to
> L3_QOS_ABMC_CFG MSR and specifying the Counter ID, Bandwidth Source, and
> Bandwidth Types. Reading L3_QOS_ABMC_DSC returns the configuration of the
> counter specified by L3_QOS_ABMC_CFG [CtrID].

This mentions the AMD architecture parts needing configuration but not what
resctrl parts are used to accomplish this configuration. It is difficult to
understand this work without this connection.

> 
> 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).
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> ---
> 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 |  1 +
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 86 ++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 88453c86474b..9d84c80104f9 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -651,6 +651,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>  void __init resctrl_file_fflags_init(const char *config,
>  				     unsigned long fflags);
>  void arch_domain_mbm_evt_config(struct rdt_hw_domain *hw_dom);
> +ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state);
>  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 7f54788a58de..cfbdaf8b5f83 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -202,6 +202,18 @@ static void assign_cntrs_init(void)
>  	assign_cntrs_free_map_len = r->mbm_assign_cntrs;
>  }
>  
> +static int assign_cntrs_alloc(void)
> +{
> +	u32 counterid = ffs(assign_cntrs_free_map);
> +
> +	if (counterid == 0)
> +		return -ENOSPC;
> +	counterid--;
> +	assign_cntrs_free_map &= ~(1 << counterid);
> +
> +	return counterid;

Use bitmap API ... eg. find_first_bit() (eliminates
need to adjust counterid), __clear_bit()

> +}
> +
>  /**
>   * rdtgroup_mode_by_closid - Return mode of resource group with closid
>   * @closid: closid if the resource group
> @@ -1848,6 +1860,80 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
>  	return ret ?: nbytes;
>  }
>  
> +static void rdtgroup_abmc_msrwrite(void *info)
> +{
> +	u64 *msrval = info;
> +
> +	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *msrval);
> +}
> +
> +static void rdtgroup_abmc_domain(struct rdt_domain *d, struct rdtgroup *rdtgrp,
> +				 u32 evtid, int index, bool assign)
> +{
> +	struct rdt_hw_domain *hw_dom = resctrl_to_arch_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.ctr_en = assign ? 1 : 0;
> +	abmc_cfg.split.ctr_id = rdtgrp->mon.abmc_ctr_id[index];
> +	abmc_cfg.split.bw_src = rdtgrp->mon.rmid;
> +
> +	/*
> +	 * Read the event configuration from the domain and pass it as
> +	 * bw_type.
> +	 */
> +	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[rdtgrp->mon.rmid];
> +	} else {
> +		abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
> +		arch_mbm = &hw_dom->arch_mbm_local[rdtgrp->mon.rmid];
> +	}
> +
> +	smp_call_function_any(&d->cpu_mask, rdtgroup_abmc_msrwrite, &abmc_cfg, 1);
> +
> +	/* Reset the internal counters */
> +	if (arch_mbm)
> +		memset(arch_mbm, 0, sizeof(struct arch_mbm_state));
> +}
> +
> +ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state)
> +{
> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> +	int counterid = 0, index;
> +	struct rdt_domain *d;
> +
> +	if (rdtgrp->mon.mon_state & mon_state) {
> +		rdt_last_cmd_puts("ABMC counter is assigned already\n");
> +		return 0;
> +	}
> +
> +	index = mon_event_config_index_get(evtid);
> +	if (index == INVALID_CONFIG_INDEX) {
> +		pr_warn_once("Invalid event id %d\n", evtid);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Allocate a new counter and update domains
> +	 */
> +	counterid = assign_cntrs_alloc();
> +	if (counterid < 0) {
> +		rdt_last_cmd_puts("Out of ABMC counters\n");
> +		return -ENOSPC;
> +	}
> +
> +	rdtgrp->mon.abmc_ctr_id[index] = counterid;
> +
> +	list_for_each_entry(d, &r->domains, list)
> +		rdtgroup_abmc_domain(d, rdtgrp, evtid, index, 1);
> +
> +	rdtgrp->mon.mon_state |= mon_state;
> +
> +	return 0;
> +}
> +
>  /* rdtgroup information files for one cache resource. */
>  static struct rftype res_common_files[] = {
>  	{

It is not clear to me where the filesystem and architecture boundaries
are, but I understand that you and Peter already discussed this and I look
forward to next version that will make this easier to understand.

Reinette
Re: [RFC PATCH v3 12/17] x86/resctrl: Add the functionality to assign the RMID
Posted by Moger, Babu 1 year, 9 months ago
Hi Reinette,

On 5/3/24 18:33, Reinette Chatre wrote:
> Hi Babu,
> 
> On 3/28/2024 6:06 PM, Babu Moger wrote:
>> With the support of ABMC (Assignable Bandwidth Monitoring Counters)
>> feature, the user has the option to assign or unassign the RMID to
>> hardware counter and monitor the bandwidth for the longer duration.
> 
> What is meant with "the longer duration" (this term is used throughout
> this series)? Perhaps "for as long as a hardware counter is assigned"?

Yes. Sure.

> 
>>
>> Provide the interface to assign the counter to the group.
>>
>> The ABMC feature implements a pair of MSRs, L3_QOS_ABMC_CFG (MSR
>> C000_03FDh) and L3_QOS_ABMC_DSC (MSR C000_3FEh). Each logical processor
>> implements a separate copy of these registers. Attempts to read or write
>> these MSRs when ABMC is not enabled will result in a #GP(0) exception.
>>
>> Individual assignable bandwidth counters are configured by writing to
>> L3_QOS_ABMC_CFG MSR and specifying the Counter ID, Bandwidth Source, and
>> Bandwidth Types. Reading L3_QOS_ABMC_DSC returns the configuration of the
>> counter specified by L3_QOS_ABMC_CFG [CtrID].
> 
> This mentions the AMD architecture parts needing configuration but not what
> resctrl parts are used to accomplish this configuration. It is difficult to
> understand this work without this connection.

Let me reword this in the next revison.

> 
>>
>> 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).
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> ---
>> 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 |  1 +
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 86 ++++++++++++++++++++++++++
>>  2 files changed, 87 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 88453c86474b..9d84c80104f9 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -651,6 +651,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>>  void __init resctrl_file_fflags_init(const char *config,
>>  				     unsigned long fflags);
>>  void arch_domain_mbm_evt_config(struct rdt_hw_domain *hw_dom);
>> +ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state);
>>  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 7f54788a58de..cfbdaf8b5f83 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -202,6 +202,18 @@ static void assign_cntrs_init(void)
>>  	assign_cntrs_free_map_len = r->mbm_assign_cntrs;
>>  }
>>  
>> +static int assign_cntrs_alloc(void)
>> +{
>> +	u32 counterid = ffs(assign_cntrs_free_map);
>> +
>> +	if (counterid == 0)
>> +		return -ENOSPC;
>> +	counterid--;
>> +	assign_cntrs_free_map &= ~(1 << counterid);
>> +
>> +	return counterid;
> 
> Use bitmap API ... eg. find_first_bit() (eliminates
> need to adjust counterid), __clear_bit()

Ok Sure.

> 
>> +}
>> +
>>  /**
>>   * rdtgroup_mode_by_closid - Return mode of resource group with closid
>>   * @closid: closid if the resource group
>> @@ -1848,6 +1860,80 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
>>  	return ret ?: nbytes;
>>  }
>>  
>> +static void rdtgroup_abmc_msrwrite(void *info)
>> +{
>> +	u64 *msrval = info;
>> +
>> +	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *msrval);
>> +}
>> +
>> +static void rdtgroup_abmc_domain(struct rdt_domain *d, struct rdtgroup *rdtgrp,
>> +				 u32 evtid, int index, bool assign)
>> +{
>> +	struct rdt_hw_domain *hw_dom = resctrl_to_arch_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.ctr_en = assign ? 1 : 0;
>> +	abmc_cfg.split.ctr_id = rdtgrp->mon.abmc_ctr_id[index];
>> +	abmc_cfg.split.bw_src = rdtgrp->mon.rmid;
>> +
>> +	/*
>> +	 * Read the event configuration from the domain and pass it as
>> +	 * bw_type.
>> +	 */
>> +	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[rdtgrp->mon.rmid];
>> +	} else {
>> +		abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
>> +		arch_mbm = &hw_dom->arch_mbm_local[rdtgrp->mon.rmid];
>> +	}
>> +
>> +	smp_call_function_any(&d->cpu_mask, rdtgroup_abmc_msrwrite, &abmc_cfg, 1);
>> +
>> +	/* Reset the internal counters */
>> +	if (arch_mbm)
>> +		memset(arch_mbm, 0, sizeof(struct arch_mbm_state));
>> +}
>> +
>> +ssize_t rdtgroup_assign_abmc(struct rdtgroup *rdtgrp, u32 evtid, int mon_state)
>> +{
>> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>> +	int counterid = 0, index;
>> +	struct rdt_domain *d;
>> +
>> +	if (rdtgrp->mon.mon_state & mon_state) {
>> +		rdt_last_cmd_puts("ABMC counter is assigned already\n");
>> +		return 0;
>> +	}
>> +
>> +	index = mon_event_config_index_get(evtid);
>> +	if (index == INVALID_CONFIG_INDEX) {
>> +		pr_warn_once("Invalid event id %d\n", evtid);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/*
>> +	 * Allocate a new counter and update domains
>> +	 */
>> +	counterid = assign_cntrs_alloc();
>> +	if (counterid < 0) {
>> +		rdt_last_cmd_puts("Out of ABMC counters\n");
>> +		return -ENOSPC;
>> +	}
>> +
>> +	rdtgrp->mon.abmc_ctr_id[index] = counterid;
>> +
>> +	list_for_each_entry(d, &r->domains, list)
>> +		rdtgroup_abmc_domain(d, rdtgrp, evtid, index, 1);
>> +
>> +	rdtgrp->mon.mon_state |= mon_state;
>> +
>> +	return 0;
>> +}
>> +
>>  /* rdtgroup information files for one cache resource. */
>>  static struct rftype res_common_files[] = {
>>  	{
> 
> It is not clear to me where the filesystem and architecture boundaries
> are, but I understand that you and Peter already discussed this and I look
> forward to next version that will make this easier to understand.

Same here. I am not very clear as well. We havent discussed anything out
of the mailing list. I see that fftype flags available for both fs and
arch layer looking at the latest patches from Dave. Will change it as
things change moving along.
-- 
Thanks
Babu Moger