Re: [PATCH v3] x86: Prioritize low memory size from Multiboot

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 10, 2022 at 09:44:36AM +0100, Tu Dinh Ngoc 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.
> 
> Prioritize the information supplied by Multiboot instead. If this is not
> available, fall back to the old BDA method.
> 
> Signed-off-by: Tu Dinh Ngoc <dinhngoc.tu@xxxxxxx>
> ---
> Changes in v3:
> - Prioritize using Multiboot's memory information.. Fall back to using
>   BDA in case MBI does not supply memory info.
> 
> Changes in v2:
> - Detect if Multiboot claims there's more than 640 KB of low memory
>   (happens with old Kexec versions), and correct the memory unit in such
>   cases.
> ---
>  xen/arch/x86/boot/head.S | 44 ++++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index dd1bea0d10..da7810060e 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -524,27 +524,41 @@ 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
> +        /* Check if Multiboot provides us with low memory size. */
> +        mov     %edx,%ecx
> +        test    %ecx,%ecx
> +        jz      1f
> +
> +        /*
> +         * Old Kexec used to report memory sizes in bytes instead of 
> kilobytes
> +         * like it's supposed to.
> +         *
> +         * If Multiboot reports more than 640 KB of low memory, assume we 
> have
> +         * this problem.

Is 640KB the absolute maximum of low memory that can be reported?

It would seem to me that reporting 1000KB or more is obviously wrong,
and would satisfy the check for old kexec reporting in bytes instead of
KB?

Do you need to also check that the adjusted value is between a sane
range also?

Thanks, Roger.