Re: [PATCH v4 1/5] efi: Introduce EFI_DIRECT flag
On 19.05.14 at 22:46, daniel.ki...@oracle.com wrote: On Mon, May 19, 2014 at 02:30:45PM +0100, Jan Beulich wrote: On 16.05.14 at 22:41, daniel.ki...@oracle.com wrote: @@ -457,6 +460,21 @@ void __init efi_free_boot_services(void) efi_unmap_memmap(); } +static void __init __iomem *efi_early_ioremap(resource_size_t phys_addr, + unsigned long size) +{ + if (efi_enabled(EFI_DIRECT)) + return early_ioremap(phys_addr, size); + + return (__force void __iomem *)phys_addr; Now that surely needs some explanation: I can't see how this can ever be correct, Xen or not being completely irrelevant. I hope that efi_enabled(EFI_DIRECT) is obvious. However, in case of !efi_enabled(EFI_DIRECT) some structures are created artificially and they live in virtual address space. So that is why they should not be mapped. If you wish I could add relevant comment here. That would be the very minimum I suppose. But I wonder whether you wouldn't be better off storing their physical addresses in the first place (and then decide whether you can stay with early_ioremap() or want/need to use early_memremap() if !EFI_DIRECT). Jan -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] xen: Put EFI machinery in place
On 16/05/14 21:41, Daniel Kiper wrote: Put EFI machinery for Xen in place. Put what machinery to do what? @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)((__u64)efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)((__u64)efi_systab_xen 32); + + x86_platform.get_wallclock = efi_get_time; x86_platform.get_wallclock should always be xen_get_wallclock(). + x86_platform.set_wallclock = efi_set_rtc_mmss; + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); --- /dev/null +++ b/drivers/xen/efi.c @@ -0,0 +1,374 @@ + * Copyright (c) 2014 Daniel Kiper, Oracle Corporation Is this really copyright by you personally and not Oracle? +#define call (op.u.efi_runtime_call) +#define DECLARE_CALL(what) \ + struct xen_platform_op op; \ + op.cmd = XENPF_efi_runtime_call; \ + call.function = XEN_EFI_##what; \ + call.misc = 0 Macros that declare local variables are awful. Use what Andrew suggested and something like struct xen_blah *call = op.u.efi_runtime_call; +static const struct efi efi_xen __initconst = { + .systab = NULL, /* Initialized later. */ + .runtime_version = 0,/* Initialized later. */ + .mps = EFI_INVALID_TABLE_ADDR, + .acpi = EFI_INVALID_TABLE_ADDR, + .acpi20 = EFI_INVALID_TABLE_ADDR, + .smbios = EFI_INVALID_TABLE_ADDR, + .sal_systab = EFI_INVALID_TABLE_ADDR, + .boot_info= EFI_INVALID_TABLE_ADDR, + .hcdp = EFI_INVALID_TABLE_ADDR, + .uga = EFI_INVALID_TABLE_ADDR, + .uv_systab= EFI_INVALID_TABLE_ADDR, + .fw_vendor= EFI_INVALID_TABLE_ADDR, + .runtime = EFI_INVALID_TABLE_ADDR, + .config_table = EFI_INVALID_TABLE_ADDR, + .get_time = xen_efi_get_time, + .set_time = xen_efi_set_time, + .get_wakeup_time = xen_efi_get_wakeup_time, + .set_wakeup_time = xen_efi_set_wakeup_time, + .get_variable = xen_efi_get_variable, + .get_next_variable= xen_efi_get_next_variable, + .set_variable = xen_efi_set_variable, + .query_variable_info = xen_efi_query_variable_info, + .update_capsule = xen_efi_update_capsule, + .query_capsule_caps = xen_efi_query_capsule_caps, + .get_next_high_mono_count = xen_efi_get_next_high_mono_count, + .reset_system = NULL, /* Functionality provided by Xen. */ Xen provides functionality to reset (just maybe not via an EFI call). Should an implementation be provided that does this? + .set_virtual_address_map = NULL, /* Not used under Xen. */ + .memmap = NULL, /* Not used under Xen. */ + .flags= 0 /* Initialized later. */ +}; + +efi_system_table_t __init *xen_efi_probe(void) +{ + struct xen_platform_op op = { + .cmd = XENPF_firmware_info, + .u.firmware_info = { + .type = XEN_FW_EFI_INFO, + .index = XEN_FW_EFI_CONFIG_TABLE + } + }; + union xenpf_efi_info *info = op.u.firmware_info.u.efi_info; + + if (!xen_initial_domain() || HYPERVISOR_dom0_op(op)) + return NULL; if (!xen_initial_domain()) return NULL; if (HYPERVISOR_dom0_op(op) 0) return NULL; + + /* Here we know that Xen runs on EFI platform. */ + + efi = efi_xen; + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_VENDOR; + info-vendor.bufsz = sizeof(vendor); + set_xen_guest_handle(info-vendor.name, vendor); + + if (!HYPERVISOR_dom0_op(op)) { if (HYPERVISOR_dom0_op(op) == 0) David -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] xen: Put EFI machinery in place
On Tue, May 20, 2014 at 10:47:00AM +0100, David Vrabel wrote: On 16/05/14 21:41, Daniel Kiper wrote: Put EFI machinery for Xen in place. Put what machinery to do what? @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)((__u64)efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)((__u64)efi_systab_xen 32); + + x86_platform.get_wallclock = efi_get_time; x86_platform.get_wallclock should always be xen_get_wallclock(). Hmmm... Make sens... Jan, why did you replace x86_platform.get_wallclock with efi_get_time() in your implementation? + x86_platform.set_wallclock = efi_set_rtc_mmss; Now I am not sure even about that. As I can see Linux Kernel running on bare metal EFI platform directly sets RTC omitting efi_set_rtc_mmss(). Am I missing something? IIRC, there was discussion about that once. But where and when? Anyway, now I think that this initialization should be done in arch/x86/xen/time.c if needed. + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); --- /dev/null +++ b/drivers/xen/efi.c @@ -0,0 +1,374 @@ + * Copyright (c) 2014 Daniel Kiper, Oracle Corporation Is this really copyright by you personally and not Oracle? As I know it is correct but I will double check it. +#define call (op.u.efi_runtime_call) +#define DECLARE_CALL(what) \ + struct xen_platform_op op; \ + op.cmd = XENPF_efi_runtime_call; \ + call.function = XEN_EFI_##what; \ + call.misc = 0 Macros that declare local variables are awful. Use what Andrew suggested and something like struct xen_blah *call = op.u.efi_runtime_call; +static const struct efi efi_xen __initconst = { + .systab = NULL, /* Initialized later. */ + .runtime_version = 0,/* Initialized later. */ + .mps = EFI_INVALID_TABLE_ADDR, + .acpi = EFI_INVALID_TABLE_ADDR, + .acpi20 = EFI_INVALID_TABLE_ADDR, + .smbios = EFI_INVALID_TABLE_ADDR, + .sal_systab = EFI_INVALID_TABLE_ADDR, + .boot_info= EFI_INVALID_TABLE_ADDR, + .hcdp = EFI_INVALID_TABLE_ADDR, + .uga = EFI_INVALID_TABLE_ADDR, + .uv_systab= EFI_INVALID_TABLE_ADDR, + .fw_vendor= EFI_INVALID_TABLE_ADDR, + .runtime = EFI_INVALID_TABLE_ADDR, + .config_table = EFI_INVALID_TABLE_ADDR, + .get_time = xen_efi_get_time, + .set_time = xen_efi_set_time, + .get_wakeup_time = xen_efi_get_wakeup_time, + .set_wakeup_time = xen_efi_set_wakeup_time, + .get_variable = xen_efi_get_variable, + .get_next_variable= xen_efi_get_next_variable, + .set_variable = xen_efi_set_variable, + .query_variable_info = xen_efi_query_variable_info, + .update_capsule = xen_efi_update_capsule, + .query_capsule_caps = xen_efi_query_capsule_caps, + .get_next_high_mono_count = xen_efi_get_next_high_mono_count, + .reset_system = NULL, /* Functionality provided by Xen. */ Xen provides functionality to reset (just maybe not via an EFI call). Should an implementation be provided that does this? I do not think so. efi.reset_system() would not be called on Xen because all reset related stuff is replaced by Xen reset specific functions. Please check arch/x86/xen/enlighten.c. Additionally, I think that situation is a bit similar to time issue. If we are running on Xen then we should ask Xen to do reset because Xen controls platform and it knows how to do that. + .set_virtual_address_map = NULL, /* Not used under Xen. */ + .memmap = NULL, /* Not used under Xen. */ + .flags= 0 /* Initialized later. */ +}; + +efi_system_table_t __init *xen_efi_probe(void) +{ + struct xen_platform_op op = { + .cmd = XENPF_firmware_info, + .u.firmware_info = { + .type = XEN_FW_EFI_INFO, + .index = XEN_FW_EFI_CONFIG_TABLE + } + }; + union xenpf_efi_info *info = op.u.firmware_info.u.efi_info; + + if (!xen_initial_domain() || HYPERVISOR_dom0_op(op)) + return NULL; if (!xen_initial_domain()) return NULL; if (HYPERVISOR_dom0_op(op) 0) return
Re: kernel 3.14.2 oops: seems related to EFI
On Mon, 19 May, at 09:09:58AM, Francis Moreau wrote: I don't know, I can't really afford to configure/compile/test this new kernel, sorry. It would be useful to know whether this issue still occurs when booting with the efi=old_map kernel parameter. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] xen: Put EFI machinery in place
On 20.05.14 at 13:29, daniel.ki...@oracle.com wrote: On Tue, May 20, 2014 at 10:47:00AM +0100, David Vrabel wrote: On 16/05/14 21:41, Daniel Kiper wrote: @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)((__u64)efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)((__u64)efi_systab_xen 32); + + x86_platform.get_wallclock = efi_get_time; x86_platform.get_wallclock should always be xen_get_wallclock(). Hmmm... Make sens... Jan, why did you replace x86_platform.get_wallclock with efi_get_time() in your implementation? On the basis that (for Dom0 only) this is the equivalent of (and actually also falls back to) mach_get_cmos_time(). If on Dom0 .get_wallclock doesn't get set to mach_get_cmos_time() on pv-ops, then that line above should also be dropped. Jan -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] xen: Put EFI machinery in place
On 20/05/14 12:29, Daniel Kiper wrote: + if (!xen_initial_domain() || HYPERVISOR_dom0_op(op)) + return NULL; if (!xen_initial_domain()) return NULL; if (HYPERVISOR_dom0_op(op) 0) return NULL; What is wrong with my if? Style. The function returns -ve on error not a boolean success/fail. + + /* Here we know that Xen runs on EFI platform. */ + + efi = efi_xen; + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_VENDOR; + info-vendor.bufsz = sizeof(vendor); + set_xen_guest_handle(info-vendor.name, vendor); + + if (!HYPERVISOR_dom0_op(op)) { if (HYPERVISOR_dom0_op(op) == 0) Ditto? Again, style. David -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel 3.14.2 oops: seems related to EFI
On 05/20/2014 01:54 PM, Matt Fleming wrote: On Mon, 19 May, at 09:09:58AM, Francis Moreau wrote: I don't know, I can't really afford to configure/compile/test this new kernel, sorry. It would be useful to know whether this issue still occurs when booting with the efi=old_map kernel parameter. ok I can try to boot with that parameter and see if the issue happens again. Unfortunately if it doesn't, we couldn't tell. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] EFI changes for arm64
On Mon, May 19, 2014 at 11:50:42AM +0100, Matt Fleming wrote: On Wed, 30 Apr, at 09:03:32PM, Matt Fleming wrote: I pulled the arm64 EFI changes into the following topic branch. Catalin was happy for this to go through tip, which definitely makes things easier for the next merge window because of the dependency these patches have on tip/x86/efi. The following changes since commit e33655a386ed3b26ad36fb97a47ebb1c2ca1e928: efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git arm64-efi for you to fetch changes up to 345c736edd07b657a8c48190baed2719b85d0938: efi/arm64: ignore dtb= when UEFI SecureBoot is enabled (2014-04-30 19:57:06 +0100) Ping? Alternatively, I can merge the arm64-efi tree and wait for tip x86/efi to go in before sending my pull request. Peter, Ingo, do you have any preference? Either way, I'd like to see this branch in -next to make sure there are no (significant) conflicts before the merging window. Thanks. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] EFI changes for v3.16
On Mon, 19 May, at 03:47:31PM, H. Peter Anvin wrote: How on earth does this solve anything? The only thing we add here is a WARN_ON_ONCE()... but the above text already tells us we have a problem. It seems, rather, that we need to figure out how to deal with a pstore in this case. There are a few possibilities: 1. We could keep an XSAVE buffer area around for this particular use. I am *assuming* we don't let more than one CPU into EFI, because I cannot for my life imagine that this is safe in typical CPUs. Correct. This is actually prohibited by the spec, so we have a lock to enforce it. 2. Drop the pstore on the floor if !irq_fpu_usable(). 3. Allow the pstore, then die (on the assumption that we're dead anyway.) Personally, I'd prefer 2, but I'm open to suggestions. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html