Re: [Xen-devel] [PATCH] x86/boot: Annotate the Real Mode entry points

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.19 at 15:27, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/boot/trampoline.S
> +++ b/xen/arch/x86/boot/trampoline.S
> @@ -38,6 +38,12 @@
>  
>          .code16
>  
> +/*
> + * do_boot_cpu() programs the Startup-IPI to point here.  Due to the SIPI
> + * format, the relocated entrypoint must be 4k aligned.
> + *
> + * It is entered in Real Mode, with %cs = wakeup_start >> 4 and %ip = 0.
> + */
>  GLOBAL(trampoline_realmode_entry)

The reference to wakeup_start looks to be a copy-and-paste
(or alike) mistake here.

> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -548,9 +548,12 @@ static int do_boot_cpu(int apicid, int cpu)
>  
>      booting_cpu = cpu;
>  
> -    /* start_eip had better be page-aligned! */
>      start_eip = setup_trampoline();
>  
> +    /* start_eip needs be page aligned, and below the 1M boundary. */
> +    if ( start_eip & ~0xff000 )
> +        panic("AP trampoline %#lx not suitably positioned\n", start_eip);

Seeing what setup_trampoline() really does, I'm not
convinced a panic() is of much value. The page-alignment
should be possible to check at build time, and the below-1M
requirement should be guaranteed by us allocating low
memory space in the first place. Nevertheless I won't insist,
so with the earlier comment corrected
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel