diff mbox series

[v3,3/4] x86/boot: Rewrite EFI/MBI2 code partly in C

Message ID 20240924102811.86884-4-frediano.ziglio@cloud.com (mailing list archive)
State Superseded
Headers show
Series x86/boot: Reduce assembly code | expand

Commit Message

Frediano Ziglio Sept. 24, 2024, 10:28 a.m. UTC
No need to have it coded in assembly.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
Changes since v1:
- update some comments;
- explain why %ebx is saved before calling efi_parse_mbi2;
- move lea before test instruction;
- removed asmlinkage from efi_multiboot2 and add to efi_parse_mbi2;
- fix line length;
- update an error message specifying "Multiboot2" instead of "Multiboot";
- use obj-bin-X instead of obj-X in Makefile;
- avoid restoring %eax (MBI magic).
---
 xen/arch/x86/boot/head.S      | 136 +++++++---------------------------
 xen/arch/x86/efi/Makefile     |   1 +
 xen/arch/x86/efi/efi-boot.h   |   6 +-
 xen/arch/x86/efi/parse-mbi2.c |  58 +++++++++++++++
 xen/arch/x86/efi/stub.c       |   3 +-
 5 files changed, 89 insertions(+), 115 deletions(-)
 create mode 100644 xen/arch/x86/efi/parse-mbi2.c

Comments

Andrew Cooper Sept. 24, 2024, 2:08 p.m. UTC | #1
On 24/09/2024 11:28 am, Frediano Ziglio wrote:
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 80bba6ff21..6d8eec554b 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -253,36 +244,30 @@ __efi64_mb2_start:
>          shr     $3, %ecx
>          xor     %eax, %eax
>          rep stosq
> -        mov     %edx, %eax
>  
> -        /* Check for Multiboot2 bootloader. */
> -        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> -        je      .Lefi_multiboot2_proto
> -
> -        /* Jump to .Lnot_multiboot after switching CPU to x86_32 mode. */
> -        lea     .Lnot_multiboot(%rip), %r15
> -        jmp     x86_32_switch
> +        /* Save Multiboot2 magic on the stack. */
> +        push    %rdx
>  
> -.Lefi_multiboot2_proto:
> -        /* Zero EFI SystemTable, EFI ImageHandle addresses and cmdline. */
> -        xor     %esi,%esi
> -        xor     %edi,%edi
> -        xor     %edx,%edx
> +        /* Save Multiboot2 pointer on the stack, keep the stack aligned. */
> +        push    %rbx

I'd merge these two pushes, so

    /* Spill MB2 magic.  Spill the pointer too, to keep the stack
aligned. */
    push %rdx
    push %rbx

and ...

>  
> -        /* Skip Multiboot2 information fixed part. */
> -        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
> -        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> +        /*
> +         * efi_parse_mbi2() is called according to System V AMD64 ABI:
> +         *   - IN:  %edi - Multiboot2 magic, %rsi - Multiboot2 pointer.

%rsi below %edi please, for readability.

> +         *   - OUT: %rax - error string.
> +         */
> +        mov     %edx, %edi
> +        mov     %rbx, %rsi
> +        call    efi_parse_mbi2
> +        lea     .Ldirect_error(%rip), %r15
> +        test    %rax, %rax
> +        jnz     x86_32_switch
>  
> -.Lefi_mb2_tsize:
> -        /* Check Multiboot2 information total size. */
> -        mov     %ecx,%r8d
> -        sub     %ebx,%r8d
> -        cmp     %r8d,MB2_fixed_total_size(%rbx)
> -        jbe     .Lrun_bs
> +        /* Restore Multiboot2 pointer. */
> +        pop     %rbx
>  
> -        /* Are EFI boot services available? */
> -        cmpl    $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx)
> -        jne     .Lefi_mb2_st
> +        /* Restore Multiboot2 magic. */
> +        pop     %rax

... merge these pops too.  (It's a shame the pushes/pops implement a
%edx -> %eax swap for magic, but it's for BSS clearing purposes.)

> diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c
> new file mode 100644
> index 0000000000..6038f35b16
> --- /dev/null
> +++ b/xen/arch/x86/efi/parse-mbi2.c
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <xen/efi.h>
> +#include <xen/init.h>
> +#include <xen/multiboot2.h>
> +#include <asm/asm_defns.h>
> +#include <asm/efibind.h>
> +#include <efi/efidef.h>
> +#include <efi/eficapsule.h>
> +#include <efi/eficon.h>
> +#include <efi/efidevp.h>
> +#include <efi/efiapi.h>
> +
> +void __init efi_multiboot2(EFI_HANDLE ImageHandle,
> +                           EFI_SYSTEM_TABLE *SystemTable,
> +                           const char *cmdline);

This wants to be in a header file seen by all references.  I see you
you've fixed up an error in the stub clearly caused by the absence of a
shared header.

> +
> +const char * asmlinkage __init
> +efi_parse_mbi2(uint32_t magic, const multiboot2_fixed_t *mbi)
> +{
> +    const multiboot2_tag_t *tag;
> +    EFI_HANDLE ImageHandle = NULL;
> +    EFI_SYSTEM_TABLE *SystemTable = NULL;
> +    const char *cmdline = NULL;
> +    bool have_bs = false;
> +
> +    if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC )
> +        return "ERR: Not a Multiboot2 bootloader!";
> +
> +    /* Skip Multiboot2 information fixed part. */
> +    tag = _p(ROUNDUP((unsigned long)(mbi + 1), MULTIBOOT2_TAG_ALIGN));
> +
> +    for ( ; (const void *)tag - (const void *)mbi < mbi->total_size
> +            && tag->type != MULTIBOOT2_TAG_TYPE_END;
> +          tag = _p(ROUNDUP((unsigned long)((const void *)tag + tag->size),
> +                   MULTIBOOT2_TAG_ALIGN)) )
> +    {
> +        if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI_BS )
> +            have_bs = true;
> +        else if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI64 )
> +            SystemTable = _p(((const multiboot2_tag_efi64_t *)tag)->pointer);
> +        else if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI64_IH )
> +            ImageHandle = _p(((const multiboot2_tag_efi64_ih_t *)tag)->pointer);
> +        else if ( tag->type == MULTIBOOT2_TAG_TYPE_CMDLINE )
> +            cmdline = ((const multiboot2_tag_string_t *)tag)->string;

switch ( tag->type ) please.  It's more lines, but more legible.

Otherwise, LGTM.  Definitely nice to move this out of asm.

~Andrew
Jan Beulich Sept. 24, 2024, 2:17 p.m. UTC | #2
On 24.09.2024 12:28, Frediano Ziglio wrote:
> No need to have it coded in assembly.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
> Changes since v1:
> - update some comments;
> - explain why %ebx is saved before calling efi_parse_mbi2;
> - move lea before test instruction;
> - removed asmlinkage from efi_multiboot2 and add to efi_parse_mbi2;
> - fix line length;
> - update an error message specifying "Multiboot2" instead of "Multiboot";
> - use obj-bin-X instead of obj-X in Makefile;
> - avoid restoring %eax (MBI magic).

Despite this long list of changes earlier comments were left unaddressed.
The new function is still named as if it did only parsing, the stub change
is still in here and (if already not separated out) not mentioned at all
in the description, and (as Andrew has now also pointed out) the
declaration of efi_multiboot2() didn't move to a header. Maybe I forgot
some more. Please make sure you address earlier comments before sending a
new version.

Jan
Frediano Ziglio Sept. 24, 2024, 2:28 p.m. UTC | #3
On Tue, Sep 24, 2024 at 3:17 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.09.2024 12:28, Frediano Ziglio wrote:
> > No need to have it coded in assembly.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> > ---
> > Changes since v1:
> > - update some comments;
> > - explain why %ebx is saved before calling efi_parse_mbi2;
> > - move lea before test instruction;
> > - removed asmlinkage from efi_multiboot2 and add to efi_parse_mbi2;
> > - fix line length;
> > - update an error message specifying "Multiboot2" instead of "Multiboot";
> > - use obj-bin-X instead of obj-X in Makefile;
> > - avoid restoring %eax (MBI magic).
>
> Despite this long list of changes earlier comments were left unaddressed.
> The new function is still named as if it did only parsing, the stub change
> is still in here and (if already not separated out) not mentioned at all
> in the description, and (as Andrew has now also pointed out) the
> declaration of efi_multiboot2() didn't move to a header. Maybe I forgot
> some more. Please make sure you address earlier comments before sending a
> new version.
>
> Jan

What about renaming efi_parse_mbi2 to "efi_multiboot2_entry_common"
and renaming "efi_multiboot2" as "efi_multiboot2_entry".
I remember I replied to the stub change and nobody get back, so I
thought it was fine as it was.
I also replied to the header asking for a location to put it, and I
don't remember any reply.

Frediano
Jan Beulich Sept. 24, 2024, 2:42 p.m. UTC | #4
On 24.09.2024 16:28, Frediano Ziglio wrote:
> On Tue, Sep 24, 2024 at 3:17 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 24.09.2024 12:28, Frediano Ziglio wrote:
>>> No need to have it coded in assembly.
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>>> ---
>>> Changes since v1:
>>> - update some comments;
>>> - explain why %ebx is saved before calling efi_parse_mbi2;
>>> - move lea before test instruction;
>>> - removed asmlinkage from efi_multiboot2 and add to efi_parse_mbi2;
>>> - fix line length;
>>> - update an error message specifying "Multiboot2" instead of "Multiboot";
>>> - use obj-bin-X instead of obj-X in Makefile;
>>> - avoid restoring %eax (MBI magic).
>>
>> Despite this long list of changes earlier comments were left unaddressed.
>> The new function is still named as if it did only parsing, the stub change
>> is still in here and (if already not separated out) not mentioned at all
>> in the description, and (as Andrew has now also pointed out) the
>> declaration of efi_multiboot2() didn't move to a header. Maybe I forgot
>> some more. Please make sure you address earlier comments before sending a
>> new version.
> 
> What about renaming efi_parse_mbi2 to "efi_multiboot2_entry_common"
> and renaming "efi_multiboot2" as "efi_multiboot2_entry".

I don't see a need to rename efi_multiboot2() as well. In
"efi_multiboot2_entry_common" I'm having trouble seeing what "common"
stands for. Imo a small improvement would already be efi_process_mbi2(),
as "process" covers parsing and the handing on of the result of the
parsing. However, if already the new code can't be folded into
efi_multiboot2(), how about naming the new function
efi_multiboot2_prelude()?

> I remember I replied to the stub change and nobody get back, so I
> thought it was fine as it was.
> I also replied to the header asking for a location to put it, and I
> don't remember any reply.

I'm sure I gave you a reply, but that was only yesterday, after I was
back from travelling / PTO.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 80bba6ff21..6d8eec554b 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -122,8 +122,6 @@  multiboot2_header:
 .Lbad_cpu_msg: .asciz "ERR: Not a 64-bit CPU!"
 .Lbad_ldr_msg: .asciz "ERR: Not a Multiboot bootloader!"
 .Lbad_ldr_nbs: .asciz "ERR: Bootloader shutdown EFI x64 boot services!"
-.Lbad_ldr_nst: .asciz "ERR: EFI SystemTable is not provided by bootloader!"
-.Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!"
 .Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!"
 .Lbag_alg_msg: .asciz "ERR: Xen must be loaded at a 2Mb boundary!"
 .Lno_nx_msg:   .asciz "ERR: Not an NX-capable CPU!"
@@ -162,17 +160,6 @@  early_error: /* Here to improve the disassembly. */
         mov     $sym_offs(.Lno_nx_msg), %ecx
         jmp     .Lget_vtb
 #endif
-.Lmb2_no_st:
-        /*
-         * Here we are on EFI platform. vga_text_buffer was zapped earlier
-         * because there is pretty good chance that VGA is unavailable.
-         */
-        mov     $sym_offs(.Lbad_ldr_nst), %ecx
-        jmp     .Lget_vtb
-.Lmb2_no_ih:
-        /* Ditto. */
-        mov     $sym_offs(.Lbad_ldr_nih), %ecx
-        jmp     .Lget_vtb
 .Lmb2_no_bs:
         /*
          * Ditto. Additionally, here there is a chance that Xen was started
@@ -190,6 +177,10 @@  early_error: /* Here to improve the disassembly. */
         mov     $sym_offs(.Lbad_efi_msg), %ecx
         xor     %edi,%edi                       # No VGA text buffer
         jmp     .Lprint_err
+.Ldirect_error:
+        mov     sym_esi(vga_text_buffer), %edi
+        mov     %eax, %esi
+        jmp     1f
 .Lget_vtb:
         mov     sym_esi(vga_text_buffer), %edi
 .Lprint_err:
@@ -236,7 +227,7 @@  __efi64_mb2_start:
 
         /*
          * Align the stack as UEFI spec requires. Keep it aligned
-         * before efi_multiboot2() call by pushing/popping even
+         * before efi_parse_mbi2() call by pushing/popping even
          * numbers of items on it.
          */
         and     $~15, %rsp
@@ -244,7 +235,7 @@  __efi64_mb2_start:
         /*
          * Initialize BSS (no nasty surprises!).
          * It must be done earlier than in BIOS case
-         * because efi_multiboot2() touches it.
+         * because efi_parse_mbi2() touches it.
          */
         mov     %eax, %edx
         lea     __bss_start(%rip), %edi
@@ -253,36 +244,30 @@  __efi64_mb2_start:
         shr     $3, %ecx
         xor     %eax, %eax
         rep stosq
-        mov     %edx, %eax
 
-        /* Check for Multiboot2 bootloader. */
-        cmp     $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
-        je      .Lefi_multiboot2_proto
-
-        /* Jump to .Lnot_multiboot after switching CPU to x86_32 mode. */
-        lea     .Lnot_multiboot(%rip), %r15
-        jmp     x86_32_switch
+        /* Save Multiboot2 magic on the stack. */
+        push    %rdx
 
-.Lefi_multiboot2_proto:
-        /* Zero EFI SystemTable, EFI ImageHandle addresses and cmdline. */
-        xor     %esi,%esi
-        xor     %edi,%edi
-        xor     %edx,%edx
+        /* Save Multiboot2 pointer on the stack, keep the stack aligned. */
+        push    %rbx
 
-        /* Skip Multiboot2 information fixed part. */
-        lea     (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
-        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
+        /*
+         * efi_parse_mbi2() is called according to System V AMD64 ABI:
+         *   - IN:  %edi - Multiboot2 magic, %rsi - Multiboot2 pointer.
+         *   - OUT: %rax - error string.
+         */
+        mov     %edx, %edi
+        mov     %rbx, %rsi
+        call    efi_parse_mbi2
+        lea     .Ldirect_error(%rip), %r15
+        test    %rax, %rax
+        jnz     x86_32_switch
 
-.Lefi_mb2_tsize:
-        /* Check Multiboot2 information total size. */
-        mov     %ecx,%r8d
-        sub     %ebx,%r8d
-        cmp     %r8d,MB2_fixed_total_size(%rbx)
-        jbe     .Lrun_bs
+        /* Restore Multiboot2 pointer. */
+        pop     %rbx
 
-        /* Are EFI boot services available? */
-        cmpl    $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx)
-        jne     .Lefi_mb2_st
+        /* Restore Multiboot2 magic. */
+        pop     %rax
 
         /* We are on EFI platform and EFI boot services are available. */
         incb    efi_platform(%rip)
@@ -292,77 +277,6 @@  __efi64_mb2_start:
          * be run on EFI platforms.
          */
         incb    skip_realmode(%rip)
-        jmp     .Lefi_mb2_next_tag
-
-.Lefi_mb2_st:
-        /* Get EFI SystemTable address from Multiboot2 information. */
-        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx)
-        cmove   MB2_efi64_st(%rcx),%rsi
-        je      .Lefi_mb2_next_tag
-
-        /* Get EFI ImageHandle address from Multiboot2 information. */
-        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64_IH,MB2_tag_type(%rcx)
-        cmove   MB2_efi64_ih(%rcx),%rdi
-        je      .Lefi_mb2_next_tag
-
-        /* Get command line from Multiboot2 information. */
-        cmpl    $MULTIBOOT2_TAG_TYPE_CMDLINE, MB2_tag_type(%rcx)
-        jne     .Lno_cmdline
-        lea     MB2_tag_string(%rcx), %rdx
-        jmp     .Lefi_mb2_next_tag
-.Lno_cmdline:
-
-        /* Is it the end of Multiboot2 information? */
-        cmpl    $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
-        je      .Lrun_bs
-
-.Lefi_mb2_next_tag:
-        /* Go to next Multiboot2 information tag. */
-        add     MB2_tag_size(%rcx),%ecx
-        add     $(MULTIBOOT2_TAG_ALIGN-1),%ecx
-        and     $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
-        jmp     .Lefi_mb2_tsize
-
-.Lrun_bs:
-        /* Are EFI boot services available? */
-        cmpb    $0,efi_platform(%rip)
-
-        /* Jump to .Lmb2_no_bs after switching CPU to x86_32 mode. */
-        lea     .Lmb2_no_bs(%rip),%r15
-        jz      x86_32_switch
-
-        /* Is EFI SystemTable address provided by boot loader? */
-        test    %rsi,%rsi
-
-        /* Jump to .Lmb2_no_st after switching CPU to x86_32 mode. */
-        lea     .Lmb2_no_st(%rip),%r15
-        jz      x86_32_switch
-
-        /* Is EFI ImageHandle address provided by boot loader? */
-        test    %rdi,%rdi
-
-        /* Jump to .Lmb2_no_ih after switching CPU to x86_32 mode. */
-        lea     .Lmb2_no_ih(%rip),%r15
-        jz      x86_32_switch
-
-        /* Save Multiboot2 magic on the stack. */
-        push    %rax
-
-        /* Save EFI ImageHandle on the stack. */
-        push    %rdi
-
-        /*
-         * efi_multiboot2() is called according to System V AMD64 ABI:
-         *   - IN:  %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
-         *          %rdx - MB2 cmdline
-         */
-        call    efi_multiboot2
-
-        /* Just pop an item from the stack. */
-        pop     %rax
-
-        /* Restore Multiboot2 magic. */
-        pop     %rax
 
         /* Jump to trampoline_setup after switching CPU to x86_32 mode. */
         lea     trampoline_setup(%rip),%r15
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 24dfecfad1..51140061fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -14,5 +14,6 @@  $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := $(cflags-stack-bounda
 obj-y := common-stub.o stub.o
 obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y))
 obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y))
+obj-bin-y += parse-mbi2.o
 extra-$(XEN_BUILD_EFI) += buildid.o relocs-dummy.o
 nocov-$(XEN_BUILD_EFI) += stub.o
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 7aa55e7aaf..859c01c13f 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -816,9 +816,9 @@  static const char *__init get_option(const char *cmd, const char *opt)
     return o;
 }
 
-void asmlinkage __init efi_multiboot2(EFI_HANDLE ImageHandle,
-                                      EFI_SYSTEM_TABLE *SystemTable,
-                                      const char *cmdline)
+void __init efi_multiboot2(EFI_HANDLE ImageHandle,
+                           EFI_SYSTEM_TABLE *SystemTable,
+                           const char *cmdline)
 {
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop;
     EFI_HANDLE gop_handle;
diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c
new file mode 100644
index 0000000000..6038f35b16
--- /dev/null
+++ b/xen/arch/x86/efi/parse-mbi2.c
@@ -0,0 +1,58 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <xen/efi.h>
+#include <xen/init.h>
+#include <xen/multiboot2.h>
+#include <asm/asm_defns.h>
+#include <asm/efibind.h>
+#include <efi/efidef.h>
+#include <efi/eficapsule.h>
+#include <efi/eficon.h>
+#include <efi/efidevp.h>
+#include <efi/efiapi.h>
+
+void __init efi_multiboot2(EFI_HANDLE ImageHandle,
+                           EFI_SYSTEM_TABLE *SystemTable,
+                           const char *cmdline);
+
+const char * asmlinkage __init
+efi_parse_mbi2(uint32_t magic, const multiboot2_fixed_t *mbi)
+{
+    const multiboot2_tag_t *tag;
+    EFI_HANDLE ImageHandle = NULL;
+    EFI_SYSTEM_TABLE *SystemTable = NULL;
+    const char *cmdline = NULL;
+    bool have_bs = false;
+
+    if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC )
+        return "ERR: Not a Multiboot2 bootloader!";
+
+    /* Skip Multiboot2 information fixed part. */
+    tag = _p(ROUNDUP((unsigned long)(mbi + 1), MULTIBOOT2_TAG_ALIGN));
+
+    for ( ; (const void *)tag - (const void *)mbi < mbi->total_size
+            && tag->type != MULTIBOOT2_TAG_TYPE_END;
+          tag = _p(ROUNDUP((unsigned long)((const void *)tag + tag->size),
+                   MULTIBOOT2_TAG_ALIGN)) )
+    {
+        if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI_BS )
+            have_bs = true;
+        else if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI64 )
+            SystemTable = _p(((const multiboot2_tag_efi64_t *)tag)->pointer);
+        else if ( tag->type == MULTIBOOT2_TAG_TYPE_EFI64_IH )
+            ImageHandle = _p(((const multiboot2_tag_efi64_ih_t *)tag)->pointer);
+        else if ( tag->type == MULTIBOOT2_TAG_TYPE_CMDLINE )
+            cmdline = ((const multiboot2_tag_string_t *)tag)->string;
+    }
+
+    if ( !have_bs )
+        return "ERR: Bootloader shutdown EFI x64 boot services!";
+    if ( !SystemTable )
+        return "ERR: EFI SystemTable is not provided by bootloader!";
+    if ( !ImageHandle )
+        return "ERR: EFI ImageHandle is not provided by bootloader!";
+
+    efi_multiboot2(ImageHandle, SystemTable, cmdline);
+
+    return NULL;
+}
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 2cd5c8d4dc..27d40964d5 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -17,7 +17,8 @@ 
  */
 
 void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
-                                    EFI_SYSTEM_TABLE *SystemTable)
+                                    EFI_SYSTEM_TABLE *SystemTable,
+                                    const char *cmdline)
 {
     static const CHAR16 __initconst err[] =
         L"Xen does not have EFI code build in!\r\nSystem halted!\r\n";