Re: [PATCH v10 05/28] x86/cea: Use array indexing to simplify exception stack access

[Sohil Mehta]

On 10/22/2026 11:07 PM, Binbin Wu wrote:
> On 9/12/2026 5:36 AM, Sohil Mehta wrote:
>> From: "Xin Li (Intel)" <xin@xxxxxxxxx>
>>
>> Refactor struct cea_exception_stacks to leverage array indexing for
>> exception stack access, improving code clarity and eliminating the
>> need for the ESTACKS_MEMBERS() macro.
>>
>> Convert __this_cpu_ist_{bottom,top}_va() from macros to functions,
>> allowing removal of the now-obsolete CEA_ESTACK_BOT and CEA_ESTACK_TOP
>> macros.
>>
>> Also drop CEA_ESTACK_SIZE, which just duplicated EXCEPTION_STKSZ.
>>
>> Signed-off-by: Xin Li (Intel) <xin@xxxxxxxxx>
>> Signed-off-by: Sohil Mehta <sohil.mehta@xxxxxxxxx>
>> Acked-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> 
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> 

Thanks!

> One nit below.
> 
> [...]>  
>> +/*
>> + * 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
>> +};
>> +
>>  /* Macro to enforce the same ordering and stack sizes */
> 
> Nit:
> Is this still hold ? ...
> 

You are right. The comment is stale now. Overall, as the changelog says
the ESTACKS_MEMBERS marco itself is redundant.

I am wondering if the remaining single use of ESTACKS_MEMBERS() should
be simplified as follows:

diff --git a/arch/x86/include/asm/cpu_entry_area.h
b/arch/x86/include/asm/cpu_entry_area.h
index 509e52fc3a0f..f34062c7bde1 100644
--- a/arch/x86/include/asm/cpu_entry_area.h
+++ b/arch/x86/include/asm/cpu_entry_area.h
@@ -31,21 +31,10 @@ enum exception_stack_ordering {

-/* Macro to enforce the same ordering and stack sizes */
-#define ESTACKS_MEMBERS(guardsize, optional_stack_size)                \
-       char    ESTACK_DF_stack_guard[guardsize];               \
-       char    ESTACK_DF_stack[EXCEPTION_STKSZ];               \
-       char    ESTACK_NMI_stack_guard[guardsize];              \
-       char    ESTACK_NMI_stack[EXCEPTION_STKSZ];              \
-       char    ESTACK_DB_stack_guard[guardsize];               \
-       char    ESTACK_DB_stack[EXCEPTION_STKSZ];               \
-       char    ESTACK_MCE_stack_guard[guardsize];              \
-       char    ESTACK_MCE_stack[EXCEPTION_STKSZ];              \
-       char    ESTACK_VC_stack_guard[guardsize];               \
-       char    ESTACK_VC_stack[optional_stack_size];           \
-       char    ESTACK_VC2_stack_guard[guardsize];              \
-       char    ESTACK_VC2_stack[optional_stack_size];          \
-       char    ESTACK_IST_top_guard[guardsize];                \
-
 /* The exception stacks' physical storage. No guard pages required */
 struct exception_stacks {
-       ESTACKS_MEMBERS(0, VC_EXCEPTION_STKSZ)
+       char    ESTACK_DF_stack[EXCEPTION_STKSZ];
+       char    ESTACK_NMI_stack[EXCEPTION_STKSZ];
+       char    ESTACK_DB_stack[EXCEPTION_STKSZ];
+       char    ESTACK_MCE_stack[EXCEPTION_STKSZ];
+       char    ESTACK_VC_stack[VC_EXCEPTION_STKSZ];
+       char    ESTACK_VC2_stack[VC_EXCEPTION_STKSZ];
 };



>>  #define ESTACKS_MEMBERS(guardsize, optional_stack_size)		\
>>  	char	ESTACK_DF_stack_guard[guardsize];		\
>> @@ -39,37 +52,22 @@ struct exception_stacks {
>>  
>>  /* The effective cpu entry area mapping with guard pages. */
>>  struct cea_exception_stacks {
>> -	ESTACKS_MEMBERS(PAGE_SIZE, EXCEPTION_STKSZ)
>> +	struct {
>> +		char stack_guard[PAGE_SIZE];
>> +		char stack[EXCEPTION_STKSZ];
>> +	} event_stacks[N_EXCEPTION_STACKS];
>> +	char IST_top_guard[PAGE_SIZE];
> 
> ... Before this change, ESTACKS_MEMBERS() is used both in exception_stacks and
> cea_exception_stacks, so ESTACKS_MEMBERS() can enforce the order and stack
> sizes.
> 
> After the change, ESTACKS_MEMBERS() only used in one place, so it doesn't have
> the capability to enforce them.
> 
> The order consistency depends on the consistency b/t ESTACKS_MEMBERS and
> enum exception_stack_ordering now.
> 

IIUC, there is no ordering requirement left to enforce. The mapping is
done per stack by name in cea_map_stack(), right? Also, the size is
already inconsistent due to VC_EXCEPTION_STKSZ being config-dependent.

I think the change in this patch doesn't change any behavior. It only
refactors the macros to improve clarity and converts a couple of them
into functions.

>>  };
> 
> [...]