[PATCH v2] x86/boot: Fix the boot time relocation calculations

[PATCH v2] x86/boot: Fix the boot time relocation calculations

[Andrew Cooper]

c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces

    mov $sym_offs(__image_base__),%esi

to the legacy boot path.  However, this is by definition 0, which means the
boot code only functions correctly when Xen is loaded at its preferred
physical address (2M at the time of writing).

Xen does cope if loaded at an alternative physical address, if the
MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
versions of Grub do fill this in appropriately, tboot does not.  (In fact,
tboot loads Xen at the preferred address, but claims a load address of 8M.)

Both Multiboot 1 and 2 specify the execution environment as being flat.  As a
result, Xen needs no help calculating the proper load address.

However, Multiboot specifies %esp as undefined.  Experimentally, using the
entry %esp is fine, but this is certainly no guarantee.  Use a temporary stack
in the first page of RAM, which is one of the safest areas to clobber.

Calculate the load address from %eip alone, and ignore
MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
various versions of tboot.

Finally, set up the stack as soon as possible, which means the BIOS path has a
usable stack for the entirety of its duration.  Use the full available stack
size, rather than limiting to an arbitrary 1k.  One side effect is that the
MB2/EFI path continues to use the EFI stack until the trampoline is entered.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Julien Grall <julien.grall@arm.com>
CC: Daniel Kiper <daniel.kiper@oracle.com>
CC: Doug Goldstein <cardoe@cardoe.com>
CC: Sergey Dyasli <sergey.dyasli@citrix.com>

This is a regression introduced in Xen 4.9, and should therefore be fixed.

v2:
 * Clobber $0x1000 rather than using the bootloader stack.
 * Load %esp earlier.
---
 xen/arch/x86/boot/head.S | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 5e84e42..fd6fc33 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -377,8 +377,26 @@ __start:
         cld
         cli
 
-        /* Load default Xen image load base address. */
-        mov     $sym_offs(__image_base__),%esi
+        /*
+         * Multiboot (both 1 and 2) specify the stack pointer as undefined
+         * when entering in BIOS circumstances.  This is unhelpful for
+         * relocatable images, where one push/pop is required to calculate
+         * images load address.
+         *
+         * On a BIOS-based system, the IVT and BDA occupy the first 5/16ths of
+         * the first page of RAM, with the rest free for use.  Use the top of
+         * this page for a temporary stack, being one of the safest locations
+         * to clobber.
+         */
+        mov     $0x1000, %esp
+
+        /* Calculate the load base address. */
+        call    1f
+1:      pop     %esi
+        sub     $sym_offs(1b), %esi
+
+        /* Set up stack. */
+        lea     STACK_SIZE + sym_esi(cpu0_stack), %esp
 
         /* Bootloaders may set multiboot{1,2}.mem_lower to a nonzero value. */
         xor     %edx,%edx
@@ -410,15 +428,6 @@ __start:
         cmp     %edi,MB2_fixed_total_size(%ebx)
         jbe     trampoline_bios_setup
 
-        /* Get Xen image load base address from Multiboot2 information. */
-        cmpl    $MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR,MB2_tag_type(%ecx)
-        jne     .Lmb2_mem_lower
-
-        mov     MB2_load_base_addr(%ecx),%esi
-        sub     $XEN_IMG_OFFSET,%esi
-        jmp     .Lmb2_next_tag
-
-.Lmb2_mem_lower:
         /* Get mem_lower from Multiboot2 information. */
         cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,MB2_tag_type(%ecx)
         cmove   MB2_mem_lower(%ecx),%edx
@@ -519,9 +528,6 @@ trampoline_setup:
         mov     %esi,sym_fs(xen_phys_start)
         mov     %esi,sym_fs(trampoline_xen_phys_start)
 
-        /* Setup stack. %ss was initialized earlier. */
-        lea     1024+sym_esi(cpu0_stack),%esp
-
         mov     sym_fs(trampoline_phys),%ecx
 
         /* Get bottom-most low-memory stack address. */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] x86/boot: Fix the boot time relocation calculations

[Jan Beulich]

>>> On 12.06.17 at 12:45, <andrew.cooper3@citrix.com> wrote:
> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
> 
>     mov $sym_offs(__image_base__),%esi
> 
> to the legacy boot path.  However, this is by definition 0, which means the
> boot code only functions correctly when Xen is loaded at its preferred
> physical address (2M at the time of writing).
> 
> Xen does cope if loaded at an alternative physical address, if the
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
> versions of Grub do fill this in appropriately, tboot does not.  (In fact,
> tboot loads Xen at the preferred address, but claims a load address of 8M.)
> 
> Both Multiboot 1 and 2 specify the execution environment as being flat.  As a
> result, Xen needs no help calculating the proper load address.
> 
> However, Multiboot specifies %esp as undefined.  Experimentally, using the
> entry %esp is fine, but this is certainly no guarantee.  Use a temporary stack
> in the first page of RAM, which is one of the safest areas to clobber.
> 
> Calculate the load address from %eip alone, and ignore
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
> various versions of tboot.
> 
> Finally, set up the stack as soon as possible, which means the BIOS path has a
> usable stack for the entirety of its duration.  Use the full available stack
> size, rather than limiting to an arbitrary 1k.  One side effect is that the
> MB2/EFI path continues to use the EFI stack until the trampoline is entered.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] x86/boot: Fix the boot time relocation calculations

[Daniel Kiper]

On Mon, Jun 12, 2017 at 11:45:43AM +0100, Andrew Cooper wrote:
> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
>
>     mov $sym_offs(__image_base__),%esi
>
> to the legacy boot path.  However, this is by definition 0, which means the
> boot code only functions correctly when Xen is loaded at its preferred
> physical address (2M at the time of writing).
>
> Xen does cope if loaded at an alternative physical address, if the
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
> versions of Grub do fill this in appropriately, tboot does not.  (In fact,
> tboot loads Xen at the preferred address, but claims a load address of 8M.)
>
> Both Multiboot 1 and 2 specify the execution environment as being flat.  As a
> result, Xen needs no help calculating the proper load address.
>
> However, Multiboot specifies %esp as undefined.  Experimentally, using the
> entry %esp is fine, but this is certainly no guarantee.  Use a temporary stack
> in the first page of RAM, which is one of the safest areas to clobber.
>
> Calculate the load address from %eip alone, and ignore
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
> various versions of tboot.
>
> Finally, set up the stack as soon as possible, which means the BIOS path has a
> usable stack for the entirety of its duration.  Use the full available stack
> size, rather than limiting to an arbitrary 1k.  One side effect is that the
> MB2/EFI path continues to use the EFI stack until the trampoline is entered.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] x86/boot: Fix the boot time relocation calculations

[Julien Grall]

Hi Andrew,

On 06/12/2017 11:45 AM, Andrew Cooper wrote:
> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
> 
>      mov $sym_offs(__image_base__),%esi
> 
> to the legacy boot path.  However, this is by definition 0, which means the
> boot code only functions correctly when Xen is loaded at its preferred
> physical address (2M at the time of writing).
> 
> Xen does cope if loaded at an alternative physical address, if the
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
> versions of Grub do fill this in appropriately, tboot does not.  (In fact,
> tboot loads Xen at the preferred address, but claims a load address of 8M.)
> 
> Both Multiboot 1 and 2 specify the execution environment as being flat.  As a
> result, Xen needs no help calculating the proper load address.
> 
> However, Multiboot specifies %esp as undefined.  Experimentally, using the
> entry %esp is fine, but this is certainly no guarantee.  Use a temporary stack
> in the first page of RAM, which is one of the safest areas to clobber.
> 
> Calculate the load address from %eip alone, and ignore
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
> various versions of tboot.
> 
> Finally, set up the stack as soon as possible, which means the BIOS path has a
> usable stack for the entirety of its duration.  Use the full available stack
> size, rather than limiting to an arbitrary 1k.  One side effect is that the
> MB2/EFI path continues to use the EFI stack until the trampoline is entered.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Daniel Kiper <daniel.kiper@oracle.com>
> CC: Doug Goldstein <cardoe@cardoe.com>
> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
> 
> This is a regression introduced in Xen 4.9, and should therefore be fixed.

This is touching early-boot code. I would like to wait a least a push 
with this patch on staging before suggesting to push in Xen 4.9.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] x86/boot: Fix the boot time relocation calculations

[Julien Grall]

On 14/06/17 11:33, Julien Grall wrote:
> Hi Andrew,
>
> On 06/12/2017 11:45 AM, Andrew Cooper wrote:
>> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
>>
>>      mov $sym_offs(__image_base__),%esi
>>
>> to the legacy boot path.  However, this is by definition 0, which
>> means the
>> boot code only functions correctly when Xen is loaded at its preferred
>> physical address (2M at the time of writing).
>>
>> Xen does cope if loaded at an alternative physical address, if the
>> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While
>> recent
>> versions of Grub do fill this in appropriately, tboot does not.  (In
>> fact,
>> tboot loads Xen at the preferred address, but claims a load address of
>> 8M.)
>>
>> Both Multiboot 1 and 2 specify the execution environment as being
>> flat.  As a
>> result, Xen needs no help calculating the proper load address.
>>
>> However, Multiboot specifies %esp as undefined.  Experimentally, using
>> the
>> entry %esp is fine, but this is certainly no guarantee.  Use a
>> temporary stack
>> in the first page of RAM, which is one of the safest areas to clobber.
>>
>> Calculate the load address from %eip alone, and ignore
>> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot
>> under
>> various versions of tboot.
>>
>> Finally, set up the stack as soon as possible, which means the BIOS
>> path has a
>> usable stack for the entirety of its duration.  Use the full available
>> stack
>> size, rather than limiting to an arbitrary 1k.  One side effect is
>> that the
>> MB2/EFI path continues to use the EFI stack until the trampoline is
>> entered.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Julien Grall <julien.grall@arm.com>
>> CC: Daniel Kiper <daniel.kiper@oracle.com>
>> CC: Doug Goldstein <cardoe@cardoe.com>
>> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
>>
>> This is a regression introduced in Xen 4.9, and should therefore be
>> fixed.
>
> This is touching early-boot code. I would like to wait a least a push
> with this patch on staging before suggesting to push in Xen 4.9.

Release-acked-by: Julien Grall <julien.grall@arm.com>

>
> Cheers,
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel