[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/boot: Rationalise stack handling during early boot
On 09.01.2020 11:43, Andrew Cooper wrote: > On 09/01/2020 10:28, Jan Beulich wrote: >> 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. > > Lets just go with i. > > "m" is also an option, and clang will probably find some way of pulling > it out of the stringtable, but anything other than an immediate encoding > here is going to be silly. No, sadly "m" is not an option when the expression is a constant: Gcc at least is unable to materialize a memory variable in this case, and will give some funny error message instead. >>> --- 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). > > In practice, this variable is never used because we never offline the BSP. traps.c uses stack_base[], for example, and hence it needs to be correct also for the BSP. Even more important is perhaps get_cpu_current()'s use. > However, the code as-is is correct. The value in stack_start has > changed in this patch, but is still the correct value for the BSP. But it's no longer the stack base (which is what stack_base[] holds for all other CPUs), or else you wouldn't have had a need to change do_boot_cpu(). > It could also be made into an initialiser, but that would cause > stack_base[] to move from BSS into data, and it is a NR_CPUS sized > datastructure. I do realize this. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |