Re: [Xen-devel] [PATCH] x86/boot: Rationalise stack handling during early boot

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2020 18:00, Andrew Cooper wrote:
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -249,23 +249,24 @@ static void __init noreturn 
> efi_arch_post_exit_boot(void)
>                     "or     $"__stringify(X86_CR4_PGE)", %[cr4]\n\t"
>                     "mov    %[cr4], %%cr4\n\t"
>  #endif
> -                   "movabs $__start_xen, %[rip]\n\t"
>                     "lgdt   boot_gdtr(%%rip)\n\t"
> -                   "mov    stack_start(%%rip), %%rsp\n\t"
>                     "mov    %[ds], %%ss\n\t"
>                     "mov    %[ds], %%ds\n\t"
>                     "mov    %[ds], %%es\n\t"
>                     "mov    %[ds], %%fs\n\t"
>                     "mov    %[ds], %%gs\n\t"
> -                   "movl   %[cs], 8(%%rsp)\n\t"
> -                   "mov    %[rip], (%%rsp)\n\t"
> -                   "lretq  %[stkoff]-16"
> +
> +                   /* Jump to higher mappings. */
> +                   "mov    stack_start(%%rip), %%rsp\n\t"
> +                   "movabs $__start_xen, %[rip]\n\t"
> +                   "push   %[cs]\n\t"

Either you need %q[cs] here (assuming q gets ignored for
immediate operands, which I didn't check) or ...

> +                   "push   %[rip]\n\t"
> +                   "lretq"
>                     : [rip] "=&r" (efer/* any dead 64-bit variable */),
>                       [cr4] "+&r" (cr4)
>                     : [cr3] "r" (idle_pg_table),
>                       [cs] "ir" (__HYPERVISOR_CS),

... you need to use just "i" here, for there not being 32-bit
PUSH forms.

> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -554,7 +554,7 @@ static int do_boot_cpu(int apicid, int cpu)
>          printk("Booting processor %d/%d eip %lx\n",
>                 cpu, apicid, start_eip);
>  
> -    stack_start = stack_base[cpu];
> +    stack_start = stack_base[cpu] + STACK_SIZE - sizeof(struct cpu_info);
>  
>      /* This grunge runs the startup process for the targeted processor. */

Further down smp_prepare_cpus() has

    stack_base[0] = stack_start;

which I think you need to also adjust (or replace, e.g. by giving
stack_base[] an initializer).

Jan

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