All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] multiboot2 protocol support
@ 2017-01-18 14:16 Doug Goldstein
  2017-01-18 14:16 ` [PATCH v4 1/5] x86: add " Doug Goldstein
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:16 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Doug Goldstein,
	pgnet.dev, ning.sun, julien.grall, Jan Beulich, qiaowei.ren,
	gang.wei, fu.wei

This is a series based on v11 of Daniel Kiper's
"x86: multiboot2 protocol support" series. It aims to collect up all the
fixes and changes that Andrew Cooper, Jan Beulich and myself discovered in
code review and testing on actual hardware. I've had problems with the
relocation portion of the series so I've dropped it as all the hardware I
am needing to support presently for my $EMPLOYER does not load anything at
the 1mb mark. To me this adds MB2 support for all pieces of hardware that
don't have things located at 1mb so it's an incremental step. I've also
dropped the early command line conversion to C as it was done in support
of the relocation changes and therefore not necessary. In the end my goal
is to help Daniel out by providing the portion of the series that works
on half a dozen physical machines I've tested with and integrates all
changes as discussed on the v11 thread. The reason I am posting this is that
Daniel has said he won't be able to address feedback and issues identified
for another 2 weeks but my requirements from my $EMPLOYER are more immediate
than that.

Feel free to grab this series at: https://github.com/cardoe/xen/tree/doug-mb2-v4

v4 - All changes contained within 5/5 along with a description there.
v3 - address review comments by Jan Beulich. They are contained within 5/5.
v2 - separate my fixes from Daniel's original series
   - add back some ACKs I accidentally dropped

Daniel Kiper (4):
  x86: add multiboot2 protocol support
  efi: build xen.gz with EFI code
  efi: create new early memory allocator
  x86: add multiboot2 protocol support for EFI platforms

Doug Goldstein (1):
  fix: add multiboot2 protocol support for EFI platforms

 xen/arch/x86/Makefile             |   2 +-
 xen/arch/x86/boot/Makefile        |   3 +-
 xen/arch/x86/boot/head.S          | 361 +++++++++++++++++++++++++++++--
 xen/arch/x86/boot/reloc.c         | 148 ++++++++++++-
 xen/arch/x86/efi/Makefile         |  12 +-
 xen/arch/x86/efi/efi-boot.h       |  63 +++--
 xen/arch/x86/efi/stub.c           |  38 +++-
 xen/arch/x86/setup.c              |   3 +-
 xen/arch/x86/x86_64/asm-offsets.c |  11 +-
 xen/arch/x86/xen.lds.S            |  15 +-
 xen/common/efi/boot.c             |  64 +++++-
 xen/common/efi/runtime.c          |   9 +-
 xen/include/xen/config.h          |   1 +-
 xen/include/xen/multiboot2.h      | 169 +++++++++++++++-
 14 files changed, 851 insertions(+), 48 deletions(-)
 create mode 100644 xen/include/xen/multiboot2.h

base-commit: 98be5ffc05e689e2131f175ed95b011a7270db67
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 1/5] x86: add multiboot2 protocol support
  2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
@ 2017-01-18 14:16 ` Doug Goldstein
  2017-01-18 14:16 ` [PATCH v4 2/5] efi: build xen.gz with EFI code Doug Goldstein
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:16 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Daniel Kiper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, Jan Beulich,
	qiaowei.ren, gang.wei, fu.wei

From: Daniel Kiper <daniel.kiper@oracle.com>

Add multiboot2 protocol support. Alter min memory limit handling as we
now may not find it from either multiboot (v1) or multiboot2.

This way we are laying the foundation for EFI + GRUB2 + Xen development.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
v9 - suggestions/fixes:
   - use .L label instead of numeric one in multiboot2 data scanning loop;
     I hope that this change does not invalidate Jan's Reviewed-by
     (suggested by Jan Beulich).

v8 - suggestions/fixes:
   - use sizeof(<var>/<expr>) instead of sizeof(<type>)
     if it is possible
     (suggested by Jan Beulich).

v7 - suggestions/fixes:
   - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively
     (suggested by Jan Beulich),
   - initialize mbi_out->flags using "|=" instead of "="
     (suggested by Jan Beulich),
   - use sizeof(*mmap_dst) instead of sizeof(memory_map_t)
     if it makes sense
     (suggested by Jan Beulich).

v6 - suggestions/fixes:
   - properly index multiboot2_tag_mmap_t.entries[]
     (suggested by Jan Beulich),
   - do not index mbi_out_mods[] beyond its end
     (suggested by Andrew Cooper),
   - reduce number of casts
     (suggested by Andrew Cooper and Jan Beulich),
   - add braces to increase code readability
     (suggested by Andrew Cooper).

v5 - suggestions/fixes:
   - check multiboot2_tag_mmap_t.entry_size before
     multiboot2_tag_mmap_t.entries[] use
     (suggested by Jan Beulich),
   - properly index multiboot2_tag_mmap_t.entries[]
     (suggested by Jan Beulich),
   - use "type name[]" instad of "type name[0]"
     in xen/include/xen/multiboot2.h
     (suggested by Jan Beulich),
   - remove unneeded comment
     (suggested by Jan Beulich).

v4 - suggestions/fixes:
   - avoid assembly usage in xen/arch/x86/boot/reloc.c,
   - fix boundary check issue and optimize
     for() loops in mbi2_mbi(),
   - move to stdcall calling convention,
   - remove unneeded typeof() from ALIGN_UP() macro
     (suggested by Jan Beulich),
   - add and use NULL definition in xen/arch/x86/boot/reloc.c
     (suggested by Jan Beulich),
   - do not read data beyond the end of multiboot2
     information in xen/arch/x86/boot/head.S
     (suggested by Jan Beulich),
   - add :req to some .macro arguments
     (suggested by Jan Beulich),
   - use cmovcc if possible,
   - add .L to multiboot2_header_end label
     (suggested by Jan Beulich),
   - add .L to multiboot2_proto label
     (suggested by Jan Beulich),
   - improve label names
     (suggested by Jan Beulich).

v3 - suggestions/fixes:
   - reorder reloc() arguments
     (suggested by Jan Beulich),
   - remove .L from multiboot2 header labels
     (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk),
   - take into account alignment when skipping multiboot2 fixed part
     (suggested by Konrad Rzeszutek Wilk),
   - create modules data if modules count != 0
     (suggested by Jan Beulich),
   - improve macros
     (suggested by Jan Beulich),
   - reduce number of casts
     (suggested by Jan Beulich),
   - use const if possible
     (suggested by Jan Beulich),
   - drop static and __used__ attribute from reloc()
     (suggested by Jan Beulich),
   - remove isolated/stray __packed attribute from
     multiboot2_memory_map_t type definition
     (suggested by Jan Beulich),
   - reformat xen/include/xen/multiboot2.h
     (suggested by Konrad Rzeszutek Wilk),
   - improve comments
     (suggested by Konrad Rzeszutek Wilk),
   - remove hard tabs
     (suggested by Jan Beulich and Konrad Rzeszutek Wilk).

v2 - suggestions/fixes:
   - generate multiboot2 header using macros
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich),
   - simplify assembly in xen/arch/x86/boot/head.S
     (suggested by Jan Beulich),
   - do not include include/xen/compiler.h
     in xen/arch/x86/boot/reloc.c
     (suggested by Jan Beulich),
   - do not read data beyond the end of multiboot2 information
     (suggested by Jan Beulich).

v2 - not fixed yet:
   - dynamic dependency generation for xen/arch/x86/boot/reloc.S;
     this requires more work; I am not sure that it pays because
     potential patch requires more changes than addition of just
     multiboot2.h to Makefile
     (suggested by Jan Beulich),
   - isolated/stray __packed attribute usage for multiboot2_memory_map_t
     (suggested by Jan Beulich).
---
---
 xen/arch/x86/boot/Makefile        |   3 +-
 xen/arch/x86/boot/head.S          | 107 +++++++++++++++++++-
 xen/arch/x86/boot/reloc.c         | 148 ++++++++++++++++++++++++++-
 xen/arch/x86/x86_64/asm-offsets.c |   9 ++-
 xen/include/xen/multiboot2.h      | 169 +++++++++++++++++++++++++++++++-
 5 files changed, 426 insertions(+), 10 deletions(-)
 create mode 100644 xen/include/xen/multiboot2.h

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 5fdb5ae..06893d8 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,6 +1,7 @@
 obj-bin-y += head.o
 
-RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h $(BASEDIR)/include/xen/multiboot.h
+RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h $(BASEDIR)/include/xen/multiboot.h \
+	     $(BASEDIR)/include/xen/multiboot2.h
 
 head.o: reloc.S
 
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 6f2c668..d423fd8 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>
@@ -19,6 +20,28 @@
 #define BOOT_PSEUDORM_CS 0x0020
 #define BOOT_PSEUDORM_DS 0x0028
 
+#define MB2_HT(name)      (MULTIBOOT2_HEADER_TAG_##name)
+#define MB2_TT(name)      (MULTIBOOT2_TAG_TYPE_##name)
+
+        .macro mb2ht_args arg:req, args:vararg
+        .long \arg
+        .ifnb \args
+        mb2ht_args \args
+        .endif
+        .endm
+
+        .macro mb2ht_init type:req, req:req, args:vararg
+        .align MULTIBOOT2_TAG_ALIGN
+.Lmb2ht_init_start\@:
+        .short \type
+        .short \req
+        .long .Lmb2ht_init_end\@ - .Lmb2ht_init_start\@
+        .ifnb \args
+        mb2ht_args \args
+        .endif
+.Lmb2ht_init_end\@:
+        .endm
+
 ENTRY(start)
         jmp     __start
 
@@ -34,6 +57,42 @@ multiboot1_header_start:       /*** MULTIBOOT1 HEADER ****/
         .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
 multiboot1_header_end:
 
+/*** MULTIBOOT2 HEADER ****/
+/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S file. */
+        .align  MULTIBOOT2_HEADER_ALIGN
+
+multiboot2_header_start:
+        /* Magic number indicating a Multiboot2 header. */
+        .long   MULTIBOOT2_HEADER_MAGIC
+        /* Architecture: i386. */
+        .long   MULTIBOOT2_ARCHITECTURE_I386
+        /* Multiboot2 header length. */
+        .long   .Lmultiboot2_header_end - multiboot2_header_start
+        /* Multiboot2 header checksum. */
+        .long   -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 + \
+                        (.Lmultiboot2_header_end - multiboot2_header_start))
+
+        /* Multiboot2 information request tag. */
+        mb2ht_init MB2_HT(INFORMATION_REQUEST), MB2_HT(REQUIRED), \
+                   MB2_TT(BASIC_MEMINFO), MB2_TT(MMAP)
+
+        /* Align modules at page boundry. */
+        mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED)
+
+        /* Console flags tag. */
+        mb2ht_init MB2_HT(CONSOLE_FLAGS), MB2_HT(OPTIONAL), \
+                   MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
+
+        /* Framebuffer tag. */
+        mb2ht_init MB2_HT(FRAMEBUFFER), MB2_HT(OPTIONAL), \
+                   0, /* Number of the columns - no preference. */ \
+                   0, /* Number of the lines - no preference. */ \
+                   0  /* Number of bits per pixel - no preference. */
+
+        /* Multiboot2 header end tag. */
+        mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
+.Lmultiboot2_header_end:
+
         .section .init.rodata, "a", @progbits
         .align 4
 
@@ -82,10 +141,52 @@ __start:
         mov     %ecx,%es
         mov     %ecx,%ss
 
-        /* Check for Multiboot bootloader */
+        /* Bootloaders may set multiboot{1,2}.mem_lower to a nonzero value. */
+        xor     %edx,%edx
+
+        /* Check for Multiboot2 bootloader. */
+        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
+        je      .Lmultiboot2_proto
+
+        /* Check for Multiboot bootloader. */
         cmp     $MULTIBOOT_BOOTLOADER_MAGIC,%eax
         jne     not_multiboot
 
+        /* Get mem_lower from Multiboot information. */
+        testb   $MBI_MEMLIMITS,MB_flags(%ebx)
+
+        /* Not available? BDA value will be fine. */
+        cmovnz  MB_mem_lower(%ebx),%edx
+        jmp     trampoline_setup
+
+.Lmultiboot2_proto:
+        /* Skip Multiboot2 information fixed part. */
+        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%ebx),%ecx
+        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+
+.Lmb2_tsize:
+        /* Check Multiboot2 information total size. */
+        mov     %ecx,%edi
+        sub     %ebx,%edi
+        cmp     %edi,MB2_fixed_total_size(%ebx)
+        jbe     trampoline_setup
+
+        /* Get mem_lower from Multiboot2 information. */
+        cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,MB2_tag_type(%ecx)
+        cmove   MB2_mem_lower(%ecx),%edx
+        je      trampoline_setup
+
+        /* Is it the end of Multiboot2 information? */
+        cmpl    $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%ecx)
+        je      trampoline_setup
+
+        /* Go to next Multiboot2 information tag. */
+        add     MB2_tag_size(%ecx),%ecx
+        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        jmp     .Lmb2_tsize
+
+trampoline_setup:
         /* Set up trampoline segment 64k below EBDA */
         movzwl  0x40e,%ecx          /* EBDA segment */
         cmp     $0xa000,%ecx        /* sanity check (high) */
@@ -100,9 +201,6 @@ __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     MB_mem_lower(%ebx),%edx
         cmp     $0x100,%edx         /* is the multiboot value too small? */
         jb      2f                  /* if so, do not use it */
         shl     $10-4,%edx
@@ -121,6 +219,7 @@ __start:
         mov     $sym_phys(cpu0_stack)+1024,%esp
         push    %ecx                /* Boot trampoline address. */
         push    %ebx                /* Multiboot information address. */
+        push    %eax                /* Multiboot magic. */
         call    reloc
         mov     %eax,sym_phys(multiboot_ptr)
 
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index ea8cb37..b22bf1e 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -5,15 +5,18 @@
  * and modules. This is most easily done early with paging disabled.
  *
  * Copyright (c) 2009, Citrix Systems, Inc.
+ * Copyright (c) 2013-2016 Oracle and/or its affiliates. All rights reserved.
  *
  * Authors:
  *    Keir Fraser <keir@xen.org>
+ *    Daniel Kiper <daniel.kiper@oracle.com>
  */
 
 /*
  * This entry point is entered from xen/arch/x86/boot/head.S with:
- *   - 0x4(%esp) = MULTIBOOT_INFORMATION_ADDRESS,
- *   - 0x8(%esp) = BOOT_TRAMPOLINE_ADDRESS.
+ *   - 0x4(%esp) = MULTIBOOT_MAGIC,
+ *   - 0x8(%esp) = MULTIBOOT_INFORMATION_ADDRESS,
+ *   - 0xc(%esp) = BOOT_TRAMPOLINE_ADDRESS.
  */
 asm (
     "    .text                         \n"
@@ -23,7 +26,12 @@ asm (
     );
 
 typedef unsigned int u32;
+typedef unsigned long long u64;
+
 #include "../../../include/xen/multiboot.h"
+#include "../../../include/xen/multiboot2.h"
+
+#define NULL		((void *)0)
 
 #define __stdcall	__attribute__((__stdcall__))
 
@@ -32,6 +40,9 @@ typedef unsigned int u32;
 
 #define _p(val)		((void *)(unsigned long)(val))
 
+#define get_mb2_data(tag, type, member)   (((multiboot2_tag_##type##_t *)(tag))->member)
+#define get_mb2_string(tag, type, member) ((u32)get_mb2_data(tag, type, member))
+
 static u32 alloc;
 
 static u32 alloc_mem(u32 bytes)
@@ -39,6 +50,12 @@ static u32 alloc_mem(u32 bytes)
     return alloc -= ALIGN_UP(bytes, 16);
 }
 
+static void zero_mem(u32 s, u32 bytes)
+{
+    while ( bytes-- )
+        *(char *)s++ = 0;
+}
+
 static u32 copy_mem(u32 src, u32 bytes)
 {
     u32 dst, dst_ret;
@@ -65,13 +82,11 @@ static u32 copy_string(u32 src)
     return copy_mem(src, p - src + 1);
 }
 
-multiboot_info_t __stdcall *reloc(u32 mbi_in, u32 trampoline)
+static multiboot_info_t *mbi_reloc(u32 mbi_in)
 {
     int i;
     multiboot_info_t *mbi_out;
 
-    alloc = trampoline;
-
     mbi_out = _p(copy_mem(mbi_in, sizeof(*mbi_out)));
 
     if ( mbi_out->flags & MBI_CMDLINE )
@@ -108,3 +123,126 @@ multiboot_info_t __stdcall *reloc(u32 mbi_in, u32 trampoline)
 
     return mbi_out;
 }
+
+static multiboot_info_t *mbi2_reloc(u32 mbi_in)
+{
+    const multiboot2_fixed_t *mbi_fix = _p(mbi_in);
+    const multiboot2_memory_map_t *mmap_src;
+    const multiboot2_tag_t *tag;
+    module_t *mbi_out_mods = NULL;
+    memory_map_t *mmap_dst;
+    multiboot_info_t *mbi_out;
+    u32 ptr;
+    unsigned int i, mod_idx = 0;
+
+    ptr = alloc_mem(sizeof(*mbi_out));
+    mbi_out = _p(ptr);
+    zero_mem(ptr, sizeof(*mbi_out));
+
+    /* Skip Multiboot2 information fixed part. */
+    ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
+
+    /* Get the number of modules. */
+    for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
+          tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+    {
+        if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
+            ++mbi_out->mods_count;
+        else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
+            break;
+    }
+
+    if ( mbi_out->mods_count )
+    {
+        mbi_out->flags |= MBI_MODULES;
+        mbi_out->mods_addr = alloc_mem(mbi_out->mods_count * sizeof(*mbi_out_mods));
+        mbi_out_mods = _p(mbi_out->mods_addr);
+    }
+
+    /* Skip Multiboot2 information fixed part. */
+    ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
+
+    /* Put all needed data into mbi_out. */
+    for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
+          tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+        switch ( tag->type )
+        {
+        case MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME:
+            mbi_out->flags |= MBI_LOADERNAME;
+            ptr = get_mb2_string(tag, string, string);
+            mbi_out->boot_loader_name = copy_string(ptr);
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_CMDLINE:
+            mbi_out->flags |= MBI_CMDLINE;
+            ptr = get_mb2_string(tag, string, string);
+            mbi_out->cmdline = copy_string(ptr);
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO:
+            mbi_out->flags |= MBI_MEMLIMITS;
+            mbi_out->mem_lower = get_mb2_data(tag, basic_meminfo, mem_lower);
+            mbi_out->mem_upper = get_mb2_data(tag, basic_meminfo, mem_upper);
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_MMAP:
+            if ( get_mb2_data(tag, mmap, entry_size) < sizeof(*mmap_src) )
+                break;
+
+            mbi_out->flags |= MBI_MEMMAP;
+            mbi_out->mmap_length = get_mb2_data(tag, mmap, size);
+            mbi_out->mmap_length -= sizeof(multiboot2_tag_mmap_t);
+            mbi_out->mmap_length /= get_mb2_data(tag, mmap, entry_size);
+            mbi_out->mmap_length *= sizeof(*mmap_dst);
+
+            mbi_out->mmap_addr = alloc_mem(mbi_out->mmap_length);
+
+            mmap_src = get_mb2_data(tag, mmap, entries);
+            mmap_dst = _p(mbi_out->mmap_addr);
+
+            for ( i = 0; i < mbi_out->mmap_length / sizeof(*mmap_dst); i++ )
+            {
+                /* Init size member properly. */
+                mmap_dst[i].size = sizeof(*mmap_dst);
+                mmap_dst[i].size -= sizeof(mmap_dst[i].size);
+                /* Now copy a given region data. */
+                mmap_dst[i].base_addr_low = (u32)mmap_src->addr;
+                mmap_dst[i].base_addr_high = (u32)(mmap_src->addr >> 32);
+                mmap_dst[i].length_low = (u32)mmap_src->len;
+                mmap_dst[i].length_high = (u32)(mmap_src->len >> 32);
+                mmap_dst[i].type = mmap_src->type;
+                mmap_src = _p(mmap_src) + get_mb2_data(tag, mmap, entry_size);
+            }
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_MODULE:
+            if ( mod_idx >= mbi_out->mods_count )
+                break;
+
+            mbi_out_mods[mod_idx].mod_start = get_mb2_data(tag, module, mod_start);
+            mbi_out_mods[mod_idx].mod_end = get_mb2_data(tag, module, mod_end);
+            ptr = get_mb2_string(tag, module, cmdline);
+            mbi_out_mods[mod_idx].string = copy_string(ptr);
+            mbi_out_mods[mod_idx].reserved = 0;
+            ++mod_idx;
+            break;
+
+        case MULTIBOOT2_TAG_TYPE_END:
+            return mbi_out;
+
+        default:
+            break;
+        }
+
+    return mbi_out;
+}
+
+multiboot_info_t __stdcall *reloc(u32 mb_magic, u32 mbi_in, u32 trampoline)
+{
+    alloc = trampoline;
+
+    if ( mb_magic == MULTIBOOT2_BOOTLOADER_MAGIC )
+        return mbi2_reloc(mbi_in);
+    else
+        return mbi_reloc(mbi_in);
+}
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 0e1f09d..92f5d81 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -13,6 +13,7 @@
 #include <asm/fixmap.h>
 #include <asm/hardirq.h>
 #include <xen/multiboot.h>
+#include <xen/multiboot2.h>
 
 #define DEFINE(_sym, _val)                                                 \
     asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
@@ -167,6 +168,14 @@ void __dummy__(void)
     OFFSET(MB_flags, multiboot_info_t, flags);
     OFFSET(MB_cmdline, multiboot_info_t, cmdline);
     OFFSET(MB_mem_lower, multiboot_info_t, mem_lower);
+    BLANK();
+
+    DEFINE(MB2_fixed_sizeof, sizeof(multiboot2_fixed_t));
+    OFFSET(MB2_fixed_total_size, multiboot2_fixed_t, total_size);
+    OFFSET(MB2_tag_type, multiboot2_tag_t, type);
+    OFFSET(MB2_tag_size, multiboot2_tag_t, size);
+    OFFSET(MB2_mem_lower, multiboot2_tag_basic_meminfo_t, mem_lower);
+    BLANK();
 
     OFFSET(DOMAIN_vm_assist, struct domain, vm_assist);
 }
diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h
new file mode 100644
index 0000000..8dd5800
--- /dev/null
+++ b/xen/include/xen/multiboot2.h
@@ -0,0 +1,169 @@
+/*
+ *  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
+#define MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI32	8
+#define MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64	9
+
+/* 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
+#define MULTIBOOT2_TAG_TYPE_EFI32_IH			19
+#define MULTIBOOT2_TAG_TYPE_EFI64_IH			20
+
+/* 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 {
+    u32 total_size;
+    u32 reserved;
+} multiboot2_fixed_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+} multiboot2_tag_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    char string[];
+} multiboot2_tag_string_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 mem_lower;
+    u32 mem_upper;
+} multiboot2_tag_basic_meminfo_t;
+
+typedef struct {
+    u64 addr;
+    u64 len;
+    u32 type;
+    u32 zero;
+} multiboot2_memory_map_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 entry_size;
+    u32 entry_version;
+    multiboot2_memory_map_t entries[];
+} multiboot2_tag_mmap_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u64 pointer;
+} multiboot2_tag_efi64_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u64 pointer;
+} multiboot2_tag_efi64_ih_t;
+
+typedef struct {
+    u32 type;
+    u32 size;
+    u32 mod_start;
+    u32 mod_end;
+    char cmdline[];
+} multiboot2_tag_module_t;
+#endif /* __ASSEMBLY__ */
+
+#endif /* __MULTIBOOT2_H__ */
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 2/5] efi: build xen.gz with EFI code
  2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
  2017-01-18 14:16 ` [PATCH v4 1/5] x86: add " Doug Goldstein
@ 2017-01-18 14:16 ` Doug Goldstein
  2017-01-18 14:17 ` [PATCH v4 3/5] efi: create new early memory allocator Doug Goldstein
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:16 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Daniel Kiper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, Jan Beulich,
	qiaowei.ren, gang.wei, fu.wei

From: Daniel Kiper <daniel.kiper@oracle.com>

Build xen.gz with EFI code. We need this to support multiboot2
protocol on EFI platforms.

If we wish to load non-ELF file using multiboot (v1) or multiboot2 then
it must contain "linear" (or "flat") representation of code and data.
This is requirement of both boot protocols. Currently, PE file contains
many sections which are not "linear" (one after another without any holes)
or even do not have representation in a file (e.g. BSS). From EFI point
of view everything is OK and works. However, this file layout cannot be
properly interpreted by multiboot protocols family. In theory there is
a chance that we could build proper PE file (from multiboot protocols POV)
using current build system. However, it means that xen.efi further diverge
from Xen ELF file (in terms of contents and build method). On the other
hand ELF has all needed properties. So, it means that this is good starting
point for further development. Additionally, I think that this is also good
starting point for further xen.efi code and build optimizations. It looks
that there is a chance that finally we can generate xen.efi directly from
Xen ELF using just simple objcopy or other tool. This way we will have one
Xen binary which can be loaded by three boot protocols: EFI native loader,
multiboot (v1) and multiboot2.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
v6 - suggestions/fixes:
   - improve efi_enabled() checks in efi_runtime_call()
     (suggested by Jan Beulich).

v5 - suggestions/fixes:
   - properly calculate efi symbol address in
     xen/arch/x86/xen.lds.S (I hope that this
     change does not invalidate Jan's ACK).

v4 - suggestions/fixes:
   - functions should return -ENOSYS instead
     of -EOPNOTSUPP if EFI runtime services
     are not available
     (suggested by Jan Beulich),
   - remove stale bits from xen/arch/x86/Makefile
     (suggested by Jan Beulich).

v3 - suggestions/fixes:
   - check for EFI platform in EFI code
     (suggested by Jan Beulich),
   - fix Makefiles
     (suggested by Jan Beulich),
   - improve commit message
     (suggested by Jan Beulich).

v2 - suggestions/fixes:
   - build EFI code only if it is supported in a given build environment
     (suggested by Jan Beulich).
---
---
 xen/arch/x86/Makefile     |  2 +-
 xen/arch/x86/efi/Makefile | 12 ++++--------
 xen/arch/x86/xen.lds.S    |  4 ++--
 xen/common/efi/boot.c     |  3 +++
 xen/common/efi/runtime.c  |  9 +++++++++
 5 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7f6b5d7..2e22cdf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,6 +219,6 @@ efi/mkreloc: efi/mkreloc.c
 clean::
 	rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
 	rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
-	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi efi/disabled efi/mkreloc
+	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
 	rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
 	rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index ad3fdf7..442f3fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,18 +1,14 @@
 CFLAGS += -fshort-wchar
 
-obj-y += stub.o
-
-create = test -e $(1) || touch -t 199901010000 $(1)
-
 efi := y$(shell rm -f disabled)
 efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c check.c 2>disabled && echo y))
 efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o)))
-
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
+efi := $(if $(efi),$(shell rm disabled)y)
 
 %.o: %.ihex
 	$(OBJCOPY) -I ihex -O binary $< $@
 
-stub.o: $(extra-y)
+obj-y := stub.o
+obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(efi) += buildid.o
 nogcov-$(efi) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7676de9..b0b1c9b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -270,10 +270,10 @@ SECTIONS
   .pad : {
     . = ALIGN(MB(16));
   } :text
-#else
-  efi = .;
 #endif
 
+  efi = DEFINED(efi) ? efi : .;
+
   /* Sections to be discarded */
   /DISCARD/ : {
        *(.exit.text)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3e5e4ab..df8c702 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1251,6 +1251,9 @@ void __init efi_init_memory(void)
     } *extra, *extra_head = NULL;
 #endif
 
+    if ( !efi_enabled(EFI_BOOT) )
+        return;
+
     printk(XENLOG_INFO "EFI memory map:%s\n",
            map_bs ? " (mapping BootServices)" : "");
     for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 8c44835..25323de 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -196,6 +196,9 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
 {
     unsigned int i, n;
 
+    if ( !efi_enabled(EFI_BOOT) )
+        return -ENOSYS;
+
     switch ( idx )
     {
     case XEN_FW_EFI_VERSION:
@@ -331,6 +334,12 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
     EFI_STATUS status = EFI_NOT_STARTED;
     int rc = 0;
 
+    if ( !efi_enabled(EFI_BOOT) )
+        return -ENOSYS;
+
+    if ( !efi_enabled(EFI_RS) )
+        return -EOPNOTSUPP;
+
     switch ( op->function )
     {
     case XEN_EFI_get_time:
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 3/5] efi: create new early memory allocator
  2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
  2017-01-18 14:16 ` [PATCH v4 1/5] x86: add " Doug Goldstein
  2017-01-18 14:16 ` [PATCH v4 2/5] efi: build xen.gz with EFI code Doug Goldstein
@ 2017-01-18 14:17 ` Doug Goldstein
  2017-01-18 14:17 ` [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
  2017-01-18 14:17 ` [PATCH v4 5/5] fix: " Doug Goldstein
  4 siblings, 0 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:17 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Daniel Kiper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, Jan Beulich,
	qiaowei.ren, gang.wei, fu.wei

From: Daniel Kiper <daniel.kiper@oracle.com>

There is a problem with place_string() which is used as early memory
allocator. It gets memory chunks starting from start symbol and goes
down. Sadly this does not work when Xen is loaded using multiboot2
protocol because then the start lives on 1 MiB address and we should
not allocate a memory from below of it. So, I tried to use mem_lower
address calculated by GRUB2. However, this solution works only on some
machines. There are machines in the wild (e.g. Dell PowerEdge R820)
which uses first ~640 KiB for boot services code or data... :-(((
Hence, we need new memory allocator for Xen EFI boot code which is
quite simple and generic and could be used by place_string() and
efi_arch_allocate_mmap_buffer(). I think about following solutions:

1) We could use native EFI allocation functions (e.g. AllocatePool()
   or AllocatePages()) to get memory chunk. However, later (somewhere
   in __start_xen()) we must copy its contents to safe place or reserve
   it in e820 memory map and map it in Xen virtual address space. This
   means that the code referring to Xen command line, loaded modules and
   EFI memory map, mostly in __start_xen(), will be further complicated
   and diverge from legacy BIOS cases. Additionally, both former things
   have to be placed below 4 GiB because their addresses are stored in
   multiboot_info_t structure which has 32-bit relevant members.

2) We may allocate memory area statically somewhere in Xen code which
   could be used as memory pool for early dynamic allocations. Looks
   quite simple. Additionally, it would not depend on EFI at all and
   could be used on legacy BIOS platforms if we need it. However, we
   must carefully choose size of this pool. We do not want increase Xen
   binary size too much and waste too much memory but also we must fit
   at least memory map on x86 EFI platforms. As I saw on small machine,
   e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more
   than 200 entries. Every entry on x86-64 platform is 40 bytes in size.
   So, it means that we need more than 8 KiB for EFI memory map only.
   Additionally, if we use this memory pool for Xen and modules command
   line storage (it would be used when xen.efi is executed as EFI application)
   then we should add, I think, about 1 KiB. In this case, to be on safe
   side, we should assume at least 64 KiB pool for early memory allocations.
   Which is about 4 times of our earlier calculations. However, during
   discussion on Xen-devel Jan Beulich suggested that just in case we should
   use 1 MiB memory pool like it is in original place_string() implementation.
   So, let's use 1 MiB as it was proposed. If we think that we should not
   waste unallocated memory in the pool on running system then we can mark
   this region as __initdata and move all required data to dynamically
   allocated places somewhere in __start_xen().

2a) We could put memory pool into .bss.page_aligned section. Then allocate
    memory chunks starting from the lowest address. After init phase we can
    free unused portion of the memory pool as in case of .init.text or .init.data
    sections. This way we do not need to allocate any space in image file and
    freeing of unused area in the memory pool is very simple.

Now #2a solution is implemented because it is quite simple and requires
limited number of changes, especially in __start_xen().

The new allocator is quite generic and can be used on ARM platforms too.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
Doug v1 - removed stale paragraph

v11 - suggestions/fixes:
    - #ifdef only EBMALLOC_SIZE from ebmalloc machinery
      (suggested by Jan Beulich).

v10 - suggestions/fixes:
    - remove unneeded ARM free_ebmalloc_unused_mem() stub.

v9 - suggestions/fixes:
   - call free_ebmalloc_unused_mem() from efi_init_memory()
     instead of xen/arch/arm/setup.c:init_done()
     (suggested by Jan Beulich),
   - improve comments.

v8 - suggestions/fixes:
   - disable whole ebmalloc machinery on ARM platforms,
   - add comment saying what should be done before
     enabling ebmalloc on ARM,
     (suggested by Julien Grall),
   - move ebmalloc code before efi-boot.h inclusion and
     remove unneeded forward declaration
     (suggested by Jan Beulich),
   - remove free_ebmalloc_unused_mem() call from
     xen/arch/arm/setup.c:init_done()
     (suggested by Julien Grall),
   - improve commit message.

v7 - suggestions/fixes:
   - enable most of ebmalloc machinery on ARM platforms
     (suggested by Jan Beulich),
   - remove unneeded cast
     (suggested by Jan Beulich),
   - wrap long line
     (suggested by Jan Beulich),
   - improve commit message.

v6 - suggestions/fixes:
   - optimize ebmalloc allocator,
   - move ebmalloc machinery to xen/common/efi/boot.c
     (suggested by Jan Beulich),
   - enforce PAGE_SIZE ebmalloc_mem alignment
     (suggested by Jan Beulich),
   - ebmalloc() must allocate properly
     aligned memory regions
     (suggested by Jan Beulich),
   - printk() should use XENLOG_INFO
     (suggested by Jan Beulich).

v4 - suggestions/fixes:
   - move from #2 solution to #2a solution,
   - improve commit message.
---
---
 xen/arch/x86/efi/efi-boot.h | 11 ++------
 xen/arch/x86/setup.c        |  3 +--
 xen/common/efi/boot.c       | 50 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 388c4ea..62c010e 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -114,7 +114,7 @@ static void __init relocate_trampoline(unsigned long phys)
 
 static void __init place_string(u32 *addr, const char *s)
 {
-    static char *__initdata alloc = start;
+    char *alloc = NULL;
 
     if ( s && *s )
     {
@@ -122,7 +122,7 @@ static void __init place_string(u32 *addr, const char *s)
         const char *old = (char *)(long)*addr;
         size_t len2 = *addr ? strlen(old) + 1 : 0;
 
-        alloc -= len1 + len2;
+        alloc = ebmalloc(len1 + len2);
         /*
          * Insert new string before already existing one. This is needed
          * for options passed on the command line to override options from
@@ -205,12 +205,7 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 
 static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
 {
-    place_string(&mbi.mem_upper, NULL);
-    mbi.mem_upper -= map_size;
-    mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);
-    if ( mbi.mem_upper < xen_phys_start )
-        return NULL;
-    return (void *)(long)mbi.mem_upper;
+    return ebmalloc(map_size);
 }
 
 static void __init efi_arch_pre_exit_boot(void)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 0ccef1d..d4b7d25 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1124,8 +1124,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     if ( !xen_phys_start )
         panic("Not enough memory to relocate Xen.");
-    reserve_e820_ram(&boot_e820, efi_enabled(EFI_LOADER) ? mbi->mem_upper : __pa(&_start),
-                     __pa(&_end));
+    reserve_e820_ram(&boot_e820, __pa(&_start), __pa(&_end));
 
     /* Late kexec reservation (dynamic start address). */
     kexec_reserve_area(&boot_e820);
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index df8c702..36dbb71 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -98,6 +98,54 @@ static CHAR16 __initdata newline[] = L"\r\n";
 #define PrintStr(s) StdOut->OutputString(StdOut, s)
 #define PrintErr(s) StdErr->OutputString(StdErr, s)
 
+#ifdef CONFIG_ARM
+/*
+ * TODO: Enable EFI boot allocator on ARM.
+ * This code can be common for x86 and ARM.
+ * Things TODO on ARM before enabling ebmalloc:
+ *   - estimate required EBMALLOC_SIZE value,
+ *   - where (in which section) ebmalloc_mem[] should live; if in
+ *     .bss.page_aligned, as it is right now, then whole BSS zeroing
+ *     have to be disabled in xen/arch/arm/arm64/head.S; though BSS
+ *     should be initialized somehow before use of variables living there,
+ *   - use ebmalloc() in ARM/common EFI boot code,
+ *   - call free_ebmalloc_unused_mem() somewhere in init code.
+ */
+#define EBMALLOC_SIZE	MB(0)
+#else
+#define EBMALLOC_SIZE	MB(1)
+#endif
+
+static char __section(".bss.page_aligned") __aligned(PAGE_SIZE)
+    ebmalloc_mem[EBMALLOC_SIZE];
+static unsigned long __initdata ebmalloc_allocated;
+
+/* EFI boot allocator. */
+static void __init __maybe_unused *ebmalloc(size_t size)
+{
+    void *ptr = ebmalloc_mem + ebmalloc_allocated;
+
+    ebmalloc_allocated += (size + sizeof(void *) - 1) & ~(sizeof(void *) - 1);
+
+    if ( ebmalloc_allocated > sizeof(ebmalloc_mem) )
+        blexit(L"Out of static memory\r\n");
+
+    return ptr;
+}
+
+static void __init __maybe_unused free_ebmalloc_unused_mem(void)
+{
+    unsigned long start, end;
+
+    start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated);
+    end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem);
+
+    destroy_xen_mappings(start, end);
+    init_xenheap_pages(__pa(start), __pa(end));
+
+    printk(XENLOG_INFO "Freed %lukB unused BSS memory\n", (end - start) >> 10);
+}
+
 /*
  * Include architecture specific implementation here, which references the
  * static globals defined above.
@@ -1251,6 +1299,8 @@ void __init efi_init_memory(void)
     } *extra, *extra_head = NULL;
 #endif
 
+    free_ebmalloc_unused_mem();
+
     if ( !efi_enabled(EFI_BOOT) )
         return;
 
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
                   ` (2 preceding siblings ...)
  2017-01-18 14:17 ` [PATCH v4 3/5] efi: create new early memory allocator Doug Goldstein
@ 2017-01-18 14:17 ` Doug Goldstein
  2017-01-18 19:51   ` Andrew Cooper
  2017-01-18 14:17 ` [PATCH v4 5/5] fix: " Doug Goldstein
  4 siblings, 1 reply; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:17 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Daniel Kiper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, Jan Beulich,
	qiaowei.ren, gang.wei, fu.wei

From: Daniel Kiper <daniel.kiper@oracle.com>

This way Xen can be loaded on EFI platforms using GRUB2 and
other boot loaders which support multiboot2 protocol.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Tested-by: Doug Goldstein <cardoe@cardoe.com>
---
Doug v2 - dropped all my changes and moved them into their own patch
Doug v1 - fix incorrect assembly (identified by Andrew Cooper)
   	- fix issue where the trampoline size was left as 0 and the
	  way the memory is allocated for the trampolines we would go to
	  the end of an available section and then subtract off the size
	  to decide where to place it. The end result was that we would
	  always copy the trampolines and the 32-bit stack into some
	  form of reserved memory after the conventional region we
	  wanted to put things into. On some systems this did not
	  manifest as a crash while on others it did. Reworked the
	  changes to always reserve 64kb for both the stack and the size
	  of the trampolines. Added an ASSERT to make sure we never blow
	  through this size.

v10 - suggestions/fixes:
    - replace ljmpl with lretq
      (suggested by Andrew Cooper),
    - introduce efi_platform to increase code readability
      (suggested by Andrew Cooper).

v9 - suggestions/fixes:
   - use .L labels instead of numeric ones in multiboot2 data scanning loops
     (suggested by Jan Beulich).

v8 - suggestions/fixes:
   - use __bss_start(%rip)/__bss_end(%rip) instead of
     of .startof.(.bss)(%rip)/$.sizeof.(.bss) because
     latter is not tested extensively in different
     built environments yet
     (suggested by Andrew Cooper),
   - fix multiboot2 data scanning loop in x86_32 code
     (suggested by Jan Beulich),
   - add check for extra mem for mbi data if Xen is loaded
     via multiboot2 protocol on EFI platform
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich).

v7 - suggestions/fixes:
   - do not allocate twice memory for trampoline if we were
     loaded via multiboot2 protocol on EFI platform,
   - wrap long line
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich).

v6 - suggestions/fixes:
   - improve label names in assembly
     error printing code
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich),
   - various minor cleanups and fixes
     (suggested by Jan Beulich).

v4 - suggestions/fixes:
   - remove redundant BSS alignment,
   - update BSS alignment check,
   - use __set_bit() instead of set_bit() if possible
     (suggested by Jan Beulich),
   - call efi_arch_cpu() from efi_multiboot2()
     even if the same work is done later in
     other place right now
     (suggested by Jan Beulich),
   - xen/arch/x86/efi/stub.c:efi_multiboot2()
     fail properly on EFI platforms,
   - do not read data beyond the end of multiboot2
     information in xen/arch/x86/boot/head.S
     (suggested by Jan Beulich),
   - use 32-bit registers in x86_64 code if possible
     (suggested by Jan Beulich),
   - multiboot2 information address is 64-bit
     in x86_64 code, so, treat it is as is
     (suggested by Jan Beulich),
   - use cmovcc if possible,
   - leave only one space between rep and stosq
     (suggested by Jan Beulich),
   - improve error handling,
   - improve early error messages,
     (suggested by Jan Beulich),
   - improve early error messages printing code,
   - improve label names
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich),
   - various minor cleanups.

v3 - suggestions/fixes:
   - take into account alignment when skipping multiboot2 fixed part
     (suggested by Konrad Rzeszutek Wilk),
   - improve segment registers initialization
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich and Konrad Rzeszutek Wilk),
   - improve commit message
     (suggested by Jan Beulich).

v2 - suggestions/fixes:
   - generate multiboot2 header using macros
     (suggested by Jan Beulich),
   - switch CPU to x86_32 mode before
     jumping to 32-bit code
     (suggested by Andrew Cooper),
   - reduce code changes to increase patch readability
     (suggested by Jan Beulich),
   - improve comments
     (suggested by Jan Beulich),
   - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform
     and find on my own multiboot2.mem_lower value,
   - stop execution if EFI platform is detected
     in legacy BIOS path.
---
---
 xen/arch/x86/boot/head.S          | 263 +++++++++++++++++++++++++++++--
 xen/arch/x86/efi/efi-boot.h       |  54 +++++-
 xen/arch/x86/efi/stub.c           |  38 ++++-
 xen/arch/x86/x86_64/asm-offsets.c |   2 +-
 xen/arch/x86/xen.lds.S            |   4 +-
 xen/common/efi/boot.c             |  11 +-
 6 files changed, 349 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index d423fd8..ac93df0 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -89,6 +89,13 @@ multiboot2_header_start:
                    0, /* Number of the lines - no preference. */ \
                    0  /* Number of bits per pixel - no preference. */
 
+        /* Inhibit bootloader from calling ExitBootServices(). */
+        mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL)
+
+        /* EFI64 entry point. */
+        mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
+                   sym_phys(__efi64_start)
+
         /* Multiboot2 header end tag. */
         mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
 .Lmultiboot2_header_end:
@@ -100,20 +107,48 @@ multiboot2_header_start:
 gdt_boot_descr:
         .word   6*8-1
         .long   sym_phys(trampoline_gdt)
+        .long   0 /* Needed for 64-bit lgdt */
+
+        .align 4
+vga_text_buffer:
+        .long   0xb8000
+
+efi_platform:
+        .byte   0
 
 .Lbad_cpu_msg: .asciz "ERR: Not a 64-bit CPU!"
 .Lbad_ldr_msg: .asciz "ERR: Not a Multiboot bootloader!"
+.Lbad_ldr_nbs: .asciz "ERR: Bootloader shutdown EFI x64 boot services!"
+.Lbad_ldr_nst: .asciz "ERR: EFI SystemTable is not provided by bootloader!"
+.Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!"
+.Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!"
 
         .section .init.text, "ax", @progbits
 
 bad_cpu:
         mov     $(sym_phys(.Lbad_cpu_msg)),%esi # Error message
-        jmp     print_err
+        jmp     .Lget_vtb
 not_multiboot:
         mov     $(sym_phys(.Lbad_ldr_msg)),%esi # Error message
-print_err:
-        mov     $0xB8000,%edi  # VGA framebuffer
-1:      mov     (%esi),%bl
+        jmp     .Lget_vtb
+.Lmb2_no_st:
+        mov     $(sym_phys(.Lbad_ldr_nst)),%esi # Error message
+        jmp     .Lget_vtb
+.Lmb2_no_ih:
+        mov     $(sym_phys(.Lbad_ldr_nih)),%esi # Error message
+        jmp     .Lget_vtb
+.Lmb2_no_bs:
+        mov     $(sym_phys(.Lbad_ldr_nbs)),%esi # Error message
+        xor     %edi,%edi                       # No VGA text buffer
+        jmp     .Lsend_chr
+.Lmb2_efi_ia_32:
+        mov     $(sym_phys(.Lbad_efi_msg)),%esi # Error message
+        xor     %edi,%edi                       # No VGA text buffer
+        jmp     .Lsend_chr
+.Lget_vtb:
+        mov     sym_phys(vga_text_buffer),%edi
+.Lsend_chr:
+        mov     (%esi),%bl
         test    %bl,%bl        # Terminate on '\0' sentinel
         je      .Lhalt
         mov     $0x3f8+5,%dx   # UART Line Status Register
@@ -123,13 +158,186 @@ print_err:
         mov     $0x3f8+0,%dx   # UART Transmit Holding Register
         mov     %bl,%al
         out     %al,%dx        # Send a character over the serial line
-        movsb                  # Write a character to the VGA framebuffer
+        test    %edi,%edi      # Is the VGA text buffer available?
+        jz      .Lsend_chr
+        movsb                  # Write a character to the VGA text buffer
         mov     $7,%al
-        stosb                  # Write an attribute to the VGA framebuffer
-        jmp     1b
+        stosb                  # Write an attribute to the VGA text buffer
+        jmp     .Lsend_chr
 .Lhalt: hlt
         jmp     .Lhalt
 
+        .code64
+
+__efi64_start:
+        cld
+
+        /* VGA is not available on EFI platforms. */
+        movl   $0,vga_text_buffer(%rip)
+
+        /* Check for Multiboot2 bootloader. */
+        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
+        je      .Lefi_multiboot2_proto
+
+        /* Jump to not_multiboot after switching CPU to x86_32 mode. */
+        lea     not_multiboot(%rip),%edi
+        jmp     x86_32_switch
+
+.Lefi_multiboot2_proto:
+        /* Zero EFI SystemTable and EFI ImageHandle addresses. */
+        xor     %esi,%esi
+        xor     %edi,%edi
+
+        /* Skip Multiboot2 information fixed part. */
+        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
+        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+
+.Lefi_mb2_tsize:
+        /* Check Multiboot2 information total size. */
+        mov     %ecx,%r8d
+        sub     %ebx,%r8d
+        cmp     %r8d,MB2_fixed_total_size(%rbx)
+        jbe     run_bs
+
+        /* Are EFI boot services available? */
+        cmpl    $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx)
+        jne     .Lefi_mb2_st
+
+        /* We are on EFI platform and EFI boot services are available. */
+        incb    efi_platform(%rip)
+
+        /*
+         * Disable real mode and other legacy stuff which should not
+         * be run on EFI platforms.
+         */
+        incb    skip_realmode(%rip)
+        jmp     .Lefi_mb2_next_tag
+
+.Lefi_mb2_st:
+        /* Get EFI SystemTable address from Multiboot2 information. */
+        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx)
+        cmove   MB2_efi64_st(%rcx),%rsi
+        je      .Lefi_mb2_next_tag
+
+        /* Get EFI ImageHandle address from Multiboot2 information. */
+        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64_IH,MB2_tag_type(%rcx)
+        cmove   MB2_efi64_ih(%rcx),%rdi
+        je      .Lefi_mb2_next_tag
+
+        /* Is it the end of Multiboot2 information? */
+        cmpl    $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
+        je      run_bs
+
+.Lefi_mb2_next_tag:
+        /* Go to next Multiboot2 information tag. */
+        add     MB2_tag_size(%rcx),%ecx
+        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        jmp     .Lefi_mb2_tsize
+
+run_bs:
+        /* Are EFI boot services available? */
+        cmpb    $0,efi_platform(%rip)
+        jnz     0f
+
+        /* Jump to .Lmb2_no_bs after switching CPU to x86_32 mode. */
+        lea     .Lmb2_no_bs(%rip),%edi
+        jmp     x86_32_switch
+
+0:
+        /* Is EFI SystemTable address provided by boot loader? */
+        test    %rsi,%rsi
+        jnz     1f
+
+        /* Jump to .Lmb2_no_st after switching CPU to x86_32 mode. */
+        lea     .Lmb2_no_st(%rip),%edi
+        jmp     x86_32_switch
+
+1:
+        /* Is EFI ImageHandle address provided by boot loader? */
+        test    %rdi,%rdi
+        jnz     2f
+
+        /* Jump to .Lmb2_no_ih after switching CPU to x86_32 mode. */
+        lea     .Lmb2_no_ih(%rip),%edi
+        jmp     x86_32_switch
+
+2:
+        push    %rax
+        push    %rdi
+
+        /*
+         * Initialize BSS (no nasty surprises!).
+         * It must be done earlier than in BIOS case
+         * because efi_multiboot2() touches it.
+         */
+        lea     __bss_start(%rip),%edi
+        lea     __bss_end(%rip),%ecx
+        sub     %edi,%ecx
+        shr     $3,%ecx
+        xor     %eax,%eax
+        rep stosq
+
+        pop     %rdi
+
+        /*
+         * efi_multiboot2() is called according to System V AMD64 ABI:
+         *   - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
+         *   - OUT: %rax - highest usable memory address below 1 MiB;
+         *                 memory above this address is reserved for trampoline;
+         *                 memory below this address is used as a storage for
+         *                 mbi struct created in reloc().
+         *
+         * MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag is not provided
+         * on EFI platforms. Hence, it could not be used like
+         * on legacy BIOS platforms.
+         */
+        call    efi_multiboot2
+
+        /* Convert memory address to bytes/16 and store it in safe place. */
+        shr     $4,%eax
+        mov     %eax,%ecx
+
+        pop     %rax
+
+        /* Jump to trampoline_setup after switching CPU to x86_32 mode. */
+        lea     trampoline_setup(%rip),%edi
+
+x86_32_switch:
+        cli
+
+        /* Initialize GDTR. */
+        lgdt    gdt_boot_descr(%rip)
+
+        /* Reload code selector. */
+        pushq   $BOOT_CS32
+        lea     cs32_switch(%rip),%edx
+        push    %rdx
+        lretq
+
+        .code32
+
+cs32_switch:
+        /* Initialize basic data segments. */
+        mov     $BOOT_DS,%edx
+        mov     %edx,%ds
+        mov     %edx,%es
+        mov     %edx,%ss
+        /* %esp is initialized later. */
+
+        /* Load null descriptor to unused segment registers. */
+        xor     %edx,%edx
+        mov     %edx,%fs
+        mov     %edx,%gs
+
+        /* Disable paging. */
+        mov     %cr0,%edx
+        and     $(~X86_CR0_PG),%edx
+        mov     %edx,%cr0
+
+        /* Jump to earlier loaded address. */
+        jmp     *%edi
+
 __start:
         cld
         cli
@@ -157,7 +365,7 @@ __start:
 
         /* Not available? BDA value will be fine. */
         cmovnz  MB_mem_lower(%ebx),%edx
-        jmp     trampoline_setup
+        jmp     trampoline_bios_setup
 
 .Lmultiboot2_proto:
         /* Skip Multiboot2 information fixed part. */
@@ -169,24 +377,33 @@ __start:
         mov     %ecx,%edi
         sub     %ebx,%edi
         cmp     %edi,MB2_fixed_total_size(%ebx)
-        jbe     trampoline_setup
+        jbe     trampoline_bios_setup
 
         /* Get mem_lower from Multiboot2 information. */
         cmpl    $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,MB2_tag_type(%ecx)
         cmove   MB2_mem_lower(%ecx),%edx
-        je      trampoline_setup
+        je      .Lmb2_next_tag
+
+        /* EFI IA-32 platforms are not supported. */
+        cmpl    $MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx)
+        je      .Lmb2_efi_ia_32
+
+        /* Bootloader shutdown EFI x64 boot services. */
+        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx)
+        je      .Lmb2_no_bs
 
         /* Is it the end of Multiboot2 information? */
         cmpl    $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%ecx)
-        je      trampoline_setup
+        je      trampoline_bios_setup
 
+.Lmb2_next_tag:
         /* Go to next Multiboot2 information tag. */
         add     MB2_tag_size(%ecx),%ecx
         add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
         and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
         jmp     .Lmb2_tsize
 
-trampoline_setup:
+trampoline_bios_setup:
         /* Set up trampoline segment 64k below EBDA */
         movzwl  0x40e,%ecx          /* EBDA segment */
         cmp     $0xa000,%ecx        /* sanity check (high) */
@@ -202,16 +419,19 @@ trampoline_setup:
          * multiboot structure (if available) and use the smallest.
          */
         cmp     $0x100,%edx         /* is the multiboot value too small? */
-        jb      2f                  /* if so, do not use it */
+        jb      trampoline_setup    /* if so, do not use it */
         shl     $10-4,%edx
         cmp     %ecx,%edx           /* compare with BDA value */
         cmovb   %edx,%ecx           /* and use the smaller */
 
-2:      /* Reserve 64kb for the trampoline */
+        /* Reserve 64kb for the trampoline. */
         sub     $0x1000,%ecx
 
         /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */
         xor     %cl, %cl
+
+trampoline_setup:
+        /* Save trampoline address for later use. */
         shl     $4, %ecx
         mov     %ecx,sym_phys(trampoline_phys)
 
@@ -223,7 +443,14 @@ trampoline_setup:
         call    reloc
         mov     %eax,sym_phys(multiboot_ptr)
 
-        /* Initialize BSS (no nasty surprises!) */
+        /*
+         * Do not zero BSS on EFI platform here.
+         * It was initialized earlier.
+         */
+        cmpb    $0,sym_phys(efi_platform)
+        jnz     1f
+
+        /* Initialize BSS (no nasty surprises!). */
         mov     $sym_phys(__bss_start),%edi
         mov     $sym_phys(__bss_end),%ecx
         sub     %edi,%ecx
@@ -231,6 +458,7 @@ trampoline_setup:
         shr     $2,%ecx
         rep stosl
 
+1:
         /* Interrogate CPU extended features via CPUID. */
         mov     $0x80000000,%eax
         cpuid
@@ -282,8 +510,13 @@ trampoline_setup:
         cmp     $sym_phys(__trampoline_seg_stop),%edi
         jb      1b
 
+        /* Do not parse command line on EFI platform here. */
+        cmpb    $0,sym_phys(efi_platform)
+        jnz     1f
+
         call    cmdline_parse_early
 
+1:
         /* Switch to low-memory stack.  */
         mov     sym_phys(trampoline_phys),%edi
         lea     0x10000(%edi),%esp
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 62c010e..dc857d8 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -146,6 +146,8 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 {
     struct e820entry *e;
     unsigned int i;
+    /* Check for extra mem for mbi data if Xen is loaded via multiboot2 protocol. */
+    UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
 
     /* Populate E820 table and check trampoline area availability. */
     e = e820map - 1;
@@ -168,7 +170,8 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
             /* fall through */
         case EfiConventionalMemory:
             if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
-                 len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
+                 len >= cfg.size + extra_mem &&
+                 desc->PhysicalStart + len > cfg.addr )
                 cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
             /* fall through */
         case EfiLoaderCode:
@@ -210,12 +213,14 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
 
 static void __init efi_arch_pre_exit_boot(void)
 {
-    if ( !trampoline_phys )
-    {
-        if ( !cfg.addr )
-            blexit(L"No memory for trampoline");
+    if ( trampoline_phys )
+        return;
+
+    if ( !cfg.addr )
+        blexit(L"No memory for trampoline");
+
+    if ( efi_enabled(EFI_LOADER) )
         relocate_trampoline(cfg.addr);
-    }
 }
 
 static void __init noreturn efi_arch_post_exit_boot(void)
@@ -653,6 +658,43 @@ static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
 
 static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
 
+paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
+{
+    EFI_GRAPHICS_OUTPUT_PROTOCOL *gop;
+    UINTN cols, gop_mode = ~0, rows;
+
+    __set_bit(EFI_BOOT, &efi_flags);
+    __set_bit(EFI_RS, &efi_flags);
+
+    efi_init(ImageHandle, SystemTable);
+
+    efi_console_set_mode();
+
+    if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
+                           &cols, &rows) == EFI_SUCCESS )
+        efi_arch_console_init(cols, rows);
+
+    gop = efi_get_gop();
+
+    if ( gop )
+        gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
+
+    efi_arch_edd();
+    efi_arch_cpu();
+
+    efi_tables();
+    setup_efi_pci();
+    efi_variables();
+
+    if ( gop )
+        efi_set_gop_mode(gop, gop_mode);
+
+    efi_exit_boot(ImageHandle, SystemTable);
+
+    /* Return highest usable memory address below 1 MiB. */
+    return cfg.addr;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 4158124..6ea6aa1 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -3,6 +3,44 @@
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <asm/page.h>
+#include <asm/efibind.h>
+#include <efi/efidef.h>
+#include <efi/eficapsule.h>
+#include <efi/eficon.h>
+#include <efi/efidevp.h>
+#include <efi/efiapi.h>
+
+/*
+ * Here we are in EFI stub. EFI calls are not supported due to lack
+ * of relevant functionality in compiler and/or linker.
+ *
+ * efi_multiboot2() is an exception. Please look below for more details.
+ */
+
+paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
+                                       EFI_SYSTEM_TABLE *SystemTable)
+{
+    CHAR16 *err = L"Xen does not have EFI code build in!!!\r\nSystem halted!!!\r\n";
+    SIMPLE_TEXT_OUTPUT_INTERFACE *StdErr;
+
+    StdErr = SystemTable->StdErr ? SystemTable->StdErr : SystemTable->ConOut;
+
+    /*
+     * Print error message and halt the system.
+     *
+     * We have to open code MS x64 calling convention
+     * in assembly because here this convention may
+     * not be directly supported by C compiler.
+     */
+    asm volatile(
+    "    call %2                      \n"
+    "0:  hlt                          \n"
+    "    jmp  0b                      \n"
+       : "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString)
+       : "rax", "r8", "r9", "r10", "r11", "memory");
+
+    unreachable();
+}
 
 bool efi_enabled(unsigned int feature)
 {
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 92f5d81..f135654 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -175,6 +175,8 @@ void __dummy__(void)
     OFFSET(MB2_tag_type, multiboot2_tag_t, type);
     OFFSET(MB2_tag_size, multiboot2_tag_t, size);
     OFFSET(MB2_mem_lower, multiboot2_tag_basic_meminfo_t, mem_lower);
+    OFFSET(MB2_efi64_st, multiboot2_tag_efi64_t, pointer);
+    OFFSET(MB2_efi64_ih, multiboot2_tag_efi64_ih_t, pointer);
     BLANK();
 
     OFFSET(DOMAIN_vm_assist, struct domain, vm_assist);
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index b0b1c9b..e0e2529 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -331,5 +331,5 @@ ASSERT(IS_ALIGNED(__init_end,   PAGE_SIZE), "__init_end misaligned")
 
 ASSERT(IS_ALIGNED(trampoline_start, 4), "trampoline_start misaligned")
 ASSERT(IS_ALIGNED(trampoline_end,   4), "trampoline_end misaligned")
-ASSERT(IS_ALIGNED(__bss_start,      4), "__bss_start misaligned")
-ASSERT(IS_ALIGNED(__bss_end,        4), "__bss_end misaligned")
+ASSERT(IS_ALIGNED(__bss_start,      8), "__bss_start misaligned")
+ASSERT(IS_ALIGNED(__bss_end,        8), "__bss_end misaligned")
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 36dbb71..b6cbdad 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -79,6 +79,17 @@ static size_t wstrlen(const CHAR16 * s);
 static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
 static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
 
+static void efi_init(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable);
+static void efi_console_set_mode(void);
+static EFI_GRAPHICS_OUTPUT_PROTOCOL *efi_get_gop(void);
+static UINTN efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
+                               UINTN cols, UINTN rows, UINTN depth);
+static void efi_tables(void);
+static void setup_efi_pci(void);
+static void efi_variables(void);
+static void efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop_mode);
+static void efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable);
+
 static const EFI_BOOT_SERVICES *__initdata efi_bs;
 static UINT32 __initdata efi_bs_revision;
 static EFI_HANDLE __initdata efi_ih;
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
                   ` (3 preceding siblings ...)
  2017-01-18 14:17 ` [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
@ 2017-01-18 14:17 ` Doug Goldstein
  2017-01-18 14:52   ` Jan Beulich
  2017-01-18 19:53   ` Andrew Cooper
  4 siblings, 2 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 14:17 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, sstabellini, konrad.wilk, Andrew Cooper, Doug Goldstein,
	pgnet.dev, ning.sun, julien.grall, Jan Beulich, qiaowei.ren,
	gang.wei, fu.wei

This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
support for EFI platforms'.

- fix incorrect assembly (identified by Andrew Cooper)
- fix issue where the trampoline size was left as 0 and the
  way the memory is allocated for the trampolines we would go to
  the end of an available section and then subtract off the size
  to decide where to place it. The end result was that we would
  always copy the trampolines and the 32-bit stack into some
  form of reserved memory after the conventional region we
  wanted to put things into. On some systems this did not
  manifest as a crash while on others it did. Reworked the
  changes to always reserve 64kb for both the stack and the size
  of the trampolines. Added a build time assert to make sure we have
  enough room always.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
Doug v4 - change wording around "stack base"
          (found by Jan Beulich)
        - added build time assert as suggested by Jan Beulich
        - added a KB() macro to make our sizes consistent with MB() and
          GB().
Doug v3 - drop ASSERTs since they are runtime only without any output.
          This should be completely mitigated by using max() and
          ensuring we have a sane value.
          (found by Jan Beulich)
        - removed extra_mem variable that was incorrectly left behind.
          (found by Jan Beulich)
        - fix comment around the "start of stack"
          (found by Jan Beulich)
Doug v2 - new in this version to help show what's changed
---
---
 xen/arch/x86/boot/head.S    | 1 +
 xen/arch/x86/efi/efi-boot.h | 8 ++++----
 xen/arch/x86/efi/stub.c     | 2 +-
 xen/arch/x86/xen.lds.S      | 7 +++++++
 xen/include/xen/config.h    | 1 +
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac93df0..df5fdab 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -519,6 +519,7 @@ trampoline_setup:
 1:
         /* Switch to low-memory stack.  */
         mov     sym_phys(trampoline_phys),%edi
+        /* The stack ends 64kb after the location of trampoline_phys */
         lea     0x10000(%edi),%esp
         lea     trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
         pushl   $BOOT_CS32
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index dc857d8..d2ebf21 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -146,8 +146,6 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 {
     struct e820entry *e;
     unsigned int i;
-    /* Check for extra mem for mbi data if Xen is loaded via multiboot2 protocol. */
-    UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
 
     /* Populate E820 table and check trampoline area availability. */
     e = e820map - 1;
@@ -170,8 +168,7 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
             /* fall through */
         case EfiConventionalMemory:
             if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
-                 len >= cfg.size + extra_mem &&
-                 desc->PhysicalStart + len > cfg.addr )
+                 len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
                 cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
             /* fall through */
         case EfiLoaderCode:
@@ -686,6 +683,9 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTa
     setup_efi_pci();
     efi_variables();
 
+    /* This is the maximum size of our trampoline + our low memory stack */
+    cfg.size = KB(64);
+
     if ( gop )
         efi_set_gop_mode(gop, gop_mode);
 
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 6ea6aa1..b81adc0 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -33,7 +33,7 @@ paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
      * not be directly supported by C compiler.
      */
     asm volatile(
-    "    call %2                      \n"
+    "    call *%2                     \n"
     "0:  hlt                          \n"
     "    jmp  0b                      \n"
        : "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e0e2529..504348e 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -333,3 +333,10 @@ ASSERT(IS_ALIGNED(trampoline_start, 4), "trampoline_start misaligned")
 ASSERT(IS_ALIGNED(trampoline_end,   4), "trampoline_end misaligned")
 ASSERT(IS_ALIGNED(__bss_start,      8), "__bss_start misaligned")
 ASSERT(IS_ALIGNED(__bss_end,        8), "__bss_end misaligned")
+
+/* The trampolines and the low memory stack must fit in 64kb. In testing
+ * the low memory stack never exceeded 1kb so just require that the
+ * trampolines fit in 63kb, leaving 1kb for the stack.
+ */
+ASSERT((trampoline_end - trampoline_start) < KB(63),
+    "not enough room for trampolines and low memory stack")
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 473c5e8..04e4da5 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -70,6 +70,7 @@
 #define __force
 #define __bitwise
 
+#define KB(_kb)     (_AC(_kb, ULL) << 10)
 #define MB(_mb)     (_AC(_mb, ULL) << 20)
 #define GB(_gb)     (_AC(_gb, ULL) << 30)
 
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:17 ` [PATCH v4 5/5] fix: " Doug Goldstein
@ 2017-01-18 14:52   ` Jan Beulich
  2017-01-18 21:25     ` Daniel Kiper
  2017-01-18 19:53   ` Andrew Cooper
  1 sibling, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2017-01-18 14:52 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	pgnet.dev, ning.sun, julien.grall, qiaowei.ren, xen-devel,
	gang.wei, fu.wei

>>> On 18.01.17 at 15:17, <cardoe@cardoe.com> wrote:
> This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
> support for EFI platforms'.
> 
> - fix incorrect assembly (identified by Andrew Cooper)
> - fix issue where the trampoline size was left as 0 and the
>   way the memory is allocated for the trampolines we would go to
>   the end of an available section and then subtract off the size
>   to decide where to place it. The end result was that we would
>   always copy the trampolines and the 32-bit stack into some
>   form of reserved memory after the conventional region we
>   wanted to put things into. On some systems this did not
>   manifest as a crash while on others it did. Reworked the
>   changes to always reserve 64kb for both the stack and the size
>   of the trampolines. Added a build time assert to make sure we have
>   enough room always.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -333,3 +333,10 @@ ASSERT(IS_ALIGNED(trampoline_start, 4), 
> "trampoline_start misaligned")
>  ASSERT(IS_ALIGNED(trampoline_end,   4), "trampoline_end misaligned")
>  ASSERT(IS_ALIGNED(__bss_start,      8), "__bss_start misaligned")
>  ASSERT(IS_ALIGNED(__bss_end,        8), "__bss_end misaligned")
> +
> +/* The trampolines and the low memory stack must fit in 64kb. In testing
> + * the low memory stack never exceeded 1kb so just require that the
> + * trampolines fit in 63kb, leaving 1kb for the stack.
> + */
> +ASSERT((trampoline_end - trampoline_start) < KB(63),
> +    "not enough room for trampolines and low memory stack")

... the comment style here fixed (which could be done upon commit
or when Daniel merges this back into his series).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:17 ` [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
@ 2017-01-18 19:51   ` Andrew Cooper
  2017-01-18 20:05     ` Doug Goldstein
  2017-01-19  9:09     ` Jan Beulich
  0 siblings, 2 replies; 18+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:51 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel, Daniel Kiper
  Cc: jgross, sstabellini, konrad.wilk, pgnet.dev, ning.sun,
	julien.grall, Jan Beulich, qiaowei.ren, gang.wei, fu.wei

On 18/01/17 14:17, Doug Goldstein wrote:
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index d423fd8..ac93df0 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -89,6 +89,13 @@ multiboot2_header_start:
>                     0, /* Number of the lines - no preference. */ \
>                     0  /* Number of bits per pixel - no preference. */
>  
> +        /* Inhibit bootloader from calling ExitBootServices(). */

/* Request that ExitBootServices() not be called. */

This tag doesn't make any guarantees.

> +        .code64
> +
> +__efi64_start:

__mb2_efi64_start:

This entry point is distinct from the PE efi64 entry point in
common/efi/boot.c

> +        cld
> +
> +        /* VGA is not available on EFI platforms. */
> +        movl   $0,vga_text_buffer(%rip)
> +
> +        /* Check for Multiboot2 bootloader. */
> +        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> +        je      .Lefi_multiboot2_proto
> +
> +        /* Jump to not_multiboot after switching CPU to x86_32 mode. */
> +        lea     not_multiboot(%rip),%edi
> +        jmp     x86_32_switch
> +
> +.Lefi_multiboot2_proto:
> +        /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> +        xor     %esi,%esi
> +        xor     %edi,%edi
> +
> +        /* Skip Multiboot2 information fixed part. */
> +        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx

Are we strictly guaranteed to have the entire image, including multiboot
tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI
capable grub2's ?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:17 ` [PATCH v4 5/5] fix: " Doug Goldstein
  2017-01-18 14:52   ` Jan Beulich
@ 2017-01-18 19:53   ` Andrew Cooper
  1 sibling, 0 replies; 18+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:53 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel
  Cc: jgross, sstabellini, konrad.wilk, pgnet.dev, ning.sun,
	julien.grall, Jan Beulich, qiaowei.ren, gang.wei, fu.wei

On 18/01/17 14:17, Doug Goldstein wrote:
> This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
> support for EFI platforms'.
>
> - fix incorrect assembly (identified by Andrew Cooper)
> - fix issue where the trampoline size was left as 0 and the
>   way the memory is allocated for the trampolines we would go to
>   the end of an available section and then subtract off the size
>   to decide where to place it. The end result was that we would
>   always copy the trampolines and the 32-bit stack into some
>   form of reserved memory after the conventional region we
>   wanted to put things into. On some systems this did not
>   manifest as a crash while on others it did. Reworked the
>   changes to always reserve 64kb for both the stack and the size
>   of the trampolines. Added a build time assert to make sure we have
>   enough room always.
>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

LGTM, but this really does want squashing.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-18 19:51   ` Andrew Cooper
@ 2017-01-18 20:05     ` Doug Goldstein
  2017-01-19  9:09     ` Jan Beulich
  1 sibling, 0 replies; 18+ messages in thread
From: Doug Goldstein @ 2017-01-18 20:05 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel, Daniel Kiper
  Cc: jgross, sstabellini, konrad.wilk, pgnet.dev, ning.sun,
	julien.grall, Jan Beulich, qiaowei.ren, gang.wei, fu.wei


[-- Attachment #1.1.1: Type: text/plain, Size: 1295 bytes --]

On 1/18/17 2:51 PM, Andrew Cooper wrote:
> On 18/01/17 14:17, Doug Goldstein wrote:
>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>> index d423fd8..ac93df0 100644
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -89,6 +89,13 @@ multiboot2_header_start:
>>                     0, /* Number of the lines - no preference. */ \
>>                     0  /* Number of bits per pixel - no preference. */
>>  
>> +        /* Inhibit bootloader from calling ExitBootServices(). */
> 
> /* Request that ExitBootServices() not be called. */
> 
> This tag doesn't make any guarantees.
>

Agreed. The multiboot2 spec is clear in that regard. I have tested the
case when the bootloader ignores this and it does work.

>> +        .code64
>> +
>> +__efi64_start:
> 
> __mb2_efi64_start:
> 
> This entry point is distinct from the PE efi64 entry point in
> common/efi/boot.c

I agree here as well. It does need one other update however. On line 97.

        /* EFI64 entry point. */
        mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
                   sym_phys(__efi64_start)

I can reroll the series or if people are comfortable with committing
this then the change can be done then.
-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-18 14:52   ` Jan Beulich
@ 2017-01-18 21:25     ` Daniel Kiper
  2017-01-19  3:40       ` Doug Goldstein
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Kiper @ 2017-01-18 21:25 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	daniel.kiper, Doug Goldstein, pgnet.dev, ning.sun, julien.grall,
	qiaowei.ren, xen-devel, gang.wei, fu.wei

On Wed, Jan 18, 2017 at 07:52:44AM -0700, Jan Beulich wrote:
> >>> On 18.01.17 at 15:17, <cardoe@cardoe.com> wrote:
> > This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
> > support for EFI platforms'.
> >
> > - fix incorrect assembly (identified by Andrew Cooper)
> > - fix issue where the trampoline size was left as 0 and the
> >   way the memory is allocated for the trampolines we would go to
> >   the end of an available section and then subtract off the size
> >   to decide where to place it. The end result was that we would
> >   always copy the trampolines and the 32-bit stack into some
> >   form of reserved memory after the conventional region we
> >   wanted to put things into. On some systems this did not
> >   manifest as a crash while on others it did. Reworked the
> >   changes to always reserve 64kb for both the stack and the size
> >   of the trampolines. Added a build time assert to make sure we have
> >   enough room always.
> >
> > Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with ...
>
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -333,3 +333,10 @@ ASSERT(IS_ALIGNED(trampoline_start, 4),
> > "trampoline_start misaligned")
> >  ASSERT(IS_ALIGNED(trampoline_end,   4), "trampoline_end misaligned")
> >  ASSERT(IS_ALIGNED(__bss_start,      8), "__bss_start misaligned")
> >  ASSERT(IS_ALIGNED(__bss_end,        8), "__bss_end misaligned")
> > +
> > +/* The trampolines and the low memory stack must fit in 64kb. In testing
> > + * the low memory stack never exceeded 1kb so just require that the
> > + * trampolines fit in 63kb, leaving 1kb for the stack.
> > + */
> > +ASSERT((trampoline_end - trampoline_start) < KB(63),
> > +    "not enough room for trampolines and low memory stack")
>
> ... the comment style here fixed (which could be done upon commit
> or when Daniel merges this back into his series).

Hmmm... Why this patch and #0 was not CC-ed to me?

Anyway, it looks that I have cleared my backlog to some extent and now I am
able to take a stab on v12. There is a chance that I will have it with Doug's
and some additional fixes on Friday or Monday.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-18 21:25     ` Daniel Kiper
@ 2017-01-19  3:40       ` Doug Goldstein
  2017-01-19 11:56         ` Daniel Kiper
  0 siblings, 1 reply; 18+ messages in thread
From: Doug Goldstein @ 2017-01-19  3:40 UTC (permalink / raw)
  To: Daniel Kiper, Jan Beulich
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	daniel.kiper, pgnet.dev, ning.sun, julien.grall, qiaowei.ren,
	xen-devel, gang.wei, fu.wei


[-- Attachment #1.1.1: Type: text/plain, Size: 1013 bytes --]

On 1/18/17 4:25 PM, Daniel Kiper wrote:
> On Wed, Jan 18, 2017 at 07:52:44AM -0700, Jan Beulich wrote:

>>
>> ... the comment style here fixed (which could be done upon commit
>> or when Daniel merges this back into his series).
> 
> Hmmm... Why this patch and #0 was not CC-ed to me?
>

You say this like I did it intentionally to upset you. I switched to
using git-series and as a result my workflow changed slightly. When I
glanced at 1/5 it had you CCed and I assumed this one was as well.

> Anyway, it looks that I have cleared my backlog to some extent and now I am
> able to take a stab on v12. There is a chance that I will have it with Doug's
> and some additional fixes on Friday or Monday.

So then there should be no issue with this landing before that or not.
The changes for you will be identical. A simple rebase --onto would
actually be easier than folding this into your larger series.

Can you tell me what were the fixes to the relocation code?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-18 19:51   ` Andrew Cooper
  2017-01-18 20:05     ` Doug Goldstein
@ 2017-01-19  9:09     ` Jan Beulich
  2017-01-19 11:37       ` Daniel Kiper
  1 sibling, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2017-01-19  9:09 UTC (permalink / raw)
  To: Doug Goldstein, Andrew Cooper, DanielKiper
  Cc: Juergen Gross, sstabellini, konrad.wilk, pgnet.dev, ning.sun,
	julien.grall, qiaowei.ren, xen-devel, gang.wei, fu.wei

>>> On 18.01.17 at 20:51, <andrew.cooper3@citrix.com> wrote:
> On 18/01/17 14:17, Doug Goldstein wrote:
>> +.Lefi_multiboot2_proto:
>> +        /* Zero EFI SystemTable and EFI ImageHandle addresses. */
>> +        xor     %esi,%esi
>> +        xor     %edi,%edi
>> +
>> +        /* Skip Multiboot2 information fixed part. */
>> +        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
>> +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> 
> Are we strictly guaranteed to have the entire image, including multiboot
> tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI
> capable grub2's ?

I can't speak for GrUB, but I've recently learned that EFI alone
doesn't appear to make such guarantees, and also doesn't look
to have ways to request such address restricted loading of an
image. Hence there'll be work needed to make at least xen.efi
cope with being loaded above that boundary.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-19  9:09     ` Jan Beulich
@ 2017-01-19 11:37       ` Daniel Kiper
  2017-01-19 12:03         ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Kiper @ 2017-01-19 11:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, qiaowei.ren,
	xen-devel, gang.wei, fu.wei

On Thu, Jan 19, 2017 at 02:09:37AM -0700, Jan Beulich wrote:
> >>> On 18.01.17 at 20:51, <andrew.cooper3@citrix.com> wrote:
> > On 18/01/17 14:17, Doug Goldstein wrote:
> >> +.Lefi_multiboot2_proto:
> >> +        /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> >> +        xor     %esi,%esi
> >> +        xor     %edi,%edi
> >> +
> >> +        /* Skip Multiboot2 information fixed part. */
> >> +        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> >> +        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> >
> > Are we strictly guaranteed to have the entire image, including multiboot
> > tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI
> > capable grub2's ?
>
> I can't speak for GrUB, but I've recently learned that EFI alone

GRUB is able to enforce 4 GiB restriction. At least right now.
However, I am afraid that sooner or later machines will appear
in the wild which do not have memory below 4 GiB (well, they must
have some below 1 MiB but everything above is not a must as I know)
or everything will be reserved/used by EFI boot/runtime services, etc.
Then GRUB will also fail and we will be forced to extend multiboot2
proto further. However, it looks that current solution should not
change much then.

> doesn't appear to make such guarantees, and also doesn't look
> to have ways to request such address restricted loading of an

Why? Due to lack of memory below 4 GiB, it being reserved for
something, EFI implementation bugs or anything else?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-19  3:40       ` Doug Goldstein
@ 2017-01-19 11:56         ` Daniel Kiper
  2017-01-19 14:25           ` Doug Goldstein
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Kiper @ 2017-01-19 11:56 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	pgnet.dev, ning.sun, julien.grall, fu.wei, Jan Beulich,
	qiaowei.ren, xen-devel, gang.wei, Daniel Kiper

On Wed, Jan 18, 2017 at 10:40:07PM -0500, Doug Goldstein wrote:
> On 1/18/17 4:25 PM, Daniel Kiper wrote:
> > On Wed, Jan 18, 2017 at 07:52:44AM -0700, Jan Beulich wrote:
> >> ... the comment style here fixed (which could be done upon commit
> >> or when Daniel merges this back into his series).
> >
> > Hmmm... Why this patch and #0 was not CC-ed to me?
>
> You say this like I did it intentionally to upset you. I switched to
> using git-series and as a result my workflow changed slightly. When I
> glanced at 1/5 it had you CCed and I assumed this one was as well.

I do not know why you read this in that way. I have just stated the fact.
No more no less.

> > Anyway, it looks that I have cleared my backlog to some extent and now I am
> > able to take a stab on v12. There is a chance that I will have it with Doug's
> > and some additional fixes on Friday or Monday.
>
> So then there should be no issue with this landing before that or not.
> The changes for you will be identical. A simple rebase --onto would
> actually be easier than folding this into your larger series.

I do not fully agree, however, I respect your point of view. Though I am not
going to discuss this any longer. I have just stated my preference clearly
earlier. Maintainers are the Supreme Court and they will decide what to do.
And I am not going to discuss this too. I prefer to focus on my work.

> Can you tell me what were the fixes to the relocation code?

Unfortunately I have not received any details from you. Just vague
statements that it does not work. So, I am not able to say anything
about that. If you provide more details I am happy to help.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
  2017-01-19 11:37       ` Daniel Kiper
@ 2017-01-19 12:03         ` Jan Beulich
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Beulich @ 2017-01-19 12:03 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	Doug Goldstein, pgnet.dev, ning.sun, julien.grall, qiaowei.ren,
	xen-devel, gang.wei, fu.wei

>>> On 19.01.17 at 12:37, <daniel.kiper@oracle.com> wrote:
> On Thu, Jan 19, 2017 at 02:09:37AM -0700, Jan Beulich wrote:
>> >>> On 18.01.17 at 20:51, <andrew.cooper3@citrix.com> wrote:
>> > Are we strictly guaranteed to have the entire image, including multiboot
>> > tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI
>> > capable grub2's ?
>>
>> I can't speak for GrUB, but I've recently learned that EFI alone
>> doesn't appear to make such guarantees, and also doesn't look
>> to have ways to request such address restricted loading of an
> 
> Why? Due to lack of memory below 4 GiB, it being reserved for
> something, EFI implementation bugs or anything else?

I don't know the precise reason in that specific case, but I did go
through the spec again after I had read that report, and I wasn't
able to find anything regarding mandated or optional address
restrictions. My recollection of there being such a restriction must
have been based on the EFI sample implementation and/or the
EDK.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-19 11:56         ` Daniel Kiper
@ 2017-01-19 14:25           ` Doug Goldstein
  2017-01-20  1:45             ` Daniel Kiper
  0 siblings, 1 reply; 18+ messages in thread
From: Doug Goldstein @ 2017-01-19 14:25 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	pgnet.dev, ning.sun, julien.grall, fu.wei, Jan Beulich,
	qiaowei.ren, xen-devel, gang.wei, Daniel Kiper


[-- Attachment #1.1.1: Type: text/plain, Size: 931 bytes --]

On 1/19/17 6:56 AM, Daniel Kiper wrote:
>> Can you tell me what were the fixes to the relocation code?
> 
> Unfortunately I have not received any details from you. Just vague
> statements that it does not work. So, I am not able to say anything
> about that. If you provide more details I am happy to help.

We discussed this on your original v11 thread. It fails on Intel NUCs,
Lenovo T430, Dell PowerEdge something (I'm on vacation so I can't see
the model number), HP Proliant ML10, QEMU (with OVMF) and some other
broads that I don't remember the model number off of my head. Its about
a dozen machines all together. They fail in two different ways:

- the other CPUs are reported as stuck and the machine boots but 'xl
info' reports 1 available CPU.
- when the non-CPUs are brought up it dies when setting paging back into
cr0.

Every single machine fails in one of these two ways.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
  2017-01-19 14:25           ` Doug Goldstein
@ 2017-01-20  1:45             ` Daniel Kiper
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Kiper @ 2017-01-20  1:45 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
	pgnet.dev, ning.sun, julien.grall, fu.wei, Jan Beulich,
	qiaowei.ren, xen-devel, gang.wei, Daniel Kiper

On Thu, Jan 19, 2017 at 09:25:08AM -0500, Doug Goldstein wrote:
> On 1/19/17 6:56 AM, Daniel Kiper wrote:
> >> Can you tell me what were the fixes to the relocation code?
> >
> > Unfortunately I have not received any details from you. Just vague
> > statements that it does not work. So, I am not able to say anything
> > about that. If you provide more details I am happy to help.
>
> We discussed this on your original v11 thread. It fails on Intel NUCs,
> Lenovo T430, Dell PowerEdge something (I'm on vacation so I can't see
> the model number), HP Proliant ML10, QEMU (with OVMF) and some other

Do you use QEMU with KVM enabled? I was hit twice by an issue (I do not
remember the detail right now) when OVMF was running in KVM. When I disabled
KVM then everything worked without any issue. It does not happen often but...

> broads that I don't remember the model number off of my head. Its about
> a dozen machines all together. They fail in two different ways:
>
> - the other CPUs are reported as stuck and the machine boots but 'xl
> info' reports 1 available CPU.
> - when the non-CPUs are brought up it dies when setting paging back into
> cr0.
>
> Every single machine fails in one of these two ways.

Hmmm... I have not seen that anywhere. And nobody except you reported such
issue. Is it happening with GRUB2 and/or iPXE?

Anyway, I have just released v12. It contains various fixes including
yours. I hope that I have not missed anything. Please take a look.

In the meantime I will try to find one of machines mentioned above and
reproduce this issue there.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-01-20  1:45 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-18 14:16 [PATCH v4 0/5] multiboot2 protocol support Doug Goldstein
2017-01-18 14:16 ` [PATCH v4 1/5] x86: add " Doug Goldstein
2017-01-18 14:16 ` [PATCH v4 2/5] efi: build xen.gz with EFI code Doug Goldstein
2017-01-18 14:17 ` [PATCH v4 3/5] efi: create new early memory allocator Doug Goldstein
2017-01-18 14:17 ` [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
2017-01-18 19:51   ` Andrew Cooper
2017-01-18 20:05     ` Doug Goldstein
2017-01-19  9:09     ` Jan Beulich
2017-01-19 11:37       ` Daniel Kiper
2017-01-19 12:03         ` Jan Beulich
2017-01-18 14:17 ` [PATCH v4 5/5] fix: " Doug Goldstein
2017-01-18 14:52   ` Jan Beulich
2017-01-18 21:25     ` Daniel Kiper
2017-01-19  3:40       ` Doug Goldstein
2017-01-19 11:56         ` Daniel Kiper
2017-01-19 14:25           ` Doug Goldstein
2017-01-20  1:45             ` Daniel Kiper
2017-01-18 19:53   ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.