[Xen-devel] [PATCH v3 1/2] x86/setup: do not relocate Xen over current Xen image placement

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

Otherwise, due to Xen code/data changes under CPU feet, Xen may crash
silently at boot.

We were hit by the issue in OVS Xen 4.4 with my earlier version of
EFI/Multiboot2 patches. Initially its implementation allowed relocation
of Xen even if it was relocated by the bootloader. This led to the
crashes on some new Oracle machines because copy destination partially
overlapped with the end of current/initial Xen image placement.

After some discussion on Xen-devel we decided to disable Xen relocation in
my EFI/Multiboot2 upstream patches if the booloader did the work for us.
Though one case is still not covered. If Xen is not relocated by the
booloader then it tries to do that by itself. If all RAM regions above
currently occupied one are unsuitable for relocation then Xen tries to move
itself higher in it. And if (end - reloc_size + XEN_IMG_OFFSET) goes below
__pa(_end) then copy/relocation destination overlaps, at least partially,
with its source.

I can agree that this should not happen on todays machines very often.
If at all. It is rather unusual to not have usable RAM regions above
~5 MiB nowadays. Though I think that we should at least consider putting
such safety measure here. Otherwise Xen may crash mysteriously without
any stack trace. It is very confusing and impairs further debugging.

Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
---
v3 - suggestions/fixes:
   - use more readable condition
     (suggested by Jan Beulich),
   - improve comment
     (suggested by Jan Beulich),
   - improve commit message.

v2 - suggestions/fixes:
   - improve comment
     (suggested by Jan Beulich),
   - improve commit message
     (suggested by Jan Beulich).
---
 xen/arch/x86/setup.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2e10c6b..9f8abd9 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -962,7 +962,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         }
         else
             end = 0;
-        if ( end > s )
+
+        /*
+         * Is the region size greater than zero and does it begin
+         * at or above the end of current Xen image placement?
+         */
+        if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
         {
             l4_pgentry_t *pl4e;
             l3_pgentry_t *pl3e;
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel