linux-kernel - RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3

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]

Date: Thu, 23 May 2024 19:04:53 +0000
From: "Luck, Tony" <tony.luck@...el.com>
To: "Chatre, Reinette" <reinette.chatre@...el.com>, "Yu, Fenghua"
	<fenghua.yu@...el.com>, "Wieczor-Retman, Maciej"
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3_cache

> >     return (is_mbm_local_enabled() &&
> > -           r->alloc_capable && is_mba_linear());
> > +           r->alloc_capable && is_mba_linear() &&
> > +           snc_nodes_per_l3_cache == 1);
> >  }
> >
> >  /*
>
> Since the software controller is a filesystem feature the above
> now requires that snc_nodes_per_l3_cache becomes part of the resctrl
> filesystem code and every architecture will need to set snc_nodes_per_l3_cache.
> Every architecture will thus need to interpret what "SNC" means for them
> using the term introduced here. That may be ok ... but the term "SNC"
> will then surely not identify an Intel feature and Intel needs to be ok
> that any architecture calls their "similar to SNC but not quite identical"
> "SNC".
>
> I assume now that as part of the fs/arch split there needs to be
> a new helper that allows different architectures to set this
> filesystem variable?

I can change this check to better reflect the underlying reason to
disable the software controller. Which is that the MBM monitor scope
does not match the MBA control scope. This seems like an architecture
neutral expression.

So code would look like this:

	struct rdt_resource *rmbm = &rdt_resources_all[RDT_RESOURCE_L3].r_rescrl;
	struct rdt_resource *rmba = &rdt_resources_all[RDT_RESOURCE_MBA].r_rescrl;

	...

	 return (is_mbm_local_enabled() &&
                r->alloc_capable && is_mba_linear() &&
                rmbm->mon_scope == rmba->ctrl_scope);

I'm also contemplating dropping snc_nodes_per_l3_cache from being a
global variable and making it a field in "struct rdt_resource" (only needed
for the RDT_RESOURCE_L3 resource). N.B. Babu had suggested it
shouldn't be global many patch versions ago.

Perhaps name it .domains_per_l3_cache or .subdomains_per_l3_cache?

Bad idea? Good idea (but you have a better name for the field)?

-Tony