| 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
| ||
|
Message-ID: <20241023051210.fjpuyxhzzima4iu7@desk>
Date: Tue, 22 Oct 2024 22:12:10 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: Mario Limonciello <mario.limonciello@....com>
Cc: 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,
daniel.sneddon@...ux.intel.com, tony.luck@...el.com,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
linux-perf-users@...r.kernel.org,
Josh Poimboeuf <jpoimboe@...nel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
Andrew Cooper <andrew.cooper3@...rix.com>,
Brice Goglin <brice.goglin@...il.com>,
Perry Yuan <Perry.Yuan@....com>,
Dapeng Mi <dapeng1.mi@...ux.intel.com>
Subject: Re: [PATCH v4 02/10] x86/cpu/topology: Add CPU type to struct
cpuinfo_topology
On Tue, Oct 22, 2024 at 09:42:37AM -0500, Mario Limonciello wrote:
> On 10/18/2024 16:11, Pawan Gupta wrote:
> > On Fri, Oct 18, 2024 at 11:28:31AM -0500, Mario Limonciello wrote:
> > > On 9/30/2024 09:47, Pawan Gupta wrote:
> > > > Sometimes it is required to take actions based on if a CPU is a performance
> > > > or efficiency core. As an example, intel_pstate driver uses the Intel
> > > > core-type to determine CPU scaling. Also, some CPU vulnerabilities only
> > > > affect a specific CPU type, like RFDS only affects Intel Atom. Hybrid
> > > > systems that have variants P+E, P-only(Core) and E-only(Atom), it is not
> > > > straightforward to identify which variant is affected by a type specific
> > > > vulnerability.
> > > >
> > > > Such processors do have CPUID field that can uniquely identify them. Like,
> > > > P+E, P-only and E-only enumerates CPUID.1A.CORE_TYPE identification, while
> > > > P+E additionally enumerates CPUID.7.HYBRID. Based on this information, it
> > > > is possible for boot CPU to identify if a system has mixed CPU types.
> > > >
> > > > Add a new field hw_cpu_type to struct cpuinfo_topology that stores the
> > > > hardware specific CPU type. This saves the overhead of IPIs to get the CPU
> > > > type of a different CPU. CPU type is populated early in the boot process,
> > > > before vulnerabilities are enumerated.
> > > >
> > > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> > > > ---
> > > > arch/x86/include/asm/cpu.h | 6 ++++++
> > > > arch/x86/include/asm/processor.h | 11 +++++++++++
> > > > arch/x86/include/asm/topology.h | 8 ++++++++
> > > > arch/x86/kernel/cpu/debugfs.c | 1 +
> > > > arch/x86/kernel/cpu/intel.c | 5 +++++
> > > > arch/x86/kernel/cpu/topology_common.c | 11 +++++++++++
> > > > 6 files changed, 42 insertions(+)
> > > >
> > > > diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> > > > index aa30fd8cad7f..2244dd86066a 100644
> > > > --- a/arch/x86/include/asm/cpu.h
> > > > +++ b/arch/x86/include/asm/cpu.h
> > > > @@ -32,6 +32,7 @@ extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
> > > > extern bool handle_guest_split_lock(unsigned long ip);
> > > > extern void handle_bus_lock(struct pt_regs *regs);
> > > > u8 get_this_hybrid_cpu_type(void);
> > > > +u32 intel_native_model_id(struct cpuinfo_x86 *c);
> > > > #else
> > > > static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
> > > > static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> > > > @@ -50,6 +51,11 @@ static inline u8 get_this_hybrid_cpu_type(void)
> > > > {
> > > > return 0;
> > > > }
> > > > +
> > > > +static u32 intel_native_model_id(struct cpuinfo_x86 *c)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > #endif
> > > > #ifdef CONFIG_IA32_FEAT_CTL
> > > > void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
> > > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> > > > index 4a686f0e5dbf..61c8336bc99b 100644
> > > > --- a/arch/x86/include/asm/processor.h
> > > > +++ b/arch/x86/include/asm/processor.h
> > > > @@ -105,6 +105,17 @@ struct cpuinfo_topology {
> > > > // Cache level topology IDs
> > > > u32 llc_id;
> > > > u32 l2c_id;
> > > > +
> > > > + // Hardware defined CPU-type
> > > > + union {
> > > > + u32 hw_cpu_type;
> > > > + struct {
> > > > + /* CPUID.1A.EAX[23-0] */
> > > > + u32 intel_core_native_model_id:24;
> > > > + /* CPUID.1A.EAX[31-24] */
> > > > + u32 intel_core_type:8;
> > > > + };
> > > > + };
> > > > };
> > > > struct cpuinfo_x86 {
> > > > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > > > index aef70336d624..faf7cb7f7d7e 100644
> > > > --- a/arch/x86/include/asm/topology.h
> > > > +++ b/arch/x86/include/asm/topology.h
> > > > @@ -114,6 +114,12 @@ enum x86_topology_domains {
> > > > TOPO_MAX_DOMAIN,
> > > > };
> > > > +enum x86_topology_hw_cpu_type {
> > > > + TOPO_HW_CPU_TYPE_UNKNOWN = 0,
> > > > + TOPO_HW_CPU_TYPE_INTEL_ATOM = 0x20,
> > > > + TOPO_HW_CPU_TYPE_INTEL_CORE = 0x40,
> > > > +};
> > >
> > > This isn't exactly generic. Unless you have a strong need to know "Atom"
> >
> > The goal was not to have generic cpu_type here, but the actual CPU type
> > that hardware enumerates. I was asked to prepend "hw_" to cpu_type to make
> > is clear that this is hardware defined, and to leave scope for generic
> > cpu_type, if we add those in future.
> >
> > > instead of "Efficient" or "Core" instead of "Performance" I think it would
> > > be better to do this as:
> > >
> > > enum x86_topology_hw_core_type {
> > > TOPO_HW_CORE_TYPE_UNKNOWN = 0,
> > > TOPO_HW_CORE_TYPE_PERFORMANT,
> > > TOPO_HW_CORE_TYPE_EFFICIENT,
> > > };
> > >
> > > Then you can do the mapping of 0x20 = Efficient and 0x40 = performant in the
> > > Intel topology lookup function.
> >
> > I can add a lookup function, but I wanted to understand the use case of
> > generic cpu_type. If we always have to lookup and map the cpu_type, then
> > why not have the actual cpu_type in the first place?
> >
> > One case where generic cpu_type can be useful is when we expose them to
> > userspace, which I think is inevitable. Overall I am fine with adding generic
> > cpu type. It may also make sense to have separate accessors for generic and
> > and hardware defined cpu_type, and the generic ones when we actually have a
> > use case. Thoughts?
> >
> > > After you land the series we can do something similar to move AMD code
> > > around and map it out to the right generic mapping.
>
> I took your patch and made the modifications that I thought made sense for a
> generic type while adding the matching AMD code and sent it out (you're on
> CC). Can you take a look and see what you think? Boris already provided
> some feedback that I'm going to spin it again.
> I think if we can align on that one we can land that patch and you can
> rebase the rest of the series on it.
I left some feedback on that series. Overall it looks good. For Intel's use
case it needs to add accessor for hardware defined cpu_type. This is needed
for cpu-type matching in affected processor list.
Powered by blists - more mailing lists