| Message ID | 20240822140044.441126-1-frediano.ziglio@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Restore memory used for IP computation | expand |
On 22.08.2024 16:00, Frediano Ziglio wrote: > We need to write in some location but no reasons to not > trying to restore what we potentially overwrote. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> > --- > xen/arch/x86/boot/head.S | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > --- > Changes since v1: > - Rewrite magic number field instead of some possible BIOS area. > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index d8ac0f0494..9b7e7b4e51 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -415,16 +415,19 @@ __pvh_start: > > /* > * We need one push/pop to determine load address. Use the same > - * absolute stack address as the native path, for lack of a better > - * alternative. > + * stack address as the native path. This isn't quite right, because ... > @@ -463,18 +466,21 @@ __start: > * 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. > + * Save and restore the magic field of start_info in ebx, and use > + * that as the stack. See also ... there simply is no start_info here. Iirc Andrew suggested to use the MB area's first slot (which effectively is what you do here, i.e. it's just the comment which is misleading), presumably on the assumption that any exception (incl NMI) in the window until a proper stack is set up will be deadly anyway (which may want mentioning in the comment or description as well). Jan
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index d8ac0f0494..9b7e7b4e51 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -415,16 +415,19 @@ __pvh_start: /* * We need one push/pop to determine load address. Use the same - * absolute stack address as the native path, for lack of a better - * alternative. + * stack address as the native path. */ - mov $0x1000, %esp + mov %ebx, %esp + pop %edx /* Calculate the load base address. */ call 1f 1: pop %esi sub $sym_offs(1b), %esi + /* Restore clobbered magic field */ + push %edx + /* Set up stack. */ lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp @@ -463,18 +466,21 @@ __start: * 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. + * Save and restore the magic field of start_info in ebx, and use + * that as the stack. See also + * https://lore.kernel.org/xen-devel/20240814195053.5564-3-jason.andryuk@amd.com/ */ - mov $0x1000, %esp + mov %ebx, %esp + pop %edx /* Calculate the load base address. */ call 1f 1: pop %esi sub $sym_offs(1b), %esi + /* Restore clobbered magic field */ + push %edx + /* Set up stack. */ lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp
We need to write in some location but no reasons to not trying to restore what we potentially overwrote. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- xen/arch/x86/boot/head.S | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) --- Changes since v1: - Rewrite magic number field instead of some possible BIOS area.