Hi Reinette,
On 12/19/2024 4:12 PM, Reinette Chatre wrote:
Hi Babu,
On 12/12/24 12:15 PM, Babu Moger wrote:
@@ -1604,33 +1645,11 @@ unsigned int mon_event_config_index_get(u32 evtid)
}
}
-static void mon_event_config_read(void *info)
-{
- struct mon_config_info *mon_info = info;
- unsigned int index;
- u64 msrval;
-
- index = mon_event_config_index_get(mon_info->evtid);
- if (index == INVALID_CONFIG_INDEX) {
- pr_warn_once("Invalid event id %d\n", mon_info->evtid);
- return;
- }
- rdmsrl(MSR_IA32_EVT_CFG_BASE + index, msrval);
-
- /* Report only the valid event configuration bits */
- mon_info->mon_config = msrval & MAX_EVT_CONFIG_BITS;
-}
-
-static void mondata_config_read(struct rdt_mon_domain *d, struct mon_config_info *mon_info)
-{
- smp_call_function_any(&d->hdr.cpu_mask, mon_event_config_read, mon_info, 1);
-}
-
static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid)
{
- struct mon_config_info mon_info;
struct rdt_mon_domain *dom;
bool sep = false;
+ u32 val;
Could this variable name be more descriptive? For example, mon_config, or config_val as
used in mbm_config_write_domain()?
Will change it to config_val.
...
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index f11d6fdfd977..c8ab3d7a0dab 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -118,6 +118,18 @@ struct rdt_mon_domain {
int cqm_work_cpu;
};
+/**
+ * struct mon_config_info - Monitoring event configuratiin details
configuratiin -> configuration
Sure.
... but actually, the motivation for moving this struct here was
to make it available for an arch to interpret the data passed
via resctrl_arch_mon_event_config_set(). This patch passes data
in this struct but a later patch modifies
resctrl_arch_mon_event_config_set() to not use struct anymore ...
and then leaves struct mon_config_info here.
Even so, considering Boris's preference this is no longer needed.
ok. I will move the "struct mon_config_info" definition where it is
used(rdtgroup.c).
+ * @d: Domain for the event
+ * @evtid: Event type
+ * @mon_config: Event configuration value
+ */
+struct mon_config_info {
+ struct rdt_mon_domain *d;
+ enum resctrl_event_id evtid;
+ u32 mon_config;
+};
+
/**
* struct resctrl_cache - Cache allocation related data
* @cbm_len: Length of the cache bit mask
@@ -352,6 +364,10 @@ void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_mon_domain *d,
*/
void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *d);
+void resctrl_arch_mon_event_config_set(void *info);
+u32 resctrl_arch_mon_event_config_get(struct rdt_mon_domain *d,
+ enum resctrl_event_id eventid);
+
Please move to internal header file instead and consider this for
all changes to include/linux/resctrl.h
Sure. Thanks
Babu