|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/6] i386/relocator: Add grub_relocator64_efi relocator
On Mon, Nov 09, 2015 at 09:05:15PM +0100, Vladimir 'Ï-coder/phcoder' Serbinenko
wrote:
> On 20.07.2015 16:35, Daniel Kiper wrote:
> > Add grub_relocator64_efi relocator. It will be used on EFI 64-bit platforms
> > when multiboot2 compatible image requests MULTIBOOT_TAG_TYPE_EFI_BS.
> > Relocator
> > will set lower parts of %rax and %rbx accordingly to multiboot2
> > specification.
> > On the other hand processor mode, just before jumping into loaded image,
> > will
> > be set accordingly to Unified Extensible Firmware Interface Specification,
> > Version 2.4 Errata B, section 2.3.4, x64 Platforms, boot services. This way
> > loaded image will be able to use EFI boot services without any issues.
> >
> > If idea is accepted I will prepare grub_relocator32_efi relocator too.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> > ---
> > grub-core/Makefile.core.def | 1 +
> > grub-core/lib/i386/relocator.c | 53 +++++++++++++++++++++++
> > grub-core/lib/i386/relocator64_efi.S | 77
> > ++++++++++++++++++++++++++++++++++
> > grub-core/loader/multiboot.c | 29 +++++++++++--
> > grub-core/loader/multiboot_mbi2.c | 19 +++++++--
> > include/grub/i386/multiboot.h | 11 +++++
> > include/grub/i386/relocator.h | 21 ++++++++++
> > include/multiboot2.h | 9 ++++
> > 8 files changed, 213 insertions(+), 7 deletions(-)
> > create mode 100644 grub-core/lib/i386/relocator64_efi.S
> >
> > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > index a6101de..d583549 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -1519,6 +1519,7 @@ module = {
> > x86 = lib/i386/relocator_common_c.c;
> > ieee1275 = lib/ieee1275/relocator.c;
> > efi = lib/efi/relocator.c;
> > + x86_64_efi = lib/i386/relocator64_efi.S;
> > mips = lib/mips/relocator_asm.S;
> > mips = lib/mips/relocator.c;
> > powerpc = lib/powerpc/relocator_asm.S;
> > diff --git a/grub-core/lib/i386/relocator.c b/grub-core/lib/i386/relocator.c
> > index 71dd4f0..459027e 100644
> > --- a/grub-core/lib/i386/relocator.c
> > +++ b/grub-core/lib/i386/relocator.c
> > @@ -69,6 +69,19 @@ extern grub_uint64_t grub_relocator64_rsi;
> > extern grub_addr_t grub_relocator64_cr3;
> > extern struct grub_i386_idt grub_relocator16_idt;
> >
> > +#ifdef GRUB_MACHINE_EFI
> > +#ifdef __x86_64__
> > +extern grub_uint8_t grub_relocator64_efi_start;
> > +extern grub_uint8_t grub_relocator64_efi_end;
> > +extern grub_uint64_t grub_relocator64_efi_rax;
> > +extern grub_uint64_t grub_relocator64_efi_rbx;
> > +extern grub_uint64_t grub_relocator64_efi_rcx;
> > +extern grub_uint64_t grub_relocator64_efi_rdx;
> > +extern grub_uint64_t grub_relocator64_efi_rip;
> > +extern grub_uint64_t grub_relocator64_efi_rsi;
> > +#endif
> > +#endif
> > +
> > #define RELOCATOR_SIZEOF(x) (&grub_relocator##x##_end -
> > &grub_relocator##x##_start)
> >
> > grub_err_t
> > @@ -214,3 +227,43 @@ grub_relocator64_boot (struct grub_relocator *rel,
> > /* Not reached. */
> > return GRUB_ERR_NONE;
> > }
> > +
> > +#ifdef GRUB_MACHINE_EFI
> > +#ifdef __x86_64__
> > +grub_err_t
> > +grub_relocator64_efi_boot (struct grub_relocator *rel,
> > + struct grub_relocator64_efi_state state)
> > +{
> > + grub_err_t err;
> > + void *relst;
> > + grub_relocator_chunk_t ch;
> > +
> > + err = grub_relocator_alloc_chunk_align (rel, &ch, 0,
> > + 0x40000000 - RELOCATOR_SIZEOF
> > (64_efi),
> > + RELOCATOR_SIZEOF (64_efi), 16,
> > + GRUB_RELOCATOR_PREFERENCE_NONE, 1);
> > + if (err)
> > + return err;
> > +
> > + grub_relocator64_efi_rax = state.rax;
> > + grub_relocator64_efi_rbx = state.rbx;
> > + grub_relocator64_efi_rcx = state.rcx;
> > + grub_relocator64_efi_rdx = state.rdx;
> > + grub_relocator64_efi_rip = state.rip;
> > + grub_relocator64_efi_rsi = state.rsi;
> > +
> > + grub_memmove (get_virtual_current_address (ch),
> > &grub_relocator64_efi_start,
> > + RELOCATOR_SIZEOF (64_efi));
> > +
> > + err = grub_relocator_prepare_relocs (rel, get_physical_target_address
> > (ch),
> > + &relst, NULL);
> > + if (err)
> > + return err;
> > +
> > + ((void (*) (void)) relst) ();
> > +
> > + /* Not reached. */
> > + return GRUB_ERR_NONE;
> > +}
> > +#endif
> > +#endif
> > diff --git a/grub-core/lib/i386/relocator64_efi.S
> > b/grub-core/lib/i386/relocator64_efi.S
> > new file mode 100644
> > index 0000000..fcd1964
> > --- /dev/null
> > +++ b/grub-core/lib/i386/relocator64_efi.S
> > @@ -0,0 +1,77 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2009,2010 Free Software Foundation, Inc.
> > + * Copyright (C) 2014,2015 Oracle Co.
> > + * Author: Daniel Kiper
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "relocator_common.S"
> > +
> > + .p2align 4 /* force 16-byte alignment */
> > +
> > +VARIABLE(grub_relocator64_efi_start)
> > + PREAMBLE
> > +
> > + .code64
> > +
> > + /* mov imm64, %rax */
> > + .byte 0x48
> > + .byte 0xb8
> > +VARIABLE(grub_relocator64_efi_rsi)
> > + .quad 0
> > +
> > + movq %rax, %rsi
> > +
> > + /* mov imm64, %rax */
> > + .byte 0x48
> > + .byte 0xb8
> > +VARIABLE(grub_relocator64_efi_rax)
> > + .quad 0
> > +
> > + /* mov imm64, %rbx */
> > + .byte 0x48
> > + .byte 0xbb
> > +VARIABLE(grub_relocator64_efi_rbx)
> > + .quad 0
> > +
> > + /* mov imm64, %rcx */
> > + .byte 0x48
> > + .byte 0xb9
> > +VARIABLE(grub_relocator64_efi_rcx)
> > + .quad 0
> > +
> > + /* mov imm64, %rdx */
> > + .byte 0x48
> > + .byte 0xba
> > +VARIABLE(grub_relocator64_efi_rdx)
> > + .quad 0
> > +
> > + /* Cleared direction flag is of no problem with any current
> > + payload and makes this implementation easier. */
> > + cld
> > +
> > +#ifdef __APPLE__
> > + .byte 0xff, 0x25
> > + .quad 0
> > +#else
> > + jmp *LOCAL(jump_addr) (%rip)
> > +#endif
> > +
> > +LOCAL(jump_addr):
> > +VARIABLE(grub_relocator64_efi_rip)
> > + .quad 0
> > +
> This repeats relocator64 almost exactly. Can we avoid code duplication?
> It's fine to compile it twice but code duplication is bad.
I will try to do that.
> > +VARIABLE(grub_relocator64_efi_end)
> > diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
> > index fd8f28e..ca7154f 100644
> > --- a/grub-core/loader/multiboot.c
> > +++ b/grub-core/loader/multiboot.c
> > @@ -118,21 +118,44 @@ grub_multiboot_set_video_mode (void)
> > return err;
> > }
> >
> > +#ifdef GRUB_MACHINE_EFI
> > +#ifdef __x86_64__
> > +#define grub_relocator_efi_boot grub_relocator64_efi_boot
> > +#endif
> > +#endif
> > +
> > static grub_err_t
> > grub_multiboot_boot (void)
> > {
> > grub_err_t err;
> > + grub_uint32_t target;
> > struct grub_relocator32_state state = MULTIBOOT_INITIAL_STATE;
> > +#ifdef GRUB_MACHINE_EFI
> > +#ifdef __x86_64__
> > + struct grub_relocator64_efi_state state_efi =
> > MULTIBOOT_EFI_INITIAL_STATE;
> > +#endif
> > +#endif
> >
> > - state.MULTIBOOT_ENTRY_REGISTER = grub_multiboot_payload_eip;
> > -
> > - err = grub_multiboot_make_mbi (&state.MULTIBOOT_MBI_REGISTER);
> > + err = grub_multiboot_make_mbi (&target);
> >
> > if (err)
> > return err;
> >
> > + state.MULTIBOOT_ENTRY_REGISTER = grub_multiboot_payload_eip;
> > + state.MULTIBOOT_MBI_REGISTER = target;
> > +
> > #if defined (__i386__) || defined (__x86_64__)
> > +#ifdef GRUB_MACHINE_EFI
> > + state_efi.MULTIBOOT_EFI_ENTRY_REGISTER = grub_multiboot_payload_eip;
> > + state_efi.MULTIBOOT_EFI_MBI_REGISTER = target;
> > +
> > + if (grub_efi_is_finished)
> > + grub_relocator32_boot (grub_multiboot_relocator, state, 0);
> > + else
> > + grub_relocator_efi_boot (grub_multiboot_relocator, state_efi);
> > +#else
> > grub_relocator32_boot (grub_multiboot_relocator, state, 0);
> > +#endif
> > #else
> > grub_relocator32_boot (grub_multiboot_relocator, state);
> > #endif
> This becomes hairy. I think it's time to split it into platform-specific
> functions and/or use tricks like
> #ifndef GRUB_MACHINE_EFI
> #define grub_efi_is_finished 0
> #endif
OK.
> > diff --git a/grub-core/loader/multiboot_mbi2.c
> > b/grub-core/loader/multiboot_mbi2.c
> > index d7c19bc..8d66e3f 100644
> > --- a/grub-core/loader/multiboot_mbi2.c
> > +++ b/grub-core/loader/multiboot_mbi2.c
> > @@ -107,8 +107,8 @@ grub_multiboot_load (grub_file_t file, const char
> > *filename)
> > grub_err_t err;
> > struct multiboot_header_tag *tag;
> > struct multiboot_header_tag_address *addr_tag = NULL;
> > - int entry_specified = 0;
> > - grub_addr_t entry = 0;
> > + int entry_specified = 0, efi_entry_specified = 0;
> > + grub_addr_t entry = 0, efi_entry = 0;
> > grub_uint32_t console_required = 0;
> > struct multiboot_header_tag_framebuffer *fbtag = NULL;
> > int accepted_consoles = GRUB_MULTIBOOT_CONSOLE_EGA_TEXT;
> > @@ -192,6 +192,13 @@ grub_multiboot_load (grub_file_t file, const char
> > *filename)
> > entry = ((struct multiboot_header_tag_entry_address *) tag)->entry_addr;
> > break;
> >
> > + case MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64:
> > +#if defined (GRUB_MACHINE_EFI) && defined (__x86_64__)
> > + efi_entry_specified = 1;
> > + efi_entry = ((struct multiboot_header_tag_entry_address_efi64 *)
> > tag)->entry_addr;
> > +#endif
> > + break;
> > +
> Why do we need separate handling of EFI entry point? Why can't we use
> the same structure?
Make sense.
> > case MULTIBOOT_HEADER_TAG_CONSOLE_FLAGS:
> > if (!(((struct multiboot_header_tag_console_flags *) tag)->console_flags
> > & MULTIBOOT_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED))
> > @@ -211,7 +218,9 @@ grub_multiboot_load (grub_file_t file, const char
> > *filename)
> > break;
> >
> > case MULTIBOOT_HEADER_TAG_EFI_BS:
> > +#ifdef GRUB_MACHINE_EFI
> > keep_bs = 1;
> > +#endif
> > break;
> >
> > default:
> > @@ -224,7 +233,7 @@ grub_multiboot_load (grub_file_t file, const char
> > *filename)
> > break;
> > }
> >
> > - if (addr_tag && !entry_specified)
> > + if (addr_tag && !entry_specified && !(keep_bs && efi_entry_specified))
> > {
> > grub_free (buffer);
> > return grub_error (GRUB_ERR_UNKNOWN_OS,
> > @@ -287,7 +296,9 @@ grub_multiboot_load (grub_file_t file, const char
> > *filename)
> > }
> > }
> >
> > - if (entry_specified)
> > + if (keep_bs && efi_entry_specified)
> > + grub_multiboot_payload_eip = efi_entry;
> > + else if (entry_specified)
> > grub_multiboot_payload_eip = entry;
> >
> This seems redundant.
What is wrong here?
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |