| Message ID | 35ad940a3da56fc39c9f24e15c9f09ef74ad3448.1611273359.git.bobbyeshleman@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support Secure Boot for multiboot2 Xen | expand |
On Thu, Jan 21, 2021 at 04:51:43PM -0800, Bobby Eshleman wrote: > From: Daniel Kiper <daniel.kiper@oracle.com> > > In comparison to ELF the PE format is not supported by the Multiboot2 > protocol. So, if we wish to load xen.mb.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 to make the header more readable. > > The Multiboot2 protocol spec can be found at > https://www.gnu.org/software/grub/manual/multiboot2/ > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > Signed-off-by: Bobby Eshleman <bobbyeshleman@gmail.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 189d91a872..f2edd182a5 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -94,6 +94,13 @@ multiboot2_header: > /* 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), /* header_addr */ \ > + sym_offs(start), /* load_addr */ \ > + sym_offs(__bss_start), /* load_end_addr */ \ > + sym_offs(__2M_rwdata_end) /* bss_end_addr */ Shouldn't this only be present when a PE binary is built? You seem to unconditionally add this to the header, even when the resulting binary will be in ELF format? According to the spec: "This information does not need to be provided if the kernel image is in ELF format", and hence Xen shouldn't require the loader to understand this tag unless it's strictly required, as the presence of the tag forces the bootloader to use the presented information in order to load the kernel, regardless of the underlying binary format. Thanks, Roger.
On 2/23/21 1:04 AM, Roger Pau Monné wrote: > On Thu, Jan 21, 2021 at 04:51:43PM -0800, Bobby Eshleman wrote: >> From: Daniel Kiper <daniel.kiper@oracle.com> >> >> In comparison to ELF the PE format is not supported by the Multiboot2 >> protocol. So, if we wish to load xen.mb.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 to make the header more readable. >> >> The Multiboot2 protocol spec can be found at >> https://www.gnu.org/software/grub/manual/multiboot2/ >> >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> >> Signed-off-by: Bobby Eshleman <bobbyeshleman@gmail.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 189d91a872..f2edd182a5 100644 >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -94,6 +94,13 @@ multiboot2_header: >> /* 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), /* header_addr */ \ >> + sym_offs(start), /* load_addr */ \ >> + sym_offs(__bss_start), /* load_end_addr */ \ >> + sym_offs(__2M_rwdata_end) /* bss_end_addr */ > > Shouldn't this only be present when a PE binary is built? > > You seem to unconditionally add this to the header, even when the > resulting binary will be in ELF format? > > According to the spec: "This information does not need to be provided > if the kernel image is in ELF format", and hence Xen shouldn't require > the loader to understand this tag unless it's strictly required, as > the presence of the tag forces the bootloader to use the presented > information in order to load the kernel, regardless of the underlying > binary format. > > Thanks, Roger. > Ah yes, this is true. It may have made more sense to do this with v2 trying to step us in the direction of a single unified binary, but it certainly isn't required with v3. Thanks, Bob
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 189d91a872..f2edd182a5 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -94,6 +94,13 @@ multiboot2_header: /* 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), /* 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. */ \ @@ -101,6 +108,14 @@ multiboot2_header: 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 @@ -114,10 +129,6 @@ multiboot2_header: /* 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: