From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Zvsh1-00029d-IA for mharc-grub-devel@gnu.org; Mon, 09 Nov 2015 15:05:31 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvsgx-000230-Qb for grub-devel@gnu.org; Mon, 09 Nov 2015 15:05:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zvsgv-0002C5-Lm for grub-devel@gnu.org; Mon, 09 Nov 2015 15:05:27 -0500 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:33544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zvsgv-0002C1-Bc for grub-devel@gnu.org; Mon, 09 Nov 2015 15:05:25 -0500 Received: by wmec201 with SMTP id c201so99332661wme.0 for ; Mon, 09 Nov 2015 12:05:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-type; bh=fkFF9YHhSgAVO/kdARFoPqp2LBZUnN3kQSUiuF66JUI=; b=LY4I0kjTZ7xd7cNo6ja4EMtES330Z6U7IJ5cTchNhjN+MsZmJdua/TpYsBqhzj93aN 3NrnoFF/0GQJ/6XYVLnjlkl912i/wQqVBt48j0FHPRDp35eV8uQZ9TJlpDrXfmqrO2+s BwVWKGvf+oPUYVedOsP/WHyCHqbvM390/viLL/bO66/T7Csj1hhSbreduUsEFcywJ0GT vTHlkVxuDZwDgENQCsaNWD/cPyCFk7gt9z9oErna2ku9oMn1oHui/yQhRKnd2NiM+gXu BAWXu/GXcqDGy02uC6jH3gYk2gU3cfndlMr+GHpbAuu7xEEsO2u8eb2zbeZeSwh+37Q4 y1ng== X-Received: by 10.28.16.203 with SMTP id 194mr310810wmq.55.1447099524741; Mon, 09 Nov 2015 12:05:24 -0800 (PST) Received: from ?IPv6:2a02:1205:34c8:dc00:863a:4bff:fe50:abc4? ([2a02:1205:34c8:dc00:863a:4bff:fe50:abc4]) by smtp.gmail.com with ESMTPSA id t194sm341042wmt.11.2015.11.09.12.05.22 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Nov 2015 12:05:23 -0800 (PST) Subject: Re: [PATCH v2 3/6] i386/relocator: Add grub_relocator64_efi relocator To: Daniel Kiper , xen-devel@lists.xenproject.org, grub-devel@gnu.org References: <1437402954-7375-1-git-send-email-daniel.kiper@oracle.com> <1437402954-7375-4-git-send-email-daniel.kiper@oracle.com> From: =?UTF-8?Q?Vladimir_'=cf=86-coder/phcoder'_Serbinenko?= X-Enigmail-Draft-Status: N1110 Message-ID: <5640FC7B.9080206@gmail.com> Date: Mon, 9 Nov 2015 21:05:15 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.3.0 MIME-Version: 1.0 In-Reply-To: <1437402954-7375-4-git-send-email-daniel.kiper@oracle.com> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Ox4FeQiX1Ixjh4bWGAvJVMubriSukeAaL" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c09::22d Cc: jgross@suse.com, keir@xen.org, ian.campbell@citrix.com, andrew.cooper3@citrix.com, stefano.stabellini@eu.citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, jbeulich@suse.com, wei.liu2@citrix.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Nov 2015 20:05:30 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ox4FeQiX1Ixjh4bWGAvJVMubriSukeAaL Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 20.07.2015 16:35, Daniel Kiper wrote: > Add grub_relocator64_efi relocator. It will be used on EFI 64-bit platf= orms > when multiboot2 compatible image requests MULTIBOOT_TAG_TYPE_EFI_BS. Re= locator > will set lower parts of %rax and %rbx accordingly to multiboot2 specifi= cation. > On the other hand processor mode, just before jumping into loaded image= , will > be set accordingly to Unified Extensible Firmware Interface Specificati= on, > 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. >=20 > If idea is accepted I will prepare grub_relocator32_efi relocator too. >=20 > Signed-off-by: Daniel Kiper > --- > 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 >=20 > 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 =3D { > x86 =3D lib/i386/relocator_common_c.c; > ieee1275 =3D lib/ieee1275/relocator.c; > efi =3D lib/efi/relocator.c; > + x86_64_efi =3D lib/i386/relocator64_efi.S; > mips =3D lib/mips/relocator_asm.S; > mips =3D lib/mips/relocator.c; > powerpc =3D lib/powerpc/relocator_asm.S; > diff --git a/grub-core/lib/i386/relocator.c b/grub-core/lib/i386/reloca= tor.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; > =20 > +#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_relocato= r##x##_start) > =20 > 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 =3D 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 =3D state.rax; > + grub_relocator64_efi_rbx =3D state.rbx; > + grub_relocator64_efi_rcx =3D state.rcx; > + grub_relocator64_efi_rdx =3D state.rdx; > + grub_relocator64_efi_rip =3D state.rip; > + grub_relocator64_efi_rsi =3D state.rsi; > + > + grub_memmove (get_virtual_current_address (ch), &grub_relocator64_ef= i_start, > + RELOCATOR_SIZEOF (64_efi)); > + > + err =3D grub_relocator_prepare_relocs (rel, get_physical_target_addr= ess (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 . > + */ > + > +#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. > +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; > } > =20 > +#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 =3D MULTIBOOT_INITIAL_STATE; > +#ifdef GRUB_MACHINE_EFI > +#ifdef __x86_64__ > + struct grub_relocator64_efi_state state_efi =3D MULTIBOOT_EFI_INITIA= L_STATE; > +#endif > +#endif > =20 > - state.MULTIBOOT_ENTRY_REGISTER =3D grub_multiboot_payload_eip; > - > - err =3D grub_multiboot_make_mbi (&state.MULTIBOOT_MBI_REGISTER); > + err =3D grub_multiboot_make_mbi (&target); > =20 > if (err) > return err; > =20 > + state.MULTIBOOT_ENTRY_REGISTER =3D grub_multiboot_payload_eip; > + state.MULTIBOOT_MBI_REGISTER =3D target; > + > #if defined (__i386__) || defined (__x86_64__) > +#ifdef GRUB_MACHINE_EFI > + state_efi.MULTIBOOT_EFI_ENTRY_REGISTER =3D grub_multiboot_payload_ei= p; > + state_efi.MULTIBOOT_EFI_MBI_REGISTER =3D 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 > diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multi= boot_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 =3D NULL; > - int entry_specified =3D 0; > - grub_addr_t entry =3D 0; > + int entry_specified =3D 0, efi_entry_specified =3D 0; > + grub_addr_t entry =3D 0, efi_entry =3D 0; > grub_uint32_t console_required =3D 0; > struct multiboot_header_tag_framebuffer *fbtag =3D NULL; > int accepted_consoles =3D GRUB_MULTIBOOT_CONSOLE_EGA_TEXT; > @@ -192,6 +192,13 @@ grub_multiboot_load (grub_file_t file, const char = *filename) > entry =3D ((struct multiboot_header_tag_entry_address *) tag)->entry_= addr; > break; > =20 > + case MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64: > +#if defined (GRUB_MACHINE_EFI) && defined (__x86_64__) > + efi_entry_specified =3D 1; > + efi_entry =3D ((struct multiboot_header_tag_entry_address_efi64 *) ta= g)->entry_addr; > +#endif > + break; > + Why do we need separate handling of EFI entry point? Why can't we use the same structure? > case MULTIBOOT_HEADER_TAG_CONSOLE_FLAGS: > if (!(((struct multiboot_header_tag_console_flags *) tag)->console_fl= ags > & MULTIBOOT_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED)) > @@ -211,7 +218,9 @@ grub_multiboot_load (grub_file_t file, const char *= filename) > break; > =20 > case MULTIBOOT_HEADER_TAG_EFI_BS: > +#ifdef GRUB_MACHINE_EFI > keep_bs =3D 1; > +#endif > break; > =20 > default: > @@ -224,7 +233,7 @@ grub_multiboot_load (grub_file_t file, const char *= filename) > break; > } > =20 > - 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) > } > } > =20 > - if (entry_specified) > + if (keep_bs && efi_entry_specified) > + grub_multiboot_payload_eip =3D efi_entry; > + else if (entry_specified) > grub_multiboot_payload_eip =3D entry; > =20 This seems redundant. > if (fbtag) > diff --git a/include/grub/i386/multiboot.h b/include/grub/i386/multiboo= t.h > index a1b9488..807a1de 100644 > --- a/include/grub/i386/multiboot.h > +++ b/include/grub/i386/multiboot.h > @@ -30,6 +30,17 @@ > #define MULTIBOOT_MBI_REGISTER ebx > #define MULTIBOOT_ARCHITECTURE_CURRENT MULTIBOOT_ARCHITECTURE_I386 > =20 > +#ifdef GRUB_MACHINE_EFI > +#ifdef __x86_64__ > +#define MULTIBOOT_EFI_INITIAL_STATE { .rax =3D MULTIBOOT_BOOTLOADER_M= AGIC, \ > + .rcx =3D 0, \ > + .rdx =3D 0, \ > + } > +#define MULTIBOOT_EFI_ENTRY_REGISTER rip > +#define MULTIBOOT_EFI_MBI_REGISTER rbx > +#endif > +#endif > + > #define MULTIBOOT_ELF32_MACHINE EM_386 > #define MULTIBOOT_ELF64_MACHINE EM_X86_64 > =20 > diff --git a/include/grub/i386/relocator.h b/include/grub/i386/relocato= r.h > index 5f89a7e..2a56c3b 100644 > --- a/include/grub/i386/relocator.h > +++ b/include/grub/i386/relocator.h > @@ -65,6 +65,20 @@ struct grub_relocator64_state > grub_addr_t cr3; > }; > =20 > +#ifdef GRUB_MACHINE_EFI > +#ifdef __x86_64__ > +struct grub_relocator64_efi_state > +{ > + grub_uint64_t rax; > + grub_uint64_t rbx; > + grub_uint64_t rcx; > + grub_uint64_t rdx; > + grub_uint64_t rip; > + grub_uint64_t rsi; > +}; > +#endif > +#endif > + > grub_err_t grub_relocator16_boot (struct grub_relocator *rel, > struct grub_relocator16_state state); > =20 > @@ -76,4 +90,11 @@ grub_err_t grub_relocator64_boot (struct grub_reloca= tor *rel, > struct grub_relocator64_state state, > grub_addr_t min_addr, grub_addr_t max_addr); > =20 > +#ifdef GRUB_MACHINE_EFI > +#ifdef __x86_64__ > +grub_err_t grub_relocator64_efi_boot (struct grub_relocator *rel, > + struct grub_relocator64_efi_state state); > +#endif > +#endif > + > #endif /* ! GRUB_RELOCATOR_CPU_HEADER */ > diff --git a/include/multiboot2.h b/include/multiboot2.h > index 9d48627..b3977e3 100644 > --- a/include/multiboot2.h > +++ b/include/multiboot2.h > @@ -69,6 +69,7 @@ > #define MULTIBOOT_HEADER_TAG_FRAMEBUFFER 5 > #define MULTIBOOT_HEADER_TAG_MODULE_ALIGN 6 > #define MULTIBOOT_HEADER_TAG_EFI_BS 7 > +#define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64 9 > =20 > #define MULTIBOOT_ARCHITECTURE_I386 0 > #define MULTIBOOT_ARCHITECTURE_MIPS32 4 > @@ -133,6 +134,14 @@ struct multiboot_header_tag_entry_address > multiboot_uint32_t entry_addr; > }; > =20 > +struct multiboot_header_tag_entry_address_efi64 > +{ > + multiboot_uint16_t type; > + multiboot_uint16_t flags; > + multiboot_uint32_t size; > + multiboot_uint32_t entry_addr; > +}; > + Ditto > struct multiboot_header_tag_console_flags > { > multiboot_uint16_t type; >=20 --Ox4FeQiX1Ixjh4bWGAvJVMubriSukeAaL Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAlZA/IIACgkQmBXlbbo5nOtoeQEAleDiAU5zMsMJ90z3Lj65hiIL 36NMJGIlnPuxfG5SUjEBAIYJuvb65NOtbz9aDTDohoyXl6pRT2/UcDN7cXByHrdN =6vax -----END PGP SIGNATURE----- --Ox4FeQiX1Ixjh4bWGAvJVMubriSukeAaL--