This series came from part of the work of removing duplications between boot code and rewriting part of code from assembly to C. First 2 patches rework BIOS/PVH paths to reuse some code. Third 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. Frediano Ziglio (4): x86/boot: Initialise BSS sooner 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 | 288 +++++++++++----------------------- xen/arch/x86/efi/Makefile | 1 + xen/arch/x86/efi/efi-boot.h | 6 +- xen/arch/x86/efi/parse-mbi2.c | 61 +++++++ xen/arch/x86/efi/stub.c | 3 +- 5 files changed, 155 insertions(+), 204 deletions(-) create mode 100644 xen/arch/x86/efi/parse-mbi2.c -- 2.34.1 Allows to call C code earlier. In order to safely call C code we need to setup stack, selectors and BSS. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- Changes since v1: - improve commit message; - improve some comments; - fix some code style (spacing); - set trampoline_phys as 32 bit value; - use PAGE_SIZE mnemonic instead of 0x1000; - use local label. --- xen/arch/x86/boot/head.S | 77 ++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 38 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 @@ __efi64_mb2_start: /* VGA is not available on EFI platforms. */ movl $0,vga_text_buffer(%rip) + /* + * Align the stack as UEFI spec requires. Keep it aligned + * before efi_multiboot2() call by pushing/popping even + * numbers of items on it. + */ + and $~15, %rsp + + /* + * Initialize BSS (no nasty surprises!). + * It must be done earlier than in BIOS case + * because efi_multiboot2() 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 @@ -XXX,XX +XXX,XX @@ __efi64_mb2_start: lea .Lmb2_no_ih(%rip),%r15 jz x86_32_switch - /* - * Align the stack as UEFI spec requires. Keep it aligned - * before efi_multiboot2() call by pushing/popping even - * numbers of items on it. - */ - and $~15,%rsp - /* Save Multiboot2 magic on the stack. */ push %rax /* Save EFI ImageHandle on the stack. */ push %rdi - /* - * Initialize BSS (no nasty surprises!). - * It must be done earlier than in BIOS case - * because efi_multiboot2() touches it. - */ - lea __bss_start(%rip),%edi - lea __bss_end(%rip),%ecx - sub %edi,%ecx - shr $3,%ecx - xor %eax,%eax - rep stosq - - /* Keep the stack aligned. Do not pop a single item off it. */ - mov (%rsp),%rdi - /* * efi_multiboot2() is called according to System V AMD64 ABI: * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable, @@ -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. */ @@ -XXX,XX +XXX,XX @@ __pvh_start: #endif /* CONFIG_PVH_GUEST */ +.Linitialise_bss: + /* Initialise the BSS. */ + mov %eax, %edx + + lea sym_esi(__bss_start), %edi + lea sym_esi(__bss_end), %ecx + sub %edi, %ecx + xor %eax, %eax + shr $2, %ecx + rep stosl + + mov %edx, %eax + ret + __start: cld cli @@ -XXX,XX +XXX,XX @@ __start: /* Set up stack. */ lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp + call .Linitialise_bss + /* Bootloaders may set multiboot{1,2}.mem_lower to a nonzero value. */ xor %edx,%edx @@ -XXX,XX +XXX,XX @@ trampoline_setup: * reserved for trampoline code and data. */ - /* - * Do not zero BSS on EFI platform here. - * It was initialized earlier. - */ - cmpb $0, sym_esi(efi_platform) - jnz 1f - - /* Initialise the BSS. */ - lea sym_esi(__bss_start), %edi - lea sym_esi(__bss_end), %ecx - sub %edi,%ecx - xor %eax,%eax - shr $2,%ecx - rep stosl - -1: /* Interrogate CPU extended features via CPUID. */ mov $1, %eax cpuid -- 2.34.1 The 2 code paths were sharing quite some common code, reuse it instead of having duplications. Use %dl register to store boot type before running common code. Using a 8 bit register reduces code size. 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. --- xen/arch/x86/boot/head.S | 117 +++++++++++++++------------------------ 1 file changed, 45 insertions(+), 72 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 BOOT_TYPE_BIOS 1 +#define BOOT_TYPE_PVH 2 + .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 $BOOT_TYPE_PVH, %dl + jmp .Lcommon_bios_pvh +#endif /* CONFIG_PVH_GUEST */ + +__start: + mov $BOOT_TYPE_BIOS, %dl + +.Lcommon_bios_pvh: 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. + * + * Durig 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. */ - mov (%ebx), %edx + mov (%ebx), %ecx lea 4(%ebx), %esp /* Calculate the load base address. */ @@ -XXX,XX +XXX,XX @@ __pvh_start: sub $sym_offs(1b), %esi /* Restore the clobbered field. */ - mov %edx, (%ebx) + mov %ecx, (%ebx) /* 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, %es mov %ecx, %ss - /* 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 */ + /* Load null selector to unused segment registers. */ + xor %ecx, %ecx + mov %ecx, %fs + mov %ecx, %gs -.Linitialise_bss: /* Initialise the BSS. */ - mov %eax, %edx - + mov %eax, %ebp lea sym_esi(__bss_start), %edi lea sym_esi(__bss_end), %ecx sub %edi, %ecx xor %eax, %eax shr $2, %ecx rep stosl + mov %ebp, %eax - mov %edx, %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 $BOOT_TYPE_PVH, %dl + 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) + mov (%ebx), %eax /* mov $XEN_HVM_START_MAGIC_VALUE, %eax */ + 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). --- 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 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_parse_mbi2() 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_parse_mbi2() 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 + /* 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) @@ -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 */ + +#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 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"; -- 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_parse_mbi2(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_parse_mbi2(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)((const void *)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