linux-kernel - Re: [PATCH v1 10/40] x86/cpu: Remove leaf 0x2 parsing loop and add helpers

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: <Z8bHK391zKE4gUEW@gmail.com>
Date: Tue, 4 Mar 2025 10:26:03 +0100
From: Ingo Molnar <mingo@...nel.org>
To: "Ahmed S. Darwish" <darwi@...utronix.de>
Cc: Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	John Ogness <john.ogness@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Cooper <andrew.cooper3@...rix.com>, x86@...nel.org,
	x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 10/40] x86/cpu: Remove leaf 0x2 parsing loop and add
 helpers


* Ahmed S. Darwish <darwi@...utronix.de> wrote:

> Leaf 0x2 output includes a "query count" byte where it was supposed to
> specify the number of repeated cpuid leaf 0x2 subleaf 0 queries needed
> to extract all of the hardware's cache and TLB descriptors.

s/cpuid
 /CPUID

Please do this in the rest of the series too. (I did it for the first 9 
patches.)

> +++ b/arch/x86/include/asm/cpuid/types.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_CPUID_TYPES_H
> +#define _ASM_X86_CPUID_TYPES_H
> +
> +#include <linux/types.h>
> +
> +#include <asm/cpuid.h>

So that header organization is a bit messy: if <asm/cpuid.h> is 
supposed to be the main header, why is there a <asm/cpuid/types.h>?

I'd suggest we follow the FPU header structure:

  starship:~/tip/arch/x86/include/asm/fpu> ls -l
  total 48
  -rw-rw-r-- 1 mingo mingo  5732 Feb 27 19:24 api.h
  -rw-rw-r-- 1 mingo mingo   671 Feb 26 16:13 regset.h
  -rw-rw-r-- 1 mingo mingo  2203 Feb 27 13:52 sched.h
  -rw-rw-r-- 1 mingo mingo  1110 Feb 27 19:24 signal.h
  -rw-rw-r-- 1 mingo mingo 14741 Feb 27 19:24 types.h
  -rw-rw-r-- 1 mingo mingo   811 Feb 26 16:13 xcr.h
  -rw-rw-r-- 1 mingo mingo  4401 Feb 27 23:01 xstate.h

With <asm/cpuid/api.h> being the main header - established via a 
separate preparatory patch.

This followup patch can then add <asm/cpuid/types.h> which will also be 
included in <asm/cpuid/api.h>.


> +/*
> + * CPUID(0x2) parsing helpers
> + * Check for_each_leaf_0x2_desc() documentation.
> + */
> +
> +struct leaf_0x2_reg {
> +		u32		: 31,
> +			invalid	: 1;
> +};
> +
> +union leaf_0x2_regs {
> +	struct leaf_0x2_reg	reg[4];
> +	u32			regv[4];
> +	u8			desc[16];
> +};
> +
> +/**
> + * get_leaf_0x2_regs() - Return sanitized leaf 0x2 register output
> + * @regs:	Output parameter
> + *
> + * Get leaf 0x2 register output and store it in @regs.  Invalid byte
> + * descriptors returned by the hardware will be force set to zero (the
> + * NULL cache/TLB descriptor) before returning them to the caller.
> + */
> +static inline void get_leaf_0x2_regs(union leaf_0x2_regs *regs)


Please prefix all new cpuid API functions and types with cpuid_.

> +#define for_each_leaf_0x2_desc(regs, desc)				\
> +	/* Skip the first byte as it is not a descriptor */		\
> +	for (desc = &(regs).desc[1]; desc < &(regs).desc[16]; desc++)

The comment line can come before the macro.

> +	get_leaf_0x2_regs(&regs);
> +	for_each_leaf_0x2_desc(regs, desc)
> +		intel_tlb_lookup(*desc);

Nice interface otherwise.

Thanks,

	Ingo