linux-kernel - Re: [PATCH v2 3/7] x86/resctrl: Detect Smart Data Cache Injection Allocation Enforcement

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: <8f7f2839-4c96-46f5-8d39-5d2a974f6fbd@intel.com>
Date: Mon, 23 Dec 2024 13:13:51 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <fenghua.yu@...el.com>, <x86@...nel.org>, <hpa@...or.com>,
	<akpm@...ux-foundation.org>, <paulmck@...nel.org>, <thuth@...hat.com>,
	<rostedt@...dmis.org>, <xiongwei.song@...driver.com>,
	<pawan.kumar.gupta@...ux.intel.com>, <jpoimboe@...nel.org>,
	<daniel.sneddon@...ux.intel.com>, <thomas.lendacky@....com>,
	<perry.yuan@....com>, <sandipan.das@....com>, <kai.huang@...el.com>,
	<seanjc@...gle.com>, <xin3.li@...el.com>, <ebiggers@...gle.com>,
	<andrew.cooper3@...rix.com>, <mario.limonciello@....com>,
	<tan.shaopeng@...itsu.com>, <james.morse@....com>, <tony.luck@...el.com>,
	<peternewman@...gle.com>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <eranian@...gle.com>, <corbet@....net>
Subject: Re: [PATCH v2 3/7] x86/resctrl: Detect Smart Data Cache Injection
 Allocation Enforcement

Hi Babu,

On 12/18/24 1:37 PM, Babu Moger wrote:
> Introduce io_alloc_capable in struct resctrl_cache to detect SDCIAE
> (L3 Smart Data Cache Injection Allocation Enforcement) feature.

Please distinguish clearly between the resctrl feature ("io_alloc_capable")
and the architecture specific feature (SDCIAE) that backs it. This is similar
to what you have done for ABMC and makes the work much easier to understand.
When the resctrl and arch feature is used interchangeably it becomes confusing.

> 
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---
> v2: Changed sdciae_capable to io_alloc_capable to make it generic feature.
>     Also moved the io_alloc_capable in struct resctrl_cache.
> ---
>  arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
>  include/linux/resctrl.h            | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index c2450cd52511..39e110033d96 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -306,6 +306,11 @@ static void rdt_get_cdp_config(int level)
>  	rdt_resources_all[level].r_resctrl.cdp_capable = true;
>  }
>  
> +static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
> +{
> +	r->cache.io_alloc_capable = true;
> +}

rdt_get_sdciae_alloc_cfg() looks like one that should have "set" in its name, not "get".
This also does not seem architecture specific so "sdciae" should not be in the name.
rdt_set_io_alloc_capable()?

> +
>  static void rdt_get_cdp_l3_config(void)
>  {
>  	rdt_get_cdp_config(RDT_RESOURCE_L3);
> @@ -931,6 +936,8 @@ static __init bool get_rdt_alloc_resources(void)
>  		rdt_get_cache_alloc_cfg(1, r);
>  		if (rdt_cpu_has(X86_FEATURE_CDP_L3))
>  			rdt_get_cdp_l3_config();
> +		if (rdt_cpu_has(X86_FEATURE_SDCIAE))
> +			rdt_get_sdciae_alloc_cfg(r);
>  		ret = true;
>  	}
>  	if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index d94abba1c716..5837acff7442 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -129,6 +129,8 @@ struct rdt_mon_domain {
>   * @arch_has_sparse_bitmasks:	True if a bitmask like f00f is valid.
>   * @arch_has_per_cpu_cfg:	True if QOS_CFG register for this cache
>   *				level has CPU scope.
> + * @io_alloc_capable:	Smart Data Cache Injection Allocation Enforcement
> + *			capable (SDCIAE).

Please remove arch specific text here. For example,
"True if portion of the L3 cache can be allocated for I/O traffic."

>   */
>  struct resctrl_cache {
>  	unsigned int	cbm_len;
> @@ -136,6 +138,7 @@ struct resctrl_cache {
>  	unsigned int	shareable_bits;
>  	bool		arch_has_sparse_bitmasks;
>  	bool		arch_has_per_cpu_cfg;
> +	bool		io_alloc_capable;
>  };
>  
>  /**
> @@ -202,6 +205,7 @@ enum resctrl_scope {
>   * @evt_list:		List of monitoring events
>   * @fflags:		flags to choose base and info files
>   * @cdp_capable:	Is the CDP feature available on this resource
> + * @sdciae_capable:	Is SDCIAE feature available on this resource
>   */
>  struct rdt_resource {
>  	int			rid;

Leftover from previous version?

Reinette