:p
atchew
Login
This series came from part of the work of removing duplications between boot code and rewriting part of code from assembly to C. First patch rework BIOS/PVH paths to reuse some code. Second patch rewrites EFI code in pure C. Changes since v1, more details in specific commits: - style updates; - comments and descriptions improvements; - other improvements. Changes since v2: - rebased on master, resolved conflicts; - add comment on trampoline section. Changes since v3: - changed new function name; - declare efi_multiboot2 in a separate header; - distinguish entry point from using magic number; - other minor changes (see commens in commits). Changes since v4: - rebase on staging; - set %fs and %gs as other segment registers; - style and other changes. Changes since v5: - fixed a typo. Frediano Ziglio (3): x86/boot: Refactor BIOS/PVH start x86/boot: Rewrite EFI/MBI2 code partly in C x86/boot: Improve MBI2 structure check xen/arch/x86/boot/head.S | 246 +++++++++------------------------ xen/arch/x86/efi/Makefile | 1 + xen/arch/x86/efi/efi-boot.h | 6 +- xen/arch/x86/efi/parse-mbi2.c | 66 +++++++++ xen/arch/x86/efi/stub.c | 3 +- xen/arch/x86/include/asm/efi.h | 18 +++ 6 files changed, 155 insertions(+), 185 deletions(-) create mode 100644 xen/arch/x86/efi/parse-mbi2.c create mode 100644 xen/arch/x86/include/asm/efi.h -- 2.34.1
The 2 code paths were sharing quite some common code, reuse it instead of having duplications. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- Changes since v1: - use %dl instead of %ebp to reduce code size; - fold cli instruction; - update some comments. Changes since v3: - dropped %dl and constant, distinguish entry by magic. Changes since v4: - remove old sentence from commit message; - fixed a typo; - load XEN_HVM_START_MAGIC_VALUE constant directly; - set %fs and %gs as other segment registers; - remove some spurious line changes. Changes since v5: - fixed a typo. --- xen/arch/x86/boot/head.S | 104 ++++++++++++++------------------------- 1 file changed, 37 insertions(+), 67 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -XXX,XX +XXX,XX @@ #define MB2_HT(name) (MULTIBOOT2_HEADER_TAG_##name) #define MB2_TT(name) (MULTIBOOT2_TAG_TYPE_##name) +#define XEN_HVM_START_MAGIC_VALUE 0x336ec578 + .macro mb2ht_args arg:req, args:vararg .long \arg .ifnb \args @@ -XXX,XX +XXX,XX @@ cs32_switch: ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY, .long sym_offs(__pvh_start)) __pvh_start: - cld + mov $XEN_HVM_START_MAGIC_VALUE, %eax + /* + * Fall through into BIOS code. + * We will use %eax to distinguish we came from PVH entry point. + */ +#endif /* CONFIG_PVH_GUEST */ + +__start: cli + cld /* - * We need one call (i.e. push) to determine the load address. See - * __start for a discussion on how to do this safely using the PVH - * info structure. + * Multiboot (both 1 and 2) and PVH specify the stack pointer as + * undefined. This is unhelpful for relocatable images, where one + * call (i.e. push) is required to calculate the image's load address. + * + * During BIOS boot, there is one area of memory we know about with + * reasonable confidence that it isn't overlapped by Xen, and that's + * the Multiboot info structure in %ebx. Use it as a temporary stack. + * + * During PVH boot use info structure in %ebx. */ /* Preserve the field we're about to clobber. */ @@ -XXX,XX +XXX,XX @@ __pvh_start: /* Set up stack. */ lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp - call .Linitialise_bss - - mov %ebx, sym_esi(pvh_start_info_pa) - - /* Force xen console. Will revert to user choice in init code. */ - movb $-1, sym_esi(opt_console_xen) - - /* Prepare gdt and segments */ + /* Initialize GDTR and basic data segments. */ add %esi, sym_esi(gdt_boot_base) lgdt sym_esi(gdt_boot_descr) @@ -XXX,XX +XXX,XX @@ __pvh_start: mov %ecx, %ds mov %ecx, %es mov %ecx, %ss + mov %ecx, %fs + mov %ecx, %gs - /* Skip bootloader setup and bios setup, go straight to trampoline */ - movb $1, sym_esi(pvh_boot) - movb $1, sym_esi(skip_realmode) - - /* Set trampoline_phys to use mfn 1 to avoid having a mapping at VA 0 */ - movw $0x1000, sym_esi(trampoline_phys) - mov (%ebx), %eax /* mov $XEN_HVM_START_MAGIC_VALUE, %eax */ - jmp trampoline_setup - -#endif /* CONFIG_PVH_GUEST */ - -.Linitialise_bss: /* Initialise the BSS. Preserve %eax (BOOTLOADER_MAGIC). */ mov %eax, %ebp @@ -XXX,XX +XXX,XX @@ __pvh_start: rep stosl mov %ebp, %eax - ret - -__start: - cld - cli - - /* - * Multiboot (both 1 and 2) specify the stack pointer as undefined - * when entering in BIOS circumstances. This is unhelpful for - * relocatable images, where one call (i.e. push) is required to - * calculate the image's load address. - * - * This early in boot, there is one area of memory we know about with - * reasonable confidence that it isn't overlapped by Xen, and that's - * the Multiboot info structure in %ebx. Use it as a temporary stack. - */ - /* Preserve the field we're about to clobber. */ - mov (%ebx), %edx - lea 4(%ebx), %esp +#ifdef CONFIG_PVH_GUEST + cmp $XEN_HVM_START_MAGIC_VALUE, %eax + jne 1f - /* Calculate the load base address. */ - call 1f -1: pop %esi - sub $sym_offs(1b), %esi + mov %ebx, sym_esi(pvh_start_info_pa) - /* Restore the clobbered field. */ - mov %edx, (%ebx) + /* Force xen console. Will revert to user choice in init code. */ + movb $-1, sym_esi(opt_console_xen) - /* Set up stack. */ - lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp + /* Skip bootloader setup and bios setup, go straight to trampoline */ + movb $1, sym_esi(pvh_boot) + movb $1, sym_esi(skip_realmode) - call .Linitialise_bss + /* Set trampoline_phys to use mfn 1 to avoid having a mapping at VA 0 */ + movl $PAGE_SIZE, sym_esi(trampoline_phys) + jmp trampoline_setup +1: +#endif /* CONFIG_PVH_GUEST */ /* Bootloaders may set multiboot{1,2}.mem_lower to a nonzero value. */ xor %edx,%edx @@ -XXX,XX +XXX,XX @@ __start: trampoline_bios_setup: /* * Called on legacy BIOS platforms only. - * - * Initialize GDTR and basic data segments. */ - add %esi,sym_esi(gdt_boot_base) - lgdt sym_esi(gdt_boot_descr) - - mov $BOOT_DS,%ecx - mov %ecx,%ds - mov %ecx,%es - mov %ecx,%ss - /* %esp is initialized later. */ - - /* Load null descriptor to unused segment registers. */ - xor %ecx,%ecx - mov %ecx,%fs - mov %ecx,%gs /* Set up trampoline segment 64k below EBDA */ movzwl 0x40e,%ecx /* EBDA segment */ -- 2.34.1
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. Changes since v4: - fix some style and space; - fix MISRA requirement. --- 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 | 63 +++++++++++++++ xen/arch/x86/efi/stub.c | 3 +- xen/arch/x86/include/asm/efi.h | 18 +++++ 6 files changed, 115 insertions(+), 118 deletions(-) create mode 100644 xen/arch/x86/efi/parse-mbi2.c create mode 100644 xen/arch/x86/include/asm/efi.h diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -XXX,XX +XXX,XX @@ 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!" @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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: @@ -XXX,XX +XXX,XX @@ __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 @@ -XXX,XX +XXX,XX @@ __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 @@ -XXX,XX +XXX,XX @@ __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) @@ -XXX,XX +XXX,XX @@ __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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -XXX,XX +XXX,XX @@ $(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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/x86/efi/parse-mbi2.c @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#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)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; + + default: + /* Satisfy MISRA requirement. */ + 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/stub.c +++ b/xen/arch/x86/efi/stub.c @@ -XXX,XX +XXX,XX @@ */ 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 XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/x86/include/asm/efi.h @@ -XXX,XX +XXX,XX @@ +/* 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 */ -- 2.34.1
Tag structure should contain at least the tag header. Entire tag structure must be contained inside MBI2 data. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- xen/arch/x86/efi/parse-mbi2.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/efi/parse-mbi2.c b/xen/arch/x86/efi/parse-mbi2.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/parse-mbi2.c +++ b/xen/arch/x86/efi/parse-mbi2.c @@ -XXX,XX +XXX,XX @@ efi_multiboot2_prelude(uint32_t magic, const multiboot2_fixed_t *mbi) EFI_HANDLE ImageHandle = NULL; EFI_SYSTEM_TABLE *SystemTable = NULL; const char *cmdline = NULL; + const void *const mbi_end = (const void *)mbi + mbi->total_size; bool have_bs = false; if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC ) @@ -XXX,XX +XXX,XX @@ efi_multiboot2_prelude(uint32_t magic, const multiboot2_fixed_t *mbi) /* 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 && + for ( ; (const void *)(tag + 1) <= mbi_end && + tag->size >= sizeof(*tag) && + (const void *)tag + tag->size <= mbi_end && tag->type != MULTIBOOT2_TAG_TYPE_END; tag = _p(ROUNDUP((unsigned long)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) ) -- 2.34.1
This series came from part of the work of removing duplications between boot code and rewriting part of code from assembly to C. Rewrites EFI code in pure C. Changes since v1, more details in specific commits: - style updates; - comments and descriptions improvements; - other improvements. Changes since v2: - rebased on master, resolved conflicts; - add comment on trampoline section. Changes since v3: - changed new function name; - declare efi_multiboot2 in a separate header; - distinguish entry point from using magic number; - other minor changes (see commens in commits). Changes since v4: - rebase on staging; - set %fs and %gs as other segment registers; - style and other changes. Changes since v5: - fixed a typo. Changes since v6: - remove merged patch; - comment and style; - change some pointer checks to avoid overflows; - rename parse-mbi2.c to mbi2.c. Frediano Ziglio (2): x86/boot: Rewrite EFI/MBI2 code partly in C x86/boot: Improve MBI2 structure check xen/arch/x86/boot/head.S | 146 +++++++-------------------------- xen/arch/x86/efi/Makefile | 1 + xen/arch/x86/efi/efi-boot.h | 7 +- xen/arch/x86/efi/mbi2.c | 66 +++++++++++++++ xen/arch/x86/efi/stub.c | 10 +-- xen/arch/x86/include/asm/efi.h | 18 ++++ 6 files changed, 123 insertions(+), 125 deletions(-) create mode 100644 xen/arch/x86/efi/mbi2.c create mode 100644 xen/arch/x86/include/asm/efi.h -- 2.34.1
No need to have it coded in assembly. Declare efi_multiboot2 in a new header to reuse between implementations and caller. 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. Changes since v4: - fix some style and space; - fix MISRA requirement. Changes since v6: - include new header to get common declaration; - add a comment in assembly code; - rename parse-mbi2.c to mbi2.c. --- xen/arch/x86/boot/head.S | 146 +++++++-------------------------- xen/arch/x86/efi/Makefile | 1 + xen/arch/x86/efi/efi-boot.h | 7 +- xen/arch/x86/efi/mbi2.c | 63 ++++++++++++++ xen/arch/x86/efi/stub.c | 10 +-- xen/arch/x86/include/asm/efi.h | 18 ++++ 6 files changed, 120 insertions(+), 125 deletions(-) create mode 100644 xen/arch/x86/efi/mbi2.c create mode 100644 xen/arch/x86/include/asm/efi.h diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -XXX,XX +XXX,XX @@ 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!" @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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: @@ -XXX,XX +XXX,XX @@ __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 + /* Save magic number, we need it later but we need to use %eax. */ + mov %eax, %edx + /* * 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 lea __bss_end(%rip), %ecx sub %edi, %ecx 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) @@ -XXX,XX +XXX,XX @@ __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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -XXX,XX +XXX,XX @@ $(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 += 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -XXX,XX +XXX,XX @@ #include <asm/msr.h> #include <asm/setup.h> #include <asm/trampoline.h> +#include <asm/efi.h> static struct file __initdata ucode; static multiboot_info_t __initdata mbi = { @@ -XXX,XX +XXX,XX @@ 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/mbi2.c b/xen/arch/x86/efi/mbi2.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/x86/efi/mbi2.c @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#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)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; + + default: + /* Satisfy MISRA requirement. */ + 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/stub.c +++ b/xen/arch/x86/efi/stub.c @@ -XXX,XX +XXX,XX @@ #include <xen/efi.h> #include <xen/init.h> #include <asm/asm_defns.h> -#include <asm/efibind.h> +#include <asm/efi.h> #include <asm/page.h> -#include <efi/efidef.h> -#include <efi/eficapsule.h> -#include <efi/eficon.h> -#include <efi/efidevp.h> -#include <efi/efiapi.h> /* * Here we are in EFI stub. EFI calls are not supported due to lack @@ -XXX,XX +XXX,XX @@ */ 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 XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/x86/include/asm/efi.h @@ -XXX,XX +XXX,XX @@ +/* 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 */ -- 2.34.1
Tag structure should contain at least the tag header. Entire tag structure must be contained inside MBI2 data. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- Changes since v6: - compare against total_size every time to avoid overflows. --- xen/arch/x86/efi/mbi2.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/efi/mbi2.c b/xen/arch/x86/efi/mbi2.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/mbi2.c +++ b/xen/arch/x86/efi/mbi2.c @@ -XXX,XX +XXX,XX @@ efi_multiboot2_prelude(uint32_t magic, const multiboot2_fixed_t *mbi) EFI_HANDLE ImageHandle = NULL; EFI_SYSTEM_TABLE *SystemTable = NULL; const char *cmdline = NULL; + const void *const mbi_raw = (const void *)mbi; bool have_bs = false; if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC ) @@ -XXX,XX +XXX,XX @@ efi_multiboot2_prelude(uint32_t magic, const multiboot2_fixed_t *mbi) /* 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 && + for ( ; (const void *)(tag + 1) - mbi_raw <= mbi->total_size && + tag->size >= sizeof(*tag) && + (const void *)tag + tag->size - mbi_raw <= mbi->total_size && tag->type != MULTIBOOT2_TAG_TYPE_END; tag = _p(ROUNDUP((unsigned long)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) ) -- 2.34.1