| Message ID | 1410310325-4509-5-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > status = efi_bs->ExitBootServices(ImageHandle, mmap_key); > if ( EFI_ERROR(status) ) > PrintErrMesg(L"Cannot exit boot services", status); > > - /* Adjust pointers into EFI. */ > - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; > -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP > - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; > -#endif > - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; > - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; > - > - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); Up to here I don't see anything arch-specific again. Jan
On Thu, Sep 11, 2014 at 7:13 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >> status = efi_bs->ExitBootServices(ImageHandle, mmap_key); >> if ( EFI_ERROR(status) ) >> PrintErrMesg(L"Cannot exit boot services", status); >> >> - /* Adjust pointers into EFI. */ >> - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; >> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP >> - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; >> -#endif >> - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; >> - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; >> - >> - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); > > Up to here I don't see anything arch-specific again. This is due to runtime services not being implemented. The "efi_rs" variable is defined in the runtime services file runtime.c, which is not yet ported to ARM. Short of including ARM runtime service support in the patchset, how would you like this handled? > > Jan >
>>> On 11.09.14 at 19:44, <roy.franz@linaro.org> wrote: > On Thu, Sep 11, 2014 at 7:13 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >>> status = efi_bs->ExitBootServices(ImageHandle, mmap_key); >>> if ( EFI_ERROR(status) ) >>> PrintErrMesg(L"Cannot exit boot services", status); >>> >>> - /* Adjust pointers into EFI. */ >>> - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; >>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP >>> - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; >>> -#endif >>> - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; >>> - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; >>> - >>> - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); >> >> Up to here I don't see anything arch-specific again. > This is due to runtime services not being implemented. The "efi_rs" > variable is defined in the runtime services > file runtime.c, which is not yet ported to ARM. > > Short of including ARM runtime service support in the patchset, how > would you like this handled? As said before, everything that's generic but not used/usable on ARM right now should stay here, but get made conditional in one or another way. Jan
On Fri, 2014-09-12 at 08:08 +0100, Jan Beulich wrote: > >>> On 11.09.14 at 19:44, <roy.franz@linaro.org> wrote: > > On Thu, Sep 11, 2014 at 7:13 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > >>> status = efi_bs->ExitBootServices(ImageHandle, mmap_key); > >>> if ( EFI_ERROR(status) ) > >>> PrintErrMesg(L"Cannot exit boot services", status); > >>> > >>> - /* Adjust pointers into EFI. */ > >>> - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; > >>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP > >>> - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; > >>> -#endif > >>> - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; > >>> - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; > >>> - > >>> - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); > >> > >> Up to here I don't see anything arch-specific again. > > This is due to runtime services not being implemented. The "efi_rs" > > variable is defined in the runtime services > > file runtime.c, which is not yet ported to ARM. > > > > Short of including ARM runtime service support in the patchset, how > > would you like this handled? > > As said before, everything that's generic but not used/usable on > ARM right now should stay here, but get made conditional in one > or another way. So in this case a HAVE_EFI_RUNTIMESERVICES ifdef would work? Ian.
>>> On 12.09.14 at 11:46, <Ian.Campbell@citrix.com> wrote: > On Fri, 2014-09-12 at 08:08 +0100, Jan Beulich wrote: >> >>> On 11.09.14 at 19:44, <roy.franz@linaro.org> wrote: >> > On Thu, Sep 11, 2014 at 7:13 AM, Jan Beulich <JBeulich@suse.com> wrote: >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >> >>> status = efi_bs->ExitBootServices(ImageHandle, mmap_key); >> >>> if ( EFI_ERROR(status) ) >> >>> PrintErrMesg(L"Cannot exit boot services", status); >> >>> >> >>> - /* Adjust pointers into EFI. */ >> >>> - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; >> >>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP >> >>> - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; >> >>> -#endif >> >>> - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; >> >>> - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; >> >>> - >> >>> - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); >> >> >> >> Up to here I don't see anything arch-specific again. >> > This is due to runtime services not being implemented. The "efi_rs" >> > variable is defined in the runtime services >> > file runtime.c, which is not yet ported to ARM. >> > >> > Short of including ARM runtime service support in the patchset, how >> > would you like this handled? >> >> As said before, everything that's generic but not used/usable on >> ARM right now should stay here, but get made conditional in one >> or another way. > > So in this case a HAVE_EFI_RUNTIMESERVICES ifdef would work? Yes (except that in the case I would use NEED_ or WANT_ instead of HAVE_). But considering this is intended to be temporary, I'd even be fine with just using CONFIG_ARM or alike here. Jan
On Fri, Sep 12, 2014 at 2:58 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>> On 12.09.14 at 11:46, <Ian.Campbell@citrix.com> wrote: > > On Fri, 2014-09-12 at 08:08 +0100, Jan Beulich wrote: > >> >>> On 11.09.14 at 19:44, <roy.franz@linaro.org> wrote: > >> > On Thu, Sep 11, 2014 at 7:13 AM, Jan Beulich <JBeulich@suse.com> > wrote: > >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > >> >>> status = efi_bs->ExitBootServices(ImageHandle, mmap_key); > >> >>> if ( EFI_ERROR(status) ) > >> >>> PrintErrMesg(L"Cannot exit boot services", status); > >> >>> > >> >>> - /* Adjust pointers into EFI. */ > >> >>> - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; > >> >>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP > >> >>> - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; > >> >>> -#endif > >> >>> - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; > >> >>> - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; > >> >>> - > >> >>> - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); > >> >> > >> >> Up to here I don't see anything arch-specific again. > >> > This is due to runtime services not being implemented. The "efi_rs" > >> > variable is defined in the runtime services > >> > file runtime.c, which is not yet ported to ARM. > >> > > >> > Short of including ARM runtime service support in the patchset, how > >> > would you like this handled? > >> > >> As said before, everything that's generic but not used/usable on > >> ARM right now should stay here, but get made conditional in one > >> or another way. > > > > So in this case a HAVE_EFI_RUNTIMESERVICES ifdef would work? > > Yes (except that in the case I would use NEED_ or WANT_ instead > of HAVE_). But considering this is intended to be temporary, I'd > even be fine with just using CONFIG_ARM or alike here. > > Jan > > OK, I'll move this back. Just having the #ifdefs in runtime.c will help a lot. Is commenting the #ifef like so acceptable: #ifndef CONFIG_ARM /* TODO - disabled until EFI runtime services implemented */ Thanks, Roy
>>> On 12.09.14 at 18:57, <roy.franz@linaro.org> wrote: >> OK, I'll move this back. Just having the #ifdefs in runtime.c will help a > lot. > > Is commenting the #ifef like so acceptable: > > #ifndef CONFIG_ARM /* TODO - disabled until EFI runtime services > implemented */ Sure. Jan
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 16ffe35..ca86beb 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -571,7 +571,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info; EFI_FILE_HANDLE dir_handle; union string section = { NULL }, name; - u64 efer; bool_t base_video = 0; UINT32 mmap_desc_ver = 0; UINTN mmap_size, mmap_desc_size, mmap_key = 0; @@ -1175,58 +1174,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) efi_arch_process_memory_map(SystemTable, mmap, mmap_size, mmap_desc_size, mmap_desc_ver); - if ( !trampoline_phys ) - { - if ( !cfg.addr ) - blexit(L"No memory for trampoline"); - relocate_trampoline(cfg.addr); - } + efi_arch_pre_exit_boot(); status = efi_bs->ExitBootServices(ImageHandle, mmap_key); if ( EFI_ERROR(status) ) PrintErrMesg(L"Cannot exit boot services", status); - /* Adjust pointers into EFI. */ - efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP - efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; -#endif - efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; - efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; - - efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); - memcpy((void *)trampoline_phys, trampoline_start, cfg.size); - - /* Set system registers and transfer control. */ - asm volatile("pushq $0\n\tpopfq"); - rdmsrl(MSR_EFER, efer); - efer |= EFER_SCE; - if ( cpuid_ext_features & (1 << (X86_FEATURE_NX & 0x1f)) ) - efer |= EFER_NX; - wrmsrl(MSR_EFER, efer); - write_cr0(X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP | - X86_CR0_AM | X86_CR0_PG); - asm volatile ( "mov %[cr4], %%cr4\n\t" - "mov %[cr3], %%cr3\n\t" - "movabs $__start_xen, %[rip]\n\t" - "lgdt gdt_descr(%%rip)\n\t" - "mov stack_start(%%rip), %%rsp\n\t" - "mov %[ds], %%ss\n\t" - "mov %[ds], %%ds\n\t" - "mov %[ds], %%es\n\t" - "mov %[ds], %%fs\n\t" - "mov %[ds], %%gs\n\t" - "movl %[cs], 8(%%rsp)\n\t" - "mov %[rip], (%%rsp)\n\t" - "lretq %[stkoff]-16" - : [rip] "=&r" (efer/* any dead 64-bit variable */) - : [cr3] "r" (idle_pg_table), - [cr4] "r" (mmu_cr4_features), - [cs] "ir" (__HYPERVISOR_CS), - [ds] "r" (__HYPERVISOR_DS), - [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)), - "D" (&mbi) - : "memory" ); + efi_arch_post_exit_boot(); for( ; ; ); /* not reached */ } diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h index 3fe6747..518c319 100644 --- a/xen/include/asm-x86/efi-boot.h +++ b/xen/include/asm-x86/efi-boot.h @@ -535,3 +535,60 @@ static void __init efi_arch_get_memory_map(UINTN *map_size, if ( EFI_ERROR(status) ) PrintErrMesg(L"Cannot obtain memory map", status); } + +static void __init efi_arch_pre_exit_boot(void) +{ + if ( !trampoline_phys ) + { + if ( !cfg.addr ) + blexit(L"No memory for trampoline"); + relocate_trampoline(cfg.addr); + } +} + +static void __init efi_arch_post_exit_boot(void) +{ + u64 efer; + + /* Adjust pointers into EFI. */ + efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; +#ifdef USE_SET_VIRTUAL_ADDRESS_MAP + efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; +#endif + efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; + efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; + + efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start); + memcpy((void *)trampoline_phys, trampoline_start, cfg.size); + + /* Set system registers and transfer control. */ + asm volatile("pushq $0\n\tpopfq"); + rdmsrl(MSR_EFER, efer); + efer |= EFER_SCE; + if ( cpuid_ext_features & (1 << (X86_FEATURE_NX & 0x1f)) ) + efer |= EFER_NX; + wrmsrl(MSR_EFER, efer); + write_cr0(X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP | + X86_CR0_AM | X86_CR0_PG); + asm volatile ( "mov %[cr4], %%cr4\n\t" + "mov %[cr3], %%cr3\n\t" + "movabs $__start_xen, %[rip]\n\t" + "lgdt gdt_descr(%%rip)\n\t" + "mov stack_start(%%rip), %%rsp\n\t" + "mov %[ds], %%ss\n\t" + "mov %[ds], %%ds\n\t" + "mov %[ds], %%es\n\t" + "mov %[ds], %%fs\n\t" + "mov %[ds], %%gs\n\t" + "movl %[cs], 8(%%rsp)\n\t" + "mov %[rip], (%%rsp)\n\t" + "lretq %[stkoff]-16" + : [rip] "=&r" (efer/* any dead 64-bit variable */) + : [cr3] "r" (idle_pg_table), + [cr4] "r" (mmu_cr4_features), + [cs] "ir" (__HYPERVISOR_CS), + [ds] "r" (__HYPERVISOR_DS), + [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)), + "D" (&mbi) + : "memory" ); +}
The UEFI ExitBootServices function is invoked to transition the system to the 'runtime' mode of operation, and is done right before transitioning from the EFI loader code into Xen proper. x86 does some arch specific memory management (trampoline) before exit boot services, and the code that transitions from the EFI application state to Xen is architecture specific. This patch adds two functions, one pre and one post ExitBootServices to allow each architecture to to handle these cases in a customized manner. Signed-off-by: Roy Franz <roy.franz@linaro.org> --- xen/common/efi/boot.c | 50 ++---------------------------------- xen/include/asm-x86/efi-boot.h | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 48 deletions(-)