From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Vladimir 'phcoder' Serbinenko" Subject: Re: [GRUB2 PATCH v3 1/4] i386/relocator: Add grub_relocator64_efi relocator Date: Fri, 11 Mar 2016 16:42:27 +0100 Message-ID: References: <1456937500-7855-1-git-send-email-daniel.kiper@oracle.com> <1456937500-7855-2-git-send-email-daniel.kiper@oracle.com> <20160311132324.GY3500@olila.local.net-space.pl> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8338390922655332758==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1aePCv-0001Dr-Ie for xen-devel@lists.xenproject.org; Fri, 11 Mar 2016 15:42:29 +0000 Received: by mail-wm0-f68.google.com with SMTP id n205so3149302wmf.2 for ; Fri, 11 Mar 2016 07:42:27 -0800 (PST) In-Reply-To: <20160311132324.GY3500@olila.local.net-space.pl> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Daniel Kiper Cc: "jgross@suse.com" , "grub-devel@gnu.org" , "eric.snowberg@oracle.com" , "arvidjaar@gmail.com" , "andrew.cooper3@citrix.com" , "cardoe@cardoe.com" , "pgnet.dev@gmail.com" , "roy.franz@linaro.org" , "ning.sun@intel.com" , "david.vrabel@citrix.com" , "jbeulich@suse.com" , "stefano.stabellini@eu.citrix.com" , "xen-devel@lists.xenproject.org" , "qiaowei.ren@intel.com" , "richard.l.maliszewski@intel.com" , "gang.wei@intel.com" , "fu.wei@linaro.org" , "seth.goldberg@oracle.com" List-Id: xen-devel@lists.xenproject.org --===============8338390922655332758== Content-Type: multipart/alternative; boundary=001a11c28706336411052dc7c7bd --001a11c28706336411052dc7c7bd Content-Type: text/plain; charset=UTF-8 On Friday, March 11, 2016, Daniel Kiper wrote: > On Thu, Mar 10, 2016 at 09:23:23PM +0100, Vladimir 'phcoder' Serbinenko > wrote: > > On Wednesday, March 2, 2016, 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. > > OK, as I can see idea in general is accepted. Do you want > grub_relocator32_efi in 2.02 or 2.03 is OK? > Is there already a user for it? Or someone who is expected to adopt it shortly? > > > > Signed-off-by: Daniel Kiper > > > > > --- > > > v3 - suggestions/fixes: > > > - reuse grub-core/lib/i386/relocator64.S code > > > instead of creating separate assembly file > > > (suggested by Vladimir 'phcoder' Serbinenko), > > > - grub_multiboot_boot() cleanup > > > (suggested by Vladimir 'phcoder' Serbinenko), > > > - reuse multiboot_header_tag_entry_address struct instead > > > of creating new one for EFI 64-bit entry point > > > (suggested by Vladimir 'phcoder' Serbinenko). > > > --- > > > grub-core/lib/i386/relocator.c | 48 > > > ++++++++++++++++++++++++++++++++++ > > > grub-core/lib/i386/relocator64.S | 3 +++ > > > grub-core/loader/multiboot.c | 51 > > > +++++++++++++++++++++++++++++++++---- > > > grub-core/loader/multiboot_mbi2.c | 19 +++++++++++--- > > > include/grub/i386/multiboot.h | 11 ++++++++ > > > include/grub/i386/relocator.h | 21 +++++++++++++++ > > > include/multiboot2.h | 1 + > > > 7 files changed, 145 insertions(+), 9 deletions(-) > > > > > > diff --git a/grub-core/lib/i386/relocator.c > > > b/grub-core/lib/i386/relocator.c > > > index 71dd4f0..2b0c260 100644 > > > --- a/grub-core/lib/i386/relocator.c > > > +++ b/grub-core/lib/i386/relocator.c > > > @@ -69,6 +69,13 @@ 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; > > > +#endif > > > +#endif > > > + > > > > > Can we move this and all to a separate file to avoid too many #ifdef ? > > Do you think about grub-core/lib/i386/relocator-efi.c or something like > that? > If yes then I do not think it is a problem. > > Daniel > -- Regards Vladimir 'phcoder' Serbinenko --001a11c28706336411052dc7c7bd Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On Friday, March 11, 2016, Daniel Kiper <daniel.kiper@oracle.com> wrote:
On Thu, Mar 10, 2016 at 09:23:23PM +0100, Vladimir 'p= hcoder' Serbinenko wrote:
> On Wednesday, March 2, 2016, Daniel Kiper <d= aniel.kiper@oracle.com> 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 Speci= fication,
> > 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 is= sues.
> >
> > If idea is accepted I will prepare grub_relocator32_efi relocator= too.

OK, as I can see idea in general is accepted. Do you want
grub_relocator32_efi in 2.02 or 2.03 is OK?
Is there a= lready a user for it? Or someone who is expected to adopt it shortly?=C2=A0=

> > Signed-off-by: Daniel Kiper <daniel.ki= per@oracle.com <javascript:;>>
> > ---
> > v3 - suggestions/fixes:
> >=C2=A0 =C2=A0 - reuse grub-core/lib/i386/relocator64.S code
> >=C2=A0 =C2=A0 =C2=A0 instead of creating separate assembly file > >=C2=A0 =C2=A0 =C2=A0 (suggested by Vladimir 'phcoder' Serb= inenko),
> >=C2=A0 =C2=A0 - grub_multiboot_boot() cleanup
> >=C2=A0 =C2=A0 =C2=A0 (suggested by Vladimir 'phcoder' Serb= inenko),
> >=C2=A0 =C2=A0 - reuse multiboot_header_tag_entry_address struct in= stead
> >=C2=A0 =C2=A0 =C2=A0 of creating new one for EFI 64-bit entry poin= t
> >=C2=A0 =C2=A0 =C2=A0 (suggested by Vladimir 'phcoder' Serb= inenko).
> > ---
> >=C2=A0 grub-core/lib/i386/relocator.c=C2=A0 =C2=A0 |=C2=A0 =C2=A04= 8
> > ++++++++++++++++++++++++++++++++++
> >=C2=A0 grub-core/lib/i386/relocator64.S=C2=A0 |=C2=A0 =C2=A0 3 +++=
> >=C2=A0 grub-core/loader/multiboot.c=C2=A0 =C2=A0 =C2=A0 |=C2=A0 = =C2=A051
> > +++++++++++++++++++++++++++++++++----
> >=C2=A0 grub-core/loader/multiboot_mbi2.c |=C2=A0 =C2=A019 ++++++++= +++---
> >=C2=A0 include/grub/i386/multiboot.h=C2=A0 =C2=A0 =C2=A0|=C2=A0 = =C2=A011 ++++++++
> >=C2=A0 include/grub/i386/relocator.h=C2=A0 =C2=A0 =C2=A0|=C2=A0 = =C2=A021 +++++++++++++++
> >=C2=A0 include/multiboot2.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 |=C2=A0 =C2=A0 1 +
> >=C2=A0 7 files changed, 145 insertions(+), 9 deletions(-)
> >
> > diff --git a/grub-core/lib/i386/relocator.c
> > b/grub-core/lib/i386/relocator.c
> > index 71dd4f0..2b0c260 100644
> > --- a/grub-core/lib/i386/relocator.c
> > +++ b/grub-core/lib/i386/relocator.c
> > @@ -69,6 +69,13 @@ extern grub_uint64_t grub_relocator64_rsi;
> >=C2=A0 extern grub_addr_t grub_relocator64_cr3;
> >=C2=A0 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;
> > +#endif
> > +#endif
> > +
> >
> Can we move this and all to a separate file to avoid too many #ifdef ?=

Do you think about grub-core/lib/i386/relocator-efi.c or something like tha= t?
If yes then I do not think it is a problem.

Daniel


--
Regards
Vladimir 'phcoder' Serbinenk= o

--001a11c28706336411052dc7c7bd-- --===============8338390922655332758== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============8338390922655332758==--