[RFC PATCH v3 09/17] x86/resctrl: Introduce assign state for the mon group

Babu Moger posted 17 patches 1 year, 10 months ago
There is a newer version of this series
[RFC PATCH v3 09/17] x86/resctrl: Introduce assign state for the mon group
Posted by Babu Moger 1 year, 10 months ago
The ABMC feature provides an option to the user to assign an RMID to
the hardware counter and monitor the bandwidth for the longer duration.
The assigned RMID will be active until user unassigns the RMID.

Add a new field assign_state in mongroup data structure to represent the
assignment state of the group. This will be when ABMC feature is enabled.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v3: Changed the field name to mon_state. Also thie state is not visible to
    users directly as part of out global assign approach.

v2: Added check to display "Unsupported" when user tries to access
    monitor state when ABMC is not enabled.
---
 arch/x86/kernel/cpu/resctrl/internal.h | 9 +++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 8238ee437369..b559b3a4555e 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -99,6 +99,13 @@ cpumask_any_housekeeping(const struct cpumask *mask, int exclude_cpu)
 /* ABMC ENABLE */
 #define ABMC_ENABLE			BIT(0)
 
+/*
+ * monitor group's state when ABMC is supported
+ */
+#define ASSIGN_NONE			0
+#define ASSIGN_TOTAL			BIT(0)
+#define ASSIGN_LOCAL			BIT(1)
+
 struct rdt_fs_context {
 	struct kernfs_fs_context	kfc;
 	bool				enable_cdpl2;
@@ -202,12 +209,14 @@ enum rdtgrp_mode {
  * @parent:			parent rdtgrp
  * @crdtgrp_list:		child rdtgroup node list
  * @rmid:			rmid for this rdtgroup
+ * @mon_state:			Assignment state of the group
  */
 struct mongroup {
 	struct kernfs_node	*mon_data_kn;
 	struct rdtgroup		*parent;
 	struct list_head	crdtgrp_list;
 	u32			rmid;
+	u32			mon_state;
 };
 
 /**
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2c7583e7b541..54ae2e6bf612 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2473,6 +2473,7 @@ static void resctrl_abmc_msrwrite(void *arg)
 static int resctrl_abmc_setup(enum resctrl_res_level l, bool enable)
 {
 	struct rdt_resource *r = &rdt_resources_all[l].r_resctrl;
+	struct rdtgroup *prgrp, *crgrp;
 	struct rdt_domain *d;
 
 	/* Reset the counters bitmap */
@@ -2484,6 +2485,13 @@ static int resctrl_abmc_setup(enum resctrl_res_level l, bool enable)
 		resctrl_arch_reset_rmid_all(r, d);
 	}
 
+	/* Reset assign state for all the monitor groups */
+	list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
+		prgrp->mon.mon_state = ASSIGN_NONE;
+		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list)
+			crgrp->mon.mon_state = ASSIGN_NONE;
+	}
+
 	return 0;
 }
 
-- 
2.34.1
Re: [RFC PATCH v3 09/17] x86/resctrl: Introduce assign state for the mon group
Posted by Peter Newman 1 year, 10 months ago
Hi Babu,

On Thu, Mar 28, 2024 at 6:08 PM Babu Moger <babu.moger@amd.com> wrote:
>
> +/*
> + * monitor group's state when ABMC is supported
> + */
> +#define ASSIGN_NONE                    0
> +#define ASSIGN_TOTAL                   BIT(0)
> +#define ASSIGN_LOCAL                   BIT(1)

We already have an enumeration for the monitoring events (i.e.,
QOS_L3_MBM_TOTAL_EVENT_ID), which should already be suitable for
maintaining a bitmap of which events have assigned monitors.

-Peter
Re: [RFC PATCH v3 09/17] x86/resctrl: Introduce assign state for the mon group
Posted by Moger, Babu 1 year, 10 months ago
Hi Peter,

On 4/16/24 13:52, Peter Newman wrote:
> Hi Babu,
> 
> On Thu, Mar 28, 2024 at 6:08 PM Babu Moger <babu.moger@amd.com> wrote:
>>
>> +/*
>> + * monitor group's state when ABMC is supported
>> + */
>> +#define ASSIGN_NONE                    0
>> +#define ASSIGN_TOTAL                   BIT(0)
>> +#define ASSIGN_LOCAL                   BIT(1)
> 
> We already have an enumeration for the monitoring events (i.e.,
> QOS_L3_MBM_TOTAL_EVENT_ID), which should already be suitable for
> maintaining a bitmap of which events have assigned monitors.
> 

/*
 * Event IDs, the values match those used to program IA32_QM_EVTSEL before
 * reading IA32_QM_CTR on RDT systems.
 */
enum resctrl_event_id {
        QOS_L3_OCCUP_EVENT_ID           = 0x01,
        QOS_L3_MBM_TOTAL_EVENT_ID       = 0x02,
        QOS_L3_MBM_LOCAL_EVENT_ID       = 0x03,
};

I think you are referring to this definition. We need just one bit for
each event. The QOS_L3_MBM_LOCAL_EVENT_ID definition(both bit 0 and bit 1
set here) does not work for us here.

I can change the definition to something like this.

+#define ASSIGN_NONE                    0
+#define ASSIGN_TOTAL                   BIT(QOS_L3_MBM_TOTAL_EVENT_ID)
+#define ASSIGN_LOCAL                   BIT(QOS_L3_MBM_LOCAL_EVENT_ID)

Is that what you meant?

-- 
Thanks
Babu Moger