Hi Peter,
On 4/21/2023 7:17 AM, Peter Newman wrote:
To implement soft RMIDs, each CPU needs a HW RMID that is unique within its L3 cache domain. This is the minimum number of RMIDs needed to monitor all CPUs.
This is accomplished by determining the rank of each CPU's mask bit within its L3 shared_cpu_mask in resctrl_online_cpu().
Signed-off-by: Peter Newman peternewman@google.com
arch/x86/kernel/cpu/resctrl/core.c | 39 +++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 47b1c37a81f8..b0d873231b1e 100644 --- a/arch/x86/kernel/cpu/resctrl/core.c +++ b/arch/x86/kernel/cpu/resctrl/core.c @@ -596,6 +596,38 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r) } } +/* Assign each CPU an RMID that is unique within its cache domain. */ +static u32 determine_hw_rmid_for_cpu(int cpu)
This code tends to use the verb "get", something like "get_hw_rmid()" could work.
+{
- struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
- struct cacheinfo *l3ci = NULL;
- u32 rmid;
- int i;
- /* Locate the cacheinfo for this CPU's L3 cache. */
- for (i = 0; i < ci->num_leaves; i++) {
if (ci->info_list[i].level == 3 &&
(ci->info_list[i].attributes & CACHE_ID)) {
l3ci = &ci->info_list[i];
break;
}
- }
- WARN_ON(!l3ci);
- if (!l3ci)
return 0;
You can use "if (WARN_ON(..))"
- /* Use the position of cpu in its shared_cpu_mask as its RMID. */
(please use "CPU" instead of "cpu" in comments and changelogs)
- rmid = 0;
- for_each_cpu(i, &l3ci->shared_cpu_map) {
if (i == cpu)
break;
rmid++;
- }
- return rmid;
+}
I do not see any impact to the (soft) RMIDs that can be assigned to monitor groups, yet from what I understand a generic "RMID" is used as index to MBM state. Is this correct? A hardware RMID and software RMID would thus share the same MBM state. If this is correct I think we need to work on making the boundaries between hard and soft RMID more clear.
static void clear_closid_rmid(int cpu) { struct resctrl_pqr_state *state = this_cpu_ptr(&pqr_state); @@ -604,7 +636,12 @@ static void clear_closid_rmid(int cpu) state->default_rmid = 0; state->cur_closid = 0; state->cur_rmid = 0;
- wrmsr(MSR_IA32_PQR_ASSOC, 0, 0);
- state->hw_rmid = 0;
- if (static_branch_likely(&rdt_soft_rmid_enable_key))
state->hw_rmid = determine_hw_rmid_for_cpu(cpu);
- wrmsr(MSR_IA32_PQR_ASSOC, state->hw_rmid, 0);
} static int resctrl_online_cpu(unsigned int cpu)
Reinette