On 23.08.2024 15:10, Andrew Cooper wrote:
> From: Frediano Ziglio <frediano.zig...@cloud.com>
>
> Right now, Xen clobbers the value at 0xffc when performing it's load-base
> calculation. We've got plenty of free registers at this point, so the value
> can be preserved easily.
>
> This fixes a real bug booting under Coreboot+SeaBIOS, where 0xffc happens to
> be the cbmem pointer (e.g. Coreboot's dmesg ring, among other things).
>
> However, there's also a better choice of memory location to use than 0xffc, as
> all our supported boot protocols have a pointer to an info structure in %ebx.
>
> Update the documentation to match.
>
> Fixes: 1695e53851e5 ("x86/boot: Fix the boot time relocation calculations")
> Fixes: d96bb172e8c9 ("x86/entry: Early PVH boot code")
> Signed-off-by: Frediano Ziglio <frediano.zig...@cloud.com>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
with two nits:
> --- a/docs/hypervisor-guide/x86/how-xen-boots.rst
> +++ b/docs/hypervisor-guide/x86/how-xen-boots.rst
> @@ -96,6 +96,12 @@ Xen, once loaded into memory, identifies its position in
> order to relocate
> system structures. For 32bit entrypoints, this necessarily requires a call
> instruction, and therefore a stack, but none of the ABIs provide one.
>
> -Overall, given that on a BIOS-based system, the IVT and BDA occupy the first
> -5/16ths of the first page of RAM, with the rest free to use, Xen assumes the
> -top of the page is safe to use.
> +In each supported 32bit entry protocol, ``%ebx`` is a pointer to an info
> +structure, and it is highly likely that this structure does not overlap with
> +Xen. Therefore we use this as as a temporary stack, preserving the prior
Double "as".
> @@ -460,21 +466,26 @@ __start:
> /*
> * 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.
> + * relocatable images, where one call (i.e. push) is required to
> + * calculate images load address.
Perhaps "the" after "calculate" and (albeit you're the native speaker) isn't
it "image's" then?
Jan
