On 02/05/2019 14:50, Jan Beulich wrote:
>>>> On 02.05.19 at 15:27, <[email protected]> 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.
Oops, indeed. Fixed.
>
>> --- 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.
Sadly it cant.
We have a number of alignment issues (well - confusions at least), most
obviously that trampoline_{start,end} in the linked Xen image has no
particular alignment, but the relocated trampoline_start has a 4k
requirement as a consequence of its alias with trampoline_realmode_entry.
All it takes is one error in the 32bit asm which relocates the
trampoline and this ends up exploding in a way which can only be
detected at runtime.
> Nevertheless I won't insist,
> so with the earlier comment corrected
> Reviewed-by: Jan Beulich <[email protected]>
Thanks,
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
