Re: [patch V3 11/32] x86/exceptions: Add structs for exception stacks

Sean Christopherson
2019-04-16T18:20:21Z

On Sun, Apr 14, 2019 at 05:59:47PM +0200, Thomas Gleixner wrote:
> At the moment everything assumes a full linear mapping of the various
> exception stacks. Adding guard pages to the cpu entry area mapping of the
> exception stacks will break that assumption.
>
> As a preparatory step convert both the real storage and the effective
> mapping in the cpu entry area from character arrays to structures.
>
> To ensure that both arrays have the same ordering and the same size of the
> individual stacks fill the members with a macro. The guard size is the only
> difference between the two resulting structures. For now both have guard
> size 0 until the preparation of all usage sites is done.
>
> Provide a couple of helper macros which are used in the following
> conversions.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---

One comment nit below, but other than that:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> V2 -> V3: Move the guards below the stacks and add a top guard. Fix the
> TOP() macro.
> ---
> arch/x86/include/asm/cpu_entry_area.h | 52 ++++++++++++++++++++++++++++++----
> arch/x86/kernel/cpu/common.c | 2 -
> arch/x86/mm/cpu_entry_area.c | 8 +----
> 3 files changed, 51 insertions(+), 11 deletions(-)
>
> --- a/arch/x86/include/asm/cpu_entry_area.h
> +++ b/arch/x86/include/asm/cpu_entry_area.h
> @@ -7,6 +7,51 @@
> #include <asm/processor.h>
> #include <asm/intel_ds.h>
>
> +#ifdef CONFIG_X86_64
> +
> +/* Macro to enforce the same ordering and stack sizes */
> +#define ESTACKS_MEMBERS(guardsize) \
> + char DF_stack_guard[guardsize]; \
> + char DF_stack[EXCEPTION_STKSZ]; \
> + char NMI_stack_guard[guardsize]; \
> + char NMI_stack[EXCEPTION_STKSZ]; \
> + char DB_stack_guard[guardsize]; \
> + char DB_stack[DEBUG_STKSZ]; \
> + char MCE_stack_guard[guardsize]; \
> + char MCE_stack[EXCEPTION_STKSZ]; \
> + char IST_top_guard[guardsize]; \
> +
> +/* The exception stacks linear storage. No guard pages required */

How about s/linear/physical? "linear" is confusing for us Intel wonks
(or maybe just me) because in Intel's parlance it means virtual addresses
for all intents and purposes. And an apostrophe to show possession would
help readability, e.g.:

/* The exception stacks' physical storage. No guard pages required */

> +struct exception_stacks {
> + ESTACKS_MEMBERS(0)
> +};
>