Re: [GRUB2 PATCH v3 1/4] i386/relocator: Add grub_relocator64_efi relocator

From: Daniel Kiper <daniel.kiper@oracle.com>
To: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
Cc: "jgross@suse.com" <jgross@suse.com>,
	"grub-devel@gnu.org" <grub-devel@gnu.org>,
	"eric.snowberg@oracle.com" <eric.snowberg@oracle.com>,
	"arvidjaar@gmail.com" <arvidjaar@gmail.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"cardoe@cardoe.com" <cardoe@cardoe.com>,
	"pgnet.dev@gmail.com" <pgnet.dev@gmail.com>,
	"roy.franz@linaro.org" <roy.franz@linaro.org>,
	"ning.sun@intel.com" <ning.sun@intel.com>,
	"david.vrabel@citrix.com" <david.vrabel@citrix.com>,
	"jbeulich@suse.com" <jbeulich@suse.com>,
	"stefano.stabellini@eu.citrix.com"
	<stefano.stabellini@eu.citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"qiaowei.ren@intel.com" <qiaowei.ren@intel.com>,
	"richard.l.maliszewski@intel.com"
	<richard.l.maliszewski@intel.com>,
	"gang.wei@intel.com" <gang.wei@intel.com>,
	"fu.wei@linaro.org" <fu.wei@linaro.org>,
	"seth.goldberg@oracle.com" <seth>
Subject: Re: [GRUB2 PATCH v3 1/4] i386/relocator: Add grub_relocator64_efi relocator
Date: Fri, 11 Mar 2016 14:23:24 +0100	[thread overview]
Message-ID: <20160311132324.GY3500@olila.local.net-space.pl> (raw)
In-Reply-To: <CAEaD8JOUj_dF5EwpCzYt8a3T76T3wOzfVCTOy=WLhvR5XqqyAw@mail.gmail.com>

On Thu, Mar 10, 2016 at 09:23:23PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> On Wednesday, March 2, 2016, Daniel Kiper <daniel.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 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?

> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com <javascript:;>>
> > ---
> > 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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel