Re: [Xen-devel] [PATCH v3 15/16 - RFC] x86: make Xen early boot code relocatable

2016-05-25 Thread Jan Beulich 
>>> On 15.04.16 at 14:33,  wrote:
> Every multiboot protocol (regardless of version) compatible image must
> specify its load address (in ELF or multiboot header). Multiboot protocol
> compatible loader have to load image at specified address. However, there
> is no guarantee that the requested memory region (in case of Xen it starts
> at 1 MiB and ends at 17 MiB) where image should be loaded initially is a RAM
> and it is free (legacy BIOS platforms are merciful for Xen but I found at
> least one EFI platform on which Xen load address conflicts with EFI boot
> services; it is Dell PowerEdge R820 with latest firmware). To cope with
> that problem we must make Xen early boot code relocatable. This patch does
> that.

I don't follow: If we have to specify a load address, and if the
loader is required to put us there or fail, how does the code being
made relocatable help? And how does moving ourselves from 1Mb
to 2Mb make it any less likely that the designated address range is
actually free? Perhaps it's just the description that's misleading
here...

Even with that resolved I expect - as a result of the discussion on
the hackathon - quite a bit of change to this patch, so I don't view
actually reviewing the code as usefully spent time.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 15/16 - RFC] x86: make Xen early boot code relocatable

2016-04-15 Thread Daniel Kiper 
Every multiboot protocol (regardless of version) compatible image must
specify its load address (in ELF or multiboot header). Multiboot protocol
compatible loader have to load image at specified address. However, there
is no guarantee that the requested memory region (in case of Xen it starts
at 1 MiB and ends at 17 MiB) where image should be loaded initially is a RAM
and it is free (legacy BIOS platforms are merciful for Xen but I found at
least one EFI platform on which Xen load address conflicts with EFI boot
services; it is Dell PowerEdge R820 with latest firmware). To cope with
that problem we must make Xen early boot code relocatable. This patch does
that. However, it does not add multiboot2 protocol interface which is done
in "x86: add multiboot2 protocol support for relocatable images" patch.

This patch changes following things:
  - default load address is changed from 1 MiB to 2 MiB; I did that because
initial page tables are using 2 MiB huge pages and this way required
updates for them are quite easy; it means that e.g. we avoid spacial
cases for start and end of required memory region if it live at address
not aligned to 2 MiB,
  - %esi and %r15d registers are used as a storage for Xen image load base
address (%r15d shortly because %rsi is used for EFI SystemTable address
in 64-bit code); both registers are (%esi is mostly) unused in early
boot code and preserved during C functions calls,
  - %esi is used as base for Xen data relative addressing in 32-bit code.

Signed-off-by: Daniel Kiper 
---
v3 - suggestions/fixes:
   - improve segment registers initialization
 (suggested by Jan Beulich),
   - simplify Xen image load base address calculation
 (suggested by Jan Beulich),
   - use %esi and %r15d instead of %ebp to store
 Xen image load base address,
   - use %esi instead of %fs for relative addressing;
 this way we get shorter and simpler code,
   - rename some variables and constants
 (suggested by Jan Beulich),
   - improve comments
 (suggested by Konrad Rzeszutek Wilk),
   - improve commit message
 (suggested by Jan Beulich).

v3 - not fixed yet:
   - small issue with remapping code in xen/arch/x86/setup.c,
   -  mkelf32 argument should
 be calculated dynamically; this issue has
 minimal impact on other parts of this patch.
---
 xen/arch/x86/Makefile  |6 +-
 xen/arch/x86/Rules.mk  |4 +
 xen/arch/x86/boot/head.S   |  162 ++--
 xen/arch/x86/boot/trampoline.S |6 +-
 xen/arch/x86/boot/wakeup.S |6 +-
 xen/arch/x86/boot/x86_64.S |   44 +--
 xen/arch/x86/setup.c   |   44 ++-
 xen/arch/x86/xen.lds.S |2 +-
 xen/include/asm-x86/config.h   |1 +
 xen/include/asm-x86/page.h |2 +-
 10 files changed, 186 insertions(+), 91 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 32d2407..0cc6f5f 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -71,8 +71,10 @@ efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h 
-o \
  echo '$(TARGET).efi'; fi)
 
 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
-   ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
-   `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
+#  THIS IS UGLY HACK! PLEASE DO NOT COMPLAIN. I WILL FIX IT IN NEXT 
RELEASE.
+   ./boot/mkelf32 $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) 
0x82d08100
+#  ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \
+#  `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
 
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o 
$(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 3139886..7c76f80 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -1,6 +1,10 @@
 
 # x86-specific definitions
 
+XEN_IMG_OFFSET = 0x20
+
+CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
+
 CFLAGS += -I$(BASEDIR)/include
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 964851b..e322270 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -12,7 +12,7 @@
 .text
 .code32
 
-#define sym_phys(sym) ((sym) - __XEN_VIRT_START)
+#define sym_offset(sym)   ((sym) - __XEN_VIRT_START)
 
 #define BOOT_CS320x0008
 #define BOOT_CS640x0010
@@ -94,7 +94,7 @@ multiboot2_header_start:
 
 /* EFI64 entry point. */
 mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
-   sym_phys(__efi64_start)
+   sym_offset(__efi64_start)
 
 /* Multiboot2 header end tag. */
 mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
@@ -106,11 +106,12 @@ multiboot2_header_end: