Previously, Xen used information from the BDA to detect the amount of
available low memory. This does not work on some scenarios such as
Coreboot, or when booting from Kexec on a UEFI system without CSM.
Prioritize the information supplied by Multiboot instead. If this is not
available, fall back to the old BDA method.
Signed-off-by: Tu Dinh Ngoc <dinhngoc.tu@irit.fr>
---
Changes in v3:
- Prioritize using Multiboot's memory information.. Fall back to using
BDA in case MBI does not supply memory info.
Changes in v2:
- Detect if Multiboot claims there's more than 640 KB of low memory
(happens with old Kexec versions), and correct the memory unit in such
cases.
---
xen/arch/x86/boot/head.S | 44 ++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index dd1bea0d10..da7810060e 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -524,27 +524,41 @@ trampoline_bios_setup:
mov %ecx,%fs
mov %ecx,%gs
- /* Set up trampoline segment 64k below EBDA */
- movzwl 0x40e,%ecx /* EBDA segment */
- cmp $0xa000,%ecx /* sanity check (high) */
- jae 0f
- cmp $0x4000,%ecx /* sanity check (low) */
- jae 1f
+ /* Check if Multiboot provides us with low memory size. */
+ mov %edx,%ecx
+ test %ecx,%ecx
+ jz 1f
+
+ /*
+ * Old Kexec used to report memory sizes in bytes instead of kilobytes
+ * like it's supposed to.
+ *
+ * If Multiboot reports more than 640 KB of low memory, assume we have
+ * this problem.
+ */
+ cmp $640,%ecx
+ jbe 0f
+ shr $10,%ecx
0:
- movzwl 0x413,%ecx /* use base memory size on failure */
+ /* %ecx now contains the low memory size in kilobytes. */
shl $10-4,%ecx
+ jmp 3f
+
1:
/*
- * Compare the value in the BDA with the information from the
- * multiboot structure (if available) and use the smallest.
+ * Multiboot doesn't provide us with memory info. Set up trampoline
+ * segment 64k below EBDA as fallback.
*/
- cmp $0x100,%edx /* is the multiboot value too small? */
- jb 2f /* if so, do not use it */
- shl $10-4,%edx
- cmp %ecx,%edx /* compare with BDA value */
- cmovb %edx,%ecx /* and use the smaller */
-
+ movzwl 0x40e,%ecx /* EBDA segment */
+ cmp $0xa000,%ecx /* sanity check (high) */
+ jae 2f
+ cmp $0x4000,%ecx /* sanity check (low) */
+ jae 3f
2:
+ movzwl 0x413,%ecx /* use base memory size on failure */
+ shl $10-4,%ecx
+
+3:
/* Reserve memory for the trampoline and the low-memory stack. */
sub $((TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE)>>4),%ecx
--
2.25.1
On 10.02.2022 09:44, Tu Dinh Ngoc wrote: > Previously, Xen used information from the BDA to detect the amount of > available low memory. This does not work on some scenarios such as > Coreboot, or when booting from Kexec on a UEFI system without CSM. > > Prioritize the information supplied by Multiboot instead. If this is not > available, fall back to the old BDA method. You're still failing to provide information on why this would be a safe thing to do. In the absence of such, prior behavior has to be retained, and only the special case you're after wants adjusting for. This is first and foremost (but not limited to) you moving to ... > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -524,27 +524,41 @@ trampoline_bios_setup: > mov %ecx,%fs > mov %ecx,%gs > > - /* Set up trampoline segment 64k below EBDA */ > - movzwl 0x40e,%ecx /* EBDA segment */ > - cmp $0xa000,%ecx /* sanity check (high) */ > - jae 0f > - cmp $0x4000,%ecx /* sanity check (low) */ > - jae 1f > + /* Check if Multiboot provides us with low memory size. */ > + mov %edx,%ecx > + test %ecx,%ecx > + jz 1f ... checking for just zero, when originally ... > + /* > + * Old Kexec used to report memory sizes in bytes instead of kilobytes > + * like it's supposed to. > + * > + * If Multiboot reports more than 640 KB of low memory, assume we have > + * this problem. > + */ > + cmp $640,%ecx > + jbe 0f > + shr $10,%ecx > 0: > - movzwl 0x413,%ecx /* use base memory size on failure */ > + /* %ecx now contains the low memory size in kilobytes. */ > shl $10-4,%ecx > + jmp 3f > + > 1: > /* > - * Compare the value in the BDA with the information from the > - * multiboot structure (if available) and use the smallest. > + * Multiboot doesn't provide us with memory info. Set up trampoline > + * segment 64k below EBDA as fallback. > */ > - cmp $0x100,%edx /* is the multiboot value too small? */ > - jb 2f /* if so, do not use it */ ... the boundary was 0x100. It was for this reason why in reply to v1 I did ask "Is the kexec case recognizable by any means (including [...]), such that we could skip using the BDA value in that case?" If it wasn't clear, I did mean _just_ in this case. > - shl $10-4,%edx > - cmp %ecx,%edx /* compare with BDA value */ > - cmovb %edx,%ecx /* and use the smaller */ > - > + movzwl 0x40e,%ecx /* EBDA segment */ > + cmp $0xa000,%ecx /* sanity check (high) */ > + jae 2f > + cmp $0x4000,%ecx /* sanity check (low) */ > + jae 3f > 2: > + movzwl 0x413,%ecx /* use base memory size on failure */ > + shl $10-4,%ecx I don't see why this shift can't be folded with the other one, by moving it ... > + > +3: ... below here (and removing the one further up). Jan > /* Reserve memory for the trampoline and the low-memory stack. */ > sub $((TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE)>>4),%ecx >
On Thu, Feb 10, 2022 at 09:44:36AM +0100, Tu Dinh Ngoc wrote: > Previously, Xen used information from the BDA to detect the amount of > available low memory. This does not work on some scenarios such as > Coreboot, or when booting from Kexec on a UEFI system without CSM. > > Prioritize the information supplied by Multiboot instead. If this is not > available, fall back to the old BDA method. > > Signed-off-by: Tu Dinh Ngoc <dinhngoc.tu@irit.fr> > --- > Changes in v3: > - Prioritize using Multiboot's memory information.. Fall back to using > BDA in case MBI does not supply memory info. > > Changes in v2: > - Detect if Multiboot claims there's more than 640 KB of low memory > (happens with old Kexec versions), and correct the memory unit in such > cases. > --- > xen/arch/x86/boot/head.S | 44 ++++++++++++++++++++++++++-------------- > 1 file changed, 29 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index dd1bea0d10..da7810060e 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -524,27 +524,41 @@ trampoline_bios_setup: > mov %ecx,%fs > mov %ecx,%gs > > - /* Set up trampoline segment 64k below EBDA */ > - movzwl 0x40e,%ecx /* EBDA segment */ > - cmp $0xa000,%ecx /* sanity check (high) */ > - jae 0f > - cmp $0x4000,%ecx /* sanity check (low) */ > - jae 1f > + /* Check if Multiboot provides us with low memory size. */ > + mov %edx,%ecx > + test %ecx,%ecx > + jz 1f > + > + /* > + * Old Kexec used to report memory sizes in bytes instead of kilobytes > + * like it's supposed to. > + * > + * If Multiboot reports more than 640 KB of low memory, assume we have > + * this problem. Is 640KB the absolute maximum of low memory that can be reported? It would seem to me that reporting 1000KB or more is obviously wrong, and would satisfy the check for old kexec reporting in bytes instead of KB? Do you need to also check that the adjusted value is between a sane range also? Thanks, Roger.
© 2016 - 2024 Red Hat, Inc.