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

["Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 11340 bytes --]

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.
>
> 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 ?


>  #define RELOCATOR_SIZEOF(x)    (&grub_relocator##x##_end -
> &grub_relocator##x##_start)
>
>  grub_err_t
> @@ -214,3 +221,44 @@ 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;
> +
> +  /* Do not touch %rsp! It points to EFI created stack. */
> +  grub_relocator64_rax = state.rax;
> +  grub_relocator64_rbx = state.rbx;
> +  grub_relocator64_rcx = state.rcx;
> +  grub_relocator64_rdx = state.rdx;
> +  grub_relocator64_rip = state.rip;
> +  grub_relocator64_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.S
> b/grub-core/lib/i386/relocator64.S
> index e4648d8..7a06b16 100644
> --- a/grub-core/lib/i386/relocator64.S
> +++ b/grub-core/lib/i386/relocator64.S
> @@ -73,6 +73,7 @@ VARIABLE(grub_relocator64_rsp)
>
>         movq    %rax, %rsp
>
> +VARIABLE(grub_relocator64_efi_start)
>
Very nice code reuse. Can you add a comment that whoever modifies this file
needs to take care to preserve this function-in function structure
correctly.

>         /* mov imm64, %rax */
>         .byte   0x48
>         .byte   0xb8
> @@ -120,6 +121,8 @@ LOCAL(jump_addr):
>  VARIABLE(grub_relocator64_rip)
>         .quad   0
>
> +VARIABLE(grub_relocator64_efi_end)
> +
>  #ifndef __x86_64__
>         .p2align        4
>  LOCAL(gdt):
> diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
> index 73aa0aa..18038fd 100644
> --- a/grub-core/loader/multiboot.c
> +++ b/grub-core/loader/multiboot.c
> @@ -118,6 +118,48 @@ grub_multiboot_set_video_mode (void)
>    return err;
>  }
>
> +#ifdef GRUB_MACHINE_EFI
> +#ifdef __x86_64__
> +#define grub_relocator_efi_boot                grub_relocator64_efi_boot
> +#define grub_relocator_efi_state       grub_relocator64_efi_state
> +#endif
> +#endif
> +
> +#ifdef grub_relocator_efi_boot
> +static void
> +efi_boot (struct grub_relocator *rel,
> +         grub_uint32_t target)
> +{
> +  struct grub_relocator_efi_state state_efi = MULTIBOOT_EFI_INITIAL_STATE;
> +
> +  state_efi.MULTIBOOT_EFI_ENTRY_REGISTER = grub_multiboot_payload_eip;
> +  state_efi.MULTIBOOT_EFI_MBI_REGISTER = target;
> +
> +  grub_relocator_efi_boot (rel, state_efi);
> +}
> +#else
> +#define grub_efi_is_finished   1
> +static void
> +efi_boot (struct grub_relocator *rel __attribute__ ((unused)),
> +         grub_uint32_t target __attribute__ ((unused)))
> +{
> +}
> +#endif
> +
> +#if defined (__i386__) || defined (__x86_64__)
> +static void
> +normal_boot (struct grub_relocator *rel, struct grub_relocator32_state
> state)
> +{
> +  grub_relocator32_boot (rel, state, 0);
> +}
> +#else
> +static void
> +normal_boot (struct grub_relocator *rel, struct grub_relocator32_state
> state)
> +{
> +  grub_relocator32_boot (rel, state);
> +}
> +#endif
> +
>  static grub_err_t
>  grub_multiboot_boot (void)
>  {
> @@ -131,11 +173,10 @@ grub_multiboot_boot (void)
>    if (err)
>      return err;
>
> -#if defined (__i386__) || defined (__x86_64__)
> -  grub_relocator32_boot (grub_multiboot_relocator, state, 0);
> -#else
> -  grub_relocator32_boot (grub_multiboot_relocator, state);
> -#endif
> +  if (grub_efi_is_finished)
> +    normal_boot (grub_multiboot_relocator, state);
> +  else
> +    efi_boot (grub_multiboot_relocator, state.MULTIBOOT_MBI_REGISTER);
>
>    /* Not reached.  */
>    return GRUB_ERR_NONE;
> diff --git a/grub-core/loader/multiboot_mbi2.c
> b/grub-core/loader/multiboot_mbi2.c
> index f147d67..a3dca90 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 *)
> tag)->entry_addr;
> +#endif
> +       break;
> +
>        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;
>
>    if (fbtag)
> diff --git a/include/grub/i386/multiboot.h b/include/grub/i386/multiboot.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
>
> +#ifdef GRUB_MACHINE_EFI
> +#ifdef __x86_64__
> +#define MULTIBOOT_EFI_INITIAL_STATE  { .rax =
> MULTIBOOT_BOOTLOADER_MAGIC,      \
> +    .rcx = 0,
>       \
> +    .rdx = 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
>
> diff --git a/include/grub/i386/relocator.h b/include/grub/i386/relocator.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;
>  };
>
> +#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);
>
> @@ -76,4 +90,11 @@ grub_err_t grub_relocator64_boot (struct grub_relocator
> *rel,
>                                   struct grub_relocator64_state state,
>                                   grub_addr_t min_addr, grub_addr_t
> max_addr);
>
> +#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..d96aa40 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
>
>  #define MULTIBOOT_ARCHITECTURE_I386  0
>  #define MULTIBOOT_ARCHITECTURE_MIPS32  4
> --
> 1.7.10.4
>
>

-- 
Regards
Vladimir 'phcoder' Serbinenko

[-- Attachment #1.2: Type: text/html, Size: 13609 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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