[PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Daniel Kiper]

Hi,

This patch series breaks multiboot (v1) protocol dependency and adds
multiboot2 support. It lays down the foundation for EFI + GRUB2 + Xen
development. Detailed description of ideas and thoughts you will
find in commit message for every patch. If something is not obvious
please drop me a line.

Most of the requested things are fixed but there are still some minor
outstanding issues (multiboot2 tags generation, excessive amount of casts
in xen/arch/x86/boot/reloc.c, etc.; please check commit messages for
more details). If something is not fixed yet it means that I do not have
good idea how to do that. In case you spot something which was mentioned
during previous review and still think that your comment is valid
in particular case please notify me.

Below you can find reply for Konrad's questions in regards to
exception request for Xen 4.5 release.

> Couple of questions:
>
>  - Since this is mostly XBI code, is there a lot of overlap with the ARM/x86
>    refactoring of the EFI code? As in, will it require a lot of
>    rebasing/fixing it up?

I did not checked that right now. However, Roy once told me that this should
not be very big issue because overlap is not so big. Please check this email
for more details: http://lists.xen.org/archives/html/xen-devel/2014-08/msg01173.html

Sadly, it was sent more then one month ago so maybe something has changed.
Roy, any comments on that?

Anyway, I am more than happy to work on this issue with Roy.

>  - What is the disadvantage of having this in Xen 4.6?

Well, it will require to rebase all this work on latest developments.
It could be quite difficult because my patches touches a lot of places
and as I can see there is a chance for substantial changes in some of them.

>  - What is the advantage of having this in Xen 4.5?
>    [My understanding is that it would allow us to thrash out a lot of
>     the early bootup issues _now_ and fix them up instead of
>     having to deal in Xen 4.6 with this, EFI, and SecureBoot - which
>     is a lot of more moving pieces]

More or less. Additionally, we will have support for multiboot2 protocol
in place. There is not very big demand for that but as I know at least Intel
guys were looking for that. IIRC, they needed that for TPM support.

Daniel

 xen/arch/x86/Makefile             |    1 +
 xen/arch/x86/boot/cmdline.S       |    9 +-
 xen/arch/x86/boot/head.S          |  150 ++++++++++++++++++++++++++++-----
 xen/arch/x86/boot/reloc.c         |  248 ++++++++++++++++++++++++++++++++++++++++++++-----------
 xen/arch/x86/boot/x86_64.S        |   10 ++-
 xen/arch/x86/boot_info.c          |  258 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/dmi_scan.c           |    7 +-
 xen/arch/x86/domain_build.c       |   24 +++---
 xen/arch/x86/e820.c               |    8 +-
 xen/arch/x86/efi/boot.c           |  214 +++++++++++++++++++++++------------------------
 xen/arch/x86/efi/efi.h            |    3 -
 xen/arch/x86/efi/runtime.c        |   52 +++++++++---
 xen/arch/x86/microcode.c          |   39 ++++-----
 xen/arch/x86/mpparse.c            |    9 +-
 xen/arch/x86/platform_hypercall.c |   19 ++---
 xen/arch/x86/setup.c              |  370 +++++++++++++++++++++++++++-------------------------------------------------------
 xen/arch/x86/x86_64/asm-offsets.c |    5 +-
 xen/drivers/acpi/osl.c            |    9 +-
 xen/drivers/video/vesa.c          |    7 +-
 xen/drivers/video/vga.c           |   18 ++--
 xen/include/asm-x86/boot_info.h   |  126 ++++++++++++++++++++++++++++
 xen/include/asm-x86/config.h      |    2 -
 xen/include/asm-x86/e820.h        |   10 +--
 xen/include/asm-x86/edd.h         |    6 --
 xen/include/asm-x86/mbd.h         |   70 ++++++++++++++++
 xen/include/asm-x86/setup.h       |   10 +--
 xen/include/xen/efi.h             |   10 ---
 xen/include/xen/multiboot2.h      |  354 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/vga.h             |   18 ----
 xen/include/xsm/xsm.h             |   14 ++--
 xen/xsm/xsm_core.c                |    6 +-
 xen/xsm/xsm_policy.c              |   14 ++--
 32 files changed, 1510 insertions(+), 590 deletions(-)

Daniel Kiper (5):
      xen/x86: Introduce MultiBoot Data (MBD) type
      xen/x86: Define e820 entries counter as unsigned int
      xen/x86: Migrate to boot_info structure
      xen/x86: Use constant as multiboot protocol identifier
      xen/x86: Add multiboot2 protocol support

[PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Daniel Kiper]

Introduce MultiBoot Data (MBD) type. It is used to define variable
which carry over data from multiboot protocol (any version) through
Xen preloader stage. Later all or parts of this data is used
to initialize boot_info structure. boot_info is introduced
in later patches.

MBD helps to break multiboot (v1) protocol dependency. Using it
we are able to save space on trampoline (we do not allocate space
for unused data what happens in current preloader implementation).
Additionally, we are able to easily add new members to MBD if we
want support for new features or protocols.

There is not plan to share MBD among architectures. It will be
nice if boot_info will be shared among architectures. Please
check later patches for more details.

Code found in xen/arch/x86/boot_info.c moves MBD data to mbi struct
which is referenced from main Xen code. This is temporary solution
which helps to split patches into logical parts. Later it will be
replaced by final version of boot_info support.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
v2 - suggestions/fixes:
   - improve inline assembly
     (suggested by Andrew Cooper and Jan Beulich),
   - use __used attribute
     (suggested by Andrew Cooper),
   - patch split rearrangement
     (suggested by Andrew Cooper and Jan Beulich).
---
 xen/arch/x86/Makefile             |    1 +
 xen/arch/x86/boot/cmdline.S       |    9 +--
 xen/arch/x86/boot/head.S          |   31 ++++----
 xen/arch/x86/boot/reloc.c         |  147 +++++++++++++++++++++++++------------
 xen/arch/x86/boot/x86_64.S        |   10 ++-
 xen/arch/x86/boot_info.c          |   58 +++++++++++++++
 xen/arch/x86/setup.c              |   41 +++++++----
 xen/arch/x86/x86_64/asm-offsets.c |    5 +-
 xen/include/asm-x86/mbd.h         |   70 ++++++++++++++++++
 9 files changed, 284 insertions(+), 88 deletions(-)
 create mode 100644 xen/arch/x86/boot_info.c
 create mode 100644 xen/include/asm-x86/mbd.h

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index c1e244d..c465bb3 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -42,6 +42,7 @@ obj-y += numa.o
 obj-y += pci.o
 obj-y += percpu.o
 obj-y += physdev.o
+obj-y += boot_info.o
 obj-y += setup.o
 obj-y += shutdown.o
 obj-y += smp.o
diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S
index 00687eb..7316011 100644
--- a/xen/arch/x86/boot/cmdline.S
+++ b/xen/arch/x86/boot/cmdline.S
@@ -152,17 +152,14 @@ cmdline_parse_early:
         pusha
 
         /* Bail if there is no command line to parse. */
-        mov     sym_phys(multiboot_ptr),%ebx
-        mov     MB_flags(%ebx),%eax
-        test    $4,%al
-        jz      .Lcmdline_exit
-        mov     MB_cmdline(%ebx),%eax
+        mov     sym_phys(mbd_ptr),%ebx
+        mov     MBD_cmdline(%ebx),%eax
         test    %eax,%eax
         jz      .Lcmdline_exit
 
         /* Check for 'no-real-mode' command-line option. */
         pushl   $sym_phys(.Lno_rm_opt)
-        pushl   MB_cmdline(%ebx)
+        pushl   MBD_cmdline(%ebx)
         call    .Lfind_option
         test    %eax,%eax
         setnz   %al
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 10ecf51..79bce3c 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -86,14 +86,14 @@ __start:
         jne     not_multiboot
 
         /* Set up trampoline segment 64k below EBDA */
-        movzwl  0x40e,%eax          /* EBDA segment */
-        cmp     $0xa000,%eax        /* sanity check (high) */
+        movzwl  0x40e,%ecx          /* EBDA segment */
+        cmp     $0xa000,%ecx        /* sanity check (high) */
         jae     0f
-        cmp     $0x4000,%eax        /* sanity check (low) */
+        cmp     $0x4000,%ecx        /* sanity check (low) */
         jae     1f
 0:
-        movzwl  0x413,%eax          /* use base memory size on failure */
-        shl     $10-4,%eax
+        movzwl  0x413,%ecx          /* use base memory size on failure */
+        shl     $10-4,%ecx
 1:
         /*
          * Compare the value in the BDA with the information from the
@@ -105,22 +105,23 @@ __start:
         cmp     $0x100,%edx         /* is the multiboot value too small? */
         jb      2f                  /* if so, do not use it */
         shl     $10-4,%edx
-        cmp     %eax,%edx           /* compare with BDA value */
-        cmovb   %edx,%eax           /* and use the smaller */
+        cmp     %ecx,%edx           /* compare with BDA value */
+        cmovb   %edx,%ecx           /* and use the smaller */
 
 2:      /* Reserve 64kb for the trampoline */
-        sub     $0x1000,%eax
+        sub     $0x1000,%ecx
 
         /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */
-        xor     %al, %al
-        shl     $4, %eax
-        mov     %eax,sym_phys(trampoline_phys)
+        xor     %cl, %cl
+        shl     $4, %ecx
+        mov     %ecx,sym_phys(trampoline_phys)
 
-        /* Save the Multiboot info struct (after relocation) for later use. */
+        /* Save the Multiboot data (after relocation) for later use. */
         mov     $sym_phys(cpu0_stack)+1024,%esp
-        push    %ebx
-        call    reloc
-        mov     %eax,sym_phys(multiboot_ptr)
+        push    %eax                /* Multiboot magic */
+        push    %ebx                /* Multiboot information address */
+        call    reloc               /* %ecx contains trampoline address */
+        mov     %eax,sym_phys(mbd_ptr)
 
         /* Initialize BSS (no nasty surprises!) */
         mov     $sym_phys(__bss_start),%edi
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index fa0fb6b..35a991e 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -1,40 +1,58 @@
-/******************************************************************************
+/*
  * reloc.c
- * 
+ *
  * 32-bit flat memory-map routines for relocating Multiboot structures
  * and modules. This is most easily done early with paging disabled.
- * 
+ *
  * Copyright (c) 2009, Citrix Systems, Inc.
- * 
+ * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
+ *
  * Authors:
  *    Keir Fraser <keir@xen.org>
+ *    Daniel Kiper
  */
 
-/* entered with %eax = BOOT_TRAMPOLINE */
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned long u32;
+typedef unsigned long long u64;
+
+#include "../../../include/xen/compiler.h"
+#include "../../../include/xen/multiboot.h"
+
+#include "../../../include/asm/mbd.h"
+
+/*
+ * __HERE__ IS ENTRY POINT!!!
+ *
+ * It is entered from xen/arch/x86/boot/head.S with:
+ *   - %eax = MULTIBOOT_MAGIC,
+ *   - %ebx = MULTIBOOT_INFORMATION_ADDRESS,
+ *   - %ecx = BOOT_TRAMPOLINE.
+ */
 asm (
     "    .text                         \n"
     "    .globl _start                 \n"
     "_start:                           \n"
     "    call 1f                       \n"
     "1:  pop  %ebx                     \n"
-    "    mov  %eax,alloc-1b(%ebx)      \n"
-    "    jmp  reloc                    \n"
+    "    mov  %ecx,alloc-1b(%ebx)      \n"
+    "    jmp  __reloc                  \n"
     );
 
-/* This is our data.  Because the code must be relocatable, no BSS is
- * allowed.  All data is accessed PC-relative with inline assembly.
+/*
+ * This is our data. Because the code must be relocatable, no BSS is
+ * allowed. All data is accessed PC-relative with inline assembly.
  */
 asm (
     "alloc:                            \n"
     "    .long 0                       \n"
     );
 
-typedef unsigned int u32;
-#include "../../../include/xen/multiboot.h"
-
-static void *reloc_mbi_struct(void *old, unsigned int bytes)
+static u32 alloc_struct(u32 bytes)
 {
-    void *new;
+    u32 s;
+
     asm(
     "    call 1f                      \n"
     "1:  pop  %%edx                   \n"
@@ -42,58 +60,95 @@ static void *reloc_mbi_struct(void *old, unsigned int bytes)
     "    sub  %1,%0                   \n"
     "    and  $~15,%0                 \n"
     "    mov  %0,alloc-1b(%%edx)      \n"
-    "    mov  %0,%%edi                \n"
+       : "=&r" (s) : "r" (bytes) : "edx");
+
+    return s;
+}
+
+static void zero_struct(u32 s, u32 bytes)
+{
+    asm volatile(
+    "    rep  stosb                   \n"
+       : "+D" (s), "+c" (bytes) : "a" (0));
+}
+
+static u32 copy_struct(u32 src, u32 bytes)
+{
+    u32 dst, dst_asm;
+
+    dst = alloc_struct(bytes);
+    dst_asm = dst;
+
+    asm volatile(
     "    rep  movsb                   \n"
-       : "=&r" (new), "+c" (bytes), "+S" (old)
-	: : "edx", "edi");
-    return new;
+       : "+S" (src), "+D" (dst_asm), "+c" (bytes));
+
+    return dst;
 }
 
-static char *reloc_mbi_string(char *old)
+static u32 copy_string(u32 src)
 {
     char *p;
-    for ( p = old; *p != '\0'; p++ )
+
+    if ( src == 0 )
+        return 0;
+
+    for ( p = (char *)src; *p != '\0'; p++ )
         continue;
-    return reloc_mbi_struct(old, p - old + 1);
+
+    return copy_struct(src, p - (char *)src + 1);
 }
 
-multiboot_info_t *reloc(multiboot_info_t *mbi_old)
+static mbd_t *mb_mbd(mbd_t *mbd, multiboot_info_t *mbi)
 {
-    multiboot_info_t *mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi));
-    int i;
+    boot_module_t *mbd_mods;
+    module_t *mbi_mods;
+    u32 i;
+
+    if ( mbi->flags & MBI_LOADERNAME )
+        mbd->boot_loader_name = copy_string(mbi->boot_loader_name);
 
     if ( mbi->flags & MBI_CMDLINE )
-        mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
+        mbd->cmdline = copy_string(mbi->cmdline);
+
+    if ( mbi->flags & MBI_MEMLIMITS )
+    {
+        mbd->mem_lower = mbi->mem_lower;
+        mbd->mem_upper = mbi->mem_upper;
+    }
+
+    if ( mbi->flags & MBI_MEMMAP )
+    {
+        mbd->mmap_size = mbi->mmap_length;
+        mbd->mmap = copy_struct(mbi->mmap_addr, mbi->mmap_length);
+    }
 
     if ( mbi->flags & MBI_MODULES )
     {
-        module_t *mods = reloc_mbi_struct(
-            (module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_t));
+        mbd->mods_nr = mbi->mods_count;
+        mbd->mods = alloc_struct(mbi->mods_count * sizeof(boot_module_t));
 
-        mbi->mods_addr = (u32)mods;
+        mbi_mods = (module_t *)mbi->mods_addr;
+        mbd_mods = (boot_module_t *)mbd->mods;
 
         for ( i = 0; i < mbi->mods_count; i++ )
         {
-            if ( mods[i].string )
-                mods[i].string = (u32)reloc_mbi_string((char *)mods[i].string);
+            mbd_mods[i].start = mbi_mods[i].mod_start;
+            mbd_mods[i].end = mbi_mods[i].mod_end;
+            mbd_mods[i].cmdline = copy_string(mbi_mods[i].string);
+            mbd_mods[i].relocated = 0;
         }
     }
 
-    if ( mbi->flags & MBI_MEMMAP )
-        mbi->mmap_addr = (u32)reloc_mbi_struct(
-            (memory_map_t *)mbi->mmap_addr, mbi->mmap_length);
+    return mbd;
+}
 
-    if ( mbi->flags & MBI_LOADERNAME )
-        mbi->boot_loader_name = (u32)reloc_mbi_string(
-            (char *)mbi->boot_loader_name);
-
-    /* Mask features we don't understand or don't relocate. */
-    mbi->flags &= (MBI_MEMLIMITS |
-                   MBI_BOOTDEV |
-                   MBI_CMDLINE |
-                   MBI_MODULES |
-                   MBI_MEMMAP |
-                   MBI_LOADERNAME);
-
-    return mbi;
+static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
+{
+    mbd_t *mbd;
+
+    mbd = (mbd_t *)alloc_struct(sizeof(mbd_t));
+    zero_struct((u32)mbd, sizeof(mbd_t));
+
+    return mb_mbd(mbd, mbi);
 }
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index bfbafd2..34243b1 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -29,8 +29,12 @@
         test    %ebx,%ebx
         jnz     start_secondary
 
-        /* Pass off the Multiboot info structure to C land. */
-        mov     multiboot_ptr(%rip),%edi
+        /* Init boot_info. */
+        mov     mbd_ptr(%rip),%edi
+        call    __init_boot_info
+
+        /* Pass off the boot_info to C land. */
+        movq    %rax,%rdi
         call    __start_xen
         ud2     /* Force a panic (invalid opcode). */
 
@@ -38,7 +42,7 @@
 
         .data
         .align 8
-multiboot_ptr:
+mbd_ptr:
         .long   0
 
         .word   0
diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c
new file mode 100644
index 0000000..70830c2
--- /dev/null
+++ b/xen/arch/x86/boot_info.c
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/types.h>
+#include <xen/init.h>
+#include <xen/multiboot.h>
+
+#include <asm/mbd.h>
+#include <asm/page.h>
+
+static multiboot_info_t mbi = {};
+
+extern void enable_exception_support(void);
+
+unsigned long __init __init_boot_info(u32 mbd_ptr)
+{
+    mbd_t *mbd = __va(mbd_ptr);
+
+    enable_exception_support();
+
+    if ( mbd->boot_loader_name ) {
+        mbi.flags = MBI_LOADERNAME;
+        mbi.boot_loader_name = mbd->boot_loader_name;
+    }
+
+    if ( mbd->cmdline ) {
+        mbi.flags |= MBI_CMDLINE;
+        mbi.cmdline = mbd->cmdline;
+    }
+
+    mbi.flags |= MBI_MEMLIMITS;
+    mbi.mem_lower = mbd->mem_lower;
+    mbi.mem_upper = mbd->mem_upper;
+
+    mbi.flags |= MBI_MEMMAP;
+    mbi.mmap_length = mbd->mmap_size;
+    mbi.mmap_addr = mbd->mmap;
+
+    mbi.flags |= MBI_MODULES;
+    mbi.mods_count = mbd->mods_nr;
+    mbi.mods_addr = mbd->mods;
+
+    return __pa(&mbi);
+}
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6a814cd..9391efa 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -539,6 +539,25 @@ static char * __init cmdline_cook(char *p, char *loader_name)
     return p;
 }
 
+void __init enable_exception_support(void)
+{
+    /* Critical region without IDT or TSS.  Any fault is deadly! */
+
+    set_processor_id(0);
+    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
+    idle_vcpu[0] = current;
+
+    percpu_init_areas();
+
+    init_idt_traps();
+    load_system_tables();
+
+    smp_prepare_boot_cpu();
+    sort_exception_tables();
+
+    /* Full exception support from here on in. */
+}
+
 void __init noreturn __start_xen(unsigned long mbi_p)
 {
     char *memmap_type = NULL;
@@ -556,21 +575,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         .stop_bits = 1
     };
 
-    /* Critical region without IDT or TSS.  Any fault is deadly! */
-
-    set_processor_id(0);
-    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
-    idle_vcpu[0] = current;
-
-    percpu_init_areas();
-
-    init_idt_traps();
-    load_system_tables();
-
-    smp_prepare_boot_cpu();
-    sort_exception_tables();
-
-    /* Full exception support from here on in. */
+    if ( !efi_enabled ) {
+        /* Exception support was enabled before __start_xen() call. */
+    }
+    else
+    {
+        enable_exception_support();
+    }
 
     loader = (mbi->flags & MBI_LOADERNAME)
         ? (char *)__va(mbi->boot_loader_name) : "unknown";
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 3994f4d..2123eb3 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -12,7 +12,7 @@
 #include <compat/xen.h>
 #include <asm/fixmap.h>
 #include <asm/hardirq.h>
-#include <xen/multiboot.h>
+#include <asm/mbd.h>
 
 #define DEFINE(_sym, _val)                                                 \
     asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
@@ -163,6 +163,5 @@ void __dummy__(void)
     OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability);
     BLANK();
 
-    OFFSET(MB_flags, multiboot_info_t, flags);
-    OFFSET(MB_cmdline, multiboot_info_t, cmdline);
+    OFFSET(MBD_cmdline, mbd_t, cmdline);
 }
diff --git a/xen/include/asm-x86/mbd.h b/xen/include/asm-x86/mbd.h
new file mode 100644
index 0000000..96e3044
--- /dev/null
+++ b/xen/include/asm-x86/mbd.h
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __MBD_H__
+#define __MBD_H__
+
+/* Module type. */
+typedef struct {
+    u32 start;
+    u32 end;
+
+    /* Pointer to a module command line. */
+    u32 cmdline;
+
+    /* If relocated != 0 then a given module was relocated. */
+    u32 relocated;
+} boot_module_t;
+
+/*
+ * MultiBoot Data (MBD) type. It is used to define variable which
+ * carry over data from multiboot protocol (any version) through
+ * Xen preloader stage. Later all or parts of this data is used
+ * to initialize boot_info structure.
+ */
+typedef struct {
+    /* Pointer to boot loader name. */
+    u32 boot_loader_name;
+
+    /* Pointer to Xen command line. */
+    u32 cmdline;
+
+    /*
+     * Amount of lower memory (in KiB) accordingly to The Multiboot
+     * Specification version 0.6.96.
+     */
+    u32 mem_lower;
+
+    /*
+     * Amount of upper memory (in KiB) accordingly to The Multiboot
+     * Specification version 0.6.96.
+     */
+    u32 mem_upper;
+
+    /* Size (in bytes) of memory map provided by bootloader. */
+    u32 mmap_size;
+
+    /* Pointer to memory map provided by bootloader. */
+    u32 mmap;
+
+    /* Number of modules. */
+    u32 mods_nr;
+
+    /* Pointer to modules description (boot_module_t *). */
+    u32 mods;
+} mbd_t;
+#endif /* __MBD_H__ */
-- 
1.7.10.4

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Andrew Cooper]

On 24/09/14 18:19, Daniel Kiper wrote:
> Introduce MultiBoot Data (MBD) type. It is used to define variable
> which carry over data from multiboot protocol (any version) through
> Xen preloader stage. Later all or parts of this data is used
> to initialize boot_info structure. boot_info is introduced
> in later patches.
>
> MBD helps to break multiboot (v1) protocol dependency. Using it
> we are able to save space on trampoline (we do not allocate space
> for unused data what happens in current preloader implementation).
> Additionally, we are able to easily add new members to MBD if we
> want support for new features or protocols.
>
> There is not plan to share MBD among architectures. It will be
> nice if boot_info will be shared among architectures. Please
> check later patches for more details.
>
> Code found in xen/arch/x86/boot_info.c moves MBD data to mbi struct
> which is referenced from main Xen code. This is temporary solution
> which helps to split patches into logical parts. Later it will be
> replaced by final version of boot_info support.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Reviewing changes to ASM code is *very* hard, and this patch is still
very complicated.

Can I suggest patches split along the following lines.

* Shuffling of registers in head.S to end up with %eax and %ecx as
indended for reloc.c, and %ebx currently unclobbered, and adjusting
reloc.c for %ecx.  Review for this is a simple case of verifying that
the changes are just a strict shuffling of registers.

* Whitespace and misc non-MBD cleanup to reloc.c

* Altering existing helper functions in preparation for MBD support

* Actually introduce mbd_t and use it.

In addition, I have some specific comments about the changes themselves.

> ---
> v2 - suggestions/fixes:
>    - improve inline assembly
>      (suggested by Andrew Cooper and Jan Beulich),
>    - use __used attribute
>      (suggested by Andrew Cooper),
>    - patch split rearrangement
>      (suggested by Andrew Cooper and Jan Beulich).
> ---
>  xen/arch/x86/Makefile             |    1 +
>  xen/arch/x86/boot/cmdline.S       |    9 +--
>  xen/arch/x86/boot/head.S          |   31 ++++----
>  xen/arch/x86/boot/reloc.c         |  147 +++++++++++++++++++++++++------------
>  xen/arch/x86/boot/x86_64.S        |   10 ++-
>  xen/arch/x86/boot_info.c          |   58 +++++++++++++++
>  xen/arch/x86/setup.c              |   41 +++++++----
>  xen/arch/x86/x86_64/asm-offsets.c |    5 +-
>  xen/include/asm-x86/mbd.h         |   70 ++++++++++++++++++
>  9 files changed, 284 insertions(+), 88 deletions(-)
>  create mode 100644 xen/arch/x86/boot_info.c
>  create mode 100644 xen/include/asm-x86/mbd.h
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index c1e244d..c465bb3 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -42,6 +42,7 @@ obj-y += numa.o
>  obj-y += pci.o
>  obj-y += percpu.o
>  obj-y += physdev.o
> +obj-y += boot_info.o
>  obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smp.o
> diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S
> index 00687eb..7316011 100644
> --- a/xen/arch/x86/boot/cmdline.S
> +++ b/xen/arch/x86/boot/cmdline.S
> @@ -152,17 +152,14 @@ cmdline_parse_early:
>          pusha
>  
>          /* Bail if there is no command line to parse. */
> -        mov     sym_phys(multiboot_ptr),%ebx
> -        mov     MB_flags(%ebx),%eax
> -        test    $4,%al
> -        jz      .Lcmdline_exit
> -        mov     MB_cmdline(%ebx),%eax
> +        mov     sym_phys(mbd_ptr),%ebx

As you are already changing the name of this symbol, please call it
mbd_pa or something to indicate that it genuinely is a physical address,
not a pointer in the C sense of the term.

> +        mov     MBD_cmdline(%ebx),%eax
>          test    %eax,%eax
>          jz      .Lcmdline_exit
>  
>          /* Check for 'no-real-mode' command-line option. */
>          pushl   $sym_phys(.Lno_rm_opt)
> -        pushl   MB_cmdline(%ebx)
> +        pushl   MBD_cmdline(%ebx)
>          call    .Lfind_option
>          test    %eax,%eax
>          setnz   %al
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 10ecf51..79bce3c 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -86,14 +86,14 @@ __start:
>          jne     not_multiboot
>  
>          /* Set up trampoline segment 64k below EBDA */
> -        movzwl  0x40e,%eax          /* EBDA segment */
> -        cmp     $0xa000,%eax        /* sanity check (high) */
> +        movzwl  0x40e,%ecx          /* EBDA segment */
> +        cmp     $0xa000,%ecx        /* sanity check (high) */
>          jae     0f
> -        cmp     $0x4000,%eax        /* sanity check (low) */
> +        cmp     $0x4000,%ecx        /* sanity check (low) */
>          jae     1f
>  0:
> -        movzwl  0x413,%eax          /* use base memory size on failure */
> -        shl     $10-4,%eax
> +        movzwl  0x413,%ecx          /* use base memory size on failure */
> +        shl     $10-4,%ecx
>  1:
>          /*
>           * Compare the value in the BDA with the information from the
> @@ -105,22 +105,23 @@ __start:
>          cmp     $0x100,%edx         /* is the multiboot value too small? */
>          jb      2f                  /* if so, do not use it */
>          shl     $10-4,%edx
> -        cmp     %eax,%edx           /* compare with BDA value */
> -        cmovb   %edx,%eax           /* and use the smaller */
> +        cmp     %ecx,%edx           /* compare with BDA value */
> +        cmovb   %edx,%ecx           /* and use the smaller */
>  
>  2:      /* Reserve 64kb for the trampoline */
> -        sub     $0x1000,%eax
> +        sub     $0x1000,%ecx
>  
>          /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */
> -        xor     %al, %al
> -        shl     $4, %eax
> -        mov     %eax,sym_phys(trampoline_phys)
> +        xor     %cl, %cl
> +        shl     $4, %ecx
> +        mov     %ecx,sym_phys(trampoline_phys)
>  
> -        /* Save the Multiboot info struct (after relocation) for later use. */
> +        /* Save the Multiboot data (after relocation) for later use. */
>          mov     $sym_phys(cpu0_stack)+1024,%esp
> -        push    %ebx
> -        call    reloc
> -        mov     %eax,sym_phys(multiboot_ptr)
> +        push    %eax                /* Multiboot magic */
> +        push    %ebx                /* Multiboot information address */
> +        call    reloc               /* %ecx contains trampoline address */
> +        mov     %eax,sym_phys(mbd_ptr)
>  
>          /* Initialize BSS (no nasty surprises!) */
>          mov     $sym_phys(__bss_start),%edi
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index fa0fb6b..35a991e 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -1,40 +1,58 @@
> -/******************************************************************************
> +/*
>   * reloc.c
> - * 
> + *
>   * 32-bit flat memory-map routines for relocating Multiboot structures
>   * and modules. This is most easily done early with paging disabled.
> - * 
> + *
>   * Copyright (c) 2009, Citrix Systems, Inc.
> - * 
> + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
> + *
>   * Authors:
>   *    Keir Fraser <keir@xen.org>
> + *    Daniel Kiper
>   */
>  
> -/* entered with %eax = BOOT_TRAMPOLINE */
> +typedef unsigned char u8;
> +typedef unsigned short u16;
> +typedef unsigned long u32;
> +typedef unsigned long long u64;
> +
> +#include "../../../include/xen/compiler.h"
> +#include "../../../include/xen/multiboot.h"
> +
> +#include "../../../include/asm/mbd.h"

How about playing with -I for this file in the Makefile to allow
#include <xen/compiler.h> and so ?

> +
> +/*
> + * __HERE__ IS ENTRY POINT!!!

I am still of the firm opinion that anyone capable of editing this file
ought to know understand the _start symbol, making this comment useless.

> + *
> + * It is entered from xen/arch/x86/boot/head.S with:
> + *   - %eax = MULTIBOOT_MAGIC,
> + *   - %ebx = MULTIBOOT_INFORMATION_ADDRESS,
> + *   - %ecx = BOOT_TRAMPOLINE.
> + */
>  asm (
>      "    .text                         \n"
>      "    .globl _start                 \n"
>      "_start:                           \n"
>      "    call 1f                       \n"
>      "1:  pop  %ebx                     \n"
> -    "    mov  %eax,alloc-1b(%ebx)      \n"
> -    "    jmp  reloc                    \n"
> +    "    mov  %ecx,alloc-1b(%ebx)      \n"
> +    "    jmp  __reloc                  \n"
>      );
>  
> -/* This is our data.  Because the code must be relocatable, no BSS is
> - * allowed.  All data is accessed PC-relative with inline assembly.
> +/*
> + * This is our data. Because the code must be relocatable, no BSS is
> + * allowed. All data is accessed PC-relative with inline assembly.
>   */
>  asm (
>      "alloc:                            \n"
>      "    .long 0                       \n"
>      );
>  
> -typedef unsigned int u32;
> -#include "../../../include/xen/multiboot.h"
> -
> -static void *reloc_mbi_struct(void *old, unsigned int bytes)
> +static u32 alloc_struct(u32 bytes)
>  {
> -    void *new;
> +    u32 s;
> +
>      asm(
>      "    call 1f                      \n"
>      "1:  pop  %%edx                   \n"
> @@ -42,58 +60,95 @@ static void *reloc_mbi_struct(void *old, unsigned int bytes)
>      "    sub  %1,%0                   \n"
>      "    and  $~15,%0                 \n"
>      "    mov  %0,alloc-1b(%%edx)      \n"
> -    "    mov  %0,%%edi                \n"
> +       : "=&r" (s) : "r" (bytes) : "edx");
> +
> +    return s;
> +}
> +
> +static void zero_struct(u32 s, u32 bytes)
> +{
> +    asm volatile(
> +    "    rep  stosb                   \n"
> +       : "+D" (s), "+c" (bytes) : "a" (0));
> +}
> +
> +static u32 copy_struct(u32 src, u32 bytes)
> +{
> +    u32 dst, dst_asm;
> +
> +    dst = alloc_struct(bytes);
> +    dst_asm = dst;
> +
> +    asm volatile(
>      "    rep  movsb                   \n"
> -       : "=&r" (new), "+c" (bytes), "+S" (old)
> -	: : "edx", "edi");
> -    return new;
> +       : "+S" (src), "+D" (dst_asm), "+c" (bytes));
> +
> +    return dst;
>  }
>  
> -static char *reloc_mbi_string(char *old)
> +static u32 copy_string(u32 src)
>  {
>      char *p;
> -    for ( p = old; *p != '\0'; p++ )
> +
> +    if ( src == 0 )
> +        return 0;
> +
> +    for ( p = (char *)src; *p != '\0'; p++ )
>          continue;
> -    return reloc_mbi_struct(old, p - old + 1);
> +
> +    return copy_struct(src, p - (char *)src + 1);
>  }
>  
> -multiboot_info_t *reloc(multiboot_info_t *mbi_old)
> +static mbd_t *mb_mbd(mbd_t *mbd, multiboot_info_t *mbi)
>  {
> -    multiboot_info_t *mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi));
> -    int i;
> +    boot_module_t *mbd_mods;
> +    module_t *mbi_mods;
> +    u32 i;
> +
> +    if ( mbi->flags & MBI_LOADERNAME )
> +        mbd->boot_loader_name = copy_string(mbi->boot_loader_name);
>  
>      if ( mbi->flags & MBI_CMDLINE )
> -        mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
> +        mbd->cmdline = copy_string(mbi->cmdline);
> +
> +    if ( mbi->flags & MBI_MEMLIMITS )
> +    {
> +        mbd->mem_lower = mbi->mem_lower;
> +        mbd->mem_upper = mbi->mem_upper;
> +    }
> +
> +    if ( mbi->flags & MBI_MEMMAP )
> +    {
> +        mbd->mmap_size = mbi->mmap_length;
> +        mbd->mmap = copy_struct(mbi->mmap_addr, mbi->mmap_length);
> +    }
>  
>      if ( mbi->flags & MBI_MODULES )
>      {
> -        module_t *mods = reloc_mbi_struct(
> -            (module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_t));
> +        mbd->mods_nr = mbi->mods_count;
> +        mbd->mods = alloc_struct(mbi->mods_count * sizeof(boot_module_t));
>  
> -        mbi->mods_addr = (u32)mods;
> +        mbi_mods = (module_t *)mbi->mods_addr;
> +        mbd_mods = (boot_module_t *)mbd->mods;
>  
>          for ( i = 0; i < mbi->mods_count; i++ )
>          {
> -            if ( mods[i].string )
> -                mods[i].string = (u32)reloc_mbi_string((char *)mods[i].string);
> +            mbd_mods[i].start = mbi_mods[i].mod_start;
> +            mbd_mods[i].end = mbi_mods[i].mod_end;
> +            mbd_mods[i].cmdline = copy_string(mbi_mods[i].string);
> +            mbd_mods[i].relocated = 0;
>          }
>      }
>  
> -    if ( mbi->flags & MBI_MEMMAP )
> -        mbi->mmap_addr = (u32)reloc_mbi_struct(
> -            (memory_map_t *)mbi->mmap_addr, mbi->mmap_length);
> +    return mbd;
> +}
>  
> -    if ( mbi->flags & MBI_LOADERNAME )
> -        mbi->boot_loader_name = (u32)reloc_mbi_string(
> -            (char *)mbi->boot_loader_name);
> -
> -    /* Mask features we don't understand or don't relocate. */
> -    mbi->flags &= (MBI_MEMLIMITS |
> -                   MBI_BOOTDEV |
> -                   MBI_CMDLINE |
> -                   MBI_MODULES |
> -                   MBI_MEMMAP |
> -                   MBI_LOADERNAME);
> -
> -    return mbi;
> +static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
> +{
> +    mbd_t *mbd;
> +
> +    mbd = (mbd_t *)alloc_struct(sizeof(mbd_t));
> +    zero_struct((u32)mbd, sizeof(mbd_t));
> +
> +    return mb_mbd(mbd, mbi);
>  }
> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
> index bfbafd2..34243b1 100644
> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -29,8 +29,12 @@
>          test    %ebx,%ebx
>          jnz     start_secondary
>  
> -        /* Pass off the Multiboot info structure to C land. */
> -        mov     multiboot_ptr(%rip),%edi
> +        /* Init boot_info. */
> +        mov     mbd_ptr(%rip),%edi
> +        call    __init_boot_info
> +
> +        /* Pass off the boot_info to C land. */
> +        movq    %rax,%rdi
>          call    __start_xen
>          ud2     /* Force a panic (invalid opcode). */
>  
> @@ -38,7 +42,7 @@
>  
>          .data
>          .align 8
> -multiboot_ptr:
> +mbd_ptr:
>          .long   0
>  
>          .word   0
> diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c
> new file mode 100644
> index 0000000..70830c2
> --- /dev/null
> +++ b/xen/arch/x86/boot_info.c
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/types.h>
> +#include <xen/init.h>
> +#include <xen/multiboot.h>
> +
> +#include <asm/mbd.h>
> +#include <asm/page.h>
> +
> +static multiboot_info_t mbi = {};

"= {}" is redundant for a static declaration.

> +
> +extern void enable_exception_support(void);
> +
> +unsigned long __init __init_boot_info(u32 mbd_ptr)
> +{
> +    mbd_t *mbd = __va(mbd_ptr);
> +
> +    enable_exception_support();
> +
> +    if ( mbd->boot_loader_name ) {
> +        mbi.flags = MBI_LOADERNAME;
> +        mbi.boot_loader_name = mbd->boot_loader_name;
> +    }
> +
> +    if ( mbd->cmdline ) {
> +        mbi.flags |= MBI_CMDLINE;
> +        mbi.cmdline = mbd->cmdline;
> +    }
> +
> +    mbi.flags |= MBI_MEMLIMITS;
> +    mbi.mem_lower = mbd->mem_lower;
> +    mbi.mem_upper = mbd->mem_upper;
> +
> +    mbi.flags |= MBI_MEMMAP;
> +    mbi.mmap_length = mbd->mmap_size;
> +    mbi.mmap_addr = mbd->mmap;
> +
> +    mbi.flags |= MBI_MODULES;
> +    mbi.mods_count = mbd->mods_nr;
> +    mbi.mods_addr = mbd->mods;
> +
> +    return __pa(&mbi);
> +}
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 6a814cd..9391efa 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -539,6 +539,25 @@ static char * __init cmdline_cook(char *p, char *loader_name)
>      return p;
>  }
>  
> +void __init enable_exception_support(void)
> +{
> +    /* Critical region without IDT or TSS.  Any fault is deadly! */
> +
> +    set_processor_id(0);
> +    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
> +    idle_vcpu[0] = current;
> +
> +    percpu_init_areas();
> +
> +    init_idt_traps();
> +    load_system_tables();
> +
> +    smp_prepare_boot_cpu();
> +    sort_exception_tables();
> +
> +    /* Full exception support from here on in. */
> +}
> +
>  void __init noreturn __start_xen(unsigned long mbi_p)
>  {
>      char *memmap_type = NULL;
> @@ -556,21 +575,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>          .stop_bits = 1
>      };
>  
> -    /* Critical region without IDT or TSS.  Any fault is deadly! */
> -
> -    set_processor_id(0);
> -    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
> -    idle_vcpu[0] = current;
> -
> -    percpu_init_areas();
> -
> -    init_idt_traps();
> -    load_system_tables();
> -
> -    smp_prepare_boot_cpu();
> -    sort_exception_tables();
> -
> -    /* Full exception support from here on in. */
> +    if ( !efi_enabled ) {
> +        /* Exception support was enabled before __start_xen() call. */
> +    }
> +    else
> +    {
> +        enable_exception_support();
> +    }

For this, absolutely not.

Pass mbi_pa into __start_xen() and have __start_xen() call
__init_boot_info() after it has set up exception support.  None of the
contents of MBD are needed beforehand.

>  
>      loader = (mbi->flags & MBI_LOADERNAME)
>          ? (char *)__va(mbi->boot_loader_name) : "unknown";
> diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
> index 3994f4d..2123eb3 100644
> --- a/xen/arch/x86/x86_64/asm-offsets.c
> +++ b/xen/arch/x86/x86_64/asm-offsets.c
> @@ -12,7 +12,7 @@
>  #include <compat/xen.h>
>  #include <asm/fixmap.h>
>  #include <asm/hardirq.h>
> -#include <xen/multiboot.h>
> +#include <asm/mbd.h>
>  
>  #define DEFINE(_sym, _val)                                                 \
>      asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
> @@ -163,6 +163,5 @@ void __dummy__(void)
>      OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability);
>      BLANK();
>  
> -    OFFSET(MB_flags, multiboot_info_t, flags);
> -    OFFSET(MB_cmdline, multiboot_info_t, cmdline);
> +    OFFSET(MBD_cmdline, mbd_t, cmdline);
>  }
> diff --git a/xen/include/asm-x86/mbd.h b/xen/include/asm-x86/mbd.h
> new file mode 100644
> index 0000000..96e3044
> --- /dev/null
> +++ b/xen/include/asm-x86/mbd.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __MBD_H__
> +#define __MBD_H__

#include <xen/types.h>

Do not rely on the translation using including this file to make u32
available for you.

> +
> +/* Module type. */
> +typedef struct {
> +    u32 start;
> +    u32 end;
> +
> +    /* Pointer to a module command line. */
> +    u32 cmdline;
> +
> +    /* If relocated != 0 then a given module was relocated. */
> +    u32 relocated;
> +} boot_module_t;
> +
> +/*
> + * MultiBoot Data (MBD) type. It is used to define variable which
> + * carry over data from multiboot protocol (any version) through
> + * Xen preloader stage. Later all or parts of this data is used
> + * to initialize boot_info structure.
> + */
> +typedef struct {
> +    /* Pointer to boot loader name. */
> +    u32 boot_loader_name;
> +
> +    /* Pointer to Xen command line. */
> +    u32 cmdline;

These need to be very clear about being physical addresses rather than
pointers.

~Andrew

> +
> +    /*
> +     * Amount of lower memory (in KiB) accordingly to The Multiboot
> +     * Specification version 0.6.96.
> +     */
> +    u32 mem_lower;
> +
> +    /*
> +     * Amount of upper memory (in KiB) accordingly to The Multiboot
> +     * Specification version 0.6.96.
> +     */
> +    u32 mem_upper;
> +
> +    /* Size (in bytes) of memory map provided by bootloader. */
> +    u32 mmap_size;
> +
> +    /* Pointer to memory map provided by bootloader. */
> +    u32 mmap;
> +
> +    /* Number of modules. */
> +    u32 mods_nr;
> +
> +    /* Pointer to modules description (boot_module_t *). */
> +    u32 mods;
> +} mbd_t;
> +#endif /* __MBD_H__ */

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Jan Beulich]

>>> On 24.09.14 at 20:40, <andrew.cooper3@citrix.com> wrote:
> On 24/09/14 18:19, Daniel Kiper wrote:
>> Introduce MultiBoot Data (MBD) type. It is used to define variable
>> which carry over data from multiboot protocol (any version) through
>> Xen preloader stage. Later all or parts of this data is used
>> to initialize boot_info structure. boot_info is introduced
>> in later patches.
>>
>> MBD helps to break multiboot (v1) protocol dependency. Using it
>> we are able to save space on trampoline (we do not allocate space
>> for unused data what happens in current preloader implementation).
>> Additionally, we are able to easily add new members to MBD if we
>> want support for new features or protocols.
>>
>> There is not plan to share MBD among architectures. It will be
>> nice if boot_info will be shared among architectures. Please
>> check later patches for more details.
>>
>> Code found in xen/arch/x86/boot_info.c moves MBD data to mbi struct
>> which is referenced from main Xen code. This is temporary solution
>> which helps to split patches into logical parts. Later it will be
>> replaced by final version of boot_info support.
>>
>> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Reviewing changes to ASM code is *very* hard, and this patch is still
> very complicated.
> 
> Can I suggest patches split along the following lines.
> 
> * Shuffling of registers in head.S to end up with %eax and %ecx as
> indended for reloc.c, and %ebx currently unclobbered, and adjusting
> reloc.c for %ecx.  Review for this is a simple case of verifying that
> the changes are just a strict shuffling of registers.
> 
> * Whitespace and misc non-MBD cleanup to reloc.c
> 
> * Altering existing helper functions in preparation for MBD support
> 
> * Actually introduce mbd_t and use it.

+1

>> --- a/xen/arch/x86/boot/reloc.c
>> +++ b/xen/arch/x86/boot/reloc.c
>> @@ -1,40 +1,58 @@
>> -/******************************************************************************
>> +/*
>>   * reloc.c
>> - * 
>> + *
>>   * 32-bit flat memory-map routines for relocating Multiboot structures
>>   * and modules. This is most easily done early with paging disabled.
>> - * 
>> + *
>>   * Copyright (c) 2009, Citrix Systems, Inc.
>> - * 
>> + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
>> + *
>>   * Authors:
>>   *    Keir Fraser <keir@xen.org>
>> + *    Daniel Kiper
>>   */
>>  
>> -/* entered with %eax = BOOT_TRAMPOLINE */
>> +typedef unsigned char u8;
>> +typedef unsigned short u16;
>> +typedef unsigned long u32;
>> +typedef unsigned long long u64;
>> +
>> +#include "../../../include/xen/compiler.h"
>> +#include "../../../include/xen/multiboot.h"
>> +
>> +#include "../../../include/asm/mbd.h"
> 
> How about playing with -I for this file in the Makefile to allow
> #include <xen/compiler.h> and so ?

Including these here is bogus anyway, even if for the ones above it's
perhaps acceptable. But expressing it to be bogus via the ../../../
prefix is quite desirable imo.

>> +
>> +/*
>> + * __HERE__ IS ENTRY POINT!!!
> 
> I am still of the firm opinion that anyone capable of editing this file
> ought to know understand the _start symbol, making this comment useless.

Indeed.

>> +/*
>> + * MultiBoot Data (MBD) type. It is used to define variable which
>> + * carry over data from multiboot protocol (any version) through
>> + * Xen preloader stage. Later all or parts of this data is used
>> + * to initialize boot_info structure.
>> + */
>> +typedef struct {
>> +    /* Pointer to boot loader name. */
>> +    u32 boot_loader_name;
>> +
>> +    /* Pointer to Xen command line. */
>> +    u32 cmdline;
> 
> These need to be very clear about being physical addresses rather than
> pointers.

Isn't their type being u32 and their placement in this structure
sufficient indication thereof?

Jan

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Daniel Kiper]

On Thu, Sep 25, 2014 at 10:22:16AM +0100, Jan Beulich wrote:
> >>> On 24.09.14 at 20:40, <andrew.cooper3@citrix.com> wrote:
> > On 24/09/14 18:19, Daniel Kiper wrote:

[...]

> >> +typedef unsigned char u8;
> >> +typedef unsigned short u16;
> >> +typedef unsigned long u32;
> >> +typedef unsigned long long u64;
> >> +
> >> +#include "../../../include/xen/compiler.h"
> >> +#include "../../../include/xen/multiboot.h"
> >> +
> >> +#include "../../../include/asm/mbd.h"
> >
> > How about playing with -I for this file in the Makefile to allow
> > #include <xen/compiler.h> and so ?
>
> Including these here is bogus anyway, even if for the ones above it's

Hmmm... Why it is bogus?

> perhaps acceptable. But expressing it to be bogus via the ../../../
> prefix is quite desirable imo.

I have been thinking about that since I saw this first time. Why we could
not use -I gcc option here? Could you enlighten me?

> >> +
> >> +/*
> >> + * __HERE__ IS ENTRY POINT!!!
> >
> > I am still of the firm opinion that anyone capable of editing this file
> > ought to know understand the _start symbol, making this comment useless.
>
> Indeed.

We know this right now. However, I spent some time to discover this at
the beginning of this work. This file is full of magic so I think that
this comment helps a bit to understand what is going on. So, please
do me a favor and let's leave it here. If you wish I can use lowercase
and remove underscore. Additionally, I removed similar comment for
__reloc() (as you requested) which does not make sense if it is
prefixed with static.

Daniel

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Jan Beulich]

>>> On 25.09.14 at 14:56, <daniel.kiper@oracle.com> wrote:
> On Thu, Sep 25, 2014 at 10:22:16AM +0100, Jan Beulich wrote:
>> >>> On 24.09.14 at 20:40, <andrew.cooper3@citrix.com> wrote:
>> > On 24/09/14 18:19, Daniel Kiper wrote:
>> >> +typedef unsigned char u8;
>> >> +typedef unsigned short u16;
>> >> +typedef unsigned long u32;
>> >> +typedef unsigned long long u64;
>> >> +
>> >> +#include "../../../include/xen/compiler.h"
>> >> +#include "../../../include/xen/multiboot.h"
>> >> +
>> >> +#include "../../../include/asm/mbd.h"
>> >
>> > How about playing with -I for this file in the Makefile to allow
>> > #include <xen/compiler.h> and so ?
>>
>> Including these here is bogus anyway, even if for the ones above it's
> 
> Hmmm... Why it is bogus?

Because the headers aren't intended for the environment this code
runs in.

>> perhaps acceptable. But expressing it to be bogus via the ../../../
>> prefix is quite desirable imo.
> 
> I have been thinking about that since I saw this first time. Why we could
> not use -I gcc option here? Could you enlighten me?

For aforementioned reason: Once we make it easy and seamless to
include further headers, the risk of depending on something this code
shouldn't depend on grows.

Apart from that you also need to deal with dependency tracking if
you include new headers here - please see the Makefile for how
this needs to be done (and which really should have got dropped
by 46fce9fd2b3 ["x86: get rid of BOOT_TRAMPOLINE"]). And yes,
the existing inclusion of multiboot.h is lacking dependency tracking
too (but as usual this is no excuse to continue with this bad practice).

Which points at another issue making it (right now at least) desirable
to don't make it a nobrainer to include further ones: Non-leaf ones
would likely cause missed dependencies as long as they need to be
tracked manually.

>> >> +
>> >> +/*
>> >> + * __HERE__ IS ENTRY POINT!!!
>> >
>> > I am still of the firm opinion that anyone capable of editing this file
>> > ought to know understand the _start symbol, making this comment useless.
>>
>> Indeed.
> 
> We know this right now. However, I spent some time to discover this at
> the beginning of this work. This file is full of magic so I think that
> this comment helps a bit to understand what is going on. So, please
> do me a favor and let's leave it here. If you wish I can use lowercase
> and remove underscore.

And remove the three exclamation marks and fix the grammar.
All in all - much preferred - just drop the comment.

> Additionally, I removed similar comment for
> __reloc() (as you requested) which does not make sense if it is
> prefixed with static.

Making a function referenced from inline assembly only static is likely
wrong anyway. No idea though why removing a similar comment
there would be an issue.

Jan

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Daniel Kiper]

On Wed, Sep 24, 2014 at 07:40:44PM +0100, Andrew Cooper wrote:
> On 24/09/14 18:19, Daniel Kiper wrote:

[...]

> > @@ -556,21 +575,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> >          .stop_bits = 1
> >      };
> >
> > -    /* Critical region without IDT or TSS.  Any fault is deadly! */
> > -
> > -    set_processor_id(0);
> > -    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
> > -    idle_vcpu[0] = current;
> > -
> > -    percpu_init_areas();
> > -
> > -    init_idt_traps();
> > -    load_system_tables();
> > -
> > -    smp_prepare_boot_cpu();
> > -    sort_exception_tables();
> > -
> > -    /* Full exception support from here on in. */
> > +    if ( !efi_enabled ) {
> > +        /* Exception support was enabled before __start_xen() call. */
> > +    }
> > +    else
> > +    {
> > +        enable_exception_support();
> > +    }
>
> For this, absolutely not.
>
> Pass mbi_pa into __start_xen() and have __start_xen() call
> __init_boot_info() after it has set up exception support.  None of the
> contents of MBD are needed beforehand.

Ehhh... I do not like this solution too. However, I want to avoid
passing mbd to __start_xen(). If we do that as you suggest then
EFI case will look bad because we would not be able to pass boot_info
directly to __start_xen(). boot_info is build directly in EFI case.
We do not need mbd. It means that __init_boot_info() is not used on
EFI platforms too. Additionally, I think that __start_xen() is firm border
between preloader and Xen main code and this as is should not see
any variables belonging to preloader (mbd is preloader stuff). Hence,
I think that __start_xen() should take boot_info as an argument.
However, I agree that we should work out better solution for
exception initialization.

> >      loader = (mbi->flags & MBI_LOADERNAME)
> >          ? (char *)__va(mbi->boot_loader_name) : "unknown";
> > diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
> > index 3994f4d..2123eb3 100644
> > --- a/xen/arch/x86/x86_64/asm-offsets.c
> > +++ b/xen/arch/x86/x86_64/asm-offsets.c
> > @@ -12,7 +12,7 @@
> >  #include <compat/xen.h>
> >  #include <asm/fixmap.h>
> >  #include <asm/hardirq.h>
> > -#include <xen/multiboot.h>
> > +#include <asm/mbd.h>
> >
> >  #define DEFINE(_sym, _val)                                                 \
> >      asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
> > @@ -163,6 +163,5 @@ void __dummy__(void)
> >      OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability);
> >      BLANK();
> >
> > -    OFFSET(MB_flags, multiboot_info_t, flags);
> > -    OFFSET(MB_cmdline, multiboot_info_t, cmdline);
> > +    OFFSET(MBD_cmdline, mbd_t, cmdline);
> >  }
> > diff --git a/xen/include/asm-x86/mbd.h b/xen/include/asm-x86/mbd.h
> > new file mode 100644
> > index 0000000..96e3044
> > --- /dev/null
> > +++ b/xen/include/asm-x86/mbd.h
> > @@ -0,0 +1,70 @@
> > +/*
> > + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef __MBD_H__
> > +#define __MBD_H__
>
> #include <xen/types.h>
>
> Do not rely on the translation using including this file to make u32
> available for you.

I did not include this header deliberately. Sadly mbd.h is used in
reloc.c and if we do that then build will be broken. However, if you
wish I could add ifndef __FOOBAR__ and then define __FOOBAR__ in reloc.c.
Or maybe you have better idea?

Daniel

Re: [PATCH v2 1/5] xen/x86: Introduce MultiBoot Data (MBD) type

[Jan Beulich]

>>> On 25.09.14 at 21:24, <daniel.kiper@oracle.com> wrote:
> On Wed, Sep 24, 2014 at 07:40:44PM +0100, Andrew Cooper wrote:
>> On 24/09/14 18:19, Daniel Kiper wrote:
>> > @@ -556,21 +575,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>> >          .stop_bits = 1
>> >      };
>> >
>> > -    /* Critical region without IDT or TSS.  Any fault is deadly! */
>> > -
>> > -    set_processor_id(0);
>> > -    set_current((struct vcpu *)0xfffff000); /* debug sanity. */
>> > -    idle_vcpu[0] = current;
>> > -
>> > -    percpu_init_areas();
>> > -
>> > -    init_idt_traps();
>> > -    load_system_tables();
>> > -
>> > -    smp_prepare_boot_cpu();
>> > -    sort_exception_tables();
>> > -
>> > -    /* Full exception support from here on in. */
>> > +    if ( !efi_enabled ) {
>> > +        /* Exception support was enabled before __start_xen() call. */
>> > +    }
>> > +    else
>> > +    {
>> > +        enable_exception_support();
>> > +    }
>>
>> For this, absolutely not.
>>
>> Pass mbi_pa into __start_xen() and have __start_xen() call
>> __init_boot_info() after it has set up exception support.  None of the
>> contents of MBD are needed beforehand.
> 
> Ehhh... I do not like this solution too. However, I want to avoid
> passing mbd to __start_xen(). If we do that as you suggest then
> EFI case will look bad because we would not be able to pass boot_info
> directly to __start_xen(). boot_info is build directly in EFI case.
> We do not need mbd. It means that __init_boot_info() is not used on
> EFI platforms too. Additionally, I think that __start_xen() is firm border
> between preloader and Xen main code and this as is should not see
> any variables belonging to preloader (mbd is preloader stuff). Hence,
> I think that __start_xen() should take boot_info as an argument.
> However, I agree that we should work out better solution for
> exception initialization.

There's no hard boundary between what the pre-__start_xen()
code in the EFI case (or the equivalent code in the non-EFI one)
can access, and I don't think there needs to be.

>> > --- /dev/null
>> > +++ b/xen/include/asm-x86/mbd.h
>> > @@ -0,0 +1,70 @@
>> > +/*
>> > + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation; either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License along
>> > + * with this program.  If not, see <http://www.gnu.org/licenses/>.
>> > + */
>> > +
>> > +#ifndef __MBD_H__
>> > +#define __MBD_H__
>>
>> #include <xen/types.h>
>>
>> Do not rely on the translation using including this file to make u32
>> available for you.
> 
> I did not include this header deliberately. Sadly mbd.h is used in
> reloc.c and if we do that then build will be broken. However, if you
> wish I could add ifndef __FOOBAR__ and then define __FOOBAR__ in reloc.c.
> Or maybe you have better idea?

Add a comment explaining why the header is deliberately not
being included? Or only use standard C types (perhaps with
suitable BUILD_BUG_ON()s put elsewhere)?

Jan

[PATCH v2 2/5] xen/x86: Define e820 entries counter as unsigned int

[Daniel Kiper]

e820 entries counter is inherently an unsigned quantity
so define it as unsigned int.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
v2 - suggestions/fixes:
   - change e820 entries counter signedness
     (suggested by Andrew Cooper).
---
 xen/arch/x86/e820.c        |    8 ++++----
 xen/include/asm-x86/e820.h |    4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index 55fe0d6..bf84bae 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -89,9 +89,9 @@ static void __init add_memory_region(unsigned long long start,
     }
 } /* add_memory_region */
 
-static void __init print_e820_memory_map(struct e820entry *map, int entries)
+static void __init print_e820_memory_map(struct e820entry *map, unsigned int entries)
 {
-    int i;
+    unsigned int i;
 
     for (i = 0; i < entries; i++) {
         printk(" %016Lx - %016Lx ",
@@ -512,7 +512,7 @@ static void __init reserve_dmi_region(void)
 }
 
 static void __init machine_specific_memory_setup(
-    struct e820entry *raw, int *raw_nr)
+    struct e820entry *raw, unsigned int *raw_nr)
 {
     unsigned long mpt_limit, ro_mpt_limit;
     uint64_t top_of_ram, size;
@@ -695,7 +695,7 @@ int __init reserve_e820_ram(struct e820map *e820, uint64_t s, uint64_t e)
 }
 
 unsigned long __init init_e820(
-    const char *str, struct e820entry *raw, int *raw_nr)
+    const char *str, struct e820entry *raw, unsigned int *raw_nr)
 {
     if ( e820_verbose )
     {
diff --git a/xen/include/asm-x86/e820.h b/xen/include/asm-x86/e820.h
index 71a804c..d9ff4eb 100644
--- a/xen/include/asm-x86/e820.h
+++ b/xen/include/asm-x86/e820.h
@@ -30,12 +30,12 @@ extern int e820_change_range_type(
     uint32_t orig_type, uint32_t new_type);
 extern int e820_add_range(
     struct e820map *, uint64_t s, uint64_t e, uint32_t type);
-extern unsigned long init_e820(const char *, struct e820entry *, int *);
+extern unsigned long init_e820(const char *, struct e820entry *, unsigned int *);
 extern struct e820map e820;
 
 /* These symbols live in the boot trampoline. */
 extern struct e820entry e820map[];
-extern int e820nr;
+extern unsigned int e820nr;
 extern unsigned int lowmem_kb, highmem_kb;
 
 #define e820_raw bootsym(e820map)
-- 
1.7.10.4

Re: [PATCH v2 2/5] xen/x86: Define e820 entries counter as unsigned int

[Andrew Cooper]

On 24/09/14 18:19, Daniel Kiper wrote:
> e820 entries counter is inherently an unsigned quantity
> so define it as unsigned int.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Although as a cleanup patch, this should be ahead of function changes in
your series.

> ---
> v2 - suggestions/fixes:
>    - change e820 entries counter signedness
>      (suggested by Andrew Cooper).
> ---
>  xen/arch/x86/e820.c        |    8 ++++----
>  xen/include/asm-x86/e820.h |    4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> index 55fe0d6..bf84bae 100644
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -89,9 +89,9 @@ static void __init add_memory_region(unsigned long long start,
>      }
>  } /* add_memory_region */
>  
> -static void __init print_e820_memory_map(struct e820entry *map, int entries)
> +static void __init print_e820_memory_map(struct e820entry *map, unsigned int entries)
>  {
> -    int i;
> +    unsigned int i;
>  
>      for (i = 0; i < entries; i++) {
>          printk(" %016Lx - %016Lx ",
> @@ -512,7 +512,7 @@ static void __init reserve_dmi_region(void)
>  }
>  
>  static void __init machine_specific_memory_setup(
> -    struct e820entry *raw, int *raw_nr)
> +    struct e820entry *raw, unsigned int *raw_nr)
>  {
>      unsigned long mpt_limit, ro_mpt_limit;
>      uint64_t top_of_ram, size;
> @@ -695,7 +695,7 @@ int __init reserve_e820_ram(struct e820map *e820, uint64_t s, uint64_t e)
>  }
>  
>  unsigned long __init init_e820(
> -    const char *str, struct e820entry *raw, int *raw_nr)
> +    const char *str, struct e820entry *raw, unsigned int *raw_nr)
>  {
>      if ( e820_verbose )
>      {
> diff --git a/xen/include/asm-x86/e820.h b/xen/include/asm-x86/e820.h
> index 71a804c..d9ff4eb 100644
> --- a/xen/include/asm-x86/e820.h
> +++ b/xen/include/asm-x86/e820.h
> @@ -30,12 +30,12 @@ extern int e820_change_range_type(
>      uint32_t orig_type, uint32_t new_type);
>  extern int e820_add_range(
>      struct e820map *, uint64_t s, uint64_t e, uint32_t type);
> -extern unsigned long init_e820(const char *, struct e820entry *, int *);
> +extern unsigned long init_e820(const char *, struct e820entry *, unsigned int *);
>  extern struct e820map e820;
>  
>  /* These symbols live in the boot trampoline. */
>  extern struct e820entry e820map[];
> -extern int e820nr;
> +extern unsigned int e820nr;
>  extern unsigned int lowmem_kb, highmem_kb;
>  
>  #define e820_raw bootsym(e820map)

[PATCH v2 3/5] xen/x86: Migrate to boot_info structure

[Daniel Kiper]

Break multiboot (v1) protocol dependency. It means that most of Xen code
(excluding preloader) could be bootloader agnostic and does not need almost
any knowledge about boot protocol. Additionally, we are able to pass all boot
data to __start_xen() in one bucket without any side channels. I do not mention
that we are also able to easily identify boot data in Xen code.

Here is boot data flow for legacy BIOS platform:

 BIOS -> GRUB -> multiboot[12]* -> __reloc() -> MBD ->-\
                                                       /
        ------<------<------<------<------<------<-----
         \
          \
           ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info
          /
 BIOS ->-/

  * multiboot2 is not implemented yet. Look for it in later patches.

Here is boot data flow for EFI platform:

  EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info

WARNING: ARM build could be broken by this patch. We need to agree boot_info
integration into ARM. Personally I think that it is worth storing all data
from any bootloader and preloader in boot_info on any architecture. This give
a chance to share more code between architectures. However, every architecture
should define its own boot_info (in relevant include/asm directory). Despite
that it looks that some parts of it could be common, e.g.  modules data,
command line arguments, boot loader name, EFI data, etc., even if types
would not be the same. So, as it was stated above a lot of code could be
shared among architectures.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
v2 - suggestions/fixes:
   - rename XBI to boot_info
     (suggested by Andrew Cooper),
   - use more meaningful types in boot_info structure
     (suggested by Andrew Cooper, Jan Beulich and Stefano Stabellini),
   - improve boot_info structure comment
     (suggested by Andrew Cooper and Jan Beulich),
   - do data shuffling after exception support initialization
     (suggested by Andrew Cooper),
   - patch split rearrangement
     (suggested by Andrew Cooper and Jan Beulich).
---
 xen/arch/x86/boot_info.c          |  240 +++++++++++++++++++++++---
 xen/arch/x86/dmi_scan.c           |    7 +-
 xen/arch/x86/domain_build.c       |   24 +--
 xen/arch/x86/efi/boot.c           |  214 +++++++++++------------
 xen/arch/x86/efi/efi.h            |    3 -
 xen/arch/x86/efi/runtime.c        |   52 ++++--
 xen/arch/x86/microcode.c          |   39 ++---
 xen/arch/x86/mpparse.c            |    9 +-
 xen/arch/x86/platform_hypercall.c |   19 +--
 xen/arch/x86/setup.c              |  341 +++++++++++--------------------------
 xen/drivers/acpi/osl.c            |    9 +-
 xen/drivers/video/vesa.c          |    7 +-
 xen/drivers/video/vga.c           |   18 +-
 xen/include/asm-x86/boot_info.h   |  126 ++++++++++++++
 xen/include/asm-x86/config.h      |    2 -
 xen/include/asm-x86/e820.h        |    8 -
 xen/include/asm-x86/edd.h         |    6 -
 xen/include/asm-x86/setup.h       |   10 +-
 xen/include/xen/efi.h             |   10 --
 xen/include/xen/vga.h             |   18 --
 xen/include/xsm/xsm.h             |   14 +-
 xen/xsm/xsm_core.c                |    6 +-
 xen/xsm/xsm_policy.c              |   14 +-
 23 files changed, 680 insertions(+), 516 deletions(-)
 create mode 100644 xen/include/asm-x86/boot_info.h
 delete mode 100644 xen/include/xen/vga.h

diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c
index 70830c2..10a1749 100644
--- a/xen/arch/x86/boot_info.c
+++ b/xen/arch/x86/boot_info.c
@@ -15,44 +15,244 @@
  * with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/*
+ * Some ideas are taken (out) from xen/arch/x86/boot/reloc.c,
+ * xen/arch/x86/efi/boot.c and xen/arch/x86/setup.c.
+ */
+
 #include <xen/types.h>
+#include <xen/cache.h>
+#include <xen/efi.h>
 #include <xen/init.h>
 #include <xen/multiboot.h>
 
+#include <asm/boot_info.h>
+#include <asm/e820.h>
 #include <asm/mbd.h>
 #include <asm/page.h>
 
-static multiboot_info_t mbi = {};
+struct boot_video_info {
+    u8  orig_x;             /* 0x00 */
+    u8  orig_y;             /* 0x01 */
+    u8  orig_video_mode;    /* 0x02 */
+    u8  orig_video_cols;    /* 0x03 */
+    u8  orig_video_lines;   /* 0x04 */
+    u8  orig_video_isVGA;   /* 0x05 */
+    u16 orig_video_points;  /* 0x06 */
+
+    /* VESA graphic mode -- linear frame buffer */
+    u32 capabilities;       /* 0x08 */
+    u16 lfb_linelength;     /* 0x0c */
+    u16 lfb_width;          /* 0x0e */
+    u16 lfb_height;         /* 0x10 */
+    u16 lfb_depth;          /* 0x12 */
+    u32 lfb_base;           /* 0x14 */
+    u32 lfb_size;           /* 0x18 */
+    u8  red_size;           /* 0x1c */
+    u8  red_pos;            /* 0x1d */
+    u8  green_size;         /* 0x1e */
+    u8  green_pos;          /* 0x1f */
+    u8  blue_size;          /* 0x20 */
+    u8  blue_pos;           /* 0x21 */
+    u8  rsvd_size;          /* 0x22 */
+    u8  rsvd_pos;           /* 0x23 */
+    u16 vesapm_seg;         /* 0x24 */
+    u16 vesapm_off;         /* 0x26 */
+    u16 vesa_attrib;        /* 0x28 */
+};
+
+/* These symbols live in the boot trampoline. Access via bootsym(). */
+extern struct e820entry e820map[];
+extern int e820nr;
+extern unsigned int lowmem_kb, highmem_kb;
+
+extern struct boot_video_info boot_vid_info;
+
+extern unsigned short boot_edid_caps;
+extern unsigned char boot_edid_info[128];
+
+extern struct edd_info boot_edd_info[];
+extern u8 boot_edd_info_nr;
+
+extern struct mbr_signature boot_mbr_signature[];
+extern u8 boot_mbr_signature_nr;
+
+static boot_info_t __read_mostly boot_info_mb = {
+    .boot_loader_name = "UNKNOWN",
+    .cmdline = NULL,
+    .mmap_type = NULL,
+    .mem_upper = 0,
+    .e820map_nr = 0,
+    .e820map = NULL,
+    .efi_mmap_size = 0,
+    .efi_mmap_desc_size = 0,
+    .efi_mmap = NULL,
+    .mps = EFI_INVALID_TABLE_ADDR,
+    .acpi = EFI_INVALID_TABLE_ADDR,
+    .acpi20 = EFI_INVALID_TABLE_ADDR,
+    .smbios = EFI_INVALID_TABLE_ADDR,
+    .efi_system_table = NULL,
+    .vga_console_info = {},
+    .edid_caps = 0,
+    .edid_info = NULL,
+    .edd_info_nr = 0,
+    .edd_info = NULL,
+    .mbr_signature_nr = 0,
+    .mbr_signature = NULL,
+    .mods_nr = 0,
+    .mods = NULL,
+    .warn_msg = NULL,
+    .err_msg = NULL
+};
+
+#define e820_raw bootsym(e820map)
+#define e820_raw_nr bootsym(e820nr)
 
 extern void enable_exception_support(void);
 
-unsigned long __init __init_boot_info(u32 mbd_ptr)
+static void __init init_mmap(boot_info_t *boot_info, mbd_t *mbd)
 {
-    mbd_t *mbd = __va(mbd_ptr);
+    int bytes = 0;
+    memory_map_t *map;
 
-    enable_exception_support();
+    if ( e820_raw_nr )
+        boot_info->mmap_type = "Xen-e820";
+    else if ( mbd->mmap_size )
+    {
+        boot_info->mmap_type = "Multiboot-e820";
+
+        while ( (bytes < mbd->mmap_size) && (e820_raw_nr < E820MAX) )
+        {
+            /*
+             * This is a gross workaround for a BIOS bug. Some bootloaders do
+             * not write e820 map entries into pre-zeroed memory. This is
+             * okay if the BIOS fills in all fields of the map entry, but
+             * some broken BIOSes do not bother to write the high word of
+             * the length field if the length is smaller than 4GB. We
+             * detect and fix this by flagging sections below 4GB that
+             * appear to be larger than 4GB in size.
+             */
+            map = __va(mbd->mmap + bytes);
+
+            if ( !map->base_addr_high && map->length_high )
+            {
+                map->length_high = 0;
+                boot_info->warn_msg = "WARNING: Buggy e820 map detected and fixed "
+                                "(truncated length fields).\n";
+            }
 
-    if ( mbd->boot_loader_name ) {
-        mbi.flags = MBI_LOADERNAME;
-        mbi.boot_loader_name = mbd->boot_loader_name;
+            e820_raw[e820_raw_nr].addr =
+                ((u64)map->base_addr_high << 32) | (u64)map->base_addr_low;
+            e820_raw[e820_raw_nr].size =
+                ((u64)map->length_high << 32) | (u64)map->length_low;
+            e820_raw[e820_raw_nr].type = map->type;
+            e820_raw_nr++;
+
+            bytes += map->size + 4;
+        }
+    }
+    else if ( bootsym(lowmem_kb) )
+    {
+        boot_info->mmap_type = "Xen-e801";
+
+        e820_raw[0].addr = 0;
+        e820_raw[0].size = bootsym(lowmem_kb) << 10;
+        e820_raw[0].type = E820_RAM;
+        e820_raw[1].addr = 0x100000;
+        e820_raw[1].size = bootsym(highmem_kb) << 10;
+        e820_raw[1].type = E820_RAM;
+        e820_raw_nr = 2;
     }
+    else if ( mbd->mem_lower || mbd->mem_upper )
+    {
+        boot_info->mmap_type = "Multiboot-e801";
 
-    if ( mbd->cmdline ) {
-        mbi.flags |= MBI_CMDLINE;
-        mbi.cmdline = mbd->cmdline;
+        e820_raw[0].addr = 0;
+        e820_raw[0].size = mbd->mem_lower << 10;
+        e820_raw[0].type = E820_RAM;
+        e820_raw[1].addr = 0x100000;
+        e820_raw[1].size = mbd->mem_upper << 10;
+        e820_raw[1].type = E820_RAM;
+        e820_raw_nr = 2;
     }
+    else
+    {
+        boot_info->err_msg = "Bootloader provided no memory information.\n";
+        return;
+    }
+
+    boot_info->mem_upper = mbd->mem_upper;
+
+    boot_info->e820map_nr = e820_raw_nr;
+    boot_info->e820map = e820_raw;
+}
+
+static void __init init_video_info(boot_info_t *boot_info)
+{
+    struct boot_video_info *bvi = &bootsym(boot_vid_info);
+
+    if ( (bvi->orig_video_isVGA == 1) && (bvi->orig_video_mode == 3) )
+    {
+        boot_info->vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
+        boot_info->vga_console_info.u.text_mode_3.font_height = bvi->orig_video_points;
+        boot_info->vga_console_info.u.text_mode_3.cursor_x = bvi->orig_x;
+        boot_info->vga_console_info.u.text_mode_3.cursor_y = bvi->orig_y;
+        boot_info->vga_console_info.u.text_mode_3.rows = bvi->orig_video_lines;
+        boot_info->vga_console_info.u.text_mode_3.columns = bvi->orig_video_cols;
+    }
+    else if ( bvi->orig_video_isVGA == 0x23 )
+    {
+        boot_info->vga_console_info.video_type = XEN_VGATYPE_VESA_LFB;
+        boot_info->vga_console_info.u.vesa_lfb.width = bvi->lfb_width;
+        boot_info->vga_console_info.u.vesa_lfb.height = bvi->lfb_height;
+        boot_info->vga_console_info.u.vesa_lfb.bytes_per_line = bvi->lfb_linelength;
+        boot_info->vga_console_info.u.vesa_lfb.bits_per_pixel = bvi->lfb_depth;
+        boot_info->vga_console_info.u.vesa_lfb.lfb_base = bvi->lfb_base;
+        boot_info->vga_console_info.u.vesa_lfb.lfb_size = bvi->lfb_size;
+        boot_info->vga_console_info.u.vesa_lfb.red_pos = bvi->red_pos;
+        boot_info->vga_console_info.u.vesa_lfb.red_size = bvi->red_size;
+        boot_info->vga_console_info.u.vesa_lfb.green_pos = bvi->green_pos;
+        boot_info->vga_console_info.u.vesa_lfb.green_size = bvi->green_size;
+        boot_info->vga_console_info.u.vesa_lfb.blue_pos = bvi->blue_pos;
+        boot_info->vga_console_info.u.vesa_lfb.blue_size = bvi->blue_size;
+        boot_info->vga_console_info.u.vesa_lfb.rsvd_pos = bvi->rsvd_pos;
+        boot_info->vga_console_info.u.vesa_lfb.rsvd_size = bvi->rsvd_size;
+        boot_info->vga_console_info.u.vesa_lfb.gbl_caps = bvi->capabilities;
+        boot_info->vga_console_info.u.vesa_lfb.mode_attrs = bvi->vesa_attrib;
+    }
+
+    boot_info->edid_caps = bootsym(boot_edid_caps);
+    boot_info->edid_info = bootsym(boot_edid_info);
+}
+
+boot_info_t __init *__init_boot_info(u32 mbd_ptr)
+{
+    mbd_t *mbd = __va(mbd_ptr);
+
+    enable_exception_support();
+
+    if ( mbd->boot_loader_name )
+        boot_info_mb.boot_loader_name = __va(mbd->boot_loader_name);
+
+    if ( mbd->cmdline )
+        boot_info_mb.cmdline = __va(mbd->cmdline);
+
+    init_mmap(&boot_info_mb, mbd);
+
+    if ( boot_info_mb.err_msg )
+        goto err;
+
+    init_video_info(&boot_info_mb);
 
-    mbi.flags |= MBI_MEMLIMITS;
-    mbi.mem_lower = mbd->mem_lower;
-    mbi.mem_upper = mbd->mem_upper;
+    boot_info_mb.edd_info_nr = bootsym(boot_edd_info_nr);
+    boot_info_mb.edd_info = bootsym(boot_edd_info);
 
-    mbi.flags |= MBI_MEMMAP;
-    mbi.mmap_length = mbd->mmap_size;
-    mbi.mmap_addr = mbd->mmap;
+    boot_info_mb.mbr_signature_nr = bootsym(boot_mbr_signature_nr);
+    boot_info_mb.mbr_signature = bootsym(boot_mbr_signature);
 
-    mbi.flags |= MBI_MODULES;
-    mbi.mods_count = mbd->mods_nr;
-    mbi.mods_addr = mbd->mods;
+    boot_info_mb.mods_nr = mbd->mods_nr;
+    boot_info_mb.mods = __va(mbd->mods);
 
-    return __pa(&mbi);
+err:
+    return &boot_info_mb;
 }
diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
index 500133a..a8153b3 100644
--- a/xen/arch/x86/dmi_scan.c
+++ b/xen/arch/x86/dmi_scan.c
@@ -12,6 +12,7 @@
 #include <xen/efi.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <asm/boot_info.h>
 
 #define bt_ioremap(b,l)  ((void *)__acpi_map_table(b,l))
 #define bt_iounmap(b,l)  ((void)0)
@@ -215,10 +216,10 @@ static int __init dmi_efi_iterate(void (*decode)(struct dmi_header *))
 	const struct smbios_eps __iomem *p;
 	int ret = -1;
 
-	if (efi.smbios == EFI_INVALID_TABLE_ADDR)
+	if (boot_info->smbios == EFI_INVALID_TABLE_ADDR)
 		return -1;
 
-	p = bt_ioremap(efi.smbios, sizeof(eps));
+	p = bt_ioremap(boot_info->smbios, sizeof(eps));
 	if (!p)
 		return -1;
 	memcpy_fromio(&eps, p, sizeof(eps));
@@ -227,7 +228,7 @@ static int __init dmi_efi_iterate(void (*decode)(struct dmi_header *))
 	if (memcmp(eps.anchor, "_SM_", 4))
 		return -1;
 
-	p = bt_ioremap(efi.smbios, eps.length);
+	p = bt_ioremap(boot_info->smbios, eps.length);
 	if (!p)
 		return -1;
 	if (dmi_checksum(p, eps.length) &&
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index d4473c1..e35260b 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -751,9 +751,9 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
 
 int __init construct_dom0(
     struct domain *d,
-    const module_t *image, unsigned long image_headroom,
-    module_t *initrd,
-    void *(*bootstrap_map)(const module_t *),
+    const boot_module_t *image, unsigned long image_headroom,
+    boot_module_t *initrd,
+    void *(*bootstrap_map)(const boot_module_t *),
     char *cmdline)
 {
     int i, cpu, rc, compatible, compat32, order, machine;
@@ -770,9 +770,9 @@ int __init construct_dom0(
     struct vcpu *v = d->vcpu[0];
     unsigned long long value;
     char *image_base = bootstrap_map(image);
-    unsigned long image_len = image->mod_end;
+    unsigned long image_len = image->end;
     char *image_start = image_base + image_headroom;
-    unsigned long initrd_len = initrd ? initrd->mod_end : 0;
+    unsigned long initrd_len = initrd ? initrd->end : 0;
     l4_pgentry_t *l4tab = NULL, *l4start = NULL;
     l3_pgentry_t *l3tab = NULL, *l3start = NULL;
     l2_pgentry_t *l2tab = NULL, *l2start = NULL;
@@ -987,7 +987,7 @@ int __init construct_dom0(
         initrd_pfn = vinitrd_start ?
                      (vinitrd_start - v_start) >> PAGE_SHIFT :
                      d->tot_pages;
-        initrd_mfn = mfn = initrd->mod_start;
+        initrd_mfn = mfn = initrd->start;
         count = PFN_UP(initrd_len);
         if ( d->arch.physaddr_bitsize &&
              ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
@@ -1002,12 +1002,12 @@ int __init construct_dom0(
                     free_domheap_pages(page, order);
                     page += 1UL << order;
                 }
-            memcpy(page_to_virt(page), mfn_to_virt(initrd->mod_start),
+            memcpy(page_to_virt(page), mfn_to_virt(initrd->start),
                    initrd_len);
-            mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
+            mpt_alloc = (paddr_t)initrd->start << PAGE_SHIFT;
             init_domheap_pages(mpt_alloc,
                                mpt_alloc + PAGE_ALIGN(initrd_len));
-            initrd->mod_start = initrd_mfn = page_to_mfn(page);
+            initrd->start = initrd_mfn = page_to_mfn(page);
         }
         else
         {
@@ -1015,7 +1015,7 @@ int __init construct_dom0(
                 if ( assign_pages(d, mfn_to_page(mfn++), 0, 0) )
                     BUG();
         }
-        initrd->mod_end = 0;
+        initrd->end = 0;
     }
 
     printk("PHYSICAL MEMORY ARRANGEMENT:\n"
@@ -1026,7 +1026,7 @@ int __init construct_dom0(
                nr_pages - d->tot_pages);
     if ( initrd )
     {
-        mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
+        mpt_alloc = (paddr_t)initrd->start << PAGE_SHIFT;
         printk("\n Init. ramdisk: %"PRIpaddr"->%"PRIpaddr,
                mpt_alloc, mpt_alloc + initrd_len);
     }
@@ -1281,7 +1281,7 @@ int __init construct_dom0(
         if ( pfn >= initrd_pfn )
         {
             if ( pfn < initrd_pfn + PFN_UP(initrd_len) )
-                mfn = initrd->mod_start + (pfn - initrd_pfn);
+                mfn = initrd->start + (pfn - initrd_pfn);
             else
                 mfn -= PFN_UP(initrd_len);
         }
diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
index 3bdc158..417ed5e 100644
--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -9,7 +9,6 @@
 #include <xen/keyhandler.h>
 #include <xen/lib.h>
 #include <xen/mm.h>
-#include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
@@ -17,7 +16,7 @@
 #endif
 #include <xen/string.h>
 #include <xen/stringify.h>
-#include <xen/vga.h>
+#include <xen/video.h>
 #include <asm/e820.h>
 #include <asm/edd.h>
 #define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
@@ -25,6 +24,7 @@
 #undef __ASSEMBLY__
 #include <asm/msr.h>
 #include <asm/processor.h>
+#include <asm/boot_info.h>
 
 /* Using SetVirtualAddressMap() is incompatible with kexec: */
 #undef USE_SET_VIRTUAL_ADDRESS_MAP
@@ -72,13 +72,12 @@ static struct file __initdata ramdisk;
 static struct file __initdata ucode;
 static struct file __initdata xsm;
 
-static multiboot_info_t __initdata mbi = {
-    .flags = MBI_MODULES | MBI_LOADERNAME
-};
-static module_t __initdata mb_modules[3];
-
 static CHAR16 __initdata newline[] = L"\r\n";
 
+extern unsigned short boot_edid_caps;
+
+extern boot_info_t __read_mostly boot_info_efi;
+
 #define PrintStr(s) StdOut->OutputString(StdOut, s)
 #define PrintErr(s) StdErr->OutputString(StdErr, s)
 
@@ -255,14 +254,14 @@ static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
     blexit(mesg);
 }
 
-static void __init place_string(u32 *addr, const char *s)
+static void __init place_string_char(char **addr, const char *s)
 {
     static char *__initdata alloc = start;
 
     if ( s && *s )
     {
         size_t len1 = strlen(s) + 1;
-        const char *old = (char *)(long)*addr;
+        const char *old = *addr;
         size_t len2 = *addr ? strlen(old) + 1 : 0;
 
         alloc -= len1 + len2;
@@ -278,7 +277,16 @@ static void __init place_string(u32 *addr, const char *s)
             memcpy(alloc + len1, old, len2);
         }
     }
-    *addr = (long)alloc;
+    *addr = alloc;
+}
+
+static void __init place_string_u32(u32 *addr, const char *s)
+{
+    char *s_new = (char *)(long)*addr;
+
+    place_string_char(&s_new, s);
+
+    *addr = (long)s_new;
 }
 
 static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
@@ -311,7 +319,7 @@ static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
                 union string rest = { .w = cmdline };
 
                 --argv;
-                place_string(&mbi.cmdline, w2s(&rest));
+                place_string_char(&boot_info_efi.cmdline, w2s(&rest));
                 break;
             }
             else
@@ -480,9 +488,9 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
             PrintStr(L"-");
             DisplayUint(file->addr + size, 2 * sizeof(file->addr));
             PrintStr(newline);
-            mb_modules[mbi.mods_count].mod_start = file->addr >> PAGE_SHIFT;
-            mb_modules[mbi.mods_count].mod_end = size;
-            ++mbi.mods_count;
+            boot_info_efi.mods[boot_info_efi.mods_nr].start = file->addr >> PAGE_SHIFT;
+            boot_info_efi.mods[boot_info_efi.mods_nr].end = size;
+            ++boot_info_efi.mods_nr;
         }
 
         file->size = size;
@@ -568,7 +576,7 @@ static void __init split_value(char *s)
 {
     while ( *s && isspace(*s) )
         ++s;
-    place_string(&mb_modules[mbi.mods_count].string, s);
+    place_string_u32(&boot_info_efi.mods[boot_info_efi.mods_nr].cmdline, s);
     while ( *s && !isspace(*s) )
         ++s;
     *s = 0;
@@ -884,10 +892,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
                            &cols, &rows) == EFI_SUCCESS )
     {
-        vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
-        vga_console_info.u.text_mode_3.columns = cols;
-        vga_console_info.u.text_mode_3.rows = rows;
-        vga_console_info.u.text_mode_3.font_height = 16;
+        boot_info_efi.vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
+        boot_info_efi.vga_console_info.u.text_mode_3.columns = cols;
+        boot_info_efi.vga_console_info.u.text_mode_3.rows = rows;
+        boot_info_efi.vga_console_info.u.text_mode_3.font_height = 16;
     }
 
     size = 0;
@@ -984,7 +992,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         name.s = get_value(&cfg, "global", "ucode");
     if ( name.s )
     {
-        microcode_set_module(mbi.mods_count);
+        microcode_set_module(boot_info_efi.mods_nr);
         split_value(name.s);
         read_file(dir_handle, s2w(&name), &ucode);
         efi_bs->FreePool(name.w);
@@ -1000,7 +1008,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     name.s = get_value(&cfg, section.s, "options");
     if ( name.s )
-        place_string(&mbi.cmdline, name.s);
+        place_string_char(&boot_info_efi.cmdline, name.s);
     /* Insert image name last, as it gets prefixed to the other options. */
     if ( argc )
     {
@@ -1009,7 +1017,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     }
     else
         name.s = "xen";
-    place_string(&mbi.cmdline, name.s);
+    place_string_char(&boot_info_efi.cmdline, name.s);
 
     cols = rows = depth = 0;
     if ( !base_video )
@@ -1075,16 +1083,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         }
     }
 
-    if ( mbi.cmdline )
-        mbi.flags |= MBI_CMDLINE;
-    /*
-     * These must not be initialized statically, since the value must
-     * not get relocated when processing base relocations below.
-     */
-    mbi.boot_loader_name = (long)"EFI";
-    mbi.mods_addr = (long)mb_modules;
+    boot_info_efi.efi_system_table = SystemTable;
+    boot_info_efi.edid_caps = boot_edid_caps;
 
-    place_string(&mbi.mem_upper, NULL);
+    place_string_u32(&boot_info_efi.mem_upper, NULL);
 
     /* Collect EDD info. */
     BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
@@ -1102,7 +1104,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     {
         EFI_BLOCK_IO *bio;
         EFI_DEV_PATH_PTR devp;
-        struct edd_info *info = boot_edd_info + boot_edd_info_nr;
+        struct edd_info *info = boot_info_efi.edd_info + boot_info_efi.edd_info_nr;
         struct edd_device_params *params = &info->edd_device_params;
         enum { root, acpi, pci, ctrlr } state = root;
 
@@ -1111,16 +1113,16 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
              bio->Media->RemovableMedia ||
              bio->Media->LogicalPartition )
             continue;
-        if ( boot_edd_info_nr < EDD_INFO_MAX )
+        if ( boot_info_efi.edd_info_nr < EDD_INFO_MAX )
         {
-            info->device = 0x80 + boot_edd_info_nr; /* fake */
+            info->device = 0x80 + boot_info_efi.edd_info_nr; /* fake */
             info->version = 0x11;
             params->length = offsetof(struct edd_device_params, dpte_ptr);
             params->number_of_sectors = bio->Media->LastBlock + 1;
             params->bytes_per_sector = bio->Media->BlockSize;
             params->dpte_ptr = ~0;
         }
-        ++boot_edd_info_nr;
+        ++boot_info_efi.edd_info_nr;
         status = efi_bs->HandleProtocol(handles[i], &devp_guid,
                                         (void **)&devp);
         if ( EFI_ERROR(status) )
@@ -1133,7 +1135,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                 const u8 *p;
 
             case ACPI_DEVICE_PATH:
-                if ( state != root || boot_edd_info_nr > EDD_INFO_MAX )
+                if ( state != root || boot_info_efi.edd_info_nr > EDD_INFO_MAX )
                     break;
                 switch ( DevicePathSubType(devp.DevPath) )
                 {
@@ -1152,7 +1154,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             case HARDWARE_DEVICE_PATH:
                 if ( state != acpi ||
                      DevicePathSubType(devp.DevPath) != HW_PCI_DP ||
-                     boot_edd_info_nr > EDD_INFO_MAX )
+                     boot_info_efi.edd_info_nr > EDD_INFO_MAX )
                     break;
                 state = pci;
                 edd_put_string(params->host_bus_type, "PCI");
@@ -1160,7 +1162,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                 params->interface_path.pci.function = devp.Pci->Function;
                 break;
             case MESSAGING_DEVICE_PATH:
-                if ( state != pci || boot_edd_info_nr > EDD_INFO_MAX )
+                if ( state != pci || boot_info_efi.edd_info_nr > EDD_INFO_MAX )
                     break;
                 state = ctrlr;
                 switch ( DevicePathSubType(devp.DevPath) )
@@ -1209,15 +1211,15 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             case MEDIA_DEVICE_PATH:
                 if ( DevicePathSubType(devp.DevPath) == MEDIA_HARDDRIVE_DP &&
                      devp.HardDrive->MBRType == MBR_TYPE_PCAT &&
-                     boot_mbr_signature_nr < EDD_MBR_SIG_MAX )
+                     boot_info_efi.mbr_signature_nr < EDD_MBR_SIG_MAX )
                 {
-                    struct mbr_signature *sig = boot_mbr_signature +
-                                                boot_mbr_signature_nr;
+                    struct mbr_signature *sig = boot_info_efi.mbr_signature +
+                                                boot_info_efi.mbr_signature_nr;
 
-                    sig->device = 0x80 + boot_edd_info_nr; /* fake */
+                    sig->device = 0x80 + boot_info_efi.edd_info_nr; /* fake */
                     memcpy(&sig->signature, devp.HardDrive->Signature,
                            sizeof(sig->signature));
-                    ++boot_mbr_signature_nr;
+                    ++boot_info_efi.mbr_signature_nr;
                 }
                 break;
             }
@@ -1225,8 +1227,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     }
     if ( handles )
         efi_bs->FreePool(handles);
-    if ( boot_edd_info_nr > EDD_INFO_MAX )
-        boot_edd_info_nr = EDD_INFO_MAX;
+    if ( boot_info_efi.edd_info_nr > EDD_INFO_MAX )
+        boot_info_efi.edd_info_nr = EDD_INFO_MAX;
 
     /* XXX Collect EDID info. */
 
@@ -1245,17 +1247,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         static EFI_GUID __initdata smbios_guid = SMBIOS_TABLE_GUID;
 
         if ( match_guid(&acpi2_guid, &efi_ct[i].VendorGuid) )
-	       efi.acpi20 = (long)efi_ct[i].VendorTable;
+	       boot_info_efi.acpi20 = (paddr_t)efi_ct[i].VendorTable;
         if ( match_guid(&acpi_guid, &efi_ct[i].VendorGuid) )
-	       efi.acpi = (long)efi_ct[i].VendorTable;
+	       boot_info_efi.acpi = (paddr_t)efi_ct[i].VendorTable;
         if ( match_guid(&mps_guid, &efi_ct[i].VendorGuid) )
-	       efi.mps = (long)efi_ct[i].VendorTable;
+	       boot_info_efi.mps = (paddr_t)efi_ct[i].VendorTable;
         if ( match_guid(&smbios_guid, &efi_ct[i].VendorGuid) )
-	       efi.smbios = (long)efi_ct[i].VendorTable;
+	       boot_info_efi.smbios = (paddr_t)efi_ct[i].VendorTable;
     }
 
-    if (efi.smbios != EFI_INVALID_TABLE_ADDR)
-        dmi_efi_get_table((void *)(long)efi.smbios);
+    if (boot_info_efi.smbios != EFI_INVALID_TABLE_ADDR)
+        dmi_efi_get_table((void *)boot_info_efi.smbios);
 
     /* Collect PCI ROM contents. */
     setup_efi_pci();
@@ -1322,40 +1324,40 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             switch ( mode_info->PixelFormat )
             {
             case PixelRedGreenBlueReserved8BitPerColor:
-                vga_console_info.u.vesa_lfb.red_pos = 0;
-                vga_console_info.u.vesa_lfb.red_size = 8;
-                vga_console_info.u.vesa_lfb.green_pos = 8;
-                vga_console_info.u.vesa_lfb.green_size = 8;
-                vga_console_info.u.vesa_lfb.blue_pos = 16;
-                vga_console_info.u.vesa_lfb.blue_size = 8;
-                vga_console_info.u.vesa_lfb.rsvd_pos = 24;
-                vga_console_info.u.vesa_lfb.rsvd_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.red_pos = 0;
+                boot_info_efi.vga_console_info.u.vesa_lfb.red_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.green_pos = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.green_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.blue_pos = 16;
+                boot_info_efi.vga_console_info.u.vesa_lfb.blue_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_pos = 24;
+                boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_size = 8;
                 bpp = 32;
                 break;
             case PixelBlueGreenRedReserved8BitPerColor:
-                vga_console_info.u.vesa_lfb.red_pos = 16;
-                vga_console_info.u.vesa_lfb.red_size = 8;
-                vga_console_info.u.vesa_lfb.green_pos = 8;
-                vga_console_info.u.vesa_lfb.green_size = 8;
-                vga_console_info.u.vesa_lfb.blue_pos = 0;
-                vga_console_info.u.vesa_lfb.blue_size = 8;
-                vga_console_info.u.vesa_lfb.rsvd_pos = 24;
-                vga_console_info.u.vesa_lfb.rsvd_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.red_pos = 16;
+                boot_info_efi.vga_console_info.u.vesa_lfb.red_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.green_pos = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.green_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.blue_pos = 0;
+                boot_info_efi.vga_console_info.u.vesa_lfb.blue_size = 8;
+                boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_pos = 24;
+                boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_size = 8;
                 bpp = 32;
                 break;
             case PixelBitMask:
                 bpp = set_color(mode_info->PixelInformation.RedMask, bpp,
-                                &vga_console_info.u.vesa_lfb.red_pos,
-                                &vga_console_info.u.vesa_lfb.red_size);
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.red_pos,
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.red_size);
                 bpp = set_color(mode_info->PixelInformation.GreenMask, bpp,
-                                &vga_console_info.u.vesa_lfb.green_pos,
-                                &vga_console_info.u.vesa_lfb.green_size);
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.green_pos,
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.green_size);
                 bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
-                                &vga_console_info.u.vesa_lfb.blue_pos,
-                                &vga_console_info.u.vesa_lfb.blue_size);
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.blue_pos,
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.blue_size);
                 bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
-                                &vga_console_info.u.vesa_lfb.rsvd_pos,
-                                &vga_console_info.u.vesa_lfb.rsvd_size);
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_pos,
+                                &boot_info_efi.vga_console_info.u.vesa_lfb.rsvd_size);
                 if ( bpp > 0 )
                     break;
                 /* fall through */
@@ -1366,37 +1368,37 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             }
         if ( !EFI_ERROR(status) )
         {
-            vga_console_info.video_type = XEN_VGATYPE_EFI_LFB;
-            vga_console_info.u.vesa_lfb.gbl_caps = 2; /* possibly non-VGA */
-            vga_console_info.u.vesa_lfb.width =
+            boot_info_efi.vga_console_info.video_type = XEN_VGATYPE_EFI_LFB;
+            boot_info_efi.vga_console_info.u.vesa_lfb.gbl_caps = 2; /* possibly non-VGA */
+            boot_info_efi.vga_console_info.u.vesa_lfb.width =
                 mode_info->HorizontalResolution;
-            vga_console_info.u.vesa_lfb.height = mode_info->VerticalResolution;
-            vga_console_info.u.vesa_lfb.bits_per_pixel = bpp;
-            vga_console_info.u.vesa_lfb.bytes_per_line =
+            boot_info_efi.vga_console_info.u.vesa_lfb.height = mode_info->VerticalResolution;
+            boot_info_efi.vga_console_info.u.vesa_lfb.bits_per_pixel = bpp;
+            boot_info_efi.vga_console_info.u.vesa_lfb.bytes_per_line =
                 (mode_info->PixelsPerScanLine * bpp + 7) >> 3;
-            vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase;
-            vga_console_info.u.vesa_lfb.lfb_size =
+            boot_info_efi.vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase;
+            boot_info_efi.vga_console_info.u.vesa_lfb.lfb_size =
                 (gop->Mode->FrameBufferSize + 0xffff) >> 16;
         }
     }
 
-    efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key,
-                         &efi_mdesc_size, &mdesc_ver);
-    mbi.mem_upper -= efi_memmap_size;
-    mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);
-    if ( mbi.mem_upper < xen_phys_start )
-        blexit(L"Out of static memory");
-    efi_memmap = (void *)(long)mbi.mem_upper;
-    status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
-                                  &efi_mdesc_size, &mdesc_ver);
+    efi_bs->GetMemoryMap(&boot_info_efi.efi_mmap_size, NULL, &map_key,
+			 &boot_info_efi.efi_mmap_desc_size, &mdesc_ver);
+    boot_info_efi.mem_upper -= boot_info_efi.efi_mmap_size;
+    boot_info_efi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);
+    if ( boot_info_efi.mem_upper < xen_phys_start )
+        blexit(L"Out of static memory\r\n");
+    boot_info_efi.efi_mmap = (void *)(long)boot_info_efi.mem_upper;
+    status = efi_bs->GetMemoryMap(&boot_info_efi.efi_mmap_size, boot_info_efi.efi_mmap, &map_key,
+                                  &boot_info_efi.efi_mmap_desc_size, &mdesc_ver);
     if ( EFI_ERROR(status) )
         PrintErrMesg(L"Cannot obtain memory map", status);
 
     /* Populate E820 table and check trampoline area availability. */
-    e = e820map - 1;
-    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+    e = boot_info_efi.e820map - 1;
+    for ( i = 0; i < boot_info_efi.efi_mmap_size; i += boot_info_efi.efi_mmap_desc_size )
     {
-        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+        EFI_MEMORY_DESCRIPTOR *desc = boot_info_efi.efi_mmap + i;
         u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
         u32 type;
 
@@ -1427,10 +1429,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             type = E820_NVS;
             break;
         }
-        if ( e820nr && type == e->type &&
+        if ( boot_info_efi.e820map_nr && type == e->type &&
              desc->PhysicalStart == e->addr + e->size )
             e->size += len;
-        else if ( !len || e820nr >= E820MAX )
+        else if ( !len || boot_info_efi.e820map_nr >= E820MAX )
             continue;
         else
         {
@@ -1438,7 +1440,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             e->addr = desc->PhysicalStart;
             e->size = len;
             e->type = type;
-            ++e820nr;
+            ++boot_info_efi.e820map_nr;
         }
     }
     if ( !trampoline_phys )
@@ -1457,7 +1459,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 #ifdef USE_SET_VIRTUAL_ADDRESS_MAP
     efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START;
 #endif
-    efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
+    boot_info_efi.efi_mmap = (void *)boot_info_efi.efi_mmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
 
     relocate_image(__XEN_VIRT_START - xen_phys_start);
@@ -1491,7 +1493,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                      [cs] "ir" (__HYPERVISOR_CS),
                      [ds] "r" (__HYPERVISOR_DS),
                      [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)),
-                     "D" (&mbi)
+                     "D" (&boot_info_efi)
                    : "memory" );
     for( ; ; ); /* not reached */
 }
@@ -1557,9 +1559,9 @@ void __init efi_init_memory(void)
 #endif
 
     printk(XENLOG_INFO "EFI memory map:\n");
-    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+    for ( i = 0; i < boot_info_efi.efi_mmap_size; i += boot_info_efi.efi_mmap_desc_size )
     {
-        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+        EFI_MEMORY_DESCRIPTOR *desc = boot_info_efi.efi_mmap + i;
         u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
         unsigned long smfn, emfn;
         unsigned int prot = PAGE_HYPERVISOR;
@@ -1634,8 +1636,8 @@ void __init efi_init_memory(void)
     }
 
 #ifdef USE_SET_VIRTUAL_ADDRESS_MAP
-    efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
-                                 mdesc_ver, efi_memmap);
+    efi_rs->SetVirtualAddressMap(boot_info_efi.efi_mmap_size, boot_info_efi.efi_mmap_desc_size,
+                                 mdesc_ver, boot_info_efi.efi_mmap);
 #else
     /* Set up 1:1 page tables to do runtime calls in "physical" mode. */
     efi_l4_pgtable = alloc_xen_pagetable();
@@ -1645,9 +1647,9 @@ void __init efi_init_memory(void)
     copy_mapping(0, max_page, ram_range_valid);
 
     /* Insert non-RAM runtime mappings inside the direct map. */
-    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+    for ( i = 0; i < boot_info_efi.efi_mmap_size; i += boot_info_efi.efi_mmap_desc_size )
     {
-        const EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+        const EFI_MEMORY_DESCRIPTOR *desc = boot_info_efi.efi_mmap + i;
 
         if ( (desc->Attribute & EFI_MEMORY_RUNTIME) &&
              desc->VirtualStart != INVALID_VIRTUAL_ADDRESS &&
diff --git a/xen/arch/x86/efi/efi.h b/xen/arch/x86/efi/efi.h
index a80d5f1..46099de 100644
--- a/xen/arch/x86/efi/efi.h
+++ b/xen/arch/x86/efi/efi.h
@@ -25,9 +25,6 @@ extern const CHAR16 *efi_fw_vendor;
 
 extern EFI_RUNTIME_SERVICES *efi_rs;
 
-extern UINTN efi_memmap_size, efi_mdesc_size;
-extern void *efi_memmap;
-
 extern l4_pgentry_t *efi_l4_pgtable;
 
 extern const struct efi_pci_rom *efi_pci_roms;
diff --git a/xen/arch/x86/efi/runtime.c b/xen/arch/x86/efi/runtime.c
index 166852d..b56332f 100644
--- a/xen/arch/x86/efi/runtime.c
+++ b/xen/arch/x86/efi/runtime.c
@@ -5,6 +5,7 @@
 #include <xen/irq.h>
 #include <xen/time.h>
 #include <asm/mc146818rtc.h>
+#include <asm/boot_info.h>
 
 DEFINE_XEN_GUEST_HANDLE(CHAR16);
 
@@ -26,25 +27,50 @@ const CHAR16 *__read_mostly efi_fw_vendor;
 EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
 static DEFINE_SPINLOCK(efi_rs_lock);
 
-UINTN __read_mostly efi_memmap_size;
-UINTN __read_mostly efi_mdesc_size;
-void *__read_mostly efi_memmap;
-
 UINT64 __read_mostly efi_boot_max_var_store_size;
 UINT64 __read_mostly efi_boot_remain_var_store_size;
 UINT64 __read_mostly efi_boot_max_var_size;
 
-struct efi __read_mostly efi = {
-	.acpi   = EFI_INVALID_TABLE_ADDR,
-	.acpi20 = EFI_INVALID_TABLE_ADDR,
-	.mps    = EFI_INVALID_TABLE_ADDR,
-	.smbios = EFI_INVALID_TABLE_ADDR,
-};
-
 l4_pgentry_t *__read_mostly efi_l4_pgtable;
 
 const struct efi_pci_rom *__read_mostly efi_pci_roms;
 
+extern struct e820entry e820map[];
+extern struct edd_info boot_edd_info[];
+extern struct mbr_signature boot_mbr_signature[];
+
+extern unsigned char boot_edid_info[128];
+
+static boot_module_t __read_mostly boot_info_mods[3] = {};
+
+boot_info_t __read_mostly boot_info_efi = {
+    .boot_loader_name = "EFI",
+    .cmdline = NULL,
+    .mmap_type = "EFI",
+    .mem_upper = 0,
+    .e820map_nr = 0,
+    .e820map = e820map,
+    .efi_mmap_size = 0,
+    .efi_mmap_desc_size = 0,
+    .efi_mmap = NULL,
+    .mps = EFI_INVALID_TABLE_ADDR,
+    .acpi = EFI_INVALID_TABLE_ADDR,
+    .acpi20 = EFI_INVALID_TABLE_ADDR,
+    .smbios = EFI_INVALID_TABLE_ADDR,
+    .efi_system_table = NULL,
+    .vga_console_info = {},
+    .edid_caps = 0,
+    .edid_info = boot_edid_info,
+    .edd_info_nr = 0,
+    .edd_info = boot_edd_info,
+    .mbr_signature_nr = 0,
+    .mbr_signature = boot_mbr_signature,
+    .mods_nr = 0,
+    .mods = boot_info_mods,
+    .warn_msg = NULL,
+    .err_msg = NULL
+};
+
 unsigned long efi_rs_enter(void)
 {
     static const u16 fcw = FCW_DEFAULT;
@@ -173,9 +199,9 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
         }
         break;
     case XEN_FW_EFI_MEM_INFO:
-        for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+        for ( i = 0; i < boot_info->efi_mmap_size; i += boot_info->efi_mmap_desc_size )
         {
-            EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+            EFI_MEMORY_DESCRIPTOR *desc = boot_info->efi_mmap + i;
             u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
 
             if ( info->mem.addr >= desc->PhysicalStart &&
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 091d5d1..0293eba 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -40,8 +40,8 @@
 #include <asm/setup.h>
 #include <asm/microcode.h>
 
-static module_t __initdata ucode_mod;
-static void *(*__initdata ucode_mod_map)(const module_t *);
+static boot_module_t __initdata ucode_mod;
+static void *(*__initdata ucode_mod_map)(const boot_module_t *);
 static signed int __initdata ucode_mod_idx;
 static bool_t __initdata ucode_mod_forced;
 static cpumask_t __initdata init_mask;
@@ -94,10 +94,9 @@ custom_param("ucode", parse_ucode);
 
 void __init microcode_scan_module(
     unsigned long *module_map,
-    const multiboot_info_t *mbi,
-    void *(*bootmap)(const module_t *))
+    const boot_info_t *boot_info,
+    void *(*bootmap)(const boot_module_t *))
 {
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     uint64_t *_blob_start;
     unsigned long _blob_size;
     struct cpio_data cd;
@@ -119,13 +118,13 @@ void __init microcode_scan_module(
     /*
      * Try all modules and see whichever could be the microcode blob.
      */
-    for ( i = 1 /* Ignore dom0 kernel */; i < mbi->mods_count; i++ )
+    for ( i = 1 /* Ignore dom0 kernel */; i < boot_info->mods_nr; i++ )
     {
         if ( !test_bit(i, module_map) )
             continue;
 
-        _blob_start = bootmap(&mod[i]);
-        _blob_size = mod[i].mod_end;
+        _blob_start = bootmap(&boot_info->mods[i]);
+        _blob_size = boot_info->mods[i].end;
         if ( !_blob_start )
         {
             printk("Could not map multiboot module #%d (size: %ld)\n",
@@ -165,21 +164,19 @@ err:
 }
 void __init microcode_grab_module(
     unsigned long *module_map,
-    const multiboot_info_t *mbi,
-    void *(*map)(const module_t *))
+    const boot_info_t *boot_info,
+    void *(*map)(const boot_module_t *))
 {
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
-
     if ( ucode_mod_idx < 0 )
-        ucode_mod_idx += mbi->mods_count;
-    if ( ucode_mod_idx <= 0 || ucode_mod_idx >= mbi->mods_count ||
+        ucode_mod_idx += boot_info->mods_nr;
+    if ( ucode_mod_idx <= 0 || ucode_mod_idx >= boot_info->mods_nr ||
          !__test_and_clear_bit(ucode_mod_idx, module_map) )
         goto scan;
-    ucode_mod = mod[ucode_mod_idx];
+    ucode_mod = boot_info->mods[ucode_mod_idx];
     ucode_mod_map = map;
 scan:
     if ( ucode_scan )
-        microcode_scan_module(module_map, mbi, map);
+        microcode_scan_module(module_map, boot_info, map);
 }
 
 const struct microcode_ops *microcode_ops;
@@ -345,7 +342,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
 static void __init _do_microcode_update(unsigned long data)
 {
     void *_data = (void *)data;
-    size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.mod_end;
+    size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.end;
 
     microcode_update_cpu(_data, len);
     cpumask_set_cpu(smp_processor_id(), &init_mask);
@@ -360,7 +357,7 @@ static int __init microcode_init(void)
     if ( !microcode_ops )
         return 0;
 
-    if ( !ucode_mod.mod_end && !ucode_blob.size )
+    if ( !ucode_mod.end && !ucode_blob.size )
         return 0;
 
     data = ucode_blob.size ? ucode_blob.data : ucode_mod_map(&ucode_mod);
@@ -414,7 +411,7 @@ static int __init microcode_presmp_init(void)
 {
     if ( microcode_ops )
     {
-        if ( ucode_mod.mod_end || ucode_blob.size )
+        if ( ucode_mod.end || ucode_blob.size )
         {
             void *data;
             size_t len;
@@ -427,7 +424,7 @@ static int __init microcode_presmp_init(void)
             }
             else
             {
-                len = ucode_mod.mod_end;
+                len = ucode_mod.end;
                 data = ucode_mod_map(&ucode_mod);
             }
             if ( data )
@@ -447,7 +444,7 @@ static int __init microcode_presmp_init(void)
                     ucode_blob.data = NULL;
                 }
                 else
-                    ucode_mod.mod_end = 0;
+                    ucode_mod.end = 0;
             }
         }
 
diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
index a38e016..54eb357 100644
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -29,6 +29,7 @@
 #include <asm/mpspec.h>
 #include <asm/io_apic.h>
 #include <asm/setup.h>
+#include <asm/boot_info.h>
 
 #include <mach_apic.h>
 #include <mach_mpparse.h>
@@ -676,18 +677,18 @@ static void __init efi_check_config(void)
 {
 	struct intel_mp_floating *mpf;
 
-	if (efi.mps == EFI_INVALID_TABLE_ADDR)
+	if (boot_info->mps == EFI_INVALID_TABLE_ADDR)
 		return;
 
-	__set_fixmap(FIX_EFI_MPF, PFN_DOWN(efi.mps), __PAGE_HYPERVISOR);
-	mpf = (void *)fix_to_virt(FIX_EFI_MPF) + ((long)efi.mps & (PAGE_SIZE-1));
+	__set_fixmap(FIX_EFI_MPF, PFN_DOWN(boot_info->mps), __PAGE_HYPERVISOR);
+	mpf = (void *)fix_to_virt(FIX_EFI_MPF) + ((long)boot_info->mps & (PAGE_SIZE-1));
 
 	if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
 	    mpf->mpf_length == 1 &&
 	    mpf_checksum((void *)mpf, 16) &&
 	    (mpf->mpf_specification == 1 || mpf->mpf_specification == 4)) {
 		smp_found_config = 1;
-		printk(KERN_INFO "SMP MP-table at %08lx\n", efi.mps);
+		printk(KERN_INFO "SMP MP-table at %08lx\n", boot_info->mps);
 		mpf_found = mpf;
 	}
 	else
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 2162811..db31b09 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -30,6 +30,7 @@
 #include <asm/mtrr.h>
 #include <asm/io_apic.h>
 #include <asm/setup.h>
+#include <asm/boot_info.h>
 #include "cpu/mtrr/mtrr.h"
 #include <xsm/xsm.h>
 
@@ -208,10 +209,10 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
             u16 length;
 
             ret = -ESRCH;
-            if ( op->u.firmware_info.index >= bootsym(boot_edd_info_nr) )
+            if ( op->u.firmware_info.index >= boot_info->edd_info_nr )
                 break;
 
-            info = bootsym(boot_edd_info) + op->u.firmware_info.index;
+            info = boot_info->edd_info + op->u.firmware_info.index;
 
             /* Transfer the EDD info block. */
             ret = -EFAULT;
@@ -247,10 +248,10 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
             const struct mbr_signature *sig;
 
             ret = -ESRCH;
-            if ( op->u.firmware_info.index >= bootsym(boot_mbr_signature_nr) )
+            if ( op->u.firmware_info.index >= boot_info->mbr_signature_nr )
                 break;
 
-            sig = bootsym(boot_mbr_signature) + op->u.firmware_info.index;
+            sig = boot_info->mbr_signature + op->u.firmware_info.index;
 
             op->u.firmware_info.u.disk_mbr_signature.device = sig->device;
             op->u.firmware_info.u.disk_mbr_signature.mbr_signature =
@@ -265,13 +266,11 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
             ret = -ESRCH;
             if ( op->u.firmware_info.index != 0 )
                 break;
-            if ( *(u32 *)bootsym(boot_edid_info) == 0x13131313 )
+            if ( *(u32 *)boot_info->edid_info == 0x13131313 )
                 break;
 
-            op->u.firmware_info.u.vbeddc_info.capabilities =
-                bootsym(boot_edid_caps);
-            op->u.firmware_info.u.vbeddc_info.edid_transfer_time =
-                bootsym(boot_edid_caps) >> 8;
+            op->u.firmware_info.u.vbeddc_info.capabilities = boot_info->edid_caps;
+            op->u.firmware_info.u.vbeddc_info.edid_transfer_time = boot_info->edid_caps >> 8;
 
             ret = 0;
             if ( __copy_field_to_guest(u_xenpf_op, op, u.firmware_info.
@@ -279,7 +278,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
                  __copy_field_to_guest(u_xenpf_op, op, u.firmware_info.
                                        u.vbeddc_info.edid_transfer_time) ||
                  copy_to_compat(op->u.firmware_info.u.vbeddc_info.edid,
-                                bootsym(boot_edid_info), 128) )
+                                boot_info->edid_info, 128) )
                 ret = -EFAULT;
             break;
         case XEN_FW_EFI_INFO:
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 9391efa..00d30a5 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -12,7 +12,6 @@
 #include <xen/console.h>
 #include <xen/serial.h>
 #include <xen/trace.h>
-#include <xen/multiboot.h>
 #include <xen/domain_page.h>
 #include <xen/version.h>
 #include <xen/gdbstub.h>
@@ -21,7 +20,7 @@
 #include <xen/keyhandler.h>
 #include <xen/numa.h>
 #include <xen/rcupdate.h>
-#include <xen/vga.h>
+#include <xen/video.h>
 #include <xen/dmi.h>
 #include <xen/pfn.h>
 #include <xen/nodemask.h>
@@ -49,6 +48,7 @@
 #include <xen/cpu.h>
 #include <asm/nmi.h>
 #include <asm/alternative.h>
+#include <asm/boot_info.h>
 
 /* opt_nosmp: If true, secondary processors are ignored. */
 static bool_t __initdata opt_nosmp;
@@ -92,6 +92,8 @@ unsigned long __initdata highmem_start;
 size_param("highmem-start", highmem_start);
 #endif
 
+boot_info_t *boot_info;
+
 cpumask_t __read_mostly cpu_present_map;
 
 unsigned long __read_mostly xen_phys_start;
@@ -137,7 +139,7 @@ static void __init parse_acpi_param(char *s)
     }
 }
 
-static const module_t *__initdata initial_images;
+static const boot_module_t *__initdata initial_images;
 static unsigned int __initdata nr_initial_images;
 
 unsigned long __init initial_images_nrpages(void)
@@ -146,7 +148,7 @@ unsigned long __init initial_images_nrpages(void)
     unsigned int i;
 
     for ( nr = i = 0; i < nr_initial_images; ++i )
-        nr += PFN_UP(initial_images[i].mod_end);
+        nr += PFN_UP(initial_images[i].end);
 
     return nr;
 }
@@ -157,10 +159,10 @@ void __init discard_initial_images(void)
 
     for ( i = 0; i < nr_initial_images; ++i )
     {
-        uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
+        uint64_t start = (uint64_t)initial_images[i].start << PAGE_SHIFT;
 
         init_domheap_pages(start,
-                           start + PAGE_ALIGN(initial_images[i].mod_end));
+                           start + PAGE_ALIGN(initial_images[i].end));
     }
 
     nr_initial_images = 0;
@@ -261,14 +263,14 @@ static void __init normalise_cpu_order(void)
  * Ensure a given physical memory range is present in the bootstrap mappings.
  * Use superpage mappings to ensure that pagetable memory needn't be allocated.
  */
-static void *__init bootstrap_map(const module_t *mod)
+static void *__init bootstrap_map(const boot_module_t *mod)
 {
     static unsigned long __initdata map_cur = BOOTSTRAP_MAP_BASE;
     uint64_t start, end, mask = (1L << L2_PAGETABLE_SHIFT) - 1;
     void *ret;
 
     if ( system_state != SYS_STATE_early_boot )
-        return mod ? mfn_to_virt(mod->mod_start) : NULL;
+        return mod ? mfn_to_virt(mod->start) : NULL;
 
     if ( !mod )
     {
@@ -277,8 +279,8 @@ static void *__init bootstrap_map(const module_t *mod)
         return NULL;
     }
 
-    start = (uint64_t)mod->mod_start << PAGE_SHIFT;
-    end = start + mod->mod_end;
+    start = (uint64_t)mod->start << PAGE_SHIFT;
+    end = start + mod->end;
     if ( start >= end )
         return NULL;
 
@@ -308,25 +310,25 @@ static void *__init move_memory(
 
     while ( size )
     {
-        module_t mod;
+        boot_module_t mod;
         unsigned int soffs = src & mask;
         unsigned int doffs = dst & mask;
         unsigned int sz;
         void *d, *s;
 
-        mod.mod_start = (src - soffs) >> PAGE_SHIFT;
-        mod.mod_end = soffs + size;
-        if ( mod.mod_end > blksz )
-            mod.mod_end = blksz;
-        sz = mod.mod_end - soffs;
+        mod.start = (src - soffs) >> PAGE_SHIFT;
+        mod.end = soffs + size;
+        if ( mod.end > blksz )
+            mod.end = blksz;
+        sz = mod.end - soffs;
         s = bootstrap_map(&mod);
 
-        mod.mod_start = (dst - doffs) >> PAGE_SHIFT;
-        mod.mod_end = doffs + size;
-        if ( mod.mod_end > blksz )
-            mod.mod_end = blksz;
-        if ( sz > mod.mod_end - doffs )
-            sz = mod.mod_end - doffs;
+        mod.start = (dst - doffs) >> PAGE_SHIFT;
+        mod.end = doffs + size;
+        if ( mod.end > blksz )
+            mod.end = blksz;
+        if ( sz > mod.end - doffs )
+            sz = mod.end - doffs;
         d = bootstrap_map(&mod);
 
         memmove(d + doffs, s + soffs, sz);
@@ -345,7 +347,7 @@ static void *__init move_memory(
 }
 
 static uint64_t __init consider_modules(
-    uint64_t s, uint64_t e, uint32_t size, const module_t *mod,
+    uint64_t s, uint64_t e, uint32_t size, const boot_module_t *mod,
     unsigned int nr_mods, unsigned int this_mod)
 {
     unsigned int i;
@@ -355,8 +357,8 @@ static uint64_t __init consider_modules(
 
     for ( i = 0; i < nr_mods ; ++i )
     {
-        uint64_t start = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
-        uint64_t end = start + PAGE_ALIGN(mod[i].mod_end);
+        uint64_t start = (uint64_t)mod[i].start << PAGE_SHIFT;
+        uint64_t end = start + PAGE_ALIGN(mod[i].end);
 
         if ( i == this_mod )
             continue;
@@ -405,76 +407,6 @@ void set_pdx_range(unsigned long smfn, unsigned long emfn)
 /* A temporary copy of the e820 map that we can mess with during bootstrap. */
 static struct e820map __initdata boot_e820;
 
-struct boot_video_info {
-    u8  orig_x;             /* 0x00 */
-    u8  orig_y;             /* 0x01 */
-    u8  orig_video_mode;    /* 0x02 */
-    u8  orig_video_cols;    /* 0x03 */
-    u8  orig_video_lines;   /* 0x04 */
-    u8  orig_video_isVGA;   /* 0x05 */
-    u16 orig_video_points;  /* 0x06 */
-
-    /* VESA graphic mode -- linear frame buffer */
-    u32 capabilities;       /* 0x08 */
-    u16 lfb_linelength;     /* 0x0c */
-    u16 lfb_width;          /* 0x0e */
-    u16 lfb_height;         /* 0x10 */
-    u16 lfb_depth;          /* 0x12 */
-    u32 lfb_base;           /* 0x14 */
-    u32 lfb_size;           /* 0x18 */
-    u8  red_size;           /* 0x1c */
-    u8  red_pos;            /* 0x1d */
-    u8  green_size;         /* 0x1e */
-    u8  green_pos;          /* 0x1f */
-    u8  blue_size;          /* 0x20 */
-    u8  blue_pos;           /* 0x21 */
-    u8  rsvd_size;          /* 0x22 */
-    u8  rsvd_pos;           /* 0x23 */
-    u16 vesapm_seg;         /* 0x24 */
-    u16 vesapm_off;         /* 0x26 */
-    u16 vesa_attrib;        /* 0x28 */
-};
-extern struct boot_video_info boot_vid_info;
-
-static void __init parse_video_info(void)
-{
-    struct boot_video_info *bvi = &bootsym(boot_vid_info);
-
-    /* The EFI loader fills vga_console_info directly. */
-    if ( efi_enabled )
-        return;
-
-    if ( (bvi->orig_video_isVGA == 1) && (bvi->orig_video_mode == 3) )
-    {
-        vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
-        vga_console_info.u.text_mode_3.font_height = bvi->orig_video_points;
-        vga_console_info.u.text_mode_3.cursor_x = bvi->orig_x;
-        vga_console_info.u.text_mode_3.cursor_y = bvi->orig_y;
-        vga_console_info.u.text_mode_3.rows = bvi->orig_video_lines;
-        vga_console_info.u.text_mode_3.columns = bvi->orig_video_cols;
-    }
-    else if ( bvi->orig_video_isVGA == 0x23 )
-    {
-        vga_console_info.video_type = XEN_VGATYPE_VESA_LFB;
-        vga_console_info.u.vesa_lfb.width = bvi->lfb_width;
-        vga_console_info.u.vesa_lfb.height = bvi->lfb_height;
-        vga_console_info.u.vesa_lfb.bytes_per_line = bvi->lfb_linelength;
-        vga_console_info.u.vesa_lfb.bits_per_pixel = bvi->lfb_depth;
-        vga_console_info.u.vesa_lfb.lfb_base = bvi->lfb_base;
-        vga_console_info.u.vesa_lfb.lfb_size = bvi->lfb_size;
-        vga_console_info.u.vesa_lfb.red_pos = bvi->red_pos;
-        vga_console_info.u.vesa_lfb.red_size = bvi->red_size;
-        vga_console_info.u.vesa_lfb.green_pos = bvi->green_pos;
-        vga_console_info.u.vesa_lfb.green_size = bvi->green_size;
-        vga_console_info.u.vesa_lfb.blue_pos = bvi->blue_pos;
-        vga_console_info.u.vesa_lfb.blue_size = bvi->blue_size;
-        vga_console_info.u.vesa_lfb.rsvd_pos = bvi->rsvd_pos;
-        vga_console_info.u.vesa_lfb.rsvd_size = bvi->rsvd_size;
-        vga_console_info.u.vesa_lfb.gbl_caps = bvi->capabilities;
-        vga_console_info.u.vesa_lfb.mode_attrs = bvi->vesa_attrib;
-    }
-}
-
 static void __init kexec_reserve_area(struct e820map *e820)
 {
     unsigned long kdump_start = kexec_crash_area.start;
@@ -558,15 +490,12 @@ void __init enable_exception_support(void)
     /* Full exception support from here on in. */
 }
 
-void __init noreturn __start_xen(unsigned long mbi_p)
+void __init noreturn __start_xen(boot_info_t *boot_info_start)
 {
-    char *memmap_type = NULL;
-    char *cmdline, *kextra, *loader;
+    char *cmdline, *kextra;
     unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity;
-    multiboot_info_t *mbi = __va(mbi_p);
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     unsigned long nr_pages, raw_max_page, modules_headroom, *module_map;
-    int i, j, e820_warn = 0, bytes = 0;
+    int i, j;
     bool_t acpi_boot_table_init_done = 0;
     struct domain *dom0;
     struct ns16550_defaults ns16550 = {
@@ -577,19 +506,19 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     if ( !efi_enabled ) {
         /* Exception support was enabled before __start_xen() call. */
+
+        boot_info = boot_info_start;
     }
     else
     {
         enable_exception_support();
-    }
 
-    loader = (mbi->flags & MBI_LOADERNAME)
-        ? (char *)__va(mbi->boot_loader_name) : "unknown";
+        boot_info = __va(boot_info_start);
+        boot_info->cmdline = __va(boot_info->cmdline);
+    }
 
     /* Parse the command-line options. */
-    cmdline = cmdline_cook((mbi->flags & MBI_CMDLINE) ?
-                           __va(mbi->cmdline) : NULL,
-                           loader);
+    cmdline = cmdline_cook(boot_info->cmdline, boot_info->boot_loader_name);
     if ( (kextra = strstr(cmdline, " -- ")) != NULL )
     {
         /*
@@ -607,8 +536,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
      * allocing any xenheap structures wanted in lower memory. */
     kexec_early_calculations();
 
-    parse_video_info();
-
     if ( cpu_has_efer )
         rdmsrl(MSR_EFER, this_cpu(efer));
     asm volatile ( "mov %%cr4,%0" : "=r" (this_cpu(cr4)) );
@@ -623,27 +550,33 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     ehci_dbgp_init();
     console_init_preirq();
 
-    printk("Bootloader: %s\n", loader);
+    if ( boot_info->err_msg )
+        panic(boot_info->err_msg);
+
+    if ( boot_info->warn_msg )
+        printk(boot_info->warn_msg);
+
+    printk("Bootloader: %s\n", boot_info->boot_loader_name);
 
-    printk("Command line: %s\n", cmdline);
+    printk("Command line: %s\n", boot_info->cmdline ? boot_info->cmdline : "NONE");
 
     printk("Video information:\n");
 
     /* Print VGA display mode information. */
-    switch ( vga_console_info.video_type )
+    switch ( boot_info->vga_console_info.video_type )
     {
     case XEN_VGATYPE_TEXT_MODE_3:
         printk(" VGA is text mode %dx%d, font 8x%d\n",
-               vga_console_info.u.text_mode_3.columns,
-               vga_console_info.u.text_mode_3.rows,
-               vga_console_info.u.text_mode_3.font_height);
+               boot_info->vga_console_info.u.text_mode_3.columns,
+               boot_info->vga_console_info.u.text_mode_3.rows,
+               boot_info->vga_console_info.u.text_mode_3.font_height);
         break;
     case XEN_VGATYPE_VESA_LFB:
     case XEN_VGATYPE_EFI_LFB:
         printk(" VGA is graphics mode %dx%d, %d bpp\n",
-               vga_console_info.u.vesa_lfb.width,
-               vga_console_info.u.vesa_lfb.height,
-               vga_console_info.u.vesa_lfb.bits_per_pixel);
+               boot_info->vga_console_info.u.vesa_lfb.width,
+               boot_info->vga_console_info.u.vesa_lfb.height,
+               boot_info->vga_console_info.u.vesa_lfb.bits_per_pixel);
         break;
     default:
         printk(" No VGA detected\n");
@@ -651,15 +584,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     }
 
     /* Print VBE/DDC EDID information. */
-    if ( bootsym(boot_edid_caps) != 0x1313 )
+    if ( boot_info->edid_caps != 0x1313 )
     {
-        u16 caps = bootsym(boot_edid_caps);
+        u16 caps = boot_info->edid_caps;
         printk(" VBE/DDC methods:%s%s%s; ",
                (caps & 1) ? " V1" : "",
                (caps & 2) ? " V2" : "",
                !(caps & 3) ? " none" : "");
         printk("EDID transfer time: %d seconds\n", caps >> 8);
-        if ( *(u32 *)bootsym(boot_edid_info) == 0x13131313 )
+        if ( *(u32 *)boot_info->edid_info == 0x13131313 )
         {
             printk(" EDID info not retrieved because ");
             if ( !(caps & 3) )
@@ -672,13 +605,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     }
 
     printk("Disc information:\n");
-    printk(" Found %d MBR signatures\n",
-           bootsym(boot_mbr_signature_nr));
-    printk(" Found %d EDD information structures\n",
-           bootsym(boot_edd_info_nr));
+    printk(" Found %d MBR signatures\n", boot_info->mbr_signature_nr);
+    printk(" Found %d EDD information structures\n", boot_info->edd_info_nr);
 
     /* Check that we have at least one Multiboot module. */
-    if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
+    if ( !boot_info->mods_nr )
         panic("dom0 kernel not specified. Check bootloader configuration.");
 
     if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
@@ -696,77 +627,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         /* Make boot page tables match non-EFI boot. */
         l3_bootmap[l3_table_offset(BOOTSTRAP_MAP_BASE)] =
             l3e_from_paddr(__pa(l2_bootmap), __PAGE_HYPERVISOR);
-
-        memmap_type = loader;
     }
-    else if ( e820_raw_nr != 0 )
-    {
-        memmap_type = "Xen-e820";
-    }
-    else if ( mbi->flags & MBI_MEMMAP )
-    {
-        memmap_type = "Multiboot-e820";
-        while ( (bytes < mbi->mmap_length) && (e820_raw_nr < E820MAX) )
-        {
-            memory_map_t *map = __va(mbi->mmap_addr + bytes);
-
-            /*
-             * This is a gross workaround for a BIOS bug. Some bootloaders do
-             * not write e820 map entries into pre-zeroed memory. This is
-             * okay if the BIOS fills in all fields of the map entry, but
-             * some broken BIOSes do not bother to write the high word of
-             * the length field if the length is smaller than 4GB. We
-             * detect and fix this by flagging sections below 4GB that
-             * appear to be larger than 4GB in size.
-             */
-            if ( (map->base_addr_high == 0) && (map->length_high != 0) )
-            {
-                if ( !e820_warn )
-                {
-                    printk("WARNING: Buggy e820 map detected and fixed "
-                           "(truncated length fields).\n");
-                    e820_warn = 1;
-                }
-                map->length_high = 0;
-            }
-
-            e820_raw[e820_raw_nr].addr = 
-                ((u64)map->base_addr_high << 32) | (u64)map->base_addr_low;
-            e820_raw[e820_raw_nr].size = 
-                ((u64)map->length_high << 32) | (u64)map->length_low;
-            e820_raw[e820_raw_nr].type = map->type;
-            e820_raw_nr++;
-
-            bytes += map->size + 4;
-        }
-    }
-    else if ( bootsym(lowmem_kb) )
-    {
-        memmap_type = "Xen-e801";
-        e820_raw[0].addr = 0;
-        e820_raw[0].size = bootsym(lowmem_kb) << 10;
-        e820_raw[0].type = E820_RAM;
-        e820_raw[1].addr = 0x100000;
-        e820_raw[1].size = bootsym(highmem_kb) << 10;
-        e820_raw[1].type = E820_RAM;
-        e820_raw_nr = 2;
-    }
-    else if ( mbi->flags & MBI_MEMLIMITS )
-    {
-        memmap_type = "Multiboot-e801";
-        e820_raw[0].addr = 0;
-        e820_raw[0].size = mbi->mem_lower << 10;
-        e820_raw[0].type = E820_RAM;
-        e820_raw[1].addr = 0x100000;
-        e820_raw[1].size = mbi->mem_upper << 10;
-        e820_raw[1].type = E820_RAM;
-        e820_raw_nr = 2;
-    }
-    else
-        panic("Bootloader provided no memory information.");
 
     /* Sanitise the raw E820 map to produce a final clean version. */
-    max_page = raw_max_page = init_e820(memmap_type, e820_raw, &e820_raw_nr);
+    max_page = raw_max_page = init_e820(boot_info->mmap_type, boot_info->e820map, &boot_info->e820map_nr);
 
     /* Create a temporary copy of the E820 map. */
     memcpy(&boot_e820, &e820, sizeof(e820));
@@ -779,8 +643,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
     kexec_reserve_area(&boot_e820);
 
-    initial_images = mod;
-    nr_initial_images = mbi->mods_count;
+    nr_initial_images = boot_info->mods_nr;
+    initial_images = boot_info->mods;
 
     /*
      * Iterate backwards over all superpage-aligned RAM regions.
@@ -795,16 +659,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
      * we can relocate the dom0 kernel and other multiboot modules. Also, on
      * x86/64, we relocate Xen to higher memory.
      */
-    for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
+    for ( i = 0; !efi_enabled && i < boot_info->mods_nr; i++ )
     {
-        if ( mod[i].mod_start & (PAGE_SIZE - 1) )
+        if ( boot_info->mods[i].start & (PAGE_SIZE - 1) )
             panic("Bootloader didn't honor module alignment request.");
-        mod[i].mod_end -= mod[i].mod_start;
-        mod[i].mod_start >>= PAGE_SHIFT;
-        mod[i].reserved = 0;
+        boot_info->mods[i].end -= boot_info->mods[i].start;
+        boot_info->mods[i].start >>= PAGE_SHIFT;
     }
 
-    modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
+    modules_headroom = bzimage_headroom(bootstrap_map(boot_info->mods), boot_info->mods->end);
     bootstrap_map(NULL);
 
 #ifndef highmem_start
@@ -845,7 +708,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         {
             /* Don't overlap with modules. */
             end = consider_modules(s, e, reloc_size + mask,
-                                   mod, mbi->mods_count, -1);
+                                   boot_info->mods, boot_info->mods_nr, -1);
             end &= ~mask;
         }
         else
@@ -933,36 +796,36 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         }
 
         /* Is the region suitable for relocating the multiboot modules? */
-        for ( j = mbi->mods_count - 1; j >= 0; j-- )
+        for ( j = boot_info->mods_nr - 1; j >= 0; j-- )
         {
             unsigned long headroom = j ? 0 : modules_headroom;
-            unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end);
+            unsigned long size = PAGE_ALIGN(headroom + boot_info->mods[j].end);
 
-            if ( mod[j].reserved )
+            if ( boot_info->mods[j].relocated )
                 continue;
 
             /* Don't overlap with other modules. */
-            end = consider_modules(s, e, size, mod, mbi->mods_count, j);
+            end = consider_modules(s, e, size, boot_info->mods, boot_info->mods_nr, j);
 
             if ( highmem_start && end > highmem_start )
                 continue;
 
             if ( s < end &&
                  (headroom ||
-                  ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) )
+                  ((end - size) >> PAGE_SHIFT) > boot_info->mods[j].start) )
             {
                 move_memory(end - size + headroom,
-                            (uint64_t)mod[j].mod_start << PAGE_SHIFT,
-                            mod[j].mod_end, 0);
-                mod[j].mod_start = (end - size) >> PAGE_SHIFT;
-                mod[j].mod_end += headroom;
-                mod[j].reserved = 1;
+                            (uint64_t)boot_info->mods[j].start << PAGE_SHIFT,
+                            boot_info->mods[j].end, 0);
+                boot_info->mods[j].start = (end - size) >> PAGE_SHIFT;
+                boot_info->mods[j].end += headroom;
+                boot_info->mods[j].relocated = 1;
             }
         }
 
         /* Don't overlap with modules. */
         e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
-                             mod, mbi->mods_count, -1);
+                             boot_info->mods, boot_info->mods_nr, -1);
         if ( !kexec_crash_area.start && (s < e) )
         {
             e = (e - kexec_crash_area.size) & PAGE_MASK;
@@ -970,18 +833,18 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         }
     }
 
-    if ( modules_headroom && !mod->reserved )
+    if ( modules_headroom && !boot_info->mods->relocated )
         panic("Not enough memory to relocate the dom0 kernel image.");
-    for ( i = 0; i < mbi->mods_count; ++i )
+    for ( i = 0; i < boot_info->mods_nr; ++i )
     {
-        uint64_t s = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
+        uint64_t s = (uint64_t)boot_info->mods[i].start << PAGE_SHIFT;
 
-        reserve_e820_ram(&boot_e820, s, s + PAGE_ALIGN(mod[i].mod_end));
+        reserve_e820_ram(&boot_e820, s, s + PAGE_ALIGN(boot_info->mods[i].end));
     }
 
     if ( !xen_phys_start )
         panic("Not enough memory to relocate Xen.");
-    reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper : __pa(&_start),
+    reserve_e820_ram(&boot_e820, efi_enabled ? boot_info->mem_upper : __pa(&_start),
                      __pa(&_end));
 
     /* Late kexec reservation (dynamic start address). */
@@ -1027,10 +890,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
                     ASSERT(j);
                 }
                 map_e = boot_e820.map[j].addr + boot_e820.map[j].size;
-                for ( j = 0; j < mbi->mods_count; ++j )
+                for ( j = 0; j < boot_info->mods_nr; ++j )
                 {
-                    uint64_t end = pfn_to_paddr(mod[j].mod_start) +
-                                   mod[j].mod_end;
+                    uint64_t end = pfn_to_paddr(boot_info->mods[j].start) +
+                                   boot_info->mods[j].end;
 
                     if ( map_e < end )
                         map_e = end;
@@ -1103,13 +966,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         }
     }
 
-    for ( i = 0; i < mbi->mods_count; ++i )
+    for ( i = 0; i < boot_info->mods_nr; ++i )
     {
-        set_pdx_range(mod[i].mod_start,
-                      mod[i].mod_start + PFN_UP(mod[i].mod_end));
-        map_pages_to_xen((unsigned long)mfn_to_virt(mod[i].mod_start),
-                         mod[i].mod_start,
-                         PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
+        set_pdx_range(boot_info->mods[i].start,
+                      boot_info->mods[i].start + PFN_UP(boot_info->mods[i].end));
+        map_pages_to_xen((unsigned long)mfn_to_virt(boot_info->mods[i].start),
+                         boot_info->mods[i].start,
+                         PFN_UP(boot_info->mods[i].end), PAGE_HYPERVISOR);
     }
 
     if ( kexec_crash_area.size )
@@ -1263,13 +1126,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     init_IRQ();
 
-    module_map = xmalloc_array(unsigned long, BITS_TO_LONGS(mbi->mods_count));
-    bitmap_fill(module_map, mbi->mods_count);
+    module_map = xmalloc_array(unsigned long, BITS_TO_LONGS(boot_info->mods_nr));
+    bitmap_fill(module_map, boot_info->mods_nr);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
 
-    xsm_multiboot_init(module_map, mbi, bootstrap_map);
+    xsm_multiboot_init(module_map, boot_info, bootstrap_map);
 
-    microcode_grab_module(module_map, mbi, bootstrap_map);
+    microcode_grab_module(module_map, boot_info, bootstrap_map);
 
     timer_init();
 
@@ -1374,12 +1237,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     dom0->target = NULL;
 
     /* Grab the DOM0 command line. */
-    cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
+    cmdline = (char *)(boot_info->mods[0].cmdline ? __va(boot_info->mods[0].cmdline) : NULL);
     if ( (cmdline != NULL) || (kextra != NULL) )
     {
         static char __initdata dom0_cmdline[MAX_GUEST_CMDLINE];
 
-        cmdline = cmdline_cook(cmdline, loader);
+        cmdline = cmdline_cook(cmdline, boot_info->boot_loader_name);
         safe_strcpy(dom0_cmdline, cmdline);
 
         if ( kextra != NULL )
@@ -1406,8 +1269,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     if ( xen_cpuidle )
         xen_processor_pmbits |= XEN_PROCESSOR_PM_CX;
 
-    initrdidx = find_first_bit(module_map, mbi->mods_count);
-    if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
+    initrdidx = find_first_bit(module_map, boot_info->mods_nr);
+    if ( bitmap_weight(module_map, boot_info->mods_nr) > 1 )
         printk(XENLOG_WARNING
                "Multiple initrd candidates, picking module #%u\n",
                initrdidx);
@@ -1424,9 +1287,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
      * We're going to setup domain0 using the module(s) that we stashed safely
      * above our heap. The second module, if present, is an initrd ramdisk.
      */
-    if ( construct_dom0(dom0, mod, modules_headroom,
-                        (initrdidx > 0) && (initrdidx < mbi->mods_count)
-                        ? mod + initrdidx : NULL,
+    if ( construct_dom0(dom0, boot_info->mods, modules_headroom,
+                        (initrdidx > 0) && (initrdidx < boot_info->mods_nr)
+                        ? boot_info->mods + initrdidx : NULL,
                         bootstrap_map, cmdline) != 0)
         panic("Could not set up DOM0 guest OS");
 
diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
index 93c983c..e328566 100644
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -39,6 +39,7 @@
 #include <xen/domain_page.h>
 #include <xen/efi.h>
 #include <xen/vmap.h>
+#include <asm/boot_info.h>
 
 #define _COMPONENT		ACPI_OS_SERVICES
 ACPI_MODULE_NAME("osl")
@@ -67,10 +68,10 @@ void __init acpi_os_vprintf(const char *fmt, va_list args)
 acpi_physical_address __init acpi_os_get_root_pointer(void)
 {
 	if (efi_enabled) {
-		if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
-			return efi.acpi20;
-		else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
-			return efi.acpi;
+		if (boot_info->acpi20 != EFI_INVALID_TABLE_ADDR)
+			return boot_info->acpi20;
+		else if (boot_info->acpi != EFI_INVALID_TABLE_ADDR)
+			return boot_info->acpi;
 		else {
 			printk(KERN_ERR PREFIX
 			       "System description tables not found\n");
diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 575db62..e95c157 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -9,13 +9,14 @@
 #include <xen/lib.h>
 #include <xen/xmalloc.h>
 #include <xen/kernel.h>
-#include <xen/vga.h>
+#include <xen/video.h>
 #include <asm/io.h>
 #include <asm/page.h>
+#include <asm/boot_info.h>
 #include "font.h"
 #include "lfb.h"
 
-#define vlfb_info    vga_console_info.u.vesa_lfb
+#define vlfb_info    (boot_info->vga_console_info.u.vesa_lfb)
 
 static void lfb_flush(void);
 
@@ -43,7 +44,7 @@ void __init vesa_early_init(void)
 {
     unsigned int vram_vmode;
 
-    vga_compat = !(vga_console_info.u.vesa_lfb.gbl_caps & 2);
+    vga_compat = !(boot_info->vga_console_info.u.vesa_lfb.gbl_caps & 2);
 
     if ( (vlfb_info.bits_per_pixel < 8) || (vlfb_info.bits_per_pixel > 32) )
         return;
diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c
index 40e5963..608d92b 100644
--- a/xen/drivers/video/vga.c
+++ b/xen/drivers/video/vga.c
@@ -8,12 +8,10 @@
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/mm.h>
-#include <xen/vga.h>
+#include <xen/video.h>
 #include <xen/pci.h>
 #include <asm/io.h>
-
-/* Filled in by arch boot code. */
-struct xen_vga_console_info vga_console_info;
+#include <asm/boot_info.h>
 
 static int vgacon_keep;
 static unsigned int xpos, ypos;
@@ -75,15 +73,15 @@ void __init video_init(void)
             vgacon_keep = 1;
     }
 
-    switch ( vga_console_info.video_type )
+    switch ( boot_info->vga_console_info.video_type )
     {
     case XEN_VGATYPE_TEXT_MODE_3:
         if ( page_is_ram_type(paddr_to_pfn(0xB8000), RAM_TYPE_CONVENTIONAL) ||
              ((video = ioremap(0xB8000, 0x8000)) == NULL) )
             return;
         outw(0x200a, 0x3d4); /* disable cursor */
-        columns = vga_console_info.u.text_mode_3.columns;
-        lines   = vga_console_info.u.text_mode_3.rows;
+        columns = boot_info->vga_console_info.u.text_mode_3.columns;
+        lines   = boot_info->vga_console_info.u.text_mode_3.rows;
         memset(video, 0, columns * lines * 2);
         video_puts = vga_text_puts;
         break;
@@ -92,7 +90,7 @@ void __init video_init(void)
         vesa_early_init();
         break;
     default:
-        memset(&vga_console_info, 0, sizeof(vga_console_info));
+        memset(&boot_info->vga_console_info, 0, sizeof(boot_info->vga_console_info));
         break;
     }
 }
@@ -163,7 +161,7 @@ void __init video_endboot(void)
             }
     }
 
-    switch ( vga_console_info.video_type )
+    switch ( boot_info->vga_console_info.video_type )
     {
     case XEN_VGATYPE_TEXT_MODE_3:
         if ( !vgacon_keep )
@@ -206,6 +204,6 @@ static void vga_text_puts(const char *s)
 
 int __init fill_console_start_info(struct dom0_vga_console_info *ci)
 {
-    memcpy(ci, &vga_console_info, sizeof(*ci));
+    memcpy(ci, &boot_info->vga_console_info, sizeof(*ci));
     return 1;
 }
diff --git a/xen/include/asm-x86/boot_info.h b/xen/include/asm-x86/boot_info.h
new file mode 100644
index 0000000..2559753
--- /dev/null
+++ b/xen/include/asm-x86/boot_info.h
@@ -0,0 +1,126 @@
+/*
+ * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __BOOT_INFO_H__
+#define __BOOT_INFO_H__
+
+#include <xen/types.h>
+#include <xen/video.h>
+
+#include <asm/e820.h>
+#include <asm/edd.h>
+#include <asm/mbd.h>
+
+/*
+ * Define boot_info type. It will be used to define variable which in turn
+ * will store data collected by bootloader and preloader. This way it will
+ * be possible to make most of Xen code bootloader agnostic.
+ *
+ * Some members should have relevant EFI/ACPI types. However, due to type
+ * conflicts among ACPI and EFI headers it is not possible to use required
+ * EFI/ACPI types here. Instead of them there are simple types in use which
+ * are compatible as much as possible with relevant EFI/ACPI types.
+ */
+typedef struct {
+    /* Boot loader name. */
+    char *boot_loader_name;
+
+    /* Xen command line. */
+    char *cmdline;
+
+    /* Memory map type (source of memory map). */
+    char *mmap_type;
+
+    /*
+     * Amount of upper memory (in KiB) accordingly to The Multiboot
+     * Specification version 0.6.96.
+     */
+    u32 mem_upper;
+
+    /* Number of memory map entries provided by Xen preloader. */
+    unsigned int e820map_nr;
+
+    /*
+     * Memory map provided by Xen preloader. It should always point
+     * to an area able to accommodate at least E820MAX entries.
+     */
+    struct e820entry *e820map;
+
+    /* Size (in bytes) of EFI memory map provided by Xen preloader. */
+    size_t efi_mmap_size;
+
+    /* Size (in bytes) of EFI memory map descriptor provided by Xen preloader. */
+    size_t efi_mmap_desc_size;
+
+    /* Pointer to EFI memory map provided by preloader. */
+    void *efi_mmap;
+
+    /* Pointer to MPS. */
+    paddr_t mps;
+
+    /* Pointer to ACPI RSDP. */
+    paddr_t acpi;
+
+    /* Pointer to ACPI 2.0 RSDP. */
+    paddr_t acpi20;
+
+    /* Pointer to SMBIOS. */
+    paddr_t smbios;
+
+    /* Pointer to EFI System Table. */
+    void *efi_system_table;
+
+    /* VGA console info. */
+    struct xen_vga_console_info vga_console_info;
+
+    /* EDID info. */
+    unsigned short edid_caps;
+    unsigned char *edid_info;
+
+    /* Number of EDD entries provided by Xen preloader. */
+    u8 edd_info_nr;
+
+    /* Pointer to EDD info. */
+    struct edd_info *edd_info;
+
+    /* Number of MBR entries provided by Xen preloader. */
+    u8 mbr_signature_nr;
+
+    /* Pointer to MBR info. */
+    struct mbr_signature *mbr_signature;
+
+    /* Number of modules. */
+    unsigned int mods_nr;
+
+    /* Pointer to modules description. */
+    boot_module_t *mods;
+
+    /*
+     * Info about warning occurred during boot_info initialization.
+     * NULL if everything went OK.
+     */
+    char *warn_msg;
+
+    /*
+     * Info about error occurred during boot_info initialization. NULL if everything
+     * went OK. Otherwise boot_info is not fully/properly initialized.
+     */
+    char *err_msg;
+} boot_info_t;
+
+extern boot_info_t *boot_info;
+#endif /* __BOOT_INFO_H__ */
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 210ff57..ae68322 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -119,8 +119,6 @@ extern unsigned int trampoline_xen_phys_start;
 extern unsigned char trampoline_cpu_started;
 extern char wakeup_start[];
 extern unsigned int video_mode, video_flags;
-extern unsigned short boot_edid_caps;
-extern unsigned char boot_edid_info[128];
 #endif
 
 #define asmlinkage
diff --git a/xen/include/asm-x86/e820.h b/xen/include/asm-x86/e820.h
index d9ff4eb..8727afb 100644
--- a/xen/include/asm-x86/e820.h
+++ b/xen/include/asm-x86/e820.h
@@ -33,12 +33,4 @@ extern int e820_add_range(
 extern unsigned long init_e820(const char *, struct e820entry *, unsigned int *);
 extern struct e820map e820;
 
-/* These symbols live in the boot trampoline. */
-extern struct e820entry e820map[];
-extern unsigned int e820nr;
-extern unsigned int lowmem_kb, highmem_kb;
-
-#define e820_raw bootsym(e820map)
-#define e820_raw_nr bootsym(e820nr)
-
 #endif /*__E820_HEADER*/
diff --git a/xen/include/asm-x86/edd.h b/xen/include/asm-x86/edd.h
index afaa237..e8361a5 100644
--- a/xen/include/asm-x86/edd.h
+++ b/xen/include/asm-x86/edd.h
@@ -143,12 +143,6 @@ struct __packed mbr_signature {
     u32 signature;
 };
 
-/* These all reside in the boot trampoline. Access via bootsym(). */
-extern struct mbr_signature boot_mbr_signature[];
-extern u8 boot_mbr_signature_nr;
-extern struct edd_info boot_edd_info[];
-extern u8 boot_edd_info_nr;
-
 #endif /* __ASSEMBLY__ */
 
 /* Maximum number of EDD information structures at boot_edd_info. */
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 8f8c6f3..75b5c5a 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -1,7 +1,7 @@
 #ifndef __X86_SETUP_H_
 #define __X86_SETUP_H_
 
-#include <xen/multiboot.h>
+#include <asm/boot_info.h>
 
 extern unsigned long xenheap_initial_phys_start;
 
@@ -27,9 +27,9 @@ void vesa_mtrr_init(void);
 
 int construct_dom0(
     struct domain *d,
-    const module_t *kernel, unsigned long kernel_headroom,
-    module_t *initrd,
-    void *(*bootstrap_map)(const module_t *),
+    const boot_module_t *kernel, unsigned long kernel_headroom,
+    boot_module_t *initrd,
+    void *(*bootstrap_map)(const boot_module_t *),
     char *cmdline);
 
 unsigned long initial_images_nrpages(void);
@@ -38,7 +38,7 @@ void discard_initial_images(void);
 int xen_in_range(unsigned long mfn);
 
 void microcode_grab_module(
-    unsigned long *, const multiboot_info_t *, void *(*)(const module_t *));
+    unsigned long *, const boot_info_t *, void *(*)(const boot_module_t *));
 
 extern uint8_t kbd_shift_flags;
 
diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
index 8a2b788..64d2dea 100644
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -9,16 +9,6 @@ extern const bool_t efi_enabled;
 
 #define EFI_INVALID_TABLE_ADDR (~0UL)
 
-/* Add fields here only if they need to be referenced from non-EFI code. */
-struct efi {
-    unsigned long mps;          /* MPS table */
-    unsigned long acpi;         /* ACPI table (IA64 ext 0.71) */
-    unsigned long acpi20;       /* ACPI table (ACPI 2.0) */
-    unsigned long smbios;       /* SM BIOS table */
-};
-
-extern struct efi efi;
-
 #ifndef __ASSEMBLY__
 
 union xenpf_efi_info;
diff --git a/xen/include/xen/vga.h b/xen/include/xen/vga.h
deleted file mode 100644
index f72b63d..0000000
--- a/xen/include/xen/vga.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- *  vga.h
- *
- *  This file is subject to the terms and conditions of the GNU General Public
- *  License.  See the file COPYING in the main directory of this archive
- *  for more details.
- */
-
-#ifndef _XEN_VGA_H
-#define _XEN_VGA_H
-
-#include <xen/video.h>
-
-#ifdef CONFIG_VGA
-extern struct xen_vga_console_info vga_console_info;
-#endif
-
-#endif /* _XEN_VGA_H */
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 6c1c079..7e333bd 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -16,7 +16,7 @@
 #define __XSM_H__
 
 #include <xen/sched.h>
-#include <xen/multiboot.h>
+#include <asm/boot_info.h>
 
 typedef void xsm_op_t;
 DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
@@ -666,11 +666,11 @@ static inline int xsm_ioport_mapping (xsm_default_t def, struct domain *d, uint3
 
 #ifdef CONFIG_MULTIBOOT
 extern int xsm_multiboot_init(unsigned long *module_map,
-                              const multiboot_info_t *mbi,
-                              void *(*bootstrap_map)(const module_t *));
+                              const boot_info_t *boot_info,
+                              void *(*bootstrap_map)(const boot_module_t *));
 extern int xsm_multiboot_policy_init(unsigned long *module_map,
-                                     const multiboot_info_t *mbi,
-                                     void *(*bootstrap_map)(const module_t *));
+                                     const boot_info_t *boot_info,
+                                     void *(*bootstrap_map)(const boot_module_t *));
 #endif
 
 #ifdef HAS_DEVICE_TREE
@@ -690,8 +690,8 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #ifdef CONFIG_MULTIBOOT
 static inline int xsm_multiboot_init (unsigned long *module_map,
-                                      const multiboot_info_t *mbi,
-                                      void *(*bootstrap_map)(const module_t *))
+                                      const boot_info_t *boot_info,
+                                      void *(*bootstrap_map)(const boot_module_t *))
 {
     return 0;
 }
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 0ac6d03..39b7ff6 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -60,8 +60,8 @@ static int __init xsm_core_init(void)
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_init(unsigned long *module_map,
-                              const multiboot_info_t *mbi,
-                              void *(*bootstrap_map)(const module_t *))
+                              const boot_info_t *boot_info,
+                              void *(*bootstrap_map)(const boot_module_t *))
 {
     int ret = 0;
 
@@ -69,7 +69,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
+        ret = xsm_multiboot_policy_init(module_map, boot_info, bootstrap_map);
         if ( ret )
         {
             bootstrap_map(NULL);
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 6e0bb78..46c694b 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -19,9 +19,6 @@
  */
 
 #include <xsm/xsm.h>
-#ifdef CONFIG_MULTIBOOT
-#include <xen/multiboot.h>
-#endif
 #include <xen/bitops.h>
 #ifdef HAS_DEVICE_TREE
 # include <asm/setup.h>
@@ -33,11 +30,10 @@ u32 __initdata policy_size = 0;
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(unsigned long *module_map,
-                                     const multiboot_info_t *mbi,
-                                     void *(*bootstrap_map)(const module_t *))
+                                     const boot_info_t *boot_info,
+                                     void *(*bootstrap_map)(const boot_module_t *))
 {
     int i;
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     int rc = 0;
     u32 *_policy_start;
     unsigned long _policy_len;
@@ -46,13 +42,13 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
      * Try all modules and see whichever could be the binary policy.
      * Adjust module_map for the module that is the binary policy.
      */
-    for ( i = mbi->mods_count-1; i >= 1; i-- )
+    for ( i = boot_info->mods_nr - 1; i >= 1; i-- )
     {
         if ( !test_bit(i, module_map) )
             continue;
 
-        _policy_start = bootstrap_map(mod + i);
-        _policy_len   = mod[i].mod_end;
+        _policy_start = bootstrap_map(boot_info->mods + i);
+        _policy_len   = boot_info->mods[i].end;
 
         if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
         {
-- 
1.7.10.4

Re: [PATCH v2 3/5] xen/x86: Migrate to boot_info structure

[Andrew Cooper]

On 24/09/14 18:19, Daniel Kiper wrote:
> Break multiboot (v1) protocol dependency. It means that most of Xen code
> (excluding preloader) could be bootloader agnostic and does not need almost
> any knowledge about boot protocol. Additionally, we are able to pass all boot
> data to __start_xen() in one bucket without any side channels. I do not mention
> that we are also able to easily identify boot data in Xen code.
>
> Here is boot data flow for legacy BIOS platform:
>
>  BIOS -> GRUB -> multiboot[12]* -> __reloc() -> MBD ->-\
>                                                        /
>         ------<------<------<------<------<------<-----
>          \
>           \
>            ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info
>           /
>  BIOS ->-/
>
>   * multiboot2 is not implemented yet. Look for it in later patches.
>
> Here is boot data flow for EFI platform:
>
>   EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info
>
> WARNING: ARM build could be broken by this patch. We need to agree boot_info
> integration into ARM. Personally I think that it is worth storing all data
> from any bootloader and preloader in boot_info on any architecture. This give
> a chance to share more code between architectures. However, every architecture
> should define its own boot_info (in relevant include/asm directory). Despite
> that it looks that some parts of it could be common, e.g.  modules data,
> command line arguments, boot loader name, EFI data, etc., even if types
> would not be the same. So, as it was stated above a lot of code could be
> shared among architectures.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

This patch is basically unreviewable.  Please split it up more, starting
with misc cleanup such as video.h vs vga.h

e.g. start with

typedef struct {
    /* Boot loader name. */
    char *boot_loader_name;
} boot_info_t;

And make some stub declarations which set and retrieve the loader name
alone.

Then over the course of the following patches, move small bits one at a
time into this new structure, such as the command line then memory map,
acpi tables, smbios tables etc.  This should probably be at least 8 patches.

~Andrew

Re: [PATCH v2 3/5] xen/x86: Migrate to boot_info structure

[Ian Campbell]

On Wed, 2014-09-24 at 19:19 +0200, Daniel Kiper wrote:
> Break multiboot (v1) protocol dependency. It means that most of Xen code
> (excluding preloader) could be bootloader agnostic and does not need almost
> any knowledge about boot protocol. Additionally, we are able to pass all boot
> data to __start_xen() in one bucket without any side channels. I do not mention
> that we are also able to easily identify boot data in Xen code.
> 
> Here is boot data flow for legacy BIOS platform:
> 
>  BIOS -> GRUB -> multiboot[12]* -> __reloc() -> MBD ->-\
>                                                        /
>         ------<------<------<------<------<------<-----
>          \
>           \
>            ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info
>           /
>  BIOS ->-/
> 
>   * multiboot2 is not implemented yet. Look for it in later patches.
> 
> Here is boot data flow for EFI platform:
> 
>   EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info
> 
> WARNING: ARM build could be broken by this patch.

The hypervisor is easy to cross compile, so please confirm and if so fix
it.

I use the cross compilers from
https://launchpad.net/linaro-toolchain-binaries 

See
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Cross_Compiling for more info.

> We need to agree boot_info
> integration into ARM. Personally I think that it is worth storing all data
> from any bootloader and preloader in boot_info on any architecture. This give
> a chance to share more code between architectures. However, every architecture
> should define its own boot_info (in relevant include/asm directory). Despite
> that it looks that some parts of it could be common, e.g.  modules data,
> command line arguments, boot loader name, EFI data, etc., even if types
> would not be the same. So, as it was stated above a lot of code could be
> shared among architectures.

As I think I've said before I'm happy to consider this in the future but
not for 4.5 at this stage. Please ensure that ARM at least still
compiles after this change.

Ian.

Re: [PATCH v2 3/5] xen/x86: Migrate to boot_info structure

[Daniel Kiper]

On Thu, Sep 25, 2014 at 10:43:36AM +0100, Ian Campbell wrote:
> On Wed, 2014-09-24 at 19:19 +0200, Daniel Kiper wrote:
> > Break multiboot (v1) protocol dependency. It means that most of Xen code
> > (excluding preloader) could be bootloader agnostic and does not need almost
> > any knowledge about boot protocol. Additionally, we are able to pass all boot
> > data to __start_xen() in one bucket without any side channels. I do not mention
> > that we are also able to easily identify boot data in Xen code.
> >
> > Here is boot data flow for legacy BIOS platform:
> >
> >  BIOS -> GRUB -> multiboot[12]* -> __reloc() -> MBD ->-\
> >                                                        /
> >         ------<------<------<------<------<------<-----
> >          \
> >           \
> >            ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info
> >           /
> >  BIOS ->-/
> >
> >   * multiboot2 is not implemented yet. Look for it in later patches.
> >
> > Here is boot data flow for EFI platform:
> >
> >   EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info
> >
> > WARNING: ARM build could be broken by this patch.
>
> The hypervisor is easy to cross compile, so please confirm and if so fix
> it.
>
> I use the cross compilers from
> https://launchpad.net/linaro-toolchain-binaries
>
> See
> http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Cross_Compiling for more info.

Thanks, wilco.

> > We need to agree boot_info
> > integration into ARM. Personally I think that it is worth storing all data
> > from any bootloader and preloader in boot_info on any architecture. This give
> > a chance to share more code between architectures. However, every architecture
> > should define its own boot_info (in relevant include/asm directory). Despite
> > that it looks that some parts of it could be common, e.g.  modules data,
> > command line arguments, boot loader name, EFI data, etc., even if types
> > would not be the same. So, as it was stated above a lot of code could be
> > shared among architectures.
>
> As I think I've said before I'm happy to consider this in the future but

Great!

> not for 4.5 at this stage. Please ensure that ARM at least still

Roger.

> compiles after this change.

Ditto.

Daniel

[PATCH v2 4/5] xen/x86: Use constant as multiboot protocol identifier

[Daniel Kiper]

Use constant as multiboot protocol identifier instead of plain number.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
v2 - suggestions/fixes:
   - patch split rearrangement
     (suggested by Andrew Cooper and Jan Beulich).
---
 xen/arch/x86/boot/head.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 79bce3c..7e48833 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -82,7 +82,7 @@ __start:
         mov     %ecx,%ss
 
         /* Check for Multiboot bootloader */
-        cmp     $0x2BADB002,%eax
+        cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
         jne     not_multiboot
 
         /* Set up trampoline segment 64k below EBDA */
-- 
1.7.10.4

Re: [PATCH v2 4/5] xen/x86: Use constant as multiboot protocol identifier

[Andrew Cooper]

On 24/09/14 18:19, Daniel Kiper wrote:
> Use constant as multiboot protocol identifier instead of plain number.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>

Again - should be ahead of functional changes in your series.

> ---
> v2 - suggestions/fixes:
>    - patch split rearrangement
>      (suggested by Andrew Cooper and Jan Beulich).
> ---
>  xen/arch/x86/boot/head.S |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 79bce3c..7e48833 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -82,7 +82,7 @@ __start:
>          mov     %ecx,%ss
>  
>          /* Check for Multiboot bootloader */
> -        cmp     $0x2BADB002,%eax
> +        cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
>          jne     not_multiboot
>  
>          /* Set up trampoline segment 64k below EBDA */

[PATCH v2 5/5] xen/x86: Add multiboot2 protocol support

[Daniel Kiper]

Add multiboot2 protocol support. This way we are laying the foundation
for EFI + GRUB2 + Xen development.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
v2 - suggestions/fixes:
   - use "for" instead of "while" for loops
     (suggested by Jan Beulich),
   - properly parenthesize macro arguments
     (suggested by Jan Beulich),
   - change some local variables types
     (suggested by Jan Beulich),
   - use meaningful labels
     (suggested by Andrew Cooper and Jan Beulich),
   - use local labels
     (suggested by Jan Beulich),
   - fix coding style
     (suggested by Jan Beulich),
   - patch split rearrangement
     (suggested by Andrew Cooper and Jan Beulich).
---
 xen/arch/x86/boot/head.S     |  117 +++++++++++++-
 xen/arch/x86/boot/reloc.c    |  103 +++++++++++-
 xen/include/xen/multiboot2.h |  354 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 567 insertions(+), 7 deletions(-)
 create mode 100644 xen/include/xen/multiboot2.h

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 7e48833..4331821 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -1,5 +1,6 @@
 #include <xen/config.h>
 #include <xen/multiboot.h>
+#include <xen/multiboot2.h>
 #include <public/xen.h>
 #include <asm/asm_defns.h>
 #include <asm/desc.h>
@@ -33,6 +34,68 @@ ENTRY(start)
         /* Checksum: must be the negated sum of the first two fields. */
         .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
 
+/*** MULTIBOOT2 HEADER ****/
+/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S file. */
+        .align  MULTIBOOT2_HEADER_ALIGN
+
+.Lmultiboot2_header:
+        /* Magic number indicating a Multiboot2 header. */
+        .long   MULTIBOOT2_HEADER_MAGIC
+        /* Architecture: i386. */
+        .long   MULTIBOOT2_ARCHITECTURE_I386
+        /* Multiboot2 header length. */
+        .long   .Lmultiboot2_header_end - .Lmultiboot2_header
+        /* Multiboot2 header checksum. */
+        .long   -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 + \
+                        (.Lmultiboot2_header_end - .Lmultiboot2_header))
+
+        /* Multiboot2 tags... */
+.Lmultiboot2_info_req:
+        /* Multiboot2 information request tag. */
+        .short  MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST
+        .short  MULTIBOOT2_HEADER_TAG_REQUIRED
+        .long   .Lmultiboot2_info_req_end - .Lmultiboot2_info_req
+        .long   MULTIBOOT2_TAG_TYPE_MMAP
+.Lmultiboot2_info_req_end:
+
+        /*
+         * Align Xen image and modules at page boundry.
+         *
+         * .balignl MULTIBOOT2_TAG_ALIGN, MULTIBOOT2_TAG_TYPE_END is a hack
+         * to avoid bug related to Multiboot2 information request tag in earlier
+         * versions of GRUB2.
+         *
+         * DO NOT MOVE THIS TAG! ANY CHANGE HERE MAY BREAK COMPATIBILITY
+         * WITH EARLIER GRUB2 VERSIONS!
+         */
+        .balignl MULTIBOOT2_TAG_ALIGN, MULTIBOOT2_TAG_TYPE_END
+        .short   MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
+        .short   MULTIBOOT2_HEADER_TAG_REQUIRED
+        .long    8 /* Tag size. */
+
+        /* Console flags tag. */
+        .align  MULTIBOOT2_TAG_ALIGN
+        .short  MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS
+        .short  MULTIBOOT2_HEADER_TAG_OPTIONAL
+        .long   12 /* Tag size. */
+        .long   MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
+
+        /* Framebuffer tag. */
+        .align  MULTIBOOT2_TAG_ALIGN
+        .short  MULTIBOOT2_HEADER_TAG_FRAMEBUFFER
+        .short  MULTIBOOT2_HEADER_TAG_OPTIONAL
+        .long   20 /* Tag size. */
+        .long   0 /* Number of the columns - no preference. */
+        .long   0 /* Number of the lines - no preference. */
+        .long   0 /* Number of bits per pixel - no preference. */
+
+        /* Multiboot2 header end tag. */
+        .align  MULTIBOOT2_TAG_ALIGN
+        .short  MULTIBOOT2_HEADER_TAG_END
+        .short  0
+        .long   8 /* Tag size. */
+.Lmultiboot2_header_end:
+
         .section .init.rodata, "a", @progbits
         .align 4
 
@@ -81,10 +144,56 @@ __start:
         mov     %ecx,%es
         mov     %ecx,%ss
 
+        /* Assume multiboot[12].mem_lower is 0 if not set by bootloader */
+        xor     %edx,%edx
+
         /* Check for Multiboot bootloader */
         cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
-        jne     not_multiboot
+        je      multiboot_proto
+
+        /* Check for Multiboot2 bootloader */
+        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
+        je      multiboot2_proto
+
+        jmp     not_multiboot
+
+multiboot_proto:
+        /* Get mem_lower from Multiboot information */
+        testb   $MBI_MEMLIMITS,(%ebx)
+        jz      trampoline_setup    /* not available? BDA value will be fine */
+
+        mov     4(%ebx),%edx
+        shl     $10-4,%edx
+        jmp     trampoline_setup
 
+multiboot2_proto:
+        /* Get Multiboot2 information address */
+        mov     %ebx,%ecx
+
+        /* Skip Multiboot2 information fixed part */
+        add     $8,%ecx
+
+0:
+        /* Get mem_lower from Multiboot2 information */
+        cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
+        jne     1f
+
+        mov     8(%ecx),%edx
+        shl     $10-4,%edx
+        jmp     trampoline_setup
+
+1:
+        /* Is it the end of Multiboot2 information? */
+        cmpl    $MULTIBOOT2_TAG_TYPE_END,(%ecx)
+        je      trampoline_setup
+
+        /* Go to next Multiboot2 information tag */
+        add     4(%ecx),%ecx
+        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        jmp     0b
+
+trampoline_setup:
         /* Set up trampoline segment 64k below EBDA */
         movzwl  0x40e,%ecx          /* EBDA segment */
         cmp     $0xa000,%ecx        /* sanity check (high) */
@@ -99,12 +208,8 @@ __start:
          * Compare the value in the BDA with the information from the
          * multiboot structure (if available) and use the smallest.
          */
-        testb   $MBI_MEMLIMITS,(%ebx)
-        jz      2f                  /* not available? BDA value will be fine */
-        mov     4(%ebx),%edx
-        cmp     $0x100,%edx         /* is the multiboot value too small? */
+        cmp     $0x4000,%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 */
 
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index 35a991e..572421f 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -19,9 +19,13 @@ typedef unsigned long long u64;
 
 #include "../../../include/xen/compiler.h"
 #include "../../../include/xen/multiboot.h"
+#include "../../../include/xen/multiboot2.h"
 
 #include "../../../include/asm/mbd.h"
 
+#define ALIGN_UP(addr, align) \
+                (((addr) + (typeof(addr))(align) - 1) & ~((typeof(addr))(align) - 1))
+
 /*
  * __HERE__ IS ENTRY POINT!!!
  *
@@ -143,6 +147,100 @@ static mbd_t *mb_mbd(mbd_t *mbd, multiboot_info_t *mbi)
     return mbd;
 }
 
+static mbd_t *mb2_mbd(mbd_t *mbd, void *mbi)
+{
+    boot_module_t *mbd_mods;
+    memory_map_t *mmap_dst;
+    multiboot2_memory_map_t *mmap_src;
+    multiboot2_tag_t *tag;
+    u32 ptr;
+    unsigned int i, mod_idx = 0;
+
+    /* Skip Multiboot2 information fixed part. */
+    tag = mbi + sizeof(u32) * 2;
+
+    for ( ; ; )
+    {
+        if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
+            ++mbd->mods_nr;
+        else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
+        {
+            mbd->mods = alloc_struct(mbd->mods_nr * sizeof(boot_module_t));
+            mbd_mods = (boot_module_t *)mbd->mods;
+            break;
+        }
+
+        /* Go to next Multiboot2 information tag. */
+        tag = (multiboot2_tag_t *)(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN));
+    }
+
+    /* Skip Multiboot2 information fixed part. */
+    tag = mbi + sizeof(u32) * 2;
+
+    for ( ; ; )
+    {
+        switch ( tag->type )
+        {
+        case MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME:
+            ptr = (u32)((multiboot2_tag_string_t *)tag)->string;
+            mbd->boot_loader_name = copy_string(ptr);
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_CMDLINE:
+            ptr = (u32)((multiboot2_tag_string_t *)tag)->string;
+            mbd->cmdline = copy_string(ptr);
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO:
+            mbd->mem_lower = ((multiboot2_tag_basic_meminfo_t *)tag)->mem_lower;
+            mbd->mem_upper = ((multiboot2_tag_basic_meminfo_t *)tag)->mem_upper;
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_MMAP:
+            mbd->mmap_size = ((multiboot2_tag_mmap_t *)tag)->size;
+            mbd->mmap_size -= sizeof(multiboot2_tag_mmap_t);
+            mbd->mmap_size += sizeof(((multiboot2_tag_mmap_t){0}).entries);
+            mbd->mmap_size /= ((multiboot2_tag_mmap_t *)tag)->entry_size;
+            mbd->mmap_size *= sizeof(memory_map_t);
+
+            mbd->mmap = alloc_struct(mbd->mmap_size);
+
+            mmap_src = ((multiboot2_tag_mmap_t *)tag)->entries;
+            mmap_dst = (memory_map_t *)mbd->mmap;
+
+            for ( i = 0; i < mbd->mmap_size / sizeof(memory_map_t); ++i )
+            {
+                mmap_dst[i].size = sizeof(memory_map_t);
+                mmap_dst[i].size -= sizeof(((memory_map_t){0}).size);
+                mmap_dst[i].base_addr_low = (u32)mmap_src[i].addr;
+                mmap_dst[i].base_addr_high = (u32)(mmap_src[i].addr >> 32);
+                mmap_dst[i].length_low = (u32)mmap_src[i].len;
+                mmap_dst[i].length_high = (u32)(mmap_src[i].len >> 32);
+                mmap_dst[i].type = mmap_src[i].type;
+            }
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_MODULE:
+            mbd_mods[mod_idx].start = (u32)((multiboot2_tag_module_t *)tag)->mod_start;
+            mbd_mods[mod_idx].end = (u32)((multiboot2_tag_module_t *)tag)->mod_end;
+            ptr = (u32)((multiboot2_tag_module_t *)tag)->cmdline;
+            mbd_mods[mod_idx].cmdline = copy_string(ptr);
+            mbd_mods[mod_idx].relocated = 0;
+            ++mod_idx;
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_END:
+            return mbd;
+
+        default:
+            break;
+        }
+
+        /* Go to next Multiboot2 information tag. */
+        tag = (multiboot2_tag_t *)(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN));
+    }
+}
+
 static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
 {
     mbd_t *mbd;
@@ -150,5 +248,8 @@ static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
     mbd = (mbd_t *)alloc_struct(sizeof(mbd_t));
     zero_struct((u32)mbd, sizeof(mbd_t));
 
-    return mb_mbd(mbd, mbi);
+    if ( mb_magic == MULTIBOOT_BOOTLOADER_MAGIC )
+        return mb_mbd(mbd, mbi);
+    else
+        return mb2_mbd(mbd, mbi);
 }
diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h
new file mode 100644
index 0000000..47ca08e
--- /dev/null
+++ b/xen/include/xen/multiboot2.h
@@ -0,0 +1,354 @@
+/*
+ *  Copyright (C) 1999,2003,2007,2008,2009,2010  Free Software Foundation, Inc.
+ *
+ *  multiboot2.h - Multiboot 2 header file.
+ *
+ *  Based on grub-2.00/include/multiboot2.h file.
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a copy
+ *  of this software and associated documentation files (the "Software"), to
+ *  deal in the Software without restriction, including without limitation the
+ *  rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ *  sell copies of the Software, and to permit persons to whom the Software is
+ *  furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in
+ *  all copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL ANY
+ *  DEVELOPER OR DISTRIBUTOR BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *  WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
+ *  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __MULTIBOOT2_H__
+#define __MULTIBOOT2_H__
+
+/* The magic field should contain this.  */
+#define MULTIBOOT2_HEADER_MAGIC			0xe85250d6
+
+/* This should be in %eax on x86 architecture.  */
+#define MULTIBOOT2_BOOTLOADER_MAGIC		0x36d76289
+
+/* How many bytes from the start of the file we search for the header.  */
+#define MULTIBOOT2_SEARCH			32768
+
+/* Multiboot 2 header alignment. */
+#define MULTIBOOT2_HEADER_ALIGN			8
+
+/* Alignment of multiboot 2 modules.  */
+#define MULTIBOOT2_MOD_ALIGN			0x00001000
+
+/* Alignment of the multiboot 2 info structure.  */
+#define MULTIBOOT2_INFO_ALIGN			0x00000008
+
+/* Multiboot 2 architectures. */
+#define MULTIBOOT2_ARCHITECTURE_I386	0
+#define MULTIBOOT2_ARCHITECTURE_MIPS32	4
+
+/* Header tag types. */
+#define MULTIBOOT2_HEADER_TAG_END			0
+#define MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST	1
+#define MULTIBOOT2_HEADER_TAG_ADDRESS			2
+#define MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS		3
+#define MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS		4
+#define MULTIBOOT2_HEADER_TAG_FRAMEBUFFER		5
+#define MULTIBOOT2_HEADER_TAG_MODULE_ALIGN		6
+#define MULTIBOOT2_HEADER_TAG_EFI_BS			7
+
+/* Header tag flags. */
+#define MULTIBOOT2_HEADER_TAG_REQUIRED	0
+#define MULTIBOOT2_HEADER_TAG_OPTIONAL	1
+
+/* Header console tag console_flags. */
+#define MULTIBOOT2_CONSOLE_FLAGS_CONSOLE_REQUIRED	1
+#define MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED	2
+
+/* Flags set in the 'flags' member of the multiboot header.  */
+#define MULTIBOOT2_TAG_TYPE_END			0
+#define MULTIBOOT2_TAG_TYPE_CMDLINE		1
+#define MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME	2
+#define MULTIBOOT2_TAG_TYPE_MODULE		3
+#define MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO	4
+#define MULTIBOOT2_TAG_TYPE_BOOTDEV		5
+#define MULTIBOOT2_TAG_TYPE_MMAP		6
+#define MULTIBOOT2_TAG_TYPE_VBE			7
+#define MULTIBOOT2_TAG_TYPE_FRAMEBUFFER		8
+#define MULTIBOOT2_TAG_TYPE_ELF_SECTIONS	9
+#define MULTIBOOT2_TAG_TYPE_APM			10
+#define MULTIBOOT2_TAG_TYPE_EFI32		11
+#define MULTIBOOT2_TAG_TYPE_EFI64		12
+#define MULTIBOOT2_TAG_TYPE_SMBIOS		13
+#define MULTIBOOT2_TAG_TYPE_ACPI_OLD		14
+#define MULTIBOOT2_TAG_TYPE_ACPI_NEW		15
+#define MULTIBOOT2_TAG_TYPE_NETWORK		16
+#define MULTIBOOT2_TAG_TYPE_EFI_MMAP		17
+#define MULTIBOOT2_TAG_TYPE_EFI_BS		18
+
+/* Multiboot 2 tag alignment. */
+#define MULTIBOOT2_TAG_ALIGN			8
+
+/* Memory types. */
+#define MULTIBOOT2_MEMORY_AVAILABLE		1
+#define MULTIBOOT2_MEMORY_RESERVED		2
+#define MULTIBOOT2_MEMORY_ACPI_RECLAIMABLE	3
+#define MULTIBOOT2_MEMORY_NVS			4
+#define MULTIBOOT2_MEMORY_BADRAM		5
+
+/* Framebuffer types. */
+#define MULTIBOOT2_FRAMEBUFFER_TYPE_INDEXED	0
+#define MULTIBOOT2_FRAMEBUFFER_TYPE_RGB		1
+#define MULTIBOOT2_FRAMEBUFFER_TYPE_EGA_TEXT	2
+
+#ifndef __ASSEMBLY__
+typedef struct {
+    /* Must be MULTIBOOT2_MAGIC - see above.  */
+    u32 magic;
+
+    /* ISA */
+    u32 architecture;
+
+    /* Total header length.  */
+    u32 header_length;
+
+    /* The above fields plus this one must equal 0 mod 2^32. */
+    u32 checksum;
+} multiboot2_header_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+} multiboot2_header_tag_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 requests[0];
+} multiboot2_header_tag_information_request_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 header_addr;
+    u32 load_addr;
+    u32 load_end_addr;
+    u32 bss_end_addr;
+} multiboot2_header_tag_address_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 entry_addr;
+} multiboot2_header_tag_entry_address_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 console_flags;
+} multiboot2_header_tag_console_flags_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 width;
+    u32 height;
+    u32 depth;
+} multiboot2_header_tag_framebuffer_t;
+
+typedef struct {
+    u16 type;
+    u16 flags;
+    u32 size;
+    u32 width;
+    u32 height;
+    u32 depth;
+} multiboot2_header_tag_module_align_t;
+
+typedef struct {
+    u8 red;
+    u8 green;
+    u8 blue;
+} multiboot2_color_t;
+
+typedef struct __packed {
+    u64 addr;
+    u64 len;
+    u32 type;
+    u32 zero;
+} multiboot2_memory_map_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+} multiboot2_tag_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    char string[0];
+} multiboot2_tag_string_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 mod_start;
+    u32 mod_end;
+    char cmdline[0];
+} multiboot2_tag_module_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 mem_lower;
+    u32 mem_upper;
+} multiboot2_tag_basic_meminfo_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 biosdev;
+    u32 slice;
+    u32 part;
+} multiboot2_tag_bootdev_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 entry_size;
+    u32 entry_version;
+    multiboot2_memory_map_t entries[0];
+} multiboot2_tag_mmap_t;
+
+typedef struct {
+    u8 external_specification[512];
+} multiboot2_vbe_info_block_t;
+
+typedef struct {
+    u8 external_specification[256];
+} multiboot2_vbe_mode_info_block_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+
+    u16 vbe_mode;
+    u16 vbe_interface_seg;
+    u16 vbe_interface_off;
+    u16 vbe_interface_len;
+
+    multiboot2_vbe_info_block_t vbe_control_info;
+    multiboot2_vbe_mode_info_block_t vbe_mode_info;
+} multiboot2_tag_vbe_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+
+    u64 framebuffer_addr;
+    u32 framebuffer_pitch;
+    u32 framebuffer_width;
+    u32 framebuffer_height;
+    u8 framebuffer_bpp;
+    u8 framebuffer_type;
+    u16 reserved;
+} multiboot2_tag_framebuffer_common_t;
+
+typedef struct {
+    multiboot2_tag_framebuffer_common_t common;
+
+    union {
+        struct {
+            u16 framebuffer_palette_num_colors;
+            multiboot2_color_t framebuffer_palette[0];
+        };
+        struct {
+            u8 framebuffer_red_field_position;
+            u8 framebuffer_red_mask_size;
+            u8 framebuffer_green_field_position;
+            u8 framebuffer_green_mask_size;
+            u8 framebuffer_blue_field_position;
+            u8 framebuffer_blue_mask_size;
+        };
+    };
+} multiboot2_tag_framebuffer_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 num;
+    u32 entsize;
+    u32 shndx;
+    char sections[0];
+} multiboot2_tag_elf_sections_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u16 version;
+    u16 cseg;
+    u32 offset;
+    u16 cseg_16;
+    u16 dseg;
+    u16 flags;
+    u16 cseg_len;
+    u16 cseg_16_len;
+    u16 dseg_len;
+} multiboot2_tag_apm_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 pointer;
+} multiboot2_tag_efi32_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u64 pointer;
+} multiboot2_tag_efi64_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u8 major;
+    u8 minor;
+    u8 reserved[6];
+    u8 tables[0];
+} multiboot2_tag_smbios_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u8 rsdp[0];
+} multiboot2_tag_old_acpi_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u8 rsdp[0];
+} multiboot2_tag_new_acpi_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u8 dhcpack[0];
+} multiboot2_tag_network_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 descr_size;
+    u32 descr_vers;
+    u8 efi_mmap[0];
+} multiboot2_tag_efi_mmap_t;
+#endif /* __ASSEMBLY__ */
+
+#endif /* __MULTIBOOT2_H__ */
-- 
1.7.10.4

Re: [PATCH v2 5/5] xen/x86: Add multiboot2 protocol support

[Andrew Cooper]

On 24/09/14 18:19, Daniel Kiper wrote:
> Add multiboot2 protocol support. This way we are laying the foundation
> for EFI + GRUB2 + Xen development.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Much clearer than before.

Can I suggest retroactively renaming the old multiboot bits to
multiboot1 to make a more marked difference between between
"multiboot1_proto" and "multiboot2_proto" ?

It would also be better to split the changing of multiboot1 and
shuffling of memory calculations away from the introduction of multiboot2.

> ---
> v2 - suggestions/fixes:
>    - use "for" instead of "while" for loops
>      (suggested by Jan Beulich),
>    - properly parenthesize macro arguments
>
>     char *boot_loader_name;
>      (suggested by Jan Beulich),
>    - change some local variables types
>      (suggested by Jan Beulich),
>    - use meaningful labels
>      (suggested by Andrew Cooper and Jan Beulich),
>    - use local labels
>      (suggested by Jan Beulich),
>    - fix coding style
>      (suggested by Jan Beulich),
>    - patch split rearrangement
>      (suggested by Andrew Cooper and Jan Beulich).
> ---
>  xen/arch/x86/boot/head.S     |  117 +++++++++++++-
>  xen/arch/x86/boot/reloc.c    |  103 +++++++++++-
>  xen/include/xen/multiboot2.h |  354 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 567 insertions(+), 7 deletions(-)
>  create mode 100644 xen/include/xen/multiboot2.h
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 7e48833..4331821 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -1,5 +1,6 @@
>  #include <xen/config.h>
>  #include <xen/multiboot.h>
> +#include <xen/multiboot2.h>
>  #include <public/xen.h>
>  #include <asm/asm_defns.h>
>  #include <asm/desc.h>
> @@ -33,6 +34,68 @@ ENTRY(start)
>          /* Checksum: must be the negated sum of the first two fields. */
>          .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
>  
> +/*** MULTIBOOT2 HEADER ****/
> +/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S file. */
> +        .align  MULTIBOOT2_HEADER_ALIGN
> +
> +.Lmultiboot2_header:
> +        /* Magic number indicating a Multiboot2 header. */
> +        .long   MULTIBOOT2_HEADER_MAGIC
> +        /* Architecture: i386. */
> +        .long   MULTIBOOT2_ARCHITECTURE_I386
> +        /* Multiboot2 header length. */
> +        .long   .Lmultiboot2_header_end - .Lmultiboot2_header
> +        /* Multiboot2 header checksum. */
> +        .long   -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 + \
> +                        (.Lmultiboot2_header_end - .Lmultiboot2_header))
> +
> +        /* Multiboot2 tags... */
> +.Lmultiboot2_info_req:
> +        /* Multiboot2 information request tag. */
> +        .short  MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST
> +        .short  MULTIBOOT2_HEADER_TAG_REQUIRED
> +        .long   .Lmultiboot2_info_req_end - .Lmultiboot2_info_req
> +        .long   MULTIBOOT2_TAG_TYPE_MMAP
> +.Lmultiboot2_info_req_end:
> +
> +        /*
> +         * Align Xen image and modules at page boundry.
> +         *
> +         * .balignl MULTIBOOT2_TAG_ALIGN, MULTIBOOT2_TAG_TYPE_END is a hack
> +         * to avoid bug related to Multiboot2 information request tag in earlier
> +         * versions of GRUB2.
> +         *
> +         * DO NOT MOVE THIS TAG! ANY CHANGE HERE MAY BREAK COMPATIBILITY
> +         * WITH EARLIER GRUB2 VERSIONS!
> +         */
> +        .balignl MULTIBOOT2_TAG_ALIGN, MULTIBOOT2_TAG_TYPE_END
> +        .short   MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
> +        .short   MULTIBOOT2_HEADER_TAG_REQUIRED
> +        .long    8 /* Tag size. */
> +
> +        /* Console flags tag. */
> +        .align  MULTIBOOT2_TAG_ALIGN
> +        .short  MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS
> +        .short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +        .long   12 /* Tag size. */
> +        .long   MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
> +
> +        /* Framebuffer tag. */
> +        .align  MULTIBOOT2_TAG_ALIGN
> +        .short  MULTIBOOT2_HEADER_TAG_FRAMEBUFFER
> +        .short  MULTIBOOT2_HEADER_TAG_OPTIONAL
> +        .long   20 /* Tag size. */
> +        .long   0 /* Number of the columns - no preference. */
> +        .long   0 /* Number of the lines - no preference. */
> +        .long   0 /* Number of bits per pixel - no preference. */
> +
> +        /* Multiboot2 header end tag. */
> +        .align  MULTIBOOT2_TAG_ALIGN
> +        .short  MULTIBOOT2_HEADER_TAG_END
> +        .short  0
> +        .long   8 /* Tag size. */
> +.Lmultiboot2_header_end:
> +
>          .section .init.rodata, "a", @progbits
>          .align 4
>  
> @@ -81,10 +144,56 @@ __start:
>          mov     %ecx,%es
>          mov     %ecx,%ss
>  
> +        /* Assume multiboot[12].mem_lower is 0 if not set by bootloader */
> +        xor     %edx,%edx
> +

This change does not match its comment, and seems to have appeared from
nowhere.

>          /* Check for Multiboot bootloader */
>          cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
> -        jne     not_multiboot
> +        je      multiboot_proto
> +
> +        /* Check for Multiboot2 bootloader */
> +        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> +        je      multiboot2_proto
> +
> +        jmp     not_multiboot
> +
> +multiboot_proto:
> +        /* Get mem_lower from Multiboot information */
> +        testb   $MBI_MEMLIMITS,(%ebx)
> +        jz      trampoline_setup    /* not available? BDA value will be fine */
> +
> +        mov     4(%ebx),%edx
> +        shl     $10-4,%edx
> +        jmp     trampoline_setup
>  
> +multiboot2_proto:
> +        /* Get Multiboot2 information address */
> +        mov     %ebx,%ecx
> +
> +        /* Skip Multiboot2 information fixed part */
> +        add     $8,%ecx
> +
> +0:
> +        /* Get mem_lower from Multiboot2 information */
> +        cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
> +        jne     1f
> +
> +        mov     8(%ecx),%edx
> +        shl     $10-4,%edx
> +        jmp     trampoline_setup
> +
> +1:
> +        /* Is it the end of Multiboot2 information? */
> +        cmpl    $MULTIBOOT2_TAG_TYPE_END,(%ecx)
> +        je      trampoline_setup
> +
> +        /* Go to next Multiboot2 information tag */
> +        add     4(%ecx),%ecx
> +        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
> +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> +        jmp     0b
> +
> +trampoline_setup:
>          /* Set up trampoline segment 64k below EBDA */
>          movzwl  0x40e,%ecx          /* EBDA segment */
>          cmp     $0xa000,%ecx        /* sanity check (high) */
> @@ -99,12 +208,8 @@ __start:
>           * Compare the value in the BDA with the information from the
>           * multiboot structure (if available) and use the smallest.
>           */
> -        testb   $MBI_MEMLIMITS,(%ebx)
> -        jz      2f                  /* not available? BDA value will be fine */
> -        mov     4(%ebx),%edx
> -        cmp     $0x100,%edx         /* is the multiboot value too small? */
> +        cmp     $0x4000,%edx        /* is the multiboot value too small? */

You are going to need some justification for changing this lower bound
of memory.

>          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 */
>  
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index 35a991e..572421f 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -19,9 +19,13 @@ typedef unsigned long long u64;
>  
>  #include "../../../include/xen/compiler.h"
>  #include "../../../include/xen/multiboot.h"
> +#include "../../../include/xen/multiboot2.h"
>  
>  #include "../../../include/asm/mbd.h"
>  
> +#define ALIGN_UP(addr, align) \
> +                (((addr) + (typeof(addr))(align) - 1) & ~((typeof(addr))(align) - 1))
> +
>  /*
>   * __HERE__ IS ENTRY POINT!!!
>   *
> @@ -143,6 +147,100 @@ static mbd_t *mb_mbd(mbd_t *mbd, multiboot_info_t *mbi)
>      return mbd;
>  }
>  
> +static mbd_t *mb2_mbd(mbd_t *mbd, void *mbi)
> +{
> +    boot_module_t *mbd_mods;
> +    memory_map_t *mmap_dst;
> +    multiboot2_memory_map_t *mmap_src;
> +    multiboot2_tag_t *tag;
> +    u32 ptr;
> +    unsigned int i, mod_idx = 0;
> +
> +    /* Skip Multiboot2 information fixed part. */
> +    tag = mbi + sizeof(u32) * 2;
> +
> +    for ( ; ; )
> +    {
> +        if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
> +            ++mbd->mods_nr;
> +        else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
> +        {
> +            mbd->mods = alloc_struct(mbd->mods_nr * sizeof(boot_module_t));
> +            mbd_mods = (boot_module_t *)mbd->mods;
> +            break;
> +        }
> +
> +        /* Go to next Multiboot2 information tag. */
> +        tag = (multiboot2_tag_t *)(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN));
> +    }
> +
> +    /* Skip Multiboot2 information fixed part. */
> +    tag = mbi + sizeof(u32) * 2;
> +
> +    for ( ; ; )
> +    {
> +        switch ( tag->type )
> +        {
> +        case MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME:
> +            ptr = (u32)((multiboot2_tag_string_t *)tag)->string;
> +            mbd->boot_loader_name = copy_string(ptr);
> +            break;
> +
> +        case MULTIBOOT2_TAG_TYPE_CMDLINE:
> +            ptr = (u32)((multiboot2_tag_string_t *)tag)->string;
> +            mbd->cmdline = copy_string(ptr);
> +            break;
> +
> +        case MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO:
> +            mbd->mem_lower = ((multiboot2_tag_basic_meminfo_t *)tag)->mem_lower;
> +            mbd->mem_upper = ((multiboot2_tag_basic_meminfo_t *)tag)->mem_upper;
> +            break;
> +
> +        case MULTIBOOT2_TAG_TYPE_MMAP:
> +            mbd->mmap_size = ((multiboot2_tag_mmap_t *)tag)->size;
> +            mbd->mmap_size -= sizeof(multiboot2_tag_mmap_t);
> +            mbd->mmap_size += sizeof(((multiboot2_tag_mmap_t){0}).entries);
> +            mbd->mmap_size /= ((multiboot2_tag_mmap_t *)tag)->entry_size;
> +            mbd->mmap_size *= sizeof(memory_map_t);
> +
> +            mbd->mmap = alloc_struct(mbd->mmap_size);
> +
> +            mmap_src = ((multiboot2_tag_mmap_t *)tag)->entries;
> +            mmap_dst = (memory_map_t *)mbd->mmap;
> +
> +            for ( i = 0; i < mbd->mmap_size / sizeof(memory_map_t); ++i )
> +            {
> +                mmap_dst[i].size = sizeof(memory_map_t);
> +                mmap_dst[i].size -= sizeof(((memory_map_t){0}).size);
> +                mmap_dst[i].base_addr_low = (u32)mmap_src[i].addr;
> +                mmap_dst[i].base_addr_high = (u32)(mmap_src[i].addr >> 32);
> +                mmap_dst[i].length_low = (u32)mmap_src[i].len;
> +                mmap_dst[i].length_high = (u32)(mmap_src[i].len >> 32);
> +                mmap_dst[i].type = mmap_src[i].type;
> +            }
> +            break;
> +
> +        case MULTIBOOT2_TAG_TYPE_MODULE:
> +            mbd_mods[mod_idx].start = (u32)((multiboot2_tag_module_t *)tag)->mod_start;
> +            mbd_mods[mod_idx].end = (u32)((multiboot2_tag_module_t *)tag)->mod_end;
> +            ptr = (u32)((multiboot2_tag_module_t *)tag)->cmdline;
> +            mbd_mods[mod_idx].cmdline = copy_string(ptr);
> +            mbd_mods[mod_idx].relocated = 0;
> +            ++mod_idx;
> +            break;
> +
> +        case MULTIBOOT2_TAG_TYPE_END:
> +            return mbd;
> +
> +        default:
> +            break;
> +        }
> +
> +        /* Go to next Multiboot2 information tag. */
> +        tag = (multiboot2_tag_t *)(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN));
> +    }
> +}
> +
>  static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
>  {
>      mbd_t *mbd;
> @@ -150,5 +248,8 @@ static mbd_t __used *__reloc(void *mbi, u32 mb_magic)
>      mbd = (mbd_t *)alloc_struct(sizeof(mbd_t));
>      zero_struct((u32)mbd, sizeof(mbd_t));
>  
> -    return mb_mbd(mbd, mbi);
> +    if ( mb_magic == MULTIBOOT_BOOTLOADER_MAGIC )
> +        return mb_mbd(mbd, mbi);
> +    else
> +        return mb2_mbd(mbd, mbi);
>  }
> diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h

This multiboot2.h is huge - can we get away with only including struct
definitions for tags we currently use/support?

~Andrew

> new file mode 100644
> index 0000000..47ca08e
> --- /dev/null
> +++ b/xen/include/xen/multiboot2.h
> @@ -0,0 +1,354 @@
> +/*
> + *  Copyright (C) 1999,2003,2007,2008,2009,2010  Free Software Foundation, Inc.
> + *
> + *  multiboot2.h - Multiboot 2 header file.
> + *
> + *  Based on grub-2.00/include/multiboot2.h file.
> + *
> + *  Permission is hereby granted, free of charge, to any person obtaining a copy
> + *  of this software and associated documentation files (the "Software"), to
> + *  deal in the Software without restriction, including without limitation the
> + *  rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + *  sell copies of the Software, and to permit persons to whom the Software is
> + *  furnished to do so, subject to the following conditions:
> + *
> + *  The above copyright notice and this permission notice shall be included in
> + *  all copies or substantial portions of the Software.
> + *
> + *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL ANY
> + *  DEVELOPER OR DISTRIBUTOR BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *  WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
> + *  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef __MULTIBOOT2_H__
> +#define __MULTIBOOT2_H__
> +
> +/* The magic field should contain this.  */
> +#define MULTIBOOT2_HEADER_MAGIC			0xe85250d6
> +
> +/* This should be in %eax on x86 architecture.  */
> +#define MULTIBOOT2_BOOTLOADER_MAGIC		0x36d76289
> +
> +/* How many bytes from the start of the file we search for the header.  */
> +#define MULTIBOOT2_SEARCH			32768
> +
> +/* Multiboot 2 header alignment. */
> +#define MULTIBOOT2_HEADER_ALIGN			8
> +
> +/* Alignment of multiboot 2 modules.  */
> +#define MULTIBOOT2_MOD_ALIGN			0x00001000
> +
> +/* Alignment of the multiboot 2 info structure.  */
> +#define MULTIBOOT2_INFO_ALIGN			0x00000008
> +
> +/* Multiboot 2 architectures. */
> +#define MULTIBOOT2_ARCHITECTURE_I386	0
> +#define MULTIBOOT2_ARCHITECTURE_MIPS32	4
> +
> +/* Header tag types. */
> +#define MULTIBOOT2_HEADER_TAG_END			0
> +#define MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST	1
> +#define MULTIBOOT2_HEADER_TAG_ADDRESS			2
> +#define MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS		3
> +#define MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS		4
> +#define MULTIBOOT2_HEADER_TAG_FRAMEBUFFER		5
> +#define MULTIBOOT2_HEADER_TAG_MODULE_ALIGN		6
> +#define MULTIBOOT2_HEADER_TAG_EFI_BS			7
> +
> +/* Header tag flags. */
> +#define MULTIBOOT2_HEADER_TAG_REQUIRED	0
> +#define MULTIBOOT2_HEADER_TAG_OPTIONAL	1
> +
> +/* Header console tag console_flags. */
> +#define MULTIBOOT2_CONSOLE_FLAGS_CONSOLE_REQUIRED	1
> +#define MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED	2
> +
> +/* Flags set in the 'flags' member of the multiboot header.  */
> +#define MULTIBOOT2_TAG_TYPE_END			0
> +#define MULTIBOOT2_TAG_TYPE_CMDLINE		1
> +#define MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME	2
> +#define MULTIBOOT2_TAG_TYPE_MODULE		3
> +#define MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO	4
> +#define MULTIBOOT2_TAG_TYPE_BOOTDEV		5
> +#define MULTIBOOT2_TAG_TYPE_MMAP		6
> +#define MULTIBOOT2_TAG_TYPE_VBE			7
> +#define MULTIBOOT2_TAG_TYPE_FRAMEBUFFER		8
> +#define MULTIBOOT2_TAG_TYPE_ELF_SECTIONS	9
> +#define MULTIBOOT2_TAG_TYPE_APM			10
> +#define MULTIBOOT2_TAG_TYPE_EFI32		11
> +#define MULTIBOOT2_TAG_TYPE_EFI64		12
> +#define MULTIBOOT2_TAG_TYPE_SMBIOS		13
> +#define MULTIBOOT2_TAG_TYPE_ACPI_OLD		14
> +#define MULTIBOOT2_TAG_TYPE_ACPI_NEW		15
> +#define MULTIBOOT2_TAG_TYPE_NETWORK		16
> +#define MULTIBOOT2_TAG_TYPE_EFI_MMAP		17
> +#define MULTIBOOT2_TAG_TYPE_EFI_BS		18
> +
> +/* Multiboot 2 tag alignment. */
> +#define MULTIBOOT2_TAG_ALIGN			8
> +
> +/* Memory types. */
> +#define MULTIBOOT2_MEMORY_AVAILABLE		1
> +#define MULTIBOOT2_MEMORY_RESERVED		2
> +#define MULTIBOOT2_MEMORY_ACPI_RECLAIMABLE	3
> +#define MULTIBOOT2_MEMORY_NVS			4
> +#define MULTIBOOT2_MEMORY_BADRAM		5
> +
> +/* Framebuffer types. */
> +#define MULTIBOOT2_FRAMEBUFFER_TYPE_INDEXED	0
> +#define MULTIBOOT2_FRAMEBUFFER_TYPE_RGB		1
> +#define MULTIBOOT2_FRAMEBUFFER_TYPE_EGA_TEXT	2
> +
> +#ifndef __ASSEMBLY__
> +typedef struct {
> +    /* Must be MULTIBOOT2_MAGIC - see above.  */
> +    u32 magic;
> +
> +    /* ISA */
> +    u32 architecture;
> +
> +    /* Total header length.  */
> +    u32 header_length;
> +
> +    /* The above fields plus this one must equal 0 mod 2^32. */
> +    u32 checksum;
> +} multiboot2_header_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +} multiboot2_header_tag_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 requests[0];
> +} multiboot2_header_tag_information_request_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 header_addr;
> +    u32 load_addr;
> +    u32 load_end_addr;
> +    u32 bss_end_addr;
> +} multiboot2_header_tag_address_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 entry_addr;
> +} multiboot2_header_tag_entry_address_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 console_flags;
> +} multiboot2_header_tag_console_flags_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 width;
> +    u32 height;
> +    u32 depth;
> +} multiboot2_header_tag_framebuffer_t;
> +
> +typedef struct {
> +    u16 type;
> +    u16 flags;
> +    u32 size;
> +    u32 width;
> +    u32 height;
> +    u32 depth;
> +} multiboot2_header_tag_module_align_t;
> +
> +typedef struct {
> +    u8 red;
> +    u8 green;
> +    u8 blue;
> +} multiboot2_color_t;
> +
> +typedef struct __packed {
> +    u64 addr;
> +    u64 len;
> +    u32 type;
> +    u32 zero;
> +} multiboot2_memory_map_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +} multiboot2_tag_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    char string[0];
> +} multiboot2_tag_string_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 mod_start;
> +    u32 mod_end;
> +    char cmdline[0];
> +} multiboot2_tag_module_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 mem_lower;
> +    u32 mem_upper;
> +} multiboot2_tag_basic_meminfo_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 biosdev;
> +    u32 slice;
> +    u32 part;
> +} multiboot2_tag_bootdev_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 entry_size;
> +    u32 entry_version;
> +    multiboot2_memory_map_t entries[0];
> +} multiboot2_tag_mmap_t;
> +
> +typedef struct {
> +    u8 external_specification[512];
> +} multiboot2_vbe_info_block_t;
> +
> +typedef struct {
> +    u8 external_specification[256];
> +} multiboot2_vbe_mode_info_block_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +
> +    u16 vbe_mode;
> +    u16 vbe_interface_seg;
> +    u16 vbe_interface_off;
> +    u16 vbe_interface_len;
> +
> +    multiboot2_vbe_info_block_t vbe_control_info;
> +    multiboot2_vbe_mode_info_block_t vbe_mode_info;
> +} multiboot2_tag_vbe_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +
> +    u64 framebuffer_addr;
> +    u32 framebuffer_pitch;
> +    u32 framebuffer_width;
> +    u32 framebuffer_height;
> +    u8 framebuffer_bpp;
> +    u8 framebuffer_type;
> +    u16 reserved;
> +} multiboot2_tag_framebuffer_common_t;
> +
> +typedef struct {
> +    multiboot2_tag_framebuffer_common_t common;
> +
> +    union {
> +        struct {
> +            u16 framebuffer_palette_num_colors;
> +            multiboot2_color_t framebuffer_palette[0];
> +        };
> +        struct {
> +            u8 framebuffer_red_field_position;
> +            u8 framebuffer_red_mask_size;
> +            u8 framebuffer_green_field_position;
> +            u8 framebuffer_green_mask_size;
> +            u8 framebuffer_blue_field_position;
> +            u8 framebuffer_blue_mask_size;
> +        };
> +    };
> +} multiboot2_tag_framebuffer_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 num;
> +    u32 entsize;
> +    u32 shndx;
> +    char sections[0];
> +} multiboot2_tag_elf_sections_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u16 version;
> +    u16 cseg;
> +    u32 offset;
> +    u16 cseg_16;
> +    u16 dseg;
> +    u16 flags;
> +    u16 cseg_len;
> +    u16 cseg_16_len;
> +    u16 dseg_len;
> +} multiboot2_tag_apm_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 pointer;
> +} multiboot2_tag_efi32_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u64 pointer;
> +} multiboot2_tag_efi64_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u8 major;
> +    u8 minor;
> +    u8 reserved[6];
> +    u8 tables[0];
> +} multiboot2_tag_smbios_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u8 rsdp[0];
> +} multiboot2_tag_old_acpi_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u8 rsdp[0];
> +} multiboot2_tag_new_acpi_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u8 dhcpack[0];
> +} multiboot2_tag_network_t;
> +
> +typedef struct {
> +    u32 type;
> +    u32 size;
> +    u32 descr_size;
> +    u32 descr_vers;
> +    u8 efi_mmap[0];
> +} multiboot2_tag_efi_mmap_t;
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __MULTIBOOT2_H__ */

Re: [PATCH v2 5/5] xen/x86: Add multiboot2 protocol support

[Daniel Kiper]

On Wed, Sep 24, 2014 at 08:14:03PM +0100, Andrew Cooper wrote:
> On 24/09/14 18:19, Daniel Kiper wrote:
> > Add multiboot2 protocol support. This way we are laying the foundation
> > for EFI + GRUB2 + Xen development.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>
> Much clearer than before.
>
> Can I suggest retroactively renaming the old multiboot bits to

Do you mean before or after this (rewritten) patch?

> multiboot1 to make a more marked difference between between
> "multiboot1_proto" and "multiboot2_proto" ?

Do you think about labels in this file only or about all
stuff related to multiboot(1) protocol?

> It would also be better to split the changing of multiboot1 and
> shuffling of memory calculations away from the introduction of multiboot2.
>
> > ---
> > v2 - suggestions/fixes:
> >    - use "for" instead of "while" for loops
> >      (suggested by Jan Beulich),
> >    - properly parenthesize macro arguments
> >
> >     char *boot_loader_name;
> >      (suggested by Jan Beulich),
> >    - change some local variables types
> >      (suggested by Jan Beulich),
> >    - use meaningful labels
> >      (suggested by Andrew Cooper and Jan Beulich),
> >    - use local labels
> >      (suggested by Jan Beulich),
> >    - fix coding style
> >      (suggested by Jan Beulich),
> >    - patch split rearrangement
> >      (suggested by Andrew Cooper and Jan Beulich).
> > ---
> >  xen/arch/x86/boot/head.S     |  117 +++++++++++++-
> >  xen/arch/x86/boot/reloc.c    |  103 +++++++++++-
> >  xen/include/xen/multiboot2.h |  354 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 567 insertions(+), 7 deletions(-)
> >  create mode 100644 xen/include/xen/multiboot2.h
> >
> > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> > index 7e48833..4331821 100644
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S

[...]

> > @@ -81,10 +144,56 @@ __start:
> >          mov     %ecx,%es
> >          mov     %ecx,%ss
> >
> > +        /* Assume multiboot[12].mem_lower is 0 if not set by bootloader */
> > +        xor     %edx,%edx
> > +
>
> This change does not match its comment, and seems to have appeared from
> nowhere.

I still do not understand what is wrong with that. We need
to set %edx to known value here (0 in this case). If MBI_MEMLIMITS
or MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO is not set by bootloader
and we do not initialize %edx then we will use uninitialized
value later.

> >          /* Check for Multiboot bootloader */
> >          cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
> > -        jne     not_multiboot
> > +        je      multiboot_proto
> > +
> > +        /* Check for Multiboot2 bootloader */
> > +        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> > +        je      multiboot2_proto
> > +
> > +        jmp     not_multiboot
> > +
> > +multiboot_proto:
> > +        /* Get mem_lower from Multiboot information */
> > +        testb   $MBI_MEMLIMITS,(%ebx)
> > +        jz      trampoline_setup    /* not available? BDA value will be fine */
> > +
> > +        mov     4(%ebx),%edx
> > +        shl     $10-4,%edx
> > +        jmp     trampoline_setup
> >
> > +multiboot2_proto:
> > +        /* Get Multiboot2 information address */
> > +        mov     %ebx,%ecx
> > +
> > +        /* Skip Multiboot2 information fixed part */
> > +        add     $8,%ecx
> > +
> > +0:
> > +        /* Get mem_lower from Multiboot2 information */
> > +        cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
> > +        jne     1f
> > +
> > +        mov     8(%ecx),%edx
> > +        shl     $10-4,%edx
> > +        jmp     trampoline_setup
> > +
> > +1:
> > +        /* Is it the end of Multiboot2 information? */
> > +        cmpl    $MULTIBOOT2_TAG_TYPE_END,(%ecx)
> > +        je      trampoline_setup
> > +
> > +        /* Go to next Multiboot2 information tag */
> > +        add     4(%ecx),%ecx
> > +        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
> > +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> > +        jmp     0b
> > +
> > +trampoline_setup:
> >          /* Set up trampoline segment 64k below EBDA */
> >          movzwl  0x40e,%ecx          /* EBDA segment */
> >          cmp     $0xa000,%ecx        /* sanity check (high) */
> > @@ -99,12 +208,8 @@ __start:
> >           * Compare the value in the BDA with the information from the
> >           * multiboot structure (if available) and use the smallest.
> >           */
> > -        testb   $MBI_MEMLIMITS,(%ebx)
> > -        jz      2f                  /* not available? BDA value will be fine */
> > -        mov     4(%ebx),%edx
> > -        cmp     $0x100,%edx         /* is the multiboot value too small? */
> > +        cmp     $0x4000,%edx        /* is the multiboot value too small? */
>
> You are going to need some justification for changing this lower bound
> of memory.

It is not changed. I did "shl $10-4,%edx" above so I must change
relevant value here too. However, it looks that it left from my
earlier work and I am able to remove that change safely.

[...]

> > diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h
>
> This multiboot2.h is huge - can we get away with only including struct
> definitions for tags we currently use/support?

I did that because multiboot.h contains all multiboot(1) related stuff.
I think that it is nice to have all defs for multiboot2 protocol too,
however, I do not insist on that.

Daniel

Re: [PATCH v2 5/5] xen/x86: Add multiboot2 protocol support

[Andrew Cooper]

On 25/09/14 19:42, Daniel Kiper wrote:
> On Wed, Sep 24, 2014 at 08:14:03PM +0100, Andrew Cooper wrote:
>> On 24/09/14 18:19, Daniel Kiper wrote:
>>> Add multiboot2 protocol support. This way we are laying the foundation
>>> for EFI + GRUB2 + Xen development.
>>>
>>> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>> Much clearer than before.
>>
>> Can I suggest retroactively renaming the old multiboot bits to
> Do you mean before or after this (rewritten) patch?

I would suggest a separate patch, if others concur that it is a sensible
idea.

>
>> multiboot1 to make a more marked difference between between
>> "multiboot1_proto" and "multiboot2_proto" ?
> Do you think about labels in this file only or about all
> stuff related to multiboot(1) protocol?

Frankly, even changing the constant names to be MULTIBOOT1_$FOO would
aid in clarity of the code.

>
>> It would also be better to split the changing of multiboot1 and
>> shuffling of memory calculations away from the introduction of multiboot2.
>>
>>> ---
>>> v2 - suggestions/fixes:
>>>    - use "for" instead of "while" for loops
>>>      (suggested by Jan Beulich),
>>>    - properly parenthesize macro arguments
>>>
>>>     char *boot_loader_name;
>>>      (suggested by Jan Beulich),
>>>    - change some local variables types
>>>      (suggested by Jan Beulich),
>>>    - use meaningful labels
>>>      (suggested by Andrew Cooper and Jan Beulich),
>>>    - use local labels
>>>      (suggested by Jan Beulich),
>>>    - fix coding style
>>>      (suggested by Jan Beulich),
>>>    - patch split rearrangement
>>>      (suggested by Andrew Cooper and Jan Beulich).
>>> ---
>>>  xen/arch/x86/boot/head.S     |  117 +++++++++++++-
>>>  xen/arch/x86/boot/reloc.c    |  103 +++++++++++-
>>>  xen/include/xen/multiboot2.h |  354 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 567 insertions(+), 7 deletions(-)
>>>  create mode 100644 xen/include/xen/multiboot2.h
>>>
>>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>>> index 7e48833..4331821 100644
>>> --- a/xen/arch/x86/boot/head.S
>>> +++ b/xen/arch/x86/boot/head.S
> [...]
>
>>> @@ -81,10 +144,56 @@ __start:
>>>          mov     %ecx,%es
>>>          mov     %ecx,%ss
>>>
>>> +        /* Assume multiboot[12].mem_lower is 0 if not set by bootloader */
>>> +        xor     %edx,%edx
>>> +
>> This change does not match its comment, and seems to have appeared from
>> nowhere.
> I still do not understand what is wrong with that. We need
> to set %edx to known value here (0 in this case). If MBI_MEMLIMITS
> or MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO is not set by bootloader
> and we do not initialize %edx then we will use uninitialized
> value later.
>
>>>          /* Check for Multiboot bootloader */
>>>          cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
>>> -        jne     not_multiboot
>>> +        je      multiboot_proto
>>> +
>>> +        /* Check for Multiboot2 bootloader */
>>> +        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
>>> +        je      multiboot2_proto
>>> +
>>> +        jmp     not_multiboot
>>> +
>>> +multiboot_proto:
>>> +        /* Get mem_lower from Multiboot information */
>>> +        testb   $MBI_MEMLIMITS,(%ebx)
>>> +        jz      trampoline_setup    /* not available? BDA value will be fine */
>>> +
>>> +        mov     4(%ebx),%edx
>>> +        shl     $10-4,%edx
>>> +        jmp     trampoline_setup
>>>
>>> +multiboot2_proto:
>>> +        /* Get Multiboot2 information address */
>>> +        mov     %ebx,%ecx
>>> +
>>> +        /* Skip Multiboot2 information fixed part */
>>> +        add     $8,%ecx
>>> +
>>> +0:
>>> +        /* Get mem_lower from Multiboot2 information */
>>> +        cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx)
>>> +        jne     1f
>>> +
>>> +        mov     8(%ecx),%edx
>>> +        shl     $10-4,%edx
>>> +        jmp     trampoline_setup
>>> +
>>> +1:
>>> +        /* Is it the end of Multiboot2 information? */
>>> +        cmpl    $MULTIBOOT2_TAG_TYPE_END,(%ecx)
>>> +        je      trampoline_setup
>>> +
>>> +        /* Go to next Multiboot2 information tag */
>>> +        add     4(%ecx),%ecx
>>> +        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
>>> +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
>>> +        jmp     0b
>>> +
>>> +trampoline_setup:
>>>          /* Set up trampoline segment 64k below EBDA */
>>>          movzwl  0x40e,%ecx          /* EBDA segment */
>>>          cmp     $0xa000,%ecx        /* sanity check (high) */
>>> @@ -99,12 +208,8 @@ __start:
>>>           * Compare the value in the BDA with the information from the
>>>           * multiboot structure (if available) and use the smallest.
>>>           */
>>> -        testb   $MBI_MEMLIMITS,(%ebx)
>>> -        jz      2f                  /* not available? BDA value will be fine */
>>> -        mov     4(%ebx),%edx
>>> -        cmp     $0x100,%edx         /* is the multiboot value too small? */
>>> +        cmp     $0x4000,%edx        /* is the multiboot value too small? */
>> You are going to need some justification for changing this lower bound
>> of memory.
> It is not changed. I did "shl $10-4,%edx" above so I must change
> relevant value here too.

And I saw that, given close inspection of the patch.  It is however not
obvious at a glance, and makes review harder.

A comment or paragraph in the commit message is perfectly fine (e.g.
"Alter min memory limit handling as we now may not find it from either
multiboot1 or multiboot2").  This makes it obvious to anyone reading the
patch that there is a real need for the memory limit handling to change,
which also covers setting a sane default of 0.

>  However, it looks that it left from my
> earlier work and I am able to remove that change safely.
>
> [...]
>
>>> diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h
>> This multiboot2.h is huge - can we get away with only including struct
>> definitions for tags we currently use/support?
> I did that because multiboot.h contains all multiboot(1) related stuff.
> I think that it is nice to have all defs for multiboot2 protocol too,
> however, I do not insist on that.

I suppose it doesn't matter either way for a header file like this.

~Andrew

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Roy Franz]

On Wed, Sep 24, 2014 at 10:19 AM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> Hi,
>
> This patch series breaks multiboot (v1) protocol dependency and adds
> multiboot2 support. It lays down the foundation for EFI + GRUB2 + Xen
> development. Detailed description of ideas and thoughts you will
> find in commit message for every patch. If something is not obvious
> please drop me a line.
>
> Most of the requested things are fixed but there are still some minor
> outstanding issues (multiboot2 tags generation, excessive amount of casts
> in xen/arch/x86/boot/reloc.c, etc.; please check commit messages for
> more details). If something is not fixed yet it means that I do not have
> good idea how to do that. In case you spot something which was mentioned
> during previous review and still think that your comment is valid
> in particular case please notify me.
>
> Below you can find reply for Konrad's questions in regards to
> exception request for Xen 4.5 release.
>
>> Couple of questions:
>>
>>  - Since this is mostly XBI code, is there a lot of overlap with the ARM/x86
>>    refactoring of the EFI code? As in, will it require a lot of
>>    rebasing/fixing it up?
>
> I did not checked that right now. However, Roy once told me that this should
> not be very big issue because overlap is not so big. Please check this email
> for more details: http://lists.xen.org/archives/html/xen-devel/2014-08/msg01173.html
>
> Sadly, it was sent more then one month ago so maybe something has changed.
> Roy, any comments on that?
There aren't fundamental conceptual conflicts, but there will be quite
a bit of refactoring/rebasing to do.  Most of my patchset is
re-organizing
x86/efi/boot.c, so rebasing it on this patchset would be a significant
amount of work.   My guess is that the first 10 of the 13 patches
in my series would need be be completely redone, as the code they are
rearranging has been changed by your series.
I think that it would be much less work for you to re-base your series
on mine, than the other way.  The x86/efi/boot.c changes
are a relatively small part of your series, but are the bulk of mine.

I should have another version of my patchset out today.


>
> Anyway, I am more than happy to work on this issue with Roy.
>
>>  - What is the disadvantage of having this in Xen 4.6?
>
> Well, it will require to rebase all this work on latest developments.
> It could be quite difficult because my patches touches a lot of places
> and as I can see there is a chance for substantial changes in some of them.
>
>>  - What is the advantage of having this in Xen 4.5?
>>    [My understanding is that it would allow us to thrash out a lot of
>>     the early bootup issues _now_ and fix them up instead of
>>     having to deal in Xen 4.6 with this, EFI, and SecureBoot - which
>>     is a lot of more moving pieces]
>
> More or less. Additionally, we will have support for multiboot2 protocol
> in place. There is not very big demand for that but as I know at least Intel
> guys were looking for that. IIRC, they needed that for TPM support.
>
> Daniel
>
>  xen/arch/x86/Makefile             |    1 +
>  xen/arch/x86/boot/cmdline.S       |    9 +-
>  xen/arch/x86/boot/head.S          |  150 ++++++++++++++++++++++++++++-----
>  xen/arch/x86/boot/reloc.c         |  248 ++++++++++++++++++++++++++++++++++++++++++++-----------
>  xen/arch/x86/boot/x86_64.S        |   10 ++-
>  xen/arch/x86/boot_info.c          |  258 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/dmi_scan.c           |    7 +-
>  xen/arch/x86/domain_build.c       |   24 +++---
>  xen/arch/x86/e820.c               |    8 +-
>  xen/arch/x86/efi/boot.c           |  214 +++++++++++++++++++++++------------------------
>  xen/arch/x86/efi/efi.h            |    3 -
>  xen/arch/x86/efi/runtime.c        |   52 +++++++++---
>  xen/arch/x86/microcode.c          |   39 ++++-----
>  xen/arch/x86/mpparse.c            |    9 +-
>  xen/arch/x86/platform_hypercall.c |   19 ++---
>  xen/arch/x86/setup.c              |  370 +++++++++++++++++++++++++++-------------------------------------------------------
>  xen/arch/x86/x86_64/asm-offsets.c |    5 +-
>  xen/drivers/acpi/osl.c            |    9 +-
>  xen/drivers/video/vesa.c          |    7 +-
>  xen/drivers/video/vga.c           |   18 ++--
>  xen/include/asm-x86/boot_info.h   |  126 ++++++++++++++++++++++++++++
>  xen/include/asm-x86/config.h      |    2 -
>  xen/include/asm-x86/e820.h        |   10 +--
>  xen/include/asm-x86/edd.h         |    6 --
>  xen/include/asm-x86/mbd.h         |   70 ++++++++++++++++
>  xen/include/asm-x86/setup.h       |   10 +--
>  xen/include/xen/efi.h             |   10 ---
>  xen/include/xen/multiboot2.h      |  354 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/vga.h             |   18 ----
>  xen/include/xsm/xsm.h             |   14 ++--
>  xen/xsm/xsm_core.c                |    6 +-
>  xen/xsm/xsm_policy.c              |   14 ++--
>  32 files changed, 1510 insertions(+), 590 deletions(-)
>
> Daniel Kiper (5):
>       xen/x86: Introduce MultiBoot Data (MBD) type
>       xen/x86: Define e820 entries counter as unsigned int
>       xen/x86: Migrate to boot_info structure
>       xen/x86: Use constant as multiboot protocol identifier
>       xen/x86: Add multiboot2 protocol support
>

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Jan Beulich]

>>> On 24.09.14 at 19:48, <roy.franz@linaro.org> wrote:
> On Wed, Sep 24, 2014 at 10:19 AM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>> Hi,
>>
>> This patch series breaks multiboot (v1) protocol dependency and adds
>> multiboot2 support. It lays down the foundation for EFI + GRUB2 + Xen
>> development. Detailed description of ideas and thoughts you will
>> find in commit message for every patch. If something is not obvious
>> please drop me a line.
>>
>> Most of the requested things are fixed but there are still some minor
>> outstanding issues (multiboot2 tags generation, excessive amount of casts
>> in xen/arch/x86/boot/reloc.c, etc.; please check commit messages for
>> more details). If something is not fixed yet it means that I do not have
>> good idea how to do that. In case you spot something which was mentioned
>> during previous review and still think that your comment is valid
>> in particular case please notify me.
>>
>> Below you can find reply for Konrad's questions in regards to
>> exception request for Xen 4.5 release.
>>
>>> Couple of questions:
>>>
>>>  - Since this is mostly XBI code, is there a lot of overlap with the ARM/x86
>>>    refactoring of the EFI code? As in, will it require a lot of
>>>    rebasing/fixing it up?
>>
>> I did not checked that right now. However, Roy once told me that this should
>> not be very big issue because overlap is not so big. Please check this email
>> for more details: 
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01173.html 
>>
>> Sadly, it was sent more then one month ago so maybe something has changed.
>> Roy, any comments on that?
> There aren't fundamental conceptual conflicts, but there will be quite
> a bit of refactoring/rebasing to do.  Most of my patchset is
> re-organizing
> x86/efi/boot.c, so rebasing it on this patchset would be a significant
> amount of work.   My guess is that the first 10 of the 13 patches
> in my series would need be be completely redone, as the code they are
> rearranging has been changed by your series.
> I think that it would be much less work for you to re-base your series
> on mine, than the other way.  The x86/efi/boot.c changes
> are a relatively small part of your series, but are the bulk of mine.
> 
> I should have another version of my patchset out today.

And when looking at both series later today in detail, I'm certainly
going to give priority to Roy's as I expect that one to be largely
ready to go in now (or at least a sufficiently large initial part of it).

Jan

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Ian Campbell]

On Thu, 2014-09-25 at 10:15 +0100, Jan Beulich wrote:
> > I should have another version of my patchset out today.
> 
> And when looking at both series later today in detail, I'm certainly
> going to give priority to Roy's as I expect that one to be largely
> ready to go in now (or at least a sufficiently large initial part of it).

Per http://lists.xen.org/archives/html/xen-devel/2014-09/msg03874.html
which I just sent I'm not in favour of trying to move ARM over to this
new scheme for 4.5 anyway.

Ian.

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Daniel Kiper]

On Thu, Sep 25, 2014 at 10:15:56AM +0100, Jan Beulich wrote:
> >>> On 24.09.14 at 19:48, <roy.franz@linaro.org> wrote:
> > On Wed, Sep 24, 2014 at 10:19 AM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> >> Hi,
> >>
> >> This patch series breaks multiboot (v1) protocol dependency and adds
> >> multiboot2 support. It lays down the foundation for EFI + GRUB2 + Xen
> >> development. Detailed description of ideas and thoughts you will
> >> find in commit message for every patch. If something is not obvious
> >> please drop me a line.
> >>
> >> Most of the requested things are fixed but there are still some minor
> >> outstanding issues (multiboot2 tags generation, excessive amount of casts
> >> in xen/arch/x86/boot/reloc.c, etc.; please check commit messages for
> >> more details). If something is not fixed yet it means that I do not have
> >> good idea how to do that. In case you spot something which was mentioned
> >> during previous review and still think that your comment is valid
> >> in particular case please notify me.
> >>
> >> Below you can find reply for Konrad's questions in regards to
> >> exception request for Xen 4.5 release.
> >>
> >>> Couple of questions:
> >>>
> >>>  - Since this is mostly XBI code, is there a lot of overlap with the ARM/x86
> >>>    refactoring of the EFI code? As in, will it require a lot of
> >>>    rebasing/fixing it up?
> >>
> >> I did not checked that right now. However, Roy once told me that this should
> >> not be very big issue because overlap is not so big. Please check this email
> >> for more details:
> > http://lists.xen.org/archives/html/xen-devel/2014-08/msg01173.html
> >>
> >> Sadly, it was sent more then one month ago so maybe something has changed.
> >> Roy, any comments on that?
> > There aren't fundamental conceptual conflicts, but there will be quite
> > a bit of refactoring/rebasing to do.  Most of my patchset is
> > re-organizing
> > x86/efi/boot.c, so rebasing it on this patchset would be a significant
> > amount of work.   My guess is that the first 10 of the 13 patches
> > in my series would need be be completely redone, as the code they are
> > rearranging has been changed by your series.
> > I think that it would be much less work for you to re-base your series
> > on mine, than the other way.  The x86/efi/boot.c changes
> > are a relatively small part of your series, but are the bulk of mine.
> >
> > I should have another version of my patchset out today.
>
> And when looking at both series later today in detail, I'm certainly
> going to give priority to Roy's as I expect that one to be largely
> ready to go in now (or at least a sufficiently large initial part of it).

OK. Roy, do you expect any bigger changes in your code right now?
May I rebase safely my work on latest release of your patches?
Should I care about anything in special way?

Daniel

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Roy Franz]

On Thu, Sep 25, 2014 at 6:01 AM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Thu, Sep 25, 2014 at 10:15:56AM +0100, Jan Beulich wrote:
>> >>> On 24.09.14 at 19:48, <roy.franz@linaro.org> wrote:
>> > On Wed, Sep 24, 2014 at 10:19 AM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>> >> Hi,
>> >>
>> >> This patch series breaks multiboot (v1) protocol dependency and adds
>> >> multiboot2 support. It lays down the foundation for EFI + GRUB2 + Xen
>> >> development. Detailed description of ideas and thoughts you will
>> >> find in commit message for every patch. If something is not obvious
>> >> please drop me a line.
>> >>
>> >> Most of the requested things are fixed but there are still some minor
>> >> outstanding issues (multiboot2 tags generation, excessive amount of casts
>> >> in xen/arch/x86/boot/reloc.c, etc.; please check commit messages for
>> >> more details). If something is not fixed yet it means that I do not have
>> >> good idea how to do that. In case you spot something which was mentioned
>> >> during previous review and still think that your comment is valid
>> >> in particular case please notify me.
>> >>
>> >> Below you can find reply for Konrad's questions in regards to
>> >> exception request for Xen 4.5 release.
>> >>
>> >>> Couple of questions:
>> >>>
>> >>>  - Since this is mostly XBI code, is there a lot of overlap with the ARM/x86
>> >>>    refactoring of the EFI code? As in, will it require a lot of
>> >>>    rebasing/fixing it up?
>> >>
>> >> I did not checked that right now. However, Roy once told me that this should
>> >> not be very big issue because overlap is not so big. Please check this email
>> >> for more details:
>> > http://lists.xen.org/archives/html/xen-devel/2014-08/msg01173.html
>> >>
>> >> Sadly, it was sent more then one month ago so maybe something has changed.
>> >> Roy, any comments on that?
>> > There aren't fundamental conceptual conflicts, but there will be quite
>> > a bit of refactoring/rebasing to do.  Most of my patchset is
>> > re-organizing
>> > x86/efi/boot.c, so rebasing it on this patchset would be a significant
>> > amount of work.   My guess is that the first 10 of the 13 patches
>> > in my series would need be be completely redone, as the code they are
>> > rearranging has been changed by your series.
>> > I think that it would be much less work for you to re-base your series
>> > on mine, than the other way.  The x86/efi/boot.c changes
>> > are a relatively small part of your series, but are the bulk of mine.
>> >
>> > I should have another version of my patchset out today.
>>
>> And when looking at both series later today in detail, I'm certainly
>> going to give priority to Roy's as I expect that one to be largely
>> ready to go in now (or at least a sufficiently large initial part of it).
>
> OK. Roy, do you expect any bigger changes in your code right now?
> May I rebase safely my work on latest release of your patches?
> Should I care about anything in special way?
>
> Daniel

I don't think there should be any more major changes, but I'm still
working through a few
last issues which may result in some code movement.  A lot of my
patches are re-organizing
boot.c, so even small changes can be a pain for patches based on that.
I'm hoping to be able
to resolve the last few issues in the next few days to get the patches
merged for 4.5, so if you
hold of for a few days you can hopefully take my changes from the master branch.

Roy

Re: [PATCH v2 0/5] xen: Break multiboot (v1) dependency and add multiboot2 support

[Daniel Kiper]

On Thu, Sep 25, 2014 at 08:56:39AM -0700, Roy Franz wrote:

[...]

> I don't think there should be any more major changes, but I'm still
> working through a few
> last issues which may result in some code movement.  A lot of my
> patches are re-organizing
> boot.c, so even small changes can be a pain for patches based on that.
> I'm hoping to be able
> to resolve the last few issues in the next few days to get the patches
> merged for 4.5, so if you
> hold of for a few days you can hopefully take my changes from the master branch.

Thanks. I will start work on MBD which does not touch EFI stuff.
I hope that in the meantime your patches will be merged into master branch.

Daniel