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

[Vladimir 'phcoder' Serbinenko]

On Friday, March 11, 2016, Daniel Kiper <address@hidden> wrote:
> On Thu, Mar 10, 2016 at 09:23:23PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > On Wednesday, March 2, 2016, Daniel Kiper <address@hidden> 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 <address@hidden <_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


-- 
Regards
Vladimir 'phcoder' Serbinenko