* [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2018-04-30 15:56 ` Jan Beulich
2017-07-08 21:53 ` [PATCH RFC 2/7] xen/x86: Manually build PE header Daniel Kiper
` (5 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
We need the POSIX time to properly fill the TimeDateStamp field in the PE header.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/Makefile | 14 ++++++++------
xen/include/xen/compile.h.in | 1 +
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/xen/Makefile b/xen/Makefile
index f6a6bc2..2424690 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -6,12 +6,13 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
export XEN_FULLVERSION = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
-include xen-version
-export XEN_WHOAMI ?= $(USER)
-export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
-export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
-export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
-export XEN_BUILD_HOST ?= $(shell hostname)
-export XEN_CONFIG_EXPERT ?= n
+export XEN_WHOAMI ?= $(USER)
+export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
+export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
+export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
+export XEN_BUILD_POSIX_TIME ?= $(shell LC_ALL=C date +%s)
+export XEN_BUILD_HOST ?= $(shell hostname)
+export XEN_CONFIG_EXPERT ?= n
export BASEDIR := $(CURDIR)
export XEN_ROOT := $(BASEDIR)/..
@@ -164,6 +165,7 @@ delete-unfresh-files:
include/xen/compile.h: include/xen/compile.h.in .banner
@sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
-e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
+ -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
-e 's/@@domain@@/$(XEN_DOMAIN)/g' \
-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 440ecb2..b2ae6f9 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -1,5 +1,6 @@
#define XEN_COMPILE_DATE "@@date@@"
#define XEN_COMPILE_TIME "@@time@@"
+#define XEN_COMPILE_POSIX_TIME @@posix_time@@
#define XEN_COMPILE_BY "@@whoami@@"
#define XEN_COMPILE_DOMAIN "@@domain@@"
#define XEN_COMPILE_HOST "@@hostname@@"
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2017-07-08 21:53 ` [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME Daniel Kiper
@ 2018-04-30 15:56 ` Jan Beulich
2018-05-08 12:18 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-04-30 15:56 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> We need the POSIX time to properly fill the TimeDateStamp field in the PE
> header.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> xen/Makefile | 14 ++++++++------
> xen/include/xen/compile.h.in | 1 +
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/xen/Makefile b/xen/Makefile
> index f6a6bc2..2424690 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -6,12 +6,13 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
> export XEN_FULLVERSION = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
> -include xen-version
>
> -export XEN_WHOAMI ?= $(USER)
> -export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
> -export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
> -export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
> -export XEN_BUILD_HOST ?= $(shell hostname)
> -export XEN_CONFIG_EXPERT ?= n
> +export XEN_WHOAMI ?= $(USER)
> +export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
> +export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
> +export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
> +export XEN_BUILD_POSIX_TIME ?= $(shell LC_ALL=C date +%s)
If you run two independent commands anyway, I don't see why you need to
obtain the POSIX time here. If you're really after the time stamps agreeing,
then you should invoke date only once (strictly speaking this also applies to
the DATE vs TIME invocation, but lets hope people sleep at midnight rather
than building Xen).
> @@ -164,6 +165,7 @@ delete-unfresh-files:
> include/xen/compile.h: include/xen/compile.h.in .banner
> @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
> -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
> + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
In order to fill a PE header, do you really need to make this available in
compile.h?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2018-04-30 15:56 ` Jan Beulich
@ 2018-05-08 12:18 ` Daniel Kiper
2018-05-14 10:30 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-08 12:18 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Mon, Apr 30, 2018 at 09:56:38AM -0600, Jan Beulich wrote:
> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> > We need the POSIX time to properly fill the TimeDateStamp field in the PE
> > header.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > ---
> > xen/Makefile | 14 ++++++++------
> > xen/include/xen/compile.h.in | 1 +
> > 2 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/Makefile b/xen/Makefile
> > index f6a6bc2..2424690 100644
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -6,12 +6,13 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
> > export XEN_FULLVERSION = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
> > -include xen-version
> >
> > -export XEN_WHOAMI ?= $(USER)
> > -export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
> > -export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
> > -export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
> > -export XEN_BUILD_HOST ?= $(shell hostname)
> > -export XEN_CONFIG_EXPERT ?= n
> > +export XEN_WHOAMI ?= $(USER)
> > +export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo [unknown]))
> > +export XEN_BUILD_DATE ?= $(shell LC_ALL=C date)
> > +export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T)
> > +export XEN_BUILD_POSIX_TIME ?= $(shell LC_ALL=C date +%s)
>
> If you run two independent commands anyway, I don't see why you need to
> obtain the POSIX time here. If you're really after the time stamps agreeing,
> then you should invoke date only once (strictly speaking this also applies to
> the DATE vs TIME invocation, but lets hope people sleep at midnight rather
> than building Xen).
If you wish I can fix this and feed the XEN_BUILD_DATE output into subsequent
date commands.
> > @@ -164,6 +165,7 @@ delete-unfresh-files:
> > include/xen/compile.h: include/xen/compile.h.in .banner
> > @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
> > -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
> > + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
>
> In order to fill a PE header, do you really need to make this available in
> compile.h?
Why not? I think that we should have all time related constants defined
in one place. Even if one of them is used just only once.
Daniel
PS Your comments come just in time. I am working on the second version
of the patches.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2018-05-08 12:18 ` Daniel Kiper
@ 2018-05-14 10:30 ` Jan Beulich
2018-05-14 16:25 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-14 10:30 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.05.18 at 14:18, <daniel.kiper@oracle.com> wrote:
> On Mon, Apr 30, 2018 at 09:56:38AM -0600, Jan Beulich wrote:
>> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> > @@ -164,6 +165,7 @@ delete-unfresh-files:
>> > include/xen/compile.h: include/xen/compile.h.in .banner
>> > @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
>> > -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
>> > + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
>>
>> In order to fill a PE header, do you really need to make this available in
>> compile.h?
>
> Why not? I think that we should have all time related constants defined
> in one place. Even if one of them is used just only once.
I don't think so, fwiw, i.e. I'd prefer you to consume XEN_BUILD_{DATE,TIME}
at the point/place you want/need the time in POSIX form.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2018-05-14 10:30 ` Jan Beulich
@ 2018-05-14 16:25 ` Daniel Kiper
2018-05-15 7:47 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-14 16:25 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Mon, May 14, 2018 at 04:30:02AM -0600, Jan Beulich wrote:
> >>> On 08.05.18 at 14:18, <daniel.kiper@oracle.com> wrote:
> > On Mon, Apr 30, 2018 at 09:56:38AM -0600, Jan Beulich wrote:
> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> >> > @@ -164,6 +165,7 @@ delete-unfresh-files:
> >> > include/xen/compile.h: include/xen/compile.h.in .banner
> >> > @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
> >> > -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
> >> > + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
> >>
> >> In order to fill a PE header, do you really need to make this available in
> >> compile.h?
> >
> > Why not? I think that we should have all time related constants defined
> > in one place. Even if one of them is used just only once.
>
> I don't think so, fwiw, i.e. I'd prefer you to consume XEN_BUILD_{DATE,TIME}
> at the point/place you want/need the time in POSIX form.
That would be perfect but TimeDateStamp in PE header requires the number of
seconds since the epoch. And XEN_BUILD_{DATE,TIME} are in different formats.
Hence, what should I do then?
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME
2018-05-14 16:25 ` Daniel Kiper
@ 2018-05-15 7:47 ` Jan Beulich
0 siblings, 0 replies; 25+ messages in thread
From: Jan Beulich @ 2018-05-15 7:47 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 14.05.18 at 18:25, <daniel.kiper@oracle.com> wrote:
> On Mon, May 14, 2018 at 04:30:02AM -0600, Jan Beulich wrote:
>> >>> On 08.05.18 at 14:18, <daniel.kiper@oracle.com> wrote:
>> > On Mon, Apr 30, 2018 at 09:56:38AM -0600, Jan Beulich wrote:
>> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> >> > @@ -164,6 +165,7 @@ delete-unfresh-files:
>> >> > include/xen/compile.h: include/xen/compile.h.in .banner
>> >> > @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
>> >> > -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
>> >> > + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
>> >>
>> >> In order to fill a PE header, do you really need to make this available in
>> >> compile.h?
>> >
>> > Why not? I think that we should have all time related constants defined
>> > in one place. Even if one of them is used just only once.
>>
>> I don't think so, fwiw, i.e. I'd prefer you to consume XEN_BUILD_{DATE,TIME}
>> at the point/place you want/need the time in POSIX form.
>
> That would be perfect but TimeDateStamp in PE header requires the number of
> seconds since the epoch. And XEN_BUILD_{DATE,TIME} are in different formats.
> Hence, what should I do then?
I'm afraid I don't understand: As long as XEN_BUILD_{DATE,TIME} properly
represent the time you're after in _some_ format, surely there's a way to
convert the format to "seconds since the epoch"?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 2/7] xen/x86: Manually build PE header
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2018-05-04 15:38 ` Jan Beulich
2017-07-08 21:53 ` [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header Daniel Kiper
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
This is the first step to get:
- one binary which can be loaded by the EFI loader,
Multiboot and Multiboot2 protocols,
- if we wish, in the future we can drop xen/xen.gz
and build xen.efi only,
- crash dumps generated by the xen.efi loaded from
the EFI loader can be analyzed by crash tool,
- simpler code,
- simpler build,
- Xen build will no longer depend on ld i386pep support.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/Rules.mk | 2 +
xen/arch/x86/boot/head.S | 145 ++++++++++++++++++++++++++++++++++++++++++++++
xen/arch/x86/xen.lds.S | 16 ++++-
3 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 568657e..b501c88 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
+CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
+CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
# Prevent floating-point variables from creeping into Xen.
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index fd6fc33..28bbc04 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -1,3 +1,4 @@
+#include <xen/compile.h>
#include <xen/multiboot.h>
#include <xen/multiboot2.h>
#include <public/xen.h>
@@ -44,6 +45,150 @@
.Lmb2ht_init_end\@:
.endm
+ .section .efi.pe.header, "a", @progbits
+
+ENTRY(efi_pe_head)
+ /*
+ * Legacy EXE header.
+ *
+ * Most of it is copied from binutils package, version 2.28,
+ * include/coff/pe.h:struct external_PEI_filehdr and
+ * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
+ *
+ * Page is equal 512 bytes here.
+ * Paragraph is equal 16 bytes here.
+ */
+ .short 0x5a4d /* EXE magic number. */
+ .short 0x90 /* Bytes on last page of file. */
+ .short 0x3 /* Pages in file. */
+ .short 0 /* Relocations. */
+ .short 0x4 /* Size of header in paragraphs. */
+ .short 0 /* Minimum extra paragraphs needed. */
+ .short 0xffff /* Maximum extra paragraphs needed. */
+ .short 0 /* Initial (relative) SS value. */
+ .short 0xb8 /* Initial SP value. */
+ .short 0 /* Checksum. */
+ .short 0 /* Initial IP value. */
+ .short 0 /* Initial (relative) CS value. */
+ .short 0x40 /* File address of relocation table. */
+ .short 0 /* Overlay number. */
+ .fill 4, 2, 0 /* Reserved words. */
+ .short 0 /* OEM identifier. */
+ .short 0 /* OEM information. */
+ .fill 10, 2, 0 /* Reserved words. */
+ .long pe_header - efi_pe_head /* File address of the PE header. */
+
+ /*
+ * DOS message.
+ *
+ * It is copied from binutils package, version 2.28,
+ * include/coff/pe.h:struct external_PEI_filehdr and
+ * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
+ */
+ .long 0x0eba1f0e
+ .long 0xcd09b400
+ .long 0x4c01b821
+ .long 0x685421cd
+ .long 0x70207369
+ .long 0x72676f72
+ .long 0x63206d61
+ .long 0x6f6e6e61
+ .long 0x65622074
+ .long 0x6e757220
+ .long 0x206e6920
+ .long 0x20534f44
+ .long 0x65646f6d
+ .long 0x0a0d0d2e
+ .long 0x24
+ .long 0
+
+ /*
+ * PE/COFF header.
+ *
+ * The PE/COFF format is defined by Microsoft, and is available from
+ * http://www.microsoft.com/whdc/system/platform/firmware/PECOFF.mspx
+ *
+ * Some ideas are taken from Linux kernel and Xen ARM64.
+ */
+
+pe_header:
+ .ascii "PE\0\0" /* PE signature. */
+ .short 0x8664 /* Machine: IMAGE_FILE_MACHINE_AMD64. */
+ .short 1 /* NumberOfSections. */
+ .long XEN_COMPILE_POSIX_TIME /* TimeDateStamp. */
+ .long 0 /* PointerToSymbolTable. */
+ .long 0 /* NumberOfSymbols. */
+ .short section_table - optional_header /* SizeOfOptionalHeader. */
+ .short 0x226 /* Characteristics:
+ * IMAGE_FILE_EXECUTABLE_IMAGE |
+ * IMAGE_FILE_LARGE_ADDRESS_AWARE |
+ * IMAGE_FILE_DEBUG_STRIPPED |
+ * IMAGE_FILE_LINE_NUMS_STRIPPED
+ */
+
+optional_header:
+ .short 0x20b /* PE format: PE32+ */
+ .byte 0x02 /* MajorLinkerVersion. */
+ .byte 0x14 /* MinorLinkerVersion. */
+ .long __2M_rwdata_end - efi_pe_head_end /* SizeOfCode. */
+ .long 0 /* SizeOfInitializedData. */
+ .long 0 /* SizeOfUninitializedData. */
+ .long sym_offs(efi_start) /* AddressOfEntryPoint. */
+ .long sym_offs(start) /* BaseOfCode. */
+ .quad sym_offs(__image_base__) /* ImageBase. */
+ .long XEN_LOAD_ALIGN /* SectionAlignment. */
+ .long XEN_FILE_ALIGN /* FileAlignment. */
+ .short 2 /* MajorOperatingSystemVersion. */
+ .short 0 /* MinorOperatingSystemVersion. */
+ .short XEN_VERSION /* MajorImageVersion. */
+ .short XEN_SUBVERSION /* MinorImageVersion. */
+ .short 2 /* MajorSubsystemVersion. */
+ .short 0 /* MinorSubsystemVersion. */
+ .long 0 /* Win32VersionValue. */
+ .long __pe_SizeOfImage /* SizeOfImage. */
+ .long efi_pe_head_end - efi_pe_head /* SizeOfHeaders. */
+ .long 0 /* CheckSum. */
+ .short 0xa /* Subsystem: EFI application. */
+ .short 0 /* DllCharacteristics. */
+ .quad 0 /* SizeOfStackReserve. */
+ .quad 0 /* SizeOfStackCommit. */
+ .quad 0 /* SizeOfHeapReserve. */
+ .quad 0 /* SizeOfHeapCommit. */
+ .long 0 /* LoaderFlags. */
+ .long 0x6 /* NumberOfRvaAndSizes. */
+
+ /* Data Directories. */
+ .quad 0 /* Export Table. */
+ .quad 0 /* Import Table. */
+ .quad 0 /* Resource Table. */
+ .quad 0 /* Exception Table. */
+ .quad 0 /* Certificate Table. */
+ .quad 0 /* Base Relocation Table. */
+
+section_table:
+ .ascii ".text\0\0\0" /* Name. */
+ .long __2M_rwdata_end - efi_pe_head_end /* VirtualSize. */
+ .long sym_offs(start) /* VirtualAddress. */
+ .long __bss_start - efi_pe_head_end /* SizeOfRawData. */
+ .long efi_pe_head_end - efi_pe_head /* PointerToRawData. */
+ .long 0 /* PointerToRelocations. */
+ .long 0 /* PointerToLinenumbers. */
+ .short 0 /* NumberOfRelocations. */
+ .short 0 /* NumberOfLinenumbers. */
+ .long 0xe0500020 /* Characteristics:
+ * IMAGE_SCN_CNT_CODE |
+ * IMAGE_SCN_ALIGN_16BYTES |
+ * IMAGE_SCN_MEM_EXECUTE |
+ * IMAGE_SCN_MEM_READ |
+ * IMAGE_SCN_MEM_WRITE
+ */
+
+ .align XEN_FILE_ALIGN
+ENTRY(efi_pe_head_end)
+
+ .text
+ .code32
+
ENTRY(start)
jmp __start
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 8289a1b..3c115b9 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -54,7 +54,12 @@ SECTIONS
__2M_text_start = .; /* Start of 2M superpages, mapped RX. */
#endif
- . = __XEN_VIRT_START + XEN_IMG_OFFSET;
+ . = __XEN_VIRT_START + XEN_IMG_OFFSET - efi_pe_head_end + efi_pe_head;
+
+ .efi.pe.header : {
+ *(.efi.pe.header)
+ } :NONE
+
_start = .;
.text : {
_stext = .; /* Text and read-only data */
@@ -234,6 +239,8 @@ SECTIONS
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
+ /* PE file must end at XEN_FILE_ALIGN boundary. */
+ . = ALIGN(XEN_FILE_ALIGN);
} :text
.bss : { /* BSS */
@@ -259,6 +266,8 @@ SECTIONS
#endif
__2M_rwdata_end = .;
+ __pe_SizeOfImage = ALIGN(. - __image_base__, XEN_LOAD_ALIGN);
+
#ifdef EFI
. = ALIGN(4);
.reloc : {
@@ -337,3 +346,8 @@ ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE - MBI_SPACE_MIN,
"not enough room for trampoline and mbi data")
ASSERT((wakeup_stack - wakeup_stack_start) >= WAKEUP_STACK_MIN,
"wakeup stack too small")
+
+ASSERT(efi_pe_head_end == _start, "PE header does not end at the beginning of .text section")
+ASSERT(_start == __XEN_VIRT_START + XEN_IMG_OFFSET, ".text section begins at wrong address")
+ASSERT(IS_ALIGNED(_start, XEN_FILE_ALIGN), "_start misaligned")
+ASSERT(IS_ALIGNED(__bss_start, XEN_FILE_ALIGN), "__bss_start misaligned")
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH RFC 2/7] xen/x86: Manually build PE header
2017-07-08 21:53 ` [PATCH RFC 2/7] xen/x86: Manually build PE header Daniel Kiper
@ 2018-05-04 15:38 ` Jan Beulich
2018-05-08 12:47 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-04 15:38 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> This is the first step to get:
> - one binary which can be loaded by the EFI loader,
> Multiboot and Multiboot2 protocols,
> - if we wish, in the future we can drop xen/xen.gz
> and build xen.efi only,
If anything, generate xen.gz from xen.efi - I see value in the compression,
but the EFI loader requires an uncompressed binary. And of course we'd have
to raise the minimal gcc version requirement.
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
> CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
> CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
> CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
> +CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
> +CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
??? (Sadly your description talks about benefits only, not about what the
patch actually does.)
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -1,3 +1,4 @@
> +#include <xen/compile.h>
> #include <xen/multiboot.h>
> #include <xen/multiboot2.h>
> #include <public/xen.h>
> @@ -44,6 +45,150 @@
> .Lmb2ht_init_end\@:
> .endm
>
> + .section .efi.pe.header, "a", @progbits
> +
> +ENTRY(efi_pe_head)
Since you put this in a separate section anyway, why don't you place it in
a C file (perhaps even of its own) with suitably declared structures?
> + /*
> + * Legacy EXE header.
> + *
> + * Most of it is copied from binutils package, version 2.28,
> + * include/coff/pe.h:struct external_PEI_filehdr and
> + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
> + *
> + * Page is equal 512 bytes here.
> + * Paragraph is equal 16 bytes here.
> + */
> + .short 0x5a4d /* EXE magic number. */
> + .short 0x90 /* Bytes on last page of file. */
> + .short 0x3 /* Pages in file. */
> + .short 0 /* Relocations. */
> + .short 0x4 /* Size of header in paragraphs. */
> + .short 0 /* Minimum extra paragraphs needed. */
> + .short 0xffff /* Maximum extra paragraphs needed. */
> + .short 0 /* Initial (relative) SS value. */
> + .short 0xb8 /* Initial SP value. */
> + .short 0 /* Checksum. */
> + .short 0 /* Initial IP value. */
> + .short 0 /* Initial (relative) CS value. */
> + .short 0x40 /* File address of relocation table. */
> + .short 0 /* Overlay number. */
> + .fill 4, 2, 0 /* Reserved words. */
> + .short 0 /* OEM identifier. */
> + .short 0 /* OEM information. */
> + .fill 10, 2, 0 /* Reserved words. */
> + .long pe_header - efi_pe_head /* File address of the PE header. */
> +
> + /*
> + * DOS message.
> + *
> + * It is copied from binutils package, version 2.28,
> + * include/coff/pe.h:struct external_PEI_filehdr and
> + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
> + */
> + .long 0x0eba1f0e
> + .long 0xcd09b400
> + .long 0x4c01b821
> + .long 0x685421cd
> + .long 0x70207369
> + .long 0x72676f72
> + .long 0x63206d61
> + .long 0x6f6e6e61
> + .long 0x65622074
> + .long 0x6e757220
> + .long 0x206e6920
> + .long 0x20534f44
> + .long 0x65646f6d
> + .long 0x0a0d0d2e
> + .long 0x24
> + .long 0
Other than what the comment says, this isn't just a message (or else you
could have used .asciz for the whole thing). I'm not convinced we need
any of this.
> @@ -259,6 +266,8 @@ SECTIONS
> #endif
> __2M_rwdata_end = .;
>
> + __pe_SizeOfImage = ALIGN(. - __image_base__, XEN_LOAD_ALIGN);
I don't think this is in line with what xen.efi currently has. Any difference
needs explaining (I think there are further fields in this category).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 2/7] xen/x86: Manually build PE header
2018-05-04 15:38 ` Jan Beulich
@ 2018-05-08 12:47 ` Daniel Kiper
2018-05-14 10:40 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-08 12:47 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Fri, May 04, 2018 at 09:38:03AM -0600, Jan Beulich wrote:
> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> > This is the first step to get:
> > - one binary which can be loaded by the EFI loader,
> > Multiboot and Multiboot2 protocols,
> > - if we wish, in the future we can drop xen/xen.gz
> > and build xen.efi only,
>
> If anything, generate xen.gz from xen.efi - I see value in the compression,
I generate both xen.gz and xen.efi from xen-syms. Anyway, as I can see
we currently depend more on ELF output than earlier. So, I do not expect
that we would be able to drop xen.gz in the near future.
> but the EFI loader requires an uncompressed binary. And of course we'd have
> to raise the minimal gcc version requirement.
>
> > --- a/xen/arch/x86/Rules.mk
> > +++ b/xen/arch/x86/Rules.mk
> > @@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
> > CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
> > +CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
> > +CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
>
> ??? (Sadly your description talks about benefits only, not about what the
> patch actually does.)
OK, I will improve the commit message. And maybe s/XEN_FILE_ALIGN/XEN_EFI_FILE_ALIGN/.
And s/PAGE_SIZE/512/. This is minimal value required by PE spec. I have used
PAGE_SIZE earlier just to be on safe side and in line with the output from ld.
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -1,3 +1,4 @@
> > +#include <xen/compile.h>
> > #include <xen/multiboot.h>
> > #include <xen/multiboot2.h>
> > #include <public/xen.h>
> > @@ -44,6 +45,150 @@
> > .Lmb2ht_init_end\@:
> > .endm
> >
> > + .section .efi.pe.header, "a", @progbits
> > +
> > +ENTRY(efi_pe_head)
>
> Since you put this in a separate section anyway, why don't you place it in
> a C file (perhaps even of its own) with suitably declared structures?
Really? I thought that it makes sense to have all bootloader headers in
one place. Additionally, C requires struct definition in advance and later
it have to be filled somehow. So, it will be twice as large. Hence, I do not
see much benefit in using C here. OK, maybe it will be a bit more readable.
> > + /*
> > + * Legacy EXE header.
> > + *
> > + * Most of it is copied from binutils package, version 2.28,
> > + * include/coff/pe.h:struct external_PEI_filehdr and
> > + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
> > + *
> > + * Page is equal 512 bytes here.
> > + * Paragraph is equal 16 bytes here.
> > + */
> > + .short 0x5a4d /* EXE magic number. */
> > + .short 0x90 /* Bytes on last page of file. */
> > + .short 0x3 /* Pages in file. */
> > + .short 0 /* Relocations. */
> > + .short 0x4 /* Size of header in paragraphs. */
> > + .short 0 /* Minimum extra paragraphs needed. */
> > + .short 0xffff /* Maximum extra paragraphs needed. */
> > + .short 0 /* Initial (relative) SS value. */
> > + .short 0xb8 /* Initial SP value. */
> > + .short 0 /* Checksum. */
> > + .short 0 /* Initial IP value. */
> > + .short 0 /* Initial (relative) CS value. */
> > + .short 0x40 /* File address of relocation table. */
> > + .short 0 /* Overlay number. */
> > + .fill 4, 2, 0 /* Reserved words. */
> > + .short 0 /* OEM identifier. */
> > + .short 0 /* OEM information. */
> > + .fill 10, 2, 0 /* Reserved words. */
> > + .long pe_header - efi_pe_head /* File address of the PE header. */
> > +
> > + /*
> > + * DOS message.
> > + *
> > + * It is copied from binutils package, version 2.28,
> > + * include/coff/pe.h:struct external_PEI_filehdr and
> > + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
> > + */
> > + .long 0x0eba1f0e
> > + .long 0xcd09b400
> > + .long 0x4c01b821
> > + .long 0x685421cd
> > + .long 0x70207369
> > + .long 0x72676f72
> > + .long 0x63206d61
> > + .long 0x6f6e6e61
> > + .long 0x65622074
> > + .long 0x6e757220
> > + .long 0x206e6920
> > + .long 0x20534f44
> > + .long 0x65646f6d
> > + .long 0x0a0d0d2e
> > + .long 0x24
> > + .long 0
>
> Other than what the comment says, this isn't just a message (or else you
> could have used .asciz for the whole thing). I'm not convinced we need
> any of this.
Potentially we can drop this. However, ld from binutils put this into
EFI binary. And IIRC this is exactly what is embedded by other linkers
into PE/COFF compatible files, e.g. *.efi, *.exe, *.dll, etc. So,
I would leave this just for the sake of compatibility.
> > @@ -259,6 +266,8 @@ SECTIONS
> > #endif
> > __2M_rwdata_end = .;
> >
> > + __pe_SizeOfImage = ALIGN(. - __image_base__, XEN_LOAD_ALIGN);
>
> I don't think this is in line with what xen.efi currently has. Any difference
> needs explaining (I think there are further fields in this category).
I am not going to build manually exact copy of current xen.efi. It does not
make sense. I would like to provide something minimalistic which works. No
more no less. However, if you wish I can provide relevant comment here.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 2/7] xen/x86: Manually build PE header
2018-05-08 12:47 ` Daniel Kiper
@ 2018-05-14 10:40 ` Jan Beulich
2018-05-14 16:52 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-14 10:40 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.05.18 at 14:47, <daniel.kiper@oracle.com> wrote:
> On Fri, May 04, 2018 at 09:38:03AM -0600, Jan Beulich wrote:
>> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> > --- a/xen/arch/x86/Rules.mk
>> > +++ b/xen/arch/x86/Rules.mk
>> > @@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
>> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
>> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
>> > CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
>> > +CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
>> > +CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
>>
>> ??? (Sadly your description talks about benefits only, not about what the
>> patch actually does.)
>
> OK, I will improve the commit message. And maybe
> s/XEN_FILE_ALIGN/XEN_EFI_FILE_ALIGN/.
> And s/PAGE_SIZE/512/. This is minimal value required by PE spec.
Just go through *.sys on any Windows, and I think you'll find several with
smaller file alignment. Iirc anything down to 0x20 is fine with the Windows
driver loader (and the EFI one as well). I don't see any reason to use
larger values than needed here, as there's no demand paging involved,
where the higher alignment indeed helps performance.
>> > --- a/xen/arch/x86/boot/head.S
>> > +++ b/xen/arch/x86/boot/head.S
>> > @@ -1,3 +1,4 @@
>> > +#include <xen/compile.h>
>> > #include <xen/multiboot.h>
>> > #include <xen/multiboot2.h>
>> > #include <public/xen.h>
>> > @@ -44,6 +45,150 @@
>> > .Lmb2ht_init_end\@:
>> > .endm
>> >
>> > + .section .efi.pe.header, "a", @progbits
>> > +
>> > +ENTRY(efi_pe_head)
>>
>> Since you put this in a separate section anyway, why don't you place it in
>> a C file (perhaps even of its own) with suitably declared structures?
>
> Really? I thought that it makes sense to have all bootloader headers in
> one place. Additionally, C requires struct definition in advance and later
> it have to be filled somehow. So, it will be twice as large. Hence, I do not
> see much benefit in using C here. OK, maybe it will be a bit more readable.
That last aspect is the important one, and the reason we try to morph
assembly code over into C where possible.
>> > + /*
>> > + * DOS message.
>> > + *
>> > + * It is copied from binutils package, version 2.28,
>> > + * include/coff/pe.h:struct external_PEI_filehdr and
>> > + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
>> > + */
>> > + .long 0x0eba1f0e
>> > + .long 0xcd09b400
>> > + .long 0x4c01b821
>> > + .long 0x685421cd
>> > + .long 0x70207369
>> > + .long 0x72676f72
>> > + .long 0x63206d61
>> > + .long 0x6f6e6e61
>> > + .long 0x65622074
>> > + .long 0x6e757220
>> > + .long 0x206e6920
>> > + .long 0x20534f44
>> > + .long 0x65646f6d
>> > + .long 0x0a0d0d2e
>> > + .long 0x24
>> > + .long 0
>>
>> Other than what the comment says, this isn't just a message (or else you
>> could have used .asciz for the whole thing). I'm not convinced we need
>> any of this.
>
> Potentially we can drop this. However, ld from binutils put this into
> EFI binary. And IIRC this is exactly what is embedded by other linkers
> into PE/COFF compatible files, e.g. *.efi, *.exe, *.dll, etc. So,
> I would leave this just for the sake of compatibility.
Having this in .exe files is indeed helpful (or at least was back when DOS still
played some sort of a role). In .dll it is already highly questionable, and
hence even more so in .efi. Let's not encode and carry cruft that's not
needed for anything.
>> > @@ -259,6 +266,8 @@ SECTIONS
>> > #endif
>> > __2M_rwdata_end = .;
>> >
>> > + __pe_SizeOfImage = ALIGN(. - __image_base__, XEN_LOAD_ALIGN);
>>
>> I don't think this is in line with what xen.efi currently has. Any difference
>> needs explaining (I think there are further fields in this category).
>
> I am not going to build manually exact copy of current xen.efi. It does not
> make sense. I would like to provide something minimalistic which works. No
> more no less. However, if you wish I can provide relevant comment here.
My remark wasn't because I expect 1:1 matching output. However, core
functionality needs to be the same, and iirc this being exactly 16Mb has a
reason in today's xen.efi (you may want to fish out the commit).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 2/7] xen/x86: Manually build PE header
2018-05-14 10:40 ` Jan Beulich
@ 2018-05-14 16:52 ` Daniel Kiper
2018-05-15 8:01 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-14 16:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Mon, May 14, 2018 at 04:40:39AM -0600, Jan Beulich wrote:
> >>> On 08.05.18 at 14:47, <daniel.kiper@oracle.com> wrote:
> > On Fri, May 04, 2018 at 09:38:03AM -0600, Jan Beulich wrote:
> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> >> > --- a/xen/arch/x86/Rules.mk
> >> > +++ b/xen/arch/x86/Rules.mk
> >> > @@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
> >> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
> >> > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
> >> > CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
> >> > +CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
> >> > +CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
> >>
> >> ??? (Sadly your description talks about benefits only, not about what the
> >> patch actually does.)
> >
> > OK, I will improve the commit message. And maybe
> > s/XEN_FILE_ALIGN/XEN_EFI_FILE_ALIGN/.
> > And s/PAGE_SIZE/512/. This is minimal value required by PE spec.
>
> Just go through *.sys on any Windows, and I think you'll find several with
> smaller file alignment. Iirc anything down to 0x20 is fine with the Windows
> driver loader (and the EFI one as well). I don't see any reason to use
> larger values than needed here, as there's no demand paging involved,
> where the higher alignment indeed helps performance.
I saw this earlier in vmlinuz. However, it looks that even MS ignores
own specs. Ehhh... OK, will change this to 0x20.
> >> > --- a/xen/arch/x86/boot/head.S
> >> > +++ b/xen/arch/x86/boot/head.S
> >> > @@ -1,3 +1,4 @@
> >> > +#include <xen/compile.h>
> >> > #include <xen/multiboot.h>
> >> > #include <xen/multiboot2.h>
> >> > #include <public/xen.h>
> >> > @@ -44,6 +45,150 @@
> >> > .Lmb2ht_init_end\@:
> >> > .endm
> >> >
> >> > + .section .efi.pe.header, "a", @progbits
> >> > +
> >> > +ENTRY(efi_pe_head)
> >>
> >> Since you put this in a separate section anyway, why don't you place it in
> >> a C file (perhaps even of its own) with suitably declared structures?
> >
> > Really? I thought that it makes sense to have all bootloader headers in
> > one place. Additionally, C requires struct definition in advance and later
> > it have to be filled somehow. So, it will be twice as large. Hence, I do not
> > see much benefit in using C here. OK, maybe it will be a bit more readable.
>
> That last aspect is the important one, and the reason we try to morph
> assembly code over into C where possible.
OK, will do that then.
> >> > + /*
> >> > + * DOS message.
> >> > + *
> >> > + * It is copied from binutils package, version 2.28,
> >> > + * include/coff/pe.h:struct external_PEI_filehdr and
> >> > + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
> >> > + */
> >> > + .long 0x0eba1f0e
> >> > + .long 0xcd09b400
> >> > + .long 0x4c01b821
> >> > + .long 0x685421cd
> >> > + .long 0x70207369
> >> > + .long 0x72676f72
> >> > + .long 0x63206d61
> >> > + .long 0x6f6e6e61
> >> > + .long 0x65622074
> >> > + .long 0x6e757220
> >> > + .long 0x206e6920
> >> > + .long 0x20534f44
> >> > + .long 0x65646f6d
> >> > + .long 0x0a0d0d2e
> >> > + .long 0x24
> >> > + .long 0
> >>
> >> Other than what the comment says, this isn't just a message (or else you
> >> could have used .asciz for the whole thing). I'm not convinced we need
> >> any of this.
> >
> > Potentially we can drop this. However, ld from binutils put this into
> > EFI binary. And IIRC this is exactly what is embedded by other linkers
> > into PE/COFF compatible files, e.g. *.efi, *.exe, *.dll, etc. So,
> > I would leave this just for the sake of compatibility.
>
> Having this in .exe files is indeed helpful (or at least was back when DOS still
> played some sort of a role). In .dll it is already highly questionable, and
> hence even more so in .efi. Let's not encode and carry cruft that's not
> needed for anything.
OK, but I think that we should leave at least one or two instructions here, e.g.
hlt and jmp back to it or something like that. Or int 0x21 with 0x4c00 in %ax.
Latter seems better for me.
> >> > @@ -259,6 +266,8 @@ SECTIONS
> >> > #endif
> >> > __2M_rwdata_end = .;
> >> >
> >> > + __pe_SizeOfImage = ALIGN(. - __image_base__, XEN_LOAD_ALIGN);
> >>
> >> I don't think this is in line with what xen.efi currently has. Any difference
> >> needs explaining (I think there are further fields in this category).
> >
> > I am not going to build manually exact copy of current xen.efi. It does not
> > make sense. I would like to provide something minimalistic which works. No
> > more no less. However, if you wish I can provide relevant comment here.
>
> My remark wasn't because I expect 1:1 matching output. However, core
> functionality needs to be the same, and iirc this being exactly 16Mb has a
> reason in today's xen.efi (you may want to fish out the commit).
OK, I will take a look.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 2/7] xen/x86: Manually build PE header
2018-05-14 16:52 ` Daniel Kiper
@ 2018-05-15 8:01 ` Jan Beulich
0 siblings, 0 replies; 25+ messages in thread
From: Jan Beulich @ 2018-05-15 8:01 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 14.05.18 at 18:52, <daniel.kiper@oracle.com> wrote:
> On Mon, May 14, 2018 at 04:40:39AM -0600, Jan Beulich wrote:
>> >>> On 08.05.18 at 14:47, <daniel.kiper@oracle.com> wrote:
>> > On Fri, May 04, 2018 at 09:38:03AM -0600, Jan Beulich wrote:
>> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> >> > + /*
>> >> > + * DOS message.
>> >> > + *
>> >> > + * It is copied from binutils package, version 2.28,
>> >> > + * include/coff/pe.h:struct external_PEI_filehdr and
>> >> > + * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
>> >> > + */
>> >> > + .long 0x0eba1f0e
>> >> > + .long 0xcd09b400
>> >> > + .long 0x4c01b821
>> >> > + .long 0x685421cd
>> >> > + .long 0x70207369
>> >> > + .long 0x72676f72
>> >> > + .long 0x63206d61
>> >> > + .long 0x6f6e6e61
>> >> > + .long 0x65622074
>> >> > + .long 0x6e757220
>> >> > + .long 0x206e6920
>> >> > + .long 0x20534f44
>> >> > + .long 0x65646f6d
>> >> > + .long 0x0a0d0d2e
>> >> > + .long 0x24
>> >> > + .long 0
>> >>
>> >> Other than what the comment says, this isn't just a message (or else you
>> >> could have used .asciz for the whole thing). I'm not convinced we need
>> >> any of this.
>> >
>> > Potentially we can drop this. However, ld from binutils put this into
>> > EFI binary. And IIRC this is exactly what is embedded by other linkers
>> > into PE/COFF compatible files, e.g. *.efi, *.exe, *.dll, etc. So,
>> > I would leave this just for the sake of compatibility.
>>
>> Having this in .exe files is indeed helpful (or at least was back when DOS still
>> played some sort of a role). In .dll it is already highly questionable, and
>> hence even more so in .efi. Let's not encode and carry cruft that's not
>> needed for anything.
>
> OK, but I think that we should leave at least one or two instructions here, e.g.
> hlt and jmp back to it or something like that. Or int 0x21 with 0x4c00 in %ax.
> Latter seems better for me.
I certainly don't mind you doing something minimalistic like what you propose.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 2/7] xen/x86: Manually build PE header Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2018-05-04 15:40 ` Jan Beulich
2017-07-08 21:53 ` [PATCH RFC 4/7] xen/x86: Add some addresses to the Multiboot2 header Daniel Kiper
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
In comparison to ELF the PE format is not supported by the Multiboot
protocol. So, if we wish to load xen.efi using this protocol we have
to put header_addr, load_addr, load_end_addr, bss_end_addr and
entry_addr data into Multiboot header.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/boot/head.S | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 28bbc04..0c603a5 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -195,13 +195,24 @@ ENTRY(start)
.align 4
multiboot1_header_start: /*** MULTIBOOT1 HEADER ****/
#define MULTIBOOT_HEADER_FLAGS (MULTIBOOT_HEADER_MODS_ALIGNED | \
- MULTIBOOT_HEADER_WANT_MEMORY)
+ MULTIBOOT_HEADER_WANT_MEMORY | \
+ MULTIBOOT_HEADER_HAS_ADDR)
/* Magic number indicating a Multiboot header. */
.long MULTIBOOT_HEADER_MAGIC
/* Flags to bootloader (see Multiboot spec). */
.long MULTIBOOT_HEADER_FLAGS
/* Checksum: must be the negated sum of the first two fields. */
.long -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
+ /* header_addr */
+ .long sym_offs(multiboot1_header_start)
+ /* load_addr */
+ .long sym_offs(start)
+ /* load_end_addr */
+ .long sym_offs(__bss_start)
+ /* bss_end_addr */
+ .long sym_offs(__2M_rwdata_end)
+ /* entry_addr */
+ .long sym_offs(__start)
multiboot1_header_end:
/*** MULTIBOOT2 HEADER ****/
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header
2017-07-08 21:53 ` [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header Daniel Kiper
@ 2018-05-04 15:40 ` Jan Beulich
2018-05-08 13:01 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-04 15:40 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> In comparison to ELF the PE format is not supported by the Multiboot
> protocol. So, if we wish to load xen.efi using this protocol we have
> to put header_addr, load_addr, load_end_addr, bss_end_addr and
> entry_addr data into Multiboot header.
Looks fine, but you will want to assure us that this non-ELF method of
loading is compatible with each and every loader able of loading Xen so
far.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header
2018-05-04 15:40 ` Jan Beulich
@ 2018-05-08 13:01 ` Daniel Kiper
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Kiper @ 2018-05-08 13:01 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Fri, May 04, 2018 at 09:40:51AM -0600, Jan Beulich wrote:
> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> > In comparison to ELF the PE format is not supported by the Multiboot
> > protocol. So, if we wish to load xen.efi using this protocol we have
> > to put header_addr, load_addr, load_end_addr, bss_end_addr and
> > entry_addr data into Multiboot header.
>
> Looks fine, but you will want to assure us that this non-ELF method of
> loading is compatible with each and every loader able of loading Xen so
> far.
We have tested internally backport of this patch series and only one
SYSLINUX bug surfaced until now. This is Multiboot implementation
issue which have to be fixed at some point in SYSLINUX. However, it
only manifests with ELF files (e.g. xen.gz) if some ELF addresses are
not in line with addresses in Multiboot header (this happened due to
not intended incorrect .efi.pe.header section placement; patch series
posted here does not have this issue). So, this does not apply to
xen.efi and everything works as expected.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 4/7] xen/x86: Add some addresses to the Multiboot2 header
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
` (2 preceding siblings ...)
2017-07-08 21:53 ` [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 5/7] efi: split out efi_shim_lock() Daniel Kiper
` (2 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
In comparison to ELF the PE format is not supported by the Multiboot2
protocol. So, if we wish to load xen.efi using this protocol we have
to add MULTIBOOT2_HEADER_TAG_ADDRESS and MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS
tags into Multiboot2 header.
Additionally, put MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS and
MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64 tags close to each other.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/boot/head.S | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 0c603a5..90db661 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -237,6 +237,13 @@ multiboot2_header_start:
/* Align modules at page boundry. */
mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED)
+ /* The address tag. */
+ mb2ht_init MB2_HT(ADDRESS), MB2_HT(REQUIRED), \
+ sym_offs(multiboot2_header_start), /* header_addr */ \
+ sym_offs(start), /* load_addr */ \
+ sym_offs(__bss_start), /* load_end_addr */ \
+ sym_offs(__2M_rwdata_end) /* bss_end_addr */
+
/* Load address preference. */
mb2ht_init MB2_HT(RELOCATABLE), MB2_HT(OPTIONAL), \
sym_offs(start), /* Min load address. */ \
@@ -244,6 +251,14 @@ multiboot2_header_start:
0x200000, /* Load address alignment (2 MiB). */ \
MULTIBOOT2_LOAD_PREFERENCE_HIGH
+ /* Multiboot2 entry point. */
+ mb2ht_init MB2_HT(ENTRY_ADDRESS), MB2_HT(REQUIRED), \
+ sym_offs(__start)
+
+ /* EFI64 Multiboot2 entry point. */
+ mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
+ sym_offs(__efi64_mb2_start)
+
/* Console flags tag. */
mb2ht_init MB2_HT(CONSOLE_FLAGS), MB2_HT(OPTIONAL), \
MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
@@ -257,10 +272,6 @@ multiboot2_header_start:
/* Request that ExitBootServices() not be called. */
mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL)
- /* EFI64 Multiboot2 entry point. */
- mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
- sym_offs(__efi64_mb2_start)
-
/* Multiboot2 header end tag. */
mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
.Lmultiboot2_header_end:
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH RFC 5/7] efi: split out efi_shim_lock()
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
` (3 preceding siblings ...)
2017-07-08 21:53 ` [PATCH RFC 4/7] xen/x86: Add some addresses to the Multiboot2 header Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2() Daniel Kiper
2017-07-08 21:53 ` [PATCH RFC 7/7] xen/x86: Build xen.mb.efi directly from xen-syms Daniel Kiper
6 siblings, 0 replies; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
..which verifies PE signatures with SHIM_LOCK protocol. We want
to re-use this code in subsequent patch in efi_multiboot2().
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/common/efi/boot.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 11bdc7a..7db3829 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -124,6 +124,7 @@ 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_shim_lock(VOID *Buffer, UINT32 Size);
static void efi_tables(void);
static void setup_efi_pci(void);
static void efi_variables(void);
@@ -797,6 +798,17 @@ static UINTN __init efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
return gop_mode;
}
+static void __init efi_shim_lock(VOID *Buffer, UINT32 Size)
+{
+ static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
+ EFI_SHIM_LOCK_PROTOCOL *shim_lock;
+ EFI_STATUS status;
+
+ if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL, (void **)&shim_lock)) &&
+ (status = shim_lock->Verify(Buffer, Size)) != EFI_SUCCESS )
+ PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+}
+
static void __init efi_tables(void)
{
unsigned int i;
@@ -1062,13 +1074,11 @@ void EFIAPI __init noreturn
efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
{
static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
- static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
EFI_LOADED_IMAGE *loaded_image;
EFI_STATUS status;
unsigned int i, argc;
CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL;
UINTN gop_mode = ~0;
- EFI_SHIM_LOCK_PROTOCOL *shim_lock;
EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
union string section = { NULL }, name;
bool base_video = false;
@@ -1225,10 +1235,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
read_file(dir_handle, s2w(&name), &kernel, option_str);
efi_bs->FreePool(name.w);
- if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
- (void **)&shim_lock)) &&
- (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
- PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+ efi_shim_lock(kernel.ptr, kernel.size);
name.s = get_value(&cfg, section.s, "ramdisk");
if ( name.s )
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
` (4 preceding siblings ...)
2017-07-08 21:53 ` [PATCH RFC 5/7] efi: split out efi_shim_lock() Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
2018-05-04 15:46 ` Jan Beulich
2017-07-08 21:53 ` [PATCH RFC 7/7] xen/x86: Build xen.mb.efi directly from xen-syms Daniel Kiper
6 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/boot/head.S | 20 ++++++++++++++++++--
xen/arch/x86/efi/efi-boot.h | 12 +++++++++++-
xen/arch/x86/efi/stub.c | 5 ++++-
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 90db661..65b3358 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -383,9 +383,13 @@ __efi64_mb2_start:
jmp x86_32_switch
.Lefi_multiboot2_proto:
- /* Zero EFI SystemTable and EFI ImageHandle addresses. */
+ /*
+ * Zero EFI SystemTable, EFI ImageHandle and
+ * dom0 kernel module struct addresses.
+ */
xor %esi,%esi
xor %edi,%edi
+ xor %r14d,%r14d
/* Skip Multiboot2 information fixed part. */
lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
@@ -423,6 +427,15 @@ __efi64_mb2_start:
cmove MB2_efi64_ih(%rcx),%rdi
je .Lefi_mb2_next_tag
+ /* Get dom0 kernel module struct address from Multiboot2 information. */
+ cmpl $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
+ jne .Lefi_mb2_end
+
+ test %r14d,%r14d
+ cmovz %ecx,%r14d
+ jmp .Lefi_mb2_next_tag
+
+.Lefi_mb2_end:
/* Is it the end of Multiboot2 information? */
cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
je .Lrun_bs
@@ -484,9 +497,12 @@ __efi64_mb2_start:
/* Keep the stack aligned. Do not pop a single item off it. */
mov (%rsp),%rdi
+ mov %r14d,%edx
+
/*
* efi_multiboot2() is called according to System V AMD64 ABI:
- * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
+ * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
+ * %rdx - dom0 kernel module struct address.
*/
call efi_multiboot2
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index bedac5c..6813196 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -3,6 +3,8 @@
* is intended to be included by common/efi/boot.c _only_, and
* therefore can define arch specific global variables.
*/
+#include <xen/types.h>
+#include <xen/multiboot2.h>
#include <xen/vga.h>
#include <asm/e820.h>
#include <asm/edd.h>
@@ -47,6 +49,7 @@ extern const struct pe_base_relocs {
static void __init efi_arch_relocate_image(unsigned long delta)
{
+#if 0
const struct pe_base_relocs *base_relocs;
for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
@@ -95,6 +98,7 @@ static void __init efi_arch_relocate_image(unsigned long delta)
}
base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
}
+#endif
}
extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
@@ -669,7 +673,9 @@ static bool __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
-void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
+void __init efi_multiboot2(EFI_HANDLE ImageHandle,
+ EFI_SYSTEM_TABLE *SystemTable,
+ multiboot2_tag_module_t *dom0_kernel)
{
EFI_GRAPHICS_OUTPUT_PROTOCOL *gop;
UINTN cols, gop_mode = ~0, rows;
@@ -687,6 +693,10 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable
gop = efi_get_gop();
+ if ( dom0_kernel && dom0_kernel->mod_end > dom0_kernel->mod_start )
+ efi_shim_lock((VOID *)(unsigned long)dom0_kernel->mod_start,
+ dom0_kernel->mod_end - dom0_kernel->mod_start);
+
if ( gop )
gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 0c481e3..d0cba1d 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -1,7 +1,9 @@
+#include <xen/types.h>
#include <xen/efi.h>
#include <xen/errno.h>
#include <xen/init.h>
#include <xen/lib.h>
+#include <xen/multiboot2.h>
#include <asm/page.h>
#include <asm/efibind.h>
#include <efi/efidef.h>
@@ -18,7 +20,8 @@
*/
void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
- EFI_SYSTEM_TABLE *SystemTable)
+ EFI_SYSTEM_TABLE *SystemTable,
+ multiboot2_tag_module_t *dom0_kernel)
{
static const CHAR16 __initconst err[] =
L"Xen does not have EFI code build in!\r\nSystem halted!\r\n";
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2017-07-08 21:53 ` [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2() Daniel Kiper
@ 2018-05-04 15:46 ` Jan Beulich
2018-05-08 13:09 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-04 15:46 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -383,9 +383,13 @@ __efi64_mb2_start:
> jmp x86_32_switch
>
> .Lefi_multiboot2_proto:
> - /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> + /*
> + * Zero EFI SystemTable, EFI ImageHandle and
> + * dom0 kernel module struct addresses.
> + */
> xor %esi,%esi
> xor %edi,%edi
> + xor %r14d,%r14d
>
> /* Skip Multiboot2 information fixed part. */
> lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> @@ -423,6 +427,15 @@ __efi64_mb2_start:
> cmove MB2_efi64_ih(%rcx),%rdi
> je .Lefi_mb2_next_tag
>
> + /* Get dom0 kernel module struct address from Multiboot2 information. */
> + cmpl $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
> + jne .Lefi_mb2_end
> +
> + test %r14d,%r14d
> + cmovz %ecx,%r14d
> + jmp .Lefi_mb2_next_tag
> +
> +.Lefi_mb2_end:
> /* Is it the end of Multiboot2 information? */
> cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
> je .Lrun_bs
> @@ -484,9 +497,12 @@ __efi64_mb2_start:
> /* Keep the stack aligned. Do not pop a single item off it. */
> mov (%rsp),%rdi
>
> + mov %r14d,%edx
> +
> /*
> * efi_multiboot2() is called according to System V AMD64 ABI:
> - * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
> + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
> + * %rdx - dom0 kernel module struct address.
How come everything further up treats this as a 32-bit quantity only?
> @@ -47,6 +49,7 @@ extern const struct pe_base_relocs {
>
> static void __init efi_arch_relocate_image(unsigned long delta)
> {
> +#if 0
> const struct pe_base_relocs *base_relocs;
>
> for ( base_relocs = __base_relocs_start; base_relocs <
> __base_relocs_end; )
> @@ -95,6 +98,7 @@ static void __init efi_arch_relocate_image(unsigned long delta)
> }
> base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
> }
> +#endif
> }
???
> @@ -669,7 +673,9 @@ static bool __init
> efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>
> static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
>
> -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> +void __init efi_multiboot2(EFI_HANDLE ImageHandle,
> + EFI_SYSTEM_TABLE *SystemTable,
> + multiboot2_tag_module_t *dom0_kernel)
const?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2018-05-04 15:46 ` Jan Beulich
@ 2018-05-08 13:09 ` Daniel Kiper
2018-05-14 10:43 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-08 13:09 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Fri, May 04, 2018 at 09:46:33AM -0600, Jan Beulich wrote:
> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -383,9 +383,13 @@ __efi64_mb2_start:
> > jmp x86_32_switch
> >
> > .Lefi_multiboot2_proto:
> > - /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> > + /*
> > + * Zero EFI SystemTable, EFI ImageHandle and
> > + * dom0 kernel module struct addresses.
> > + */
> > xor %esi,%esi
> > xor %edi,%edi
> > + xor %r14d,%r14d
> >
> > /* Skip Multiboot2 information fixed part. */
> > lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> > @@ -423,6 +427,15 @@ __efi64_mb2_start:
> > cmove MB2_efi64_ih(%rcx),%rdi
> > je .Lefi_mb2_next_tag
> >
> > + /* Get dom0 kernel module struct address from Multiboot2 information. */
> > + cmpl $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
> > + jne .Lefi_mb2_end
> > +
> > + test %r14d,%r14d
> > + cmovz %ecx,%r14d
> > + jmp .Lefi_mb2_next_tag
> > +
> > +.Lefi_mb2_end:
> > /* Is it the end of Multiboot2 information? */
> > cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
> > je .Lrun_bs
> > @@ -484,9 +497,12 @@ __efi64_mb2_start:
> > /* Keep the stack aligned. Do not pop a single item off it. */
> > mov (%rsp),%rdi
> >
> > + mov %r14d,%edx
> > +
> > /*
> > * efi_multiboot2() is called according to System V AMD64 ABI:
> > - * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
> > + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
> > + * %rdx - dom0 kernel module struct address.
>
> How come everything further up treats this as a 32-bit quantity only?
According to the Multiboot2 spec the bootloader is not allowed to
put the kernel (xen.gz) and the modules above 4 GiB boundary. And
comment below __efi64_mb2_start label is clear about that.
> > @@ -47,6 +49,7 @@ extern const struct pe_base_relocs {
> >
> > static void __init efi_arch_relocate_image(unsigned long delta)
> > {
> > +#if 0
> > const struct pe_base_relocs *base_relocs;
> >
> > for ( base_relocs = __base_relocs_start; base_relocs <
> > __base_relocs_end; )
> > @@ -95,6 +98,7 @@ static void __init efi_arch_relocate_image(unsigned long delta)
> > }
> > base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
> > }
> > +#endif
> > }
>
> ???
Please forget it. This is just an RFC. It will be fixed in v2.
> > @@ -669,7 +673,9 @@ static bool __init
> > efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
> >
> > static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
> >
> > -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> > +void __init efi_multiboot2(EFI_HANDLE ImageHandle,
> > + EFI_SYSTEM_TABLE *SystemTable,
> > + multiboot2_tag_module_t *dom0_kernel)
>
> const?
Will do.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2018-05-08 13:09 ` Daniel Kiper
@ 2018-05-14 10:43 ` Jan Beulich
2018-05-14 16:56 ` Daniel Kiper
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-05-14 10:43 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 08.05.18 at 15:09, <daniel.kiper@oracle.com> wrote:
> On Fri, May 04, 2018 at 09:46:33AM -0600, Jan Beulich wrote:
>> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> > --- a/xen/arch/x86/boot/head.S
>> > +++ b/xen/arch/x86/boot/head.S
>> > @@ -383,9 +383,13 @@ __efi64_mb2_start:
>> > jmp x86_32_switch
>> >
>> > .Lefi_multiboot2_proto:
>> > - /* Zero EFI SystemTable and EFI ImageHandle addresses. */
>> > + /*
>> > + * Zero EFI SystemTable, EFI ImageHandle and
>> > + * dom0 kernel module struct addresses.
>> > + */
>> > xor %esi,%esi
>> > xor %edi,%edi
>> > + xor %r14d,%r14d
>> >
>> > /* Skip Multiboot2 information fixed part. */
>> > lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
>> > @@ -423,6 +427,15 @@ __efi64_mb2_start:
>> > cmove MB2_efi64_ih(%rcx),%rdi
>> > je .Lefi_mb2_next_tag
>> >
>> > + /* Get dom0 kernel module struct address from Multiboot2 information. */
>> > + cmpl $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
>> > + jne .Lefi_mb2_end
>> > +
>> > + test %r14d,%r14d
>> > + cmovz %ecx,%r14d
>> > + jmp .Lefi_mb2_next_tag
>> > +
>> > +.Lefi_mb2_end:
>> > /* Is it the end of Multiboot2 information? */
>> > cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
>> > je .Lrun_bs
>> > @@ -484,9 +497,12 @@ __efi64_mb2_start:
>> > /* Keep the stack aligned. Do not pop a single item off it. */
>> > mov (%rsp),%rdi
>> >
>> > + mov %r14d,%edx
>> > +
>> > /*
>> > * efi_multiboot2() is called according to System V AMD64 ABI:
>> > - * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
>> > + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
>> > + * %rdx - dom0 kernel module struct address.
>>
>> How come everything further up treats this as a 32-bit quantity only?
>
> According to the Multiboot2 spec the bootloader is not allowed to
> put the kernel (xen.gz) and the modules above 4 GiB boundary.
Interesting - how would they load a 1Gb initrd on a system with just 1Gb
RAM below 4Gb? Not to speak of a 4Gb initrd ...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2018-05-14 10:43 ` Jan Beulich
@ 2018-05-14 16:56 ` Daniel Kiper
2018-05-15 8:06 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Kiper @ 2018-05-14 16:56 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On Mon, May 14, 2018 at 04:43:13AM -0600, Jan Beulich wrote:
> >>> On 08.05.18 at 15:09, <daniel.kiper@oracle.com> wrote:
> > On Fri, May 04, 2018 at 09:46:33AM -0600, Jan Beulich wrote:
> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
> >> > --- a/xen/arch/x86/boot/head.S
> >> > +++ b/xen/arch/x86/boot/head.S
> >> > @@ -383,9 +383,13 @@ __efi64_mb2_start:
> >> > jmp x86_32_switch
> >> >
> >> > .Lefi_multiboot2_proto:
> >> > - /* Zero EFI SystemTable and EFI ImageHandle addresses. */
> >> > + /*
> >> > + * Zero EFI SystemTable, EFI ImageHandle and
> >> > + * dom0 kernel module struct addresses.
> >> > + */
> >> > xor %esi,%esi
> >> > xor %edi,%edi
> >> > + xor %r14d,%r14d
> >> >
> >> > /* Skip Multiboot2 information fixed part. */
> >> > lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> >> > @@ -423,6 +427,15 @@ __efi64_mb2_start:
> >> > cmove MB2_efi64_ih(%rcx),%rdi
> >> > je .Lefi_mb2_next_tag
> >> >
> >> > + /* Get dom0 kernel module struct address from Multiboot2 information. */
> >> > + cmpl $MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
> >> > + jne .Lefi_mb2_end
> >> > +
> >> > + test %r14d,%r14d
> >> > + cmovz %ecx,%r14d
> >> > + jmp .Lefi_mb2_next_tag
> >> > +
> >> > +.Lefi_mb2_end:
> >> > /* Is it the end of Multiboot2 information? */
> >> > cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
> >> > je .Lrun_bs
> >> > @@ -484,9 +497,12 @@ __efi64_mb2_start:
> >> > /* Keep the stack aligned. Do not pop a single item off it. */
> >> > mov (%rsp),%rdi
> >> >
> >> > + mov %r14d,%edx
> >> > +
> >> > /*
> >> > * efi_multiboot2() is called according to System V AMD64 ABI:
> >> > - * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
> >> > + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
> >> > + * %rdx - dom0 kernel module struct address.
> >>
> >> How come everything further up treats this as a 32-bit quantity only?
> >
> > According to the Multiboot2 spec the bootloader is not allowed to
> > put the kernel (xen.gz) and the modules above 4 GiB boundary.
>
> Interesting - how would they load a 1Gb initrd on a system with just 1Gb
> RAM below 4Gb? Not to speak of a 4Gb initrd ...
That is not possible right now. This requires changes in the boot protocol.
Anyway, have you seen such setups in the wild today?
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()
2018-05-14 16:56 ` Daniel Kiper
@ 2018-05-15 8:06 ` Jan Beulich
0 siblings, 0 replies; 25+ messages in thread
From: Jan Beulich @ 2018-05-15 8:06 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Andrew Cooper, xen-devel
>>> On 14.05.18 at 18:56, <daniel.kiper@oracle.com> wrote:
> On Mon, May 14, 2018 at 04:43:13AM -0600, Jan Beulich wrote:
>> >>> On 08.05.18 at 15:09, <daniel.kiper@oracle.com> wrote:
>> > On Fri, May 04, 2018 at 09:46:33AM -0600, Jan Beulich wrote:
>> >> >>> On 08.07.17 at 23:53, <daniel.kiper@oracle.com> wrote:
>> >> > @@ -484,9 +497,12 @@ __efi64_mb2_start:
>> >> > /* Keep the stack aligned. Do not pop a single item off it. */
>> >> > mov (%rsp),%rdi
>> >> >
>> >> > + mov %r14d,%edx
>> >> > +
>> >> > /*
>> >> > * efi_multiboot2() is called according to System V AMD64 ABI:
>> >> > - * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
>> >> > + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
>> >> > + * %rdx - dom0 kernel module struct address.
>> >>
>> >> How come everything further up treats this as a 32-bit quantity only?
>> >
>> > According to the Multiboot2 spec the bootloader is not allowed to
>> > put the kernel (xen.gz) and the modules above 4 GiB boundary.
>>
>> Interesting - how would they load a 1Gb initrd on a system with just 1Gb
>> RAM below 4Gb? Not to speak of a 4Gb initrd ...
>
> That is not possible right now. This requires changes in the boot protocol.
> Anyway, have you seen such setups in the wild today?
Years ago we've already had to make our XenoLinux forward port cope with
512Mb+ initrd-s - see the commit introducing XEN_ELFNOTE_MOD_START_PFN,
which tells you that Xen itself needed to be changed for this as well. Those
folks wanted to be able to boot a full fledged distro without loading anything
from disk (or network) post-boot.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 7/7] xen/x86: Build xen.mb.efi directly from xen-syms
2017-07-08 21:53 [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2() Daniel Kiper
` (5 preceding siblings ...)
2017-07-08 21:53 ` [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2() Daniel Kiper
@ 2017-07-08 21:53 ` Daniel Kiper
6 siblings, 0 replies; 25+ messages in thread
From: Daniel Kiper @ 2017-07-08 21:53 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, jbeulich
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 93ead6e..e09f5f4 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -95,6 +95,7 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
$(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \
`$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'`
+ $(OBJCOPY) -O binary -S $(TARGET)-syms $(TARGET).mb.efi
ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread