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@xxxxxxxxxx>
> ---
> 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
