linux-kernel - Re: [PATCH v4 02/10] x86/cpu/topology: Add CPU type to struct cpuinfo

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: <20241018203053.2x6oyws3dkxfw6rm@desk>
Date: Fri, 18 Oct 2024 13:30:53 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: Borislav Petkov <bp@...en8.de>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	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>,
	Mario Limonciello <mario.limonciello@....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 Fri, Oct 18, 2024 at 06:19:56PM +0200, Borislav Petkov wrote:
> On Mon, Sep 30, 2024 at 07:47:24AM -0700, Pawan Gupta wrote:
> > Subject: Re: [PATCH v4 02/10] x86/cpu/topology: Add CPU type to struct...
> 
> x86/cpu: ...
> 
> is enough.

Ok.

> > 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] */
> 
> Might as well stick to only // comments as we do those in headers now.

Will do.

> > +			u32	intel_core_native_model_id:24;
> 
> wow, that needs a whole breath to speak: "intel_core_native_model_id".

Yes, it needs to be shortened.

> "core" and "native" look like they wanna go. What is that field supposed to
> mean even?

In combination with core_type, this field can be used to uniquely identify
the microarchitecture.

I will drop "core", but can we keep "native"? "native" is used in SDM to
define this field. Also model_id could be confused with model number.

  From Intel SDM Vol. 2A:

  Bits 23-00: Native model ID of the core. The core-type and native model
  ID can be used to uniquely identify the microarchitecture of the core.
  This native model ID is not unique across core types, and not related to
  the model ID reported in CPUID leaf 01H, and does not identify the SOC.


> > +			/* CPUID.1A.EAX[31-24] */
> > +			u32	intel_core_type:8;
> > +		};
> > +	};
> >  };
> 
> ...
> 
> > +enum x86_topology_hw_cpu_type topology_hw_cpu_type(struct cpuinfo_x86 *c)
> > +{
> > +	if (c->x86_vendor == X86_VENDOR_INTEL)
> > +		return c->topo.intel_core_type;
> > +
> > +	return c->topo.hw_cpu_type;
> 
> Huh, the other vendors are not enabled. This should return
> TOPO_HW_CPU_TYPE_UNKNOWN then.
> 
> I know, it does but make explicit pls.

Yes, topo.hw_cpu_type is initialized to TOPO_HW_CPU_TYPE_UNKNOWN. We should
not ideally need the vendor check at all. As long as topo.hw_cpu_type has
the core type, returning it should be enough here. For Intel hw_cpu_type
also has the native_model_id, that is why we need the vendor check.

If AMD or other vendors have similar use case, it makes sense to add the
explicit vendor check. Please let me know if thats the likely case.