Re: [PATCH v2] x86/trampoline: Collect other scattered trampoline symbols

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Fri, Nov 8, 2024 at 2:53 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> ... and document them too.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> CC: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
>
> v2:
>  * Rebase
>
> video.h, edd.h and e820.h also contain trampoline symbols, but they're pretty
> well contained headers already.
>
> kbd_shift_flags seems especially dubious.  It's a snapshot of the keyboard
> state when Xen happened to pass through the trampoline, and surely cannot be
> useful for dom0 in the slightest...

I suppose an OT comment here.
It's just passed as a firmware information back to dom0.
It's used by the Linux kernel. See for instance
https://gfiber.googlesource.com/kernel/bruno/+/801f6772cecea6cfc7da61aa197716ab64db5f9e/arch/x86/xen/enlighten.c.

> ---
>  xen/arch/x86/include/asm/processor.h  |  2 --
>  xen/arch/x86/include/asm/setup.h      |  2 --
>  xen/arch/x86/include/asm/trampoline.h | 15 +++++++++++++++
>  3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/processor.h 
> b/xen/arch/x86/include/asm/processor.h
> index 26e2fb13699a..877651212273 100644
> --- a/xen/arch/x86/include/asm/processor.h
> +++ b/xen/arch/x86/include/asm/processor.h
> @@ -96,8 +96,6 @@ extern void ctxt_switch_levelling(const struct vcpu *next);
>  extern void (*ctxt_switch_masking)(const struct vcpu *next);
>
>  extern bool opt_cpu_info;
> -extern u32 trampoline_efer;
> -extern u64 trampoline_misc_enable_off;
>
>  /* Maximum width of physical addresses supported by the hardware. */
>  extern unsigned int paddr_bits;
> diff --git a/xen/arch/x86/include/asm/setup.h 
> b/xen/arch/x86/include/asm/setup.h
> index d7ed4f40024c..25c15ef9140d 100644
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -48,8 +48,6 @@ int remove_xen_ranges(struct rangeset *r);
>
>  int cf_check stub_selftest(void);
>
> -extern uint8_t kbd_shift_flags;
> -
>  #ifdef NDEBUG
>  # define highmem_start 0
>  #else
> diff --git a/xen/arch/x86/include/asm/trampoline.h 
> b/xen/arch/x86/include/asm/trampoline.h
> index 57a1f85a539e..838c2f0b6fcd 100644
> --- a/xen/arch/x86/include/asm/trampoline.h
> +++ b/xen/arch/x86/include/asm/trampoline.h
> @@ -85,9 +85,24 @@ extern uint32_t trampoline_xen_phys_start;
>  /* A semaphore to indicate signs-of-life at the start of the AP boot path. */
>  extern uint8_t trampoline_cpu_started;
>
> +/*
> + * Extra MSR_EFER settings when activating Long Mode.  EFER_NXE is necessary
> + * for APs to boot if the BSP found and activated support.
> + */
> +extern uint32_t trampoline_efer;
> +
> +/*
> + * When nonzero, clear the specified bits in MSR_MISC_ENABLE.  This is
> + * necessary to clobber XD_DISABLE before trying to set MSR_EFER.NXE.
> + */
> +extern uint64_t trampoline_misc_enable_off;
> +
>  /* Quirks about video mode-setting on S3 resume. */
>  extern uint8_t video_flags;
>
> +/* BIOS Int 16h, Fn 02h.  The keyboard shift status. */
> +extern uint8_t kbd_shift_flags;
> +
>  /* Extended Display Identification Data, gathered from the BIOS. */
>  extern uint16_t boot_edid_caps;
>  extern uint8_t boot_edid_info[128];
>
> base-commit: 75035c386eeb8bd5dbed1831dbb96a35b31521f6

Reviewed-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>

Frediano