linux-kernel - Re: [PATCH v1 7/9] x86/resctrl: Assign HW RMIDs to CPUs for soft RMID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3816ccf6-4f74-6406-5ca0-580743efa2a1@intel.com>
Date:   Thu, 11 May 2023 14:39:56 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Peter Newman <peternewman@...gle.com>,
        Fenghua Yu <fenghua.yu@...el.com>
CC:     Babu Moger <babu.moger@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Stephane Eranian <eranian@...gle.com>,
        James Morse <james.morse@....com>,
        <linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v1 7/9] x86/resctrl: Assign HW RMIDs to CPUs for soft RMID

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@...gle.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