[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v14 4/9] x86: add multiboot2 protocol support for EFI platforms
On Thu, Feb 02, 2017 at 11:01:14PM +0100, Daniel Kiper wrote: > This way Xen can be loaded on EFI platforms using GRUB2 and > other boot loaders which support multiboot2 protocol. > > Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx> > --- > v14 - suggestions/fixes: > - mark .init.data section as writable; by the way we must change > similar definition in xen/arch/x86/boot/x86_64.S because otherwise > compiler complains about section types conflicts > (suggested by Jan Beulich), > - use %r15 instead of %r15d > (suggested by Jan Beulich), > - remove redundant add from UEFI stack alignment > (suggested by Jan Beulich), > - use "mov (%rsp),%rdi" instead of "pop %rdi/push %rdi" > (suggested by Jan Beulich), > - return void instead of paddr_t from efi_multiboot2() > and then simplify a bit trampoline setup assembly > (suggested by Jan Beulich), > - remove "(XEN)" from efi_multiboot2() stub error messages > (suggested by Jan Beulich), > - move err from inline assembly OutputOperands to InputOperands in > stub.c:efi_multiboot2(); this way we avoid following compile time error: > stub.c: In function ‘efi_multiboot2’: > stub.c:36:5: error: read-only variable ‘err’ used as ‘asm’ output > asm volatile( > ^~~ > issue was introduced by changing err type to "static const CHAR16 > __initconst"; > potentially we can revert this change but move to InputOperands looks > better IMO; > even if we are not able to specify %rdx in Clobbers; simply we do not > care > because next instruction after call is hlt > (discovered by Konrad Rzeszutek Wilk and Marcos Matsunaga), > - take into account MBI_SPACE_MIN in ASSERT((trampoline_end - > trampoline_start) < ...) > (suggested by Jan Beulich), > - improve comments > (suggested by Jan Beulich). Diff as Doug asked: diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 2ecdcb3..5147204 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -116,7 +116,7 @@ gdt_boot_descr: .Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!" .Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!" - .section .init.data, "a", @progbits + .section .init.data, "aw", @progbits .align 4 vga_text_buffer: @@ -173,9 +173,10 @@ not_multiboot: __efi64_mb2_start: /* - * Multiboot2 spec says that here CPU is in 64-bit mode. However, there - * is also guarantee that all code and data is always put by the bootloader - * below 4 GiB. Hence, we can safely use in most cases 32-bit addressing. + * Multiboot2 spec says that here CPU is in 64-bit mode. However, + * there is also guarantee that all code and data is always put + * by the bootloader below 4 GiB. Hence, we can safely truncate + * addresses to 32-bits in most cases below. */ cld @@ -188,7 +189,7 @@ __efi64_mb2_start: je .Lefi_multiboot2_proto /* Jump to not_multiboot after switching CPU to x86_32 mode. */ - lea not_multiboot(%rip),%r15d + lea not_multiboot(%rip),%r15 jmp x86_32_switch .Lefi_multiboot2_proto: @@ -248,28 +249,34 @@ __efi64_mb2_start: cmpb $0,efi_platform(%rip) /* Jump to .Lmb2_no_bs after switching CPU to x86_32 mode. */ - lea .Lmb2_no_bs(%rip),%r15d + 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),%r15d + 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),%r15d + lea .Lmb2_no_ih(%rip),%r15 jz x86_32_switch - /* Align the stack as UEFI spec requires. */ - add $15,%rsp + /* + * 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 /* @@ -284,34 +291,26 @@ __efi64_mb2_start: xor %eax,%eax rep stosq - pop %rdi - - /* Align the stack as UEFI spec requires. */ - push %rdi + /* 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, - * - OUT: %rax - trampoline address. - * - * MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag is not provided - * on EFI platforms. Hence, it could not be used like - * on legacy BIOS platforms. + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable. */ call efi_multiboot2 - /* Convert memory address to bytes/16 and store it in safe place. */ - shr $4,%eax - mov %eax,%ecx + /* Just pop an item from the stack. */ + pop %rax - pop %rdi + /* Restore Multiboot2 magic. */ pop %rax /* Jump to trampoline_setup after switching CPU to x86_32 mode. */ - lea trampoline_setup(%rip),%r15d + lea trampoline_setup(%rip),%r15 x86_32_switch: - mov %r15d,%edi + mov %r15,%rdi cli @@ -428,46 +427,49 @@ trampoline_bios_setup: * multiboot structure (if available) and use the smallest. */ cmp $0x100,%edx /* is the multiboot value too small? */ - jb trampoline_setup /* if so, do not use it */ + jb 2f /* if so, do not use it */ shl $10-4,%edx cmp %ecx,%edx /* compare with BDA value */ cmovb %edx,%ecx /* and use the smaller */ +2: /* Reserve memory for the trampoline and the low-memory stack. */ sub $((TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE)>>4),%ecx /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */ xor %cl, %cl - -trampoline_setup: shl $4, %ecx mov %ecx,sym_phys(trampoline_phys) - /* Get topmost low-memory stack address. */ +trampoline_setup: + mov sym_phys(trampoline_phys),%ecx + + /* Get bottom-most low-memory stack address. */ add $TRAMPOLINE_SPACE,%ecx /* Save the Multiboot info struct (after relocation) for later use. */ mov $sym_phys(cpu0_stack)+1024,%esp - push %ecx /* Topmost low-memory stack address. */ + push %ecx /* Bottom-most low-memory stack address. */ push %ebx /* Multiboot information address. */ push %eax /* Multiboot magic. */ call reloc mov %eax,sym_phys(multiboot_ptr) /* - * Now trampoline_phys points to the following structure (lowest - * address is at the top): + * Now trampoline_phys points to the following structure (lowest address + * is at the bottom): * * +------------------------+ - * | TRAMPOLINE_SPACE | - * +- - - - - - - - - - - - + - * | mbi struct | - * +------------------------+ * | TRAMPOLINE_STACK_SPACE | * +------------------------+ + * | mbi data | + * +- - - - - - - - - - - - + + * | TRAMPOLINE_SPACE | + * +------------------------+ * - * mbi struct lives at the end of TRAMPOLINE_SPACE. The rest of - * TRAMPOLINE_SPACE is reserved for trampoline code and data. + * mbi data grows downwards from the highest address of TRAMPOLINE_SPACE + * region to the end of the trampoline. The rest of TRAMPOLINE_SPACE is + * reserved for trampoline code and data. */ /* diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index 139b2ca..4d507fb 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -186,7 +186,7 @@ GLOBAL(idle_pg_table) GLOBAL(__page_tables_end) /* Init pagetables. Enough page directories to map into the bottom 1GB. */ - .section .init.data, "a", @progbits + .section .init.data, "aw", @progbits .align PAGE_SIZE, 0 GLOBAL(l2_bootmap) diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index d193847..94418bf 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -665,7 +665,7 @@ static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable) static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { } -paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) +void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) { EFI_GRAPHICS_OUTPUT_PROTOCOL *gop; UINTN cols, gop_mode = ~0, rows; @@ -698,9 +698,6 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTa efi_set_gop_mode(gop, gop_mode); efi_exit_boot(ImageHandle, SystemTable); - - /* Return trampoline address. */ - return trampoline_phys; } /* diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c index 8df9ba2..2127cce 100644 --- a/xen/arch/x86/efi/stub.c +++ b/xen/arch/x86/efi/stub.c @@ -17,11 +17,11 @@ * efi_multiboot2() is an exception. Please look below for more details. */ -paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle, - EFI_SYSTEM_TABLE *SystemTable) +void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle, + EFI_SYSTEM_TABLE *SystemTable) { static const CHAR16 __initconst err[] = - L"(XEN) Xen does not have EFI code build in!\r\n(XEN) System halted!\r\n"; + L"Xen does not have EFI code build in!\r\nSystem halted!\r\n"; SIMPLE_TEXT_OUTPUT_INTERFACE *StdErr; StdErr = SystemTable->StdErr ? SystemTable->StdErr : SystemTable->ConOut; @@ -37,7 +37,7 @@ paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle, " call *%2 \n" "0: hlt \n" " jmp 0b \n" - : "+c" (StdErr), "+d" (err) : "rm" (StdErr->OutputString) + : "+c" (StdErr) : "d" (err), "rm" (StdErr->OutputString) : "rax", "r8", "r9", "r10", "r11", "memory"); unreachable(); diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index addf2ef..76e18ab 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -334,5 +334,5 @@ ASSERT(IS_ALIGNED(trampoline_end, 4), "trampoline_end misaligned") ASSERT(IS_ALIGNED(__bss_start, 8), "__bss_start misaligned") ASSERT(IS_ALIGNED(__bss_end, 8), "__bss_end misaligned") -ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE, - "not enough room for trampoline") +ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE - MBI_SPACE_MIN, + "not enough room for trampoline and mbi data") diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h index 9cd05a2..b9a6d94 100644 --- a/xen/include/asm-x86/config.h +++ b/xen/include/asm-x86/config.h @@ -76,6 +76,8 @@ #define TRAMPOLINE_STACK_SPACE PAGE_SIZE #define TRAMPOLINE_SPACE (KB(64) - TRAMPOLINE_STACK_SPACE) +#define MBI_SPACE_MIN (2 * PAGE_SIZE) + /* Primary stack is restricted to 8kB by guard pages. */ #define PRIMARY_STACK_SIZE 8192 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |