| Message ID | 20240925060101.259244-4-frediano.ziglio@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/boot: Reduce assembly code | expand |
On 25/09/2024 7:01 am, Frediano Ziglio wrote: > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index 2d2f56ad22..859f7055dc 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -252,36 +243,30 @@ __efi64_mb2_start: > <snip> > > /* We are on EFI platform and EFI boot services are available. */ > incb efi_platform(%rip) > @@ -291,77 +276,6 @@ __efi64_mb2_start: > * be run on EFI platforms. > */ > incb skip_realmode(%rip) Well, these are two unfounded assumptions about the compile-time defaults of certain variables. Lets fix it afterwards, to save interfering with this series. > diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c > new file mode 100644 > index 0000000000..89c562cf6a > --- /dev/null > +++ b/xen/arch/x86/efi/parse-mbi2.c > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ GPL-2.0-only. The unsuffixed form is deprecated now. > + > +#include <xen/efi.h> > +#include <xen/init.h> > +#include <xen/multiboot2.h> > +#include <asm/asm_defns.h> > +#include <asm/efi.h> > + > +const char * asmlinkage __init > +efi_multiboot2_prelude(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; && on previous line. But, this can move into the switch statement and reduce the for() expression somewhat. > + tag = _p(ROUNDUP((unsigned long)((const void *)tag + tag->size), > + MULTIBOOT2_TAG_ALIGN)) ) No need to cast through (const void *) I don't think. It's byte-granular when unsigned long too. This should do: _p(ROUNDUP(((unsigned long)tag + tag->size), MULTIBOOT2_TAG_ALIGN)) > + { > + switch ( tag->type ) > + { > + case MULTIBOOT2_TAG_TYPE_EFI_BS: > + have_bs = true; > + break; Newlines after break's please. > + case MULTIBOOT2_TAG_TYPE_EFI64: > + SystemTable = _p(((const multiboot2_tag_efi64_t *)tag)->pointer); > + break; > + case MULTIBOOT2_TAG_TYPE_EFI64_IH: > + ImageHandle = _p(((const multiboot2_tag_efi64_ih_t *)tag)->pointer); > + break; > + case MULTIBOOT2_TAG_TYPE_CMDLINE: > + cmdline = ((const multiboot2_tag_string_t *)tag)->string; > + break; default: /* This comment is here because MISRA thinks you can't read the code without it. */ break; Probably not that wording, but it reflects what I think of this particular rule. I can fix all on commit, but this needs an EFI ack. ~Andrew
On 25.09.2024 22:20, Andrew Cooper wrote: > On 25/09/2024 7:01 am, Frediano Ziglio wrote: >> +const char * asmlinkage __init >> +efi_multiboot2_prelude(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; > > && on previous line. > > But, this can move into the switch statement and reduce the for() > expression somewhat. While it can in principle, it would require further adjustments to the loop body, which I'm uncertain would be desirable. I notice Frediano also didn't make any change in v5 for this particular comment. Frediano: As indicated before, sending a new version without addressing all comments isn't nice. If you don't agree with a comment and hence don't make a change requested, you will want to "address" the comment verbally. Jan
On Thu, Sep 26, 2024 at 7:58 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 25.09.2024 22:20, Andrew Cooper wrote: > > On 25/09/2024 7:01 am, Frediano Ziglio wrote: > >> +const char * asmlinkage __init > >> +efi_multiboot2_prelude(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; > > > > && on previous line. > > > > But, this can move into the switch statement and reduce the for() > > expression somewhat. > > While it can in principle, it would require further adjustments to the > loop body, which I'm uncertain would be desirable. I notice Frediano > also didn't make any change in v5 for this particular comment. Frediano: > As indicated before, sending a new version without addressing all > comments isn't nice. If you don't agree with a comment and hence don't > make a change requested, you will want to "address" the comment verbally. > > Jan I was going to reply, then I forgot. Yes, I'm trying to reply and address all comments. In this specific instance, for some reason while I was addressing the others, I forgot this. Frediano
On Wed, Sep 25, 2024 at 9:20 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 25/09/2024 7:01 am, Frediano Ziglio wrote: > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > > index 2d2f56ad22..859f7055dc 100644 > > --- a/xen/arch/x86/boot/head.S > > +++ b/xen/arch/x86/boot/head.S > > @@ -252,36 +243,30 @@ __efi64_mb2_start: > > <snip> > > > > /* We are on EFI platform and EFI boot services are available. */ > > incb efi_platform(%rip) > > @@ -291,77 +276,6 @@ __efi64_mb2_start: > > * be run on EFI platforms. > > */ > > incb skip_realmode(%rip) > > Well, these are two unfounded assumptions about the compile-time > defaults of certain variables. > > Lets fix it afterwards, to save interfering with this series. > > > diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c > > new file mode 100644 > > index 0000000000..89c562cf6a > > --- /dev/null > > +++ b/xen/arch/x86/efi/parse-mbi2.c > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > GPL-2.0-only. The unsuffixed form is deprecated now. > > > + > > +#include <xen/efi.h> > > +#include <xen/init.h> > > +#include <xen/multiboot2.h> > > +#include <asm/asm_defns.h> > > +#include <asm/efi.h> > > + > > +const char * asmlinkage __init > > +efi_multiboot2_prelude(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; > > && on previous line. > > But, this can move into the switch statement and reduce the for() > expression somewhat. > I forgot to reply this, I even though about what to write. There are multiple reasons: - having now a switch, it would require a goto/label or an additional variable to exit the loop, unless I change all other case to continue and have 2 breaks, either cases not much improving in my opinion; - that specific part of the for loop is checking for termination and that condition is doing exactly this; - there are multiple instances of this kind of loop, and I was thinking of adding a macro to simplify and reuse that loop so that form is more suitable to do it. ... omissis ... Frediano
On 26/09/2024 10:14 am, Frediano Ziglio wrote: > On Wed, Sep 25, 2024 at 9:20 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> On 25/09/2024 7:01 am, Frediano Ziglio wrote: >>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S >>> index 2d2f56ad22..859f7055dc 100644 >>> --- a/xen/arch/x86/boot/head.S >>> +++ b/xen/arch/x86/boot/head.S >>> @@ -252,36 +243,30 @@ __efi64_mb2_start: >>> <snip> >>> >>> /* We are on EFI platform and EFI boot services are available. */ >>> incb efi_platform(%rip) >>> @@ -291,77 +276,6 @@ __efi64_mb2_start: >>> * be run on EFI platforms. >>> */ >>> incb skip_realmode(%rip) >> Well, these are two unfounded assumptions about the compile-time >> defaults of certain variables. >> >> Lets fix it afterwards, to save interfering with this series. >> >>> diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c >>> new file mode 100644 >>> index 0000000000..89c562cf6a >>> --- /dev/null >>> +++ b/xen/arch/x86/efi/parse-mbi2.c >>> @@ -0,0 +1,56 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >> GPL-2.0-only. The unsuffixed form is deprecated now. >> >>> + >>> +#include <xen/efi.h> >>> +#include <xen/init.h> >>> +#include <xen/multiboot2.h> >>> +#include <asm/asm_defns.h> >>> +#include <asm/efi.h> >>> + >>> +const char * asmlinkage __init >>> +efi_multiboot2_prelude(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; >> && on previous line. >> >> But, this can move into the switch statement and reduce the for() >> expression somewhat. >> > I forgot to reply this, I even though about what to write. > There are multiple reasons: > - having now a switch, it would require a goto/label or an additional > variable to exit the loop, Oh, of course. Sorry, my mistake. Leave it as is. ~Andrew
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 2d2f56ad22..859f7055dc 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -121,8 +121,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!" @@ -161,17 +159,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 @@ -189,6 +176,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: @@ -235,7 +226,7 @@ __efi64_mb2_start: /* * Align the stack as UEFI spec requires. Keep it aligned - * before efi_multiboot2() call by pushing/popping even + * before efi_multiboot2_prelude() call by pushing/popping even * numbers of items on it. */ and $~15, %rsp @@ -243,7 +234,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_multiboot2_prelude() touches it. */ mov %eax, %edx lea __bss_start(%rip), %edi @@ -252,36 +243,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 - -.Lefi_multiboot2_proto: - /* Zero EFI SystemTable, EFI ImageHandle addresses and cmdline. */ - xor %esi,%esi - xor %edi,%edi - xor %edx,%edx - /* Skip Multiboot2 information fixed part. */ - lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx - and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx - -.Lefi_mb2_tsize: - /* Check Multiboot2 information total size. */ - mov %ecx,%r8d - sub %ebx,%r8d - cmp %r8d,MB2_fixed_total_size(%rbx) - jbe .Lrun_bs + /* + * Spill MB2 magic. + * Spill the pointer too, to keep the stack aligned. + */ + push %rdx + push %rbx - /* Are EFI boot services available? */ - cmpl $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx) - jne .Lefi_mb2_st + /* + * efi_multiboot2_prelude() 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_multiboot2_prelude + lea .Ldirect_error(%rip), %r15 + test %rax, %rax + jnz x86_32_switch + + /* Restore Multiboot2 pointer and magic. */ + pop %rbx + pop %rax /* We are on EFI platform and EFI boot services are available. */ incb efi_platform(%rip) @@ -291,77 +276,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..89c562cf6a --- /dev/null +++ b/xen/arch/x86/efi/parse-mbi2.c @@ -0,0 +1,56 @@ +/* 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/efi.h> + +const char * asmlinkage __init +efi_multiboot2_prelude(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)) ) + { + switch ( tag->type ) + { + case MULTIBOOT2_TAG_TYPE_EFI_BS: + have_bs = true; + break; + case MULTIBOOT2_TAG_TYPE_EFI64: + SystemTable = _p(((const multiboot2_tag_efi64_t *)tag)->pointer); + break; + case MULTIBOOT2_TAG_TYPE_EFI64_IH: + ImageHandle = _p(((const multiboot2_tag_efi64_ih_t *)tag)->pointer); + break; + case MULTIBOOT2_TAG_TYPE_CMDLINE: + cmdline = ((const multiboot2_tag_string_t *)tag)->string; + break; + } + } + + 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"; diff --git a/xen/arch/x86/include/asm/efi.h b/xen/arch/x86/include/asm/efi.h new file mode 100644 index 0000000000..575a33e302 --- /dev/null +++ b/xen/arch/x86/include/asm/efi.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef X86_ASM_EFI_H +#define X86_ASM_EFI_H + +#include <xen/types.h> +#include <asm/x86_64/efibind.h> +#include <efi/efidef.h> +#include <efi/eficapsule.h> +#include <efi/eficon.h> +#include <efi/efidevp.h> +#include <efi/efiapi.h> + +void efi_multiboot2(EFI_HANDLE ImageHandle, + EFI_SYSTEM_TABLE *SystemTable, + const char *cmdline); + +#endif /* X86_ASM_EFI_H */
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). Changes since v3: - rename new function to efi_multiboot2_prelude; - declare efi_multiboot2 in a separate header. --- xen/arch/x86/boot/head.S | 142 +++++++-------------------------- xen/arch/x86/efi/Makefile | 1 + xen/arch/x86/efi/efi-boot.h | 6 +- xen/arch/x86/efi/parse-mbi2.c | 56 +++++++++++++ xen/arch/x86/efi/stub.c | 3 +- xen/arch/x86/include/asm/efi.h | 18 +++++ 6 files changed, 108 insertions(+), 118 deletions(-) create mode 100644 xen/arch/x86/efi/parse-mbi2.c create mode 100644 xen/arch/x86/include/asm/efi.h