* [PATCH 1/4] x86: add multiboot2 protocol support
2017-01-13 19:21 [PATCH 0/4] multiboot2 protocol support Doug Goldstein
@ 2017-01-13 19:21 ` Doug Goldstein
2017-01-13 19:21 ` [PATCH 2/4] efi: build xen.gz with EFI code Doug Goldstein
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2017-01-13 19:21 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] 20+ messages in thread* [PATCH 2/4] efi: build xen.gz with EFI code
2017-01-13 19:21 [PATCH 0/4] multiboot2 protocol support Doug Goldstein
2017-01-13 19:21 ` [PATCH 1/4] x86: add " Doug Goldstein
@ 2017-01-13 19:21 ` Doug Goldstein
2017-01-13 19:21 ` [PATCH 3/4] efi: create new early memory allocator Doug Goldstein
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2017-01-13 19:21 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] 20+ messages in thread* [PATCH 3/4] efi: create new early memory allocator
2017-01-13 19:21 [PATCH 0/4] multiboot2 protocol support Doug Goldstein
2017-01-13 19:21 ` [PATCH 1/4] x86: add " Doug Goldstein
2017-01-13 19:21 ` [PATCH 2/4] efi: build xen.gz with EFI code Doug Goldstein
@ 2017-01-13 19:21 ` Doug Goldstein
2017-01-16 11:52 ` Jan Beulich
2017-01-13 19:21 ` [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
2017-01-16 11:56 ` [PATCH 0/4] multiboot2 protocol support Jan Beulich
4 siblings, 1 reply; 20+ messages in thread
From: Doug Goldstein @ 2017-01-13 19:21 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>
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] 20+ messages in thread* Re: [PATCH 3/4] efi: create new early memory allocator
2017-01-13 19:21 ` [PATCH 3/4] efi: create new early memory allocator Doug Goldstein
@ 2017-01-16 11:52 ` Jan Beulich
2017-01-16 13:55 ` Doug Goldstein
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-01-16 11:52 UTC (permalink / raw)
To: Doug Goldstein
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
Daniel Kiper, pgnet.dev, ning.sun, julien.grall, qiaowei.ren,
xen-devel, gang.wei, fu.wei
>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> 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>
> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
You've lost (at least) Julien's and my ack here.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] efi: create new early memory allocator
2017-01-16 11:52 ` Jan Beulich
@ 2017-01-16 13:55 ` Doug Goldstein
0 siblings, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2017-01-16 13:55 UTC (permalink / raw)
To: 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: 4151 bytes --]
On 1/16/17 6:52 AM, Jan Beulich wrote:
>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>> 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>
>> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
>
> You've lost (at least) Julien's and my ack here.
>
> Jan
>
Yes. My mistake. I pulled in the patch wholesale without adding the acks
for this patch since they were replies.
--
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] 20+ messages in thread
* [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-13 19:21 [PATCH 0/4] multiboot2 protocol support Doug Goldstein
` (2 preceding siblings ...)
2017-01-13 19:21 ` [PATCH 3/4] efi: create new early memory allocator Doug Goldstein
@ 2017-01-13 19:21 ` Doug Goldstein
2017-01-16 12:02 ` Jan Beulich
2017-01-16 11:56 ` [PATCH 0/4] multiboot2 protocol support Jan Beulich
4 siblings, 1 reply; 20+ messages in thread
From: Doug Goldstein @ 2017-01-13 19:21 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>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
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.
---
memory allocation fix
---
xen/arch/x86/boot/head.S | 264 +++++++++++++++++++++++++++++--
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, 351 insertions(+), 22 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index d423fd8..876a6b1 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,10 +510,16 @@ 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
+ /* The stack starts 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 62c010e..3871de7 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -169,6 +169,7 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
case EfiConventionalMemory:
if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
+ ASSERT(cfg.size > 0);
cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
/* fall through */
case EfiLoaderCode:
@@ -210,12 +211,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 +656,47 @@ 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();
+
+ /* This is the maximum size of our trampoline + our low memory stack */
+ cfg.size = 64 << 10;
+ ASSERT(cfg.size >= ((trampoline_end - trampoline_start) + 4096));
+
+ 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..b81adc0 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] 20+ messages in thread* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-13 19:21 ` [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
@ 2017-01-16 12:02 ` Jan Beulich
2017-01-16 12:50 ` Daniel Kiper
2017-01-16 13:42 ` Doug Goldstein
0 siblings, 2 replies; 20+ messages in thread
From: Jan Beulich @ 2017-01-16 12:02 UTC (permalink / raw)
To: Doug Goldstein
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
Daniel Kiper, pgnet.dev, ning.sun, julien.grall, qiaowei.ren,
xen-devel, gang.wei, fu.wei
>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> 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.
Without having looked at the patch in detail, but knowing I did closely
look at earlier versions (and iirc I was mostly fine with v10) the way
the above is written would require me to either inter-diff the patches,
or re-review the whole thing. For a large patch like this it would be
rather helpful to be quite a bit more specific as to where exactly in the
patch changes were made.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 12:02 ` Jan Beulich
@ 2017-01-16 12:50 ` Daniel Kiper
2017-01-16 13:41 ` Doug Goldstein
2017-01-16 13:42 ` Doug Goldstein
1 sibling, 1 reply; 20+ messages in thread
From: Daniel Kiper @ 2017-01-16 12:50 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 Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote:
> >>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> > 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.
>
> Without having looked at the patch in detail, but knowing I did closely
> look at earlier versions (and iirc I was mostly fine with v10) the way
> the above is written would require me to either inter-diff the patches,
> or re-review the whole thing. For a large patch like this it would be
> rather helpful to be quite a bit more specific as to where exactly in the
> patch changes were made.
I would prefer to not have this patch series applied because it will make me
more difficult to prepare v12. I hope that I will post it in about 2 weeks.
Though I am going to take into account all comments posted by Doug for v11.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 12:50 ` Daniel Kiper
@ 2017-01-16 13:41 ` Doug Goldstein
2017-01-16 14:11 ` Daniel Kiper
0 siblings, 1 reply; 20+ messages in thread
From: Doug Goldstein @ 2017-01-16 13:41 UTC (permalink / raw)
To: Daniel Kiper, Jan Beulich
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
pgnet.dev, ning.sun, julien.grall, qiaowei.ren, xen-devel,
gang.wei, fu.wei
[-- Attachment #1.1.1: Type: text/plain, Size: 1864 bytes --]
On 1/16/17 7:50 AM, Daniel Kiper wrote:
> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote:
>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>>> 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.
>>
>> Without having looked at the patch in detail, but knowing I did closely
>> look at earlier versions (and iirc I was mostly fine with v10) the way
>> the above is written would require me to either inter-diff the patches,
>> or re-review the whole thing. For a large patch like this it would be
>> rather helpful to be quite a bit more specific as to where exactly in the
>> patch changes were made.
>
> I would prefer to not have this patch series applied because it will make me
> more difficult to prepare v12. I hope that I will post it in about 2 weeks.
> Though I am going to take into account all comments posted by Doug for v11.
>
> Daniel
>
Why? They're the first 4 patches remaining of your series. It'll
literally be the following commands:
git fetch origin
git rebase origin/staging
--
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] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 13:41 ` Doug Goldstein
@ 2017-01-16 14:11 ` Daniel Kiper
2017-01-16 14:28 ` Doug Goldstein
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Kiper @ 2017-01-16 14:11 UTC (permalink / raw)
To: Doug Goldstein
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
pgnet.dev, ning.sun, julien.grall, Jan Beulich, qiaowei.ren,
xen-devel, gang.wei, fu.wei
On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote:
> On 1/16/17 7:50 AM, Daniel Kiper wrote:
> > On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote:
> >>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> >>> 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.
> >>
> >> Without having looked at the patch in detail, but knowing I did closely
> >> look at earlier versions (and iirc I was mostly fine with v10) the way
> >> the above is written would require me to either inter-diff the patches,
> >> or re-review the whole thing. For a large patch like this it would be
> >> rather helpful to be quite a bit more specific as to where exactly in the
> >> patch changes were made.
> >
> > I would prefer to not have this patch series applied because it will make me
> > more difficult to prepare v12. I hope that I will post it in about 2 weeks.
> > Though I am going to take into account all comments posted by Doug for v11.
> >
> > Daniel
> >
>
> Why? They're the first 4 patches remaining of your series. It'll
> literally be the following commands:
>
> git fetch origin
> git rebase origin/staging
If you changed something then probably this is not so simple.
Second, Jan asked me to reorder some patches in the series.
Last but not least, your patch series does not contain whole
functionality which is needed to properly load Xen using
multiboot2 protocol on most EFI platforms. So, as above.
Though I appreciate your feedback and I will take all
your changes into account.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 14:11 ` Daniel Kiper
@ 2017-01-16 14:28 ` Doug Goldstein
2017-01-16 15:16 ` Daniel Kiper
0 siblings, 1 reply; 20+ messages in thread
From: Doug Goldstein @ 2017-01-16 14:28 UTC (permalink / raw)
To: Daniel Kiper
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
pgnet.dev, ning.sun, julien.grall, Jan Beulich, qiaowei.ren,
xen-devel, gang.wei, fu.wei
[-- Attachment #1.1.1: Type: text/plain, Size: 3128 bytes --]
On 1/16/17 9:11 AM, Daniel Kiper wrote:
> On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote:
>> On 1/16/17 7:50 AM, Daniel Kiper wrote:
>>> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote:
>>>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>>>>> 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.
>>>>
>>>> Without having looked at the patch in detail, but knowing I did closely
>>>> look at earlier versions (and iirc I was mostly fine with v10) the way
>>>> the above is written would require me to either inter-diff the patches,
>>>> or re-review the whole thing. For a large patch like this it would be
>>>> rather helpful to be quite a bit more specific as to where exactly in the
>>>> patch changes were made.
>>>
>>> I would prefer to not have this patch series applied because it will make me
>>> more difficult to prepare v12. I hope that I will post it in about 2 weeks.
>>> Though I am going to take into account all comments posted by Doug for v11.
>>>
>>> Daniel
>>>
>>
>> Why? They're the first 4 patches remaining of your series. It'll
>> literally be the following commands:
>>
>> git fetch origin
>> git rebase origin/staging
>
> If you changed something then probably this is not so simple.
> Second, Jan asked me to reorder some patches in the series.
> Last but not least, your patch series does not contain whole
> functionality which is needed to properly load Xen using
> multiboot2 protocol on most EFI platforms. So, as above.
> Though I appreciate your feedback and I will take all
> your changes into account.
>
> Daniel
>
Yes there are some code changes which is the point of me sending this.
But the work for you is the same as having to rebase your series over
the past few years. Its still a rebase. Its the same thing that everyone
submitting patches has to do when they submit another version of their
patches.
I believe you identified 1 machine that had an issue with the 1mb
region. I've used a 12 machines to test this series and these 4 patches
work on those machines. The relocation part of the series works on NONE
of the machines I've tested with. So I would argue the opposite. I still
haven't identified what's wrong with the relocation part of the series.
--
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] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 14:28 ` Doug Goldstein
@ 2017-01-16 15:16 ` Daniel Kiper
2017-01-17 12:05 ` George Dunlap
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Kiper @ 2017-01-16 15:16 UTC (permalink / raw)
To: Doug Goldstein
Cc: Juergen Gross, sstabellini, konrad.wilk, Andrew Cooper,
pgnet.dev, ning.sun, julien.grall, Jan Beulich, qiaowei.ren,
xen-devel, gang.wei, fu.wei
On Mon, Jan 16, 2017 at 09:28:45AM -0500, Doug Goldstein wrote:
> On 1/16/17 9:11 AM, Daniel Kiper wrote:
> > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote:
> >> On 1/16/17 7:50 AM, Daniel Kiper wrote:
> >>> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote:
> >>>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> >>>>> 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.
> >>>>
> >>>> Without having looked at the patch in detail, but knowing I did closely
> >>>> look at earlier versions (and iirc I was mostly fine with v10) the way
> >>>> the above is written would require me to either inter-diff the patches,
> >>>> or re-review the whole thing. For a large patch like this it would be
> >>>> rather helpful to be quite a bit more specific as to where exactly in the
> >>>> patch changes were made.
> >>>
> >>> I would prefer to not have this patch series applied because it will make me
> >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks.
> >>> Though I am going to take into account all comments posted by Doug for v11.
> >>>
> >>> Daniel
> >>>
> >>
> >> Why? They're the first 4 patches remaining of your series. It'll
> >> literally be the following commands:
> >>
> >> git fetch origin
> >> git rebase origin/staging
> >
> > If you changed something then probably this is not so simple.
> > Second, Jan asked me to reorder some patches in the series.
> > Last but not least, your patch series does not contain whole
> > functionality which is needed to properly load Xen using
> > multiboot2 protocol on most EFI platforms. So, as above.
> > Though I appreciate your feedback and I will take all
> > your changes into account.
> >
> > Daniel
> >
>
> Yes there are some code changes which is the point of me sending this.
> But the work for you is the same as having to rebase your series over
> the past few years. Its still a rebase. Its the same thing that everyone
> submitting patches has to do when they submit another version of their
> patches.
Yes, I know. It is always easy to say. And I do not know why you are in
such a hurry. Could not you wait 1-2 weeks?
> I believe you identified 1 machine that had an issue with the 1mb
> region. I've used a 12 machines to test this series and these 4 patches
> work on those machines. The relocation part of the series works on NONE
> of the machines I've tested with. So I would argue the opposite. I still
> haven't identified what's wrong with the relocation part of the series.
This is very interesting. As I know similar thing is used on many
machines for a year or two. And I have not received any complaints
until now. Though I do not say that it is perfect. If you find
something just drop me a line and I will fix it.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 15:16 ` Daniel Kiper
@ 2017-01-17 12:05 ` George Dunlap
2017-01-17 12:45 ` Daniel Kiper
0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-01-17 12:05 UTC (permalink / raw)
To: Daniel Kiper
Cc: Juergen Gross, Stefano Stabellini, Konrad Rzeszutek Wilk,
Andrew Cooper, Doug Goldstein, PGNd _, ning.sun, Julien Grall,
committers, Jan Beulich, qiaowei.ren, xen-devel, gang.wei,
fu.wei
On Mon, Jan 16, 2017 at 3:16 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Mon, Jan 16, 2017 at 09:28:45AM -0500, Doug Goldstein wrote:
>> On 1/16/17 9:11 AM, Daniel Kiper wrote:
>> > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote:
>> >> On 1/16/17 7:50 AM, Daniel Kiper wrote:
>> >>> I would prefer to not have this patch series applied because it will make me
>> >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks.
>> >>> Though I am going to take into account all comments posted by Doug for v11.
[snip]
>> >> Why? They're the first 4 patches remaining of your series. It'll
>> >> literally be the following commands:
>> >>
>> >> git fetch origin
>> >> git rebase origin/staging
>> >
>> > If you changed something then probably this is not so simple.
>> > Second, Jan asked me to reorder some patches in the series.
>> > Last but not least, your patch series does not contain whole
>> > functionality which is needed to properly load Xen using
>> > multiboot2 protocol on most EFI platforms. So, as above.
>> > Though I appreciate your feedback and I will take all
>> > your changes into account.
[snip]
>> Yes there are some code changes which is the point of me sending this.
>> But the work for you is the same as having to rebase your series over
>> the past few years. Its still a rebase. Its the same thing that everyone
>> submitting patches has to do when they submit another version of their
>> patches.
>
> Yes, I know. It is always easy to say. And I do not know why you are in
> such a hurry. Could not you wait 1-2 weeks?
I can't speak to the technical merits of Doug's patch, but I have to
say I'm not very happy with this way of interacting. No one should be
able to block other people's valid contributions from going in to the
tree for their own convenience -- it discourages contribution and sets
the incentives all wrong. Doing a difficult rebase is something
everyone has to do sometimes; and it's not clear that it *will* be
difficult, as it sounds like Daniel hasn't even tried it.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-17 12:05 ` George Dunlap
@ 2017-01-17 12:45 ` Daniel Kiper
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Kiper @ 2017-01-17 12:45 UTC (permalink / raw)
To: George Dunlap
Cc: Juergen Gross, Stefano Stabellini, Konrad Rzeszutek Wilk,
Andrew Cooper, Doug Goldstein, PGNd _, ning.sun, Julien Grall,
committers, Jan Beulich, qiaowei.ren, xen-devel, gang.wei,
fu.wei
On Tue, Jan 17, 2017 at 12:05:21PM +0000, George Dunlap wrote:
> On Mon, Jan 16, 2017 at 3:16 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote:
[...]
> >> Yes there are some code changes which is the point of me sending this.
> >> But the work for you is the same as having to rebase your series over
> >> the past few years. Its still a rebase. Its the same thing that everyone
> >> submitting patches has to do when they submit another version of their
> >> patches.
> >
> > Yes, I know. It is always easy to say. And I do not know why you are in
> > such a hurry. Could not you wait 1-2 weeks?
>
> I can't speak to the technical merits of Doug's patch, but I have to
> say I'm not very happy with this way of interacting. No one should be
> able to block other people's valid contributions from going in to the
> tree for their own convenience -- it discourages contribution and sets
> the incentives all wrong. Doing a difficult rebase is something
> everyone has to do sometimes; and it's not clear that it *will* be
> difficult, as it sounds like Daniel hasn't even tried it.
First of all, I would like to underline that never ever I would not like
to hinder a cooperation here or anywhere else. If it looks like that then
sorry. It was not my intention. Though at least until now I try to maintain
this patch series and nothing will change (at least I do not have such plans)
until it will be fully taken into Xen tree. I appreciate any feedback and
in this case Doug's one. I am happy that more and more people testing it
and find it useful. However, I think that it makes a confusion if we step
on each other foot and everybody tries to post own version of a given patch
series. I would not have any concerns if this project would be abandoned.
However, this is not the case.
Anyway, if maintainers and in particular Jan and/or Andrew will take decision
to merge some or all Doug's patches I will not argue.
And once again, I appreciate everybody feedback and in particular Doug's one.
If I am missing something drop me a line.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
2017-01-16 12:02 ` Jan Beulich
2017-01-16 12:50 ` Daniel Kiper
@ 2017-01-16 13:42 ` Doug Goldstein
1 sibling, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2017-01-16 13:42 UTC (permalink / raw)
To: 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: 1510 bytes --]
On 1/16/17 7:02 AM, Jan Beulich wrote:
>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>> 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.
>
> Without having looked at the patch in detail, but knowing I did closely
> look at earlier versions (and iirc I was mostly fine with v10) the way
> the above is written would require me to either inter-diff the patches,
> or re-review the whole thing. For a large patch like this it would be
> rather helpful to be quite a bit more specific as to where exactly in the
> patch changes were made.
>
> Jan
>
I'll submit a diff against v11 to help show the difference. I can also
submit a difference against v10 if you want as well.
--
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] 20+ messages in thread
* Re: [PATCH 0/4] multiboot2 protocol support
2017-01-13 19:21 [PATCH 0/4] multiboot2 protocol support Doug Goldstein
` (3 preceding siblings ...)
2017-01-13 19:21 ` [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms Doug Goldstein
@ 2017-01-16 11:56 ` Jan Beulich
2017-01-18 17:38 ` George Dunlap
4 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-01-16 11:56 UTC (permalink / raw)
To: Doug Goldstein, Andrew Cooper
Cc: Juergen Gross, sstabellini, konrad.wilk, pgnet.dev, ning.sun,
julien.grall, qiaowei.ren, xen-devel, gang.wei, fu.wei
>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
> 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 its 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.
>
> You can pull this series from https://github.com/cardoe/xen/tree/doug-mb2-v1
>
> 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
Since this last patch continue to be controversial (and there's no
real point in committing the earlier three without the last one, as
only that one offers new functionality), so to be honest I'm not
sure this submission will accelerate anything. Iirc the main point
here is the not insignificant amount of new assembly code this
patch adds. Andrew - you've indicated you'd want to comment
on this and (of the original series) at least patch 11. When is
that going to happen?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/4] multiboot2 protocol support
2017-01-16 11:56 ` [PATCH 0/4] multiboot2 protocol support Jan Beulich
@ 2017-01-18 17:38 ` George Dunlap
2017-01-19 8:31 ` Jan Beulich
0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-01-18 17:38 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, Stefano Stabellini, Konrad Rzeszutek Wilk,
Andrew Cooper, Doug Goldstein, PGNd _, ning.sun, Julien Grall,
qiaowei.ren, xen-devel, gang.wei, Fu Wei
On Mon, Jan 16, 2017 at 11:56 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>> 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 its 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.
>>
>> You can pull this series from https://github.com/cardoe/xen/tree/doug-mb2-v1
>>
>> 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
>
> Since this last patch continue to be controversial (and there's no
> real point in committing the earlier three without the last one, as
> only that one offers new functionality), so to be honest I'm not
> sure this submission will accelerate anything. Iirc the main point
> here is the not insignificant amount of new assembly code this
> patch adds. Andrew - you've indicated you'd want to comment
> on this and (of the original series) at least patch 11. When is
> that going to happen?
What's controversial about it?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] multiboot2 protocol support
2017-01-18 17:38 ` George Dunlap
@ 2017-01-19 8:31 ` Jan Beulich
2017-01-19 14:30 ` Doug Goldstein
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-01-19 8:31 UTC (permalink / raw)
To: George Dunlap
Cc: Juergen Gross, StefanoStabellini, Konrad Rzeszutek Wilk,
Andrew Cooper, Doug Goldstein, PGNd _, ning.sun, Julien Grall,
qiaowei.ren, xen-devel, gang.wei, FuWei
>>> On 18.01.17 at 18:38, <george.dunlap@citrix.com> wrote:
> On Mon, Jan 16, 2017 at 11:56 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote:
>>> 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 its 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.
>>>
>>> You can pull this series from https://github.com/cardoe/xen/tree/doug-mb2-v1
>>>
>>> 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
>>
>> Since this last patch continue to be controversial (and there's no
>> real point in committing the earlier three without the last one, as
>> only that one offers new functionality), so to be honest I'm not
>> sure this submission will accelerate anything. Iirc the main point
>> here is the not insignificant amount of new assembly code this
>> patch adds. Andrew - you've indicated you'd want to comment
>> on this and (of the original series) at least patch 11. When is
>> that going to happen?
>
> What's controversial about it?
The not insignificant amount of assembly code it adds, when our
overall goal is to reduce the amount of assembly code. But
Andrew has meanwhile indicated he's okay for this to go in as is.
I will want to go over the whole patch once more though before
committing it.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] multiboot2 protocol support
2017-01-19 8:31 ` Jan Beulich
@ 2017-01-19 14:30 ` Doug Goldstein
0 siblings, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2017-01-19 14:30 UTC (permalink / raw)
To: Jan Beulich, George Dunlap
Cc: Juergen Gross, StefanoStabellini, Konrad Rzeszutek Wilk,
Andrew Cooper, PGNd _, ning.sun, Julien Grall, qiaowei.ren,
xen-devel, gang.wei, FuWei
[-- Attachment #1.1.1: Type: text/plain, Size: 777 bytes --]
On 1/19/17 3:31 AM, Jan Beulich wrote:
>>>> On 18.01.17 at 18:38, <george.dunlap@citrix.com> wrote:
>>
>> What's controversial about it?
>
> The not insignificant amount of assembly code it adds, when our
> overall goal is to reduce the amount of assembly code. But
> Andrew has meanwhile indicated he's okay for this to go in as is.
> I will want to go over the whole patch once more though before
> committing it.
I completely agree with you on the assembly vs C. I want to follow this
up with some conversions to C but my original goal was to land some
basic multiboot2 support since we've had this series outstanding for
years. I was just trying to help this series move forward and believe we
can do improvements after the fact.
--
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] 20+ messages in thread