linux-kernel - Re: [PATCH v8 05/21] x86/cea: Export API for per-CPU exception stacks for KVM

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] [day] [month] [year] [list]

Message-ID: <ba8434fb-8bbd-4d4a-bd01-1bdb2219979b@intel.com>
Date: Thu, 23 Oct 2025 08:03:44 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: "Xin Li (Intel)" <xin@...or.com>, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org, linux-doc@...r.kernel.org
Cc: pbonzini@...hat.com, seanjc@...gle.com, corbet@....net,
 tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, luto@...nel.org,
 peterz@...radead.org, andrew.cooper3@...rix.com, chao.gao@...el.com,
 hch@...radead.org
Subject: Re: [PATCH v8 05/21] x86/cea: Export API for per-CPU exception stacks
 for KVM

A couple of nits:

The subject here isn't super helpful. This is doing a *LOT* more than
just exporting something. In a perfect world, you'd probably even do the
refactoring first and export in a separate patch.

On 10/13/25 18:09, Xin Li (Intel) wrote:
> Convert the __this_cpu_ist_{top,bottom}_va() macros into proper functions,
> and export __this_cpu_ist_top_va() to allow KVM to retrieve the top of the
> per-CPU exception stack.

The key thing this does is create a data structure and move away from
open-coded size calculations to using the compiler for it. The
macro=>function conversion is barely worth mentioning, IMNHO.

> FRED introduced new fields in the host-state area of the VMCS for stack
> levels 1->3 (HOST_IA32_FRED_RSP[123]), each respectively corresponding to
> per-CPU exception stacks for #DB, NMI and #DF.  KVM must populate these
> fields each time a vCPU is loaded onto a CPU.
> 
> To simplify access to the exception stacks in struct cea_exception_stacks,
> a union is used to create an array alias, enabling array-style indexing of
> the stack entries.

Super nit here, but please use imperative voice for stuff like this.

"To simplify access to" => "Simplify access to"

 and/or

"a union is used" => "use a union"

> diff --git a/arch/x86/include/asm/cpu_entry_area.h b/arch/x86/include/asm/cpu_entry_area.h
> index d0f884c28178..58cd71144e5e 100644
> --- a/arch/x86/include/asm/cpu_entry_area.h
> +++ b/arch/x86/include/asm/cpu_entry_area.h
> @@ -16,6 +16,19 @@
>  #define VC_EXCEPTION_STKSZ	0
>  #endif
>  
> +/*
> + * The exception stack ordering in [cea_]exception_stacks
> + */
> +enum exception_stack_ordering {
> +	ESTACK_DF,
> +	ESTACK_NMI,
> +	ESTACK_DB,
> +	ESTACK_MCE,
> +	ESTACK_VC,
> +	ESTACK_VC2,
> +	N_EXCEPTION_STACKS
> +};

This creates some new duplicated logic. There's already a list in the
same order in ESTACKS_MEMBERS() of all the stacks.

Ideally, everyone would move over to the enum and wouldn't need to use
the ESTACKS_MEMBERS() route. What's preventing that?