Re: [PATCH] x86: Use low memory size directly from Multiboot

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.02.2022 12:00, dinhngoc.tu@xxxxxxx wrote:
> Previously, Xen used information from the BDA to detect the amount of
> available low memory. This does not work on some scenarios such as
> Coreboot, or when booting from Kexec on a UEFI system without CSM.
> 
> Use the information directly supplied by Multiboot boot information
> instead.
> ---
>  xen/arch/x86/boot/head.S | 30 ++++++------------------------
>  1 file changed, 6 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index dd1bea0d10..31a0663e6d 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -524,33 +524,15 @@ trampoline_bios_setup:
>          mov     %ecx,%fs
>          mov     %ecx,%gs
>  
> -        /* Set up trampoline segment 64k below EBDA */
> -        movzwl  0x40e,%ecx          /* EBDA segment */
> -        cmp     $0xa000,%ecx        /* sanity check (high) */
> -        jae     0f
> -        cmp     $0x4000,%ecx        /* sanity check (low) */
> -        jae     1f
> -0:
> -        movzwl  0x413,%ecx          /* use base memory size on failure */
> -        shl     $10-4,%ecx
> -1:
> -        /*
> -         * Compare the value in the BDA with the information from the
> -         * multiboot structure (if available) and use the smallest.
> -         */
> -        cmp     $0x100,%edx         /* is the multiboot value too small? */
> -        jb      2f                  /* if so, do not use it */

The comment here is a pretty clear indication that bad values may have
been observed, even if this was only in the distant past. But we have
to not regress even on very old boot loaders.

Is the kexec case recognizable by any means (including to distinguish
kexec properly communicating the value vs it not doing so, as iirc it
was said on irc that this didn't always work correctly there), such
that we could skip using the BDA value in that case?

Jan

> -        shl     $10-4,%edx
> -        cmp     %ecx,%edx           /* compare with BDA value */
> -        cmovb   %edx,%ecx           /* and use the smaller */
> +        /* Use lower memory size directly from Multiboot */
> +        mov     %edx,%ecx
> +        /* From arch/x86/smpboot.c: start_eip had better be page-aligned!
> */
> +        shr     $2,%ecx
>  
> -2:
>          /* Reserve memory for the trampoline and the low-memory stack. */
> -        sub     $((TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE)>>4),%ecx
> +        sub     $((TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE)>>12),%ecx
>  
> -        /* From arch/x86/smpboot.c: start_eip had better be page-aligned!
> */
> -        xor     %cl, %cl
> -        shl     $4, %ecx
> +        shl     $12,%ecx
>          mov     %ecx,sym_esi(trampoline_phys)
>  
>  trampoline_setup: