From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: efi_enabled(EFI_PARAVIRT) use Date: Fri, 29 Apr 2016 08:39:36 +0200 Message-ID: <20160429063936.GA28320@gmail.com> References: <20160429142020.4499e185@canb.auug.org.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20160429142020.4499e185@canb.auug.org.au> Sender: linux-next-owner@vger.kernel.org To: Stephen Rothwell , "Luis R. Rodriguez" Cc: Jeremy Fitzhardinge , Konrad Rzeszutek Wilk , Stefano Stabellini , Xen Devel , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Peter Zijlstra , linux-next@vger.kernel.org, linux-kernel@vger.kernel.org, Shannon Zhao , Ard Biesheuvel , Matt Fleming , Borislav Petkov List-Id: xen-devel@lists.xenproject.org * Stephen Rothwell wrote: > Hi all, > > Today's linux-next merge of the xen-tip tree got a conflict in: > > drivers/firmware/efi/arm-runtime.c > > between commit: > > 14c43be60166 ("efi/arm*: Drop writable mapping of the UEFI System table") > > from the tip tree and commit: > > 21c8dfaa2327 ("Xen: EFI: Parse DT parameters for Xen specific UEFI") > > from the xen-tip tree. (I've attached 21c8dfaa2327 below, for reference.) Argh: With considerable pain we just got rid of paravirt_enabled() in the x86 tree, and Xen is now reintroducing it in the EFI code. Please don't: if you have to do capability flags then name the flag accordingly to what it does, don't use some generic catch-all naming that will inevitably cause the kind of problems paravirt_enabled() caused... So: NAKed-by: Ingo Molnar Also, it would be nice to have all things EFI in a single tree, the conflicts are going to be painful! There's very little reason not to carry this kind of commit: arch/arm/xen/enlighten.c | 6 +++++ drivers/firmware/efi/arm-runtime.c | 17 +++++++++----- drivers/firmware/efi/efi.c | 45 ++++++++++++++++++++++++++++++++------ 3 files changed, 56 insertions(+), 12 deletions(-) in the EFI tree. Thanks, Ingo =======================> >>From 21c8dfaa23276be2ae6d580331d8d252cc41e8d9 Mon Sep 17 00:00:00 2001 From: Shannon Zhao Date: Thu, 7 Apr 2016 20:03:34 +0800 Subject: [PATCH] Xen: EFI: Parse DT parameters for Xen specific UEFI Add a new function to parse DT parameters for Xen specific UEFI just like the way for normal UEFI. Then it could reuse the existing codes. If Xen supports EFI, initialize runtime services. CC: Matt Fleming Signed-off-by: Shannon Zhao Reviewed-by: Matt Fleming Reviewed-by: Stefano Stabellini Tested-by: Julien Grall --- arch/arm/xen/enlighten.c | 6 +++++ drivers/firmware/efi/arm-runtime.c | 17 +++++++++----- drivers/firmware/efi/efi.c | 45 ++++++++++++++++++++++++++++++++------ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 13e3e9f9b094..e130562d3283 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -261,6 +261,12 @@ static int __init fdt_find_hyper_node(unsigned long node, const char *uname, !strncmp(hyper_node.prefix, s, strlen(hyper_node.prefix))) hyper_node.version = s + strlen(hyper_node.prefix); + if (IS_ENABLED(CONFIG_XEN_EFI)) { + /* Check if Xen supports EFI */ + if (of_get_flat_dt_subnode_by_name(node, "uefi") > 0) + set_bit(EFI_PARAVIRT, &efi.flags); + } + return 0; } diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 6ae21e41a429..ac609b9f0b99 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -27,6 +27,7 @@ #include #include #include +#include extern u64 efi_system_table; @@ -107,13 +108,19 @@ static int __init arm_enable_runtime_services(void) } set_bit(EFI_SYSTEM_TABLES, &efi.flags); - if (!efi_virtmap_init()) { - pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n"); - return -ENOMEM; + if (IS_ENABLED(CONFIG_XEN_EFI) && efi_enabled(EFI_PARAVIRT)) { + /* Set up runtime services function pointers for Xen Dom0 */ + xen_efi_runtime_setup(); + } else { + if (!efi_virtmap_init()) { + pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n"); + return -ENOMEM; + } + + /* Set up runtime services function pointers */ + efi_native_runtime_setup(); } - /* Set up runtime services function pointers */ - efi_native_runtime_setup(); set_bit(EFI_RUNTIME_SERVICES, &efi.flags); efi.runtime_version = efi.systab->hdr.revision; diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 3a69ed5ecfcb..519c096a7c33 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -469,12 +469,14 @@ device_initcall(efi_load_efivars); FIELD_SIZEOF(struct efi_fdt_params, field) \ } -static __initdata struct { +struct params { const char name[32]; const char propname[32]; int offset; int size; -} dt_params[] = { +}; + +static struct params fdt_params[] __initdata = { UEFI_PARAM("System Table", "linux,uefi-system-table", system_table), UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap), UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size), @@ -482,24 +484,45 @@ static __initdata struct { UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver) }; +static struct params xen_fdt_params[] __initdata = { + UEFI_PARAM("System Table", "xen,uefi-system-table", system_table), + UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap), + UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size), + UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size), + UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver) +}; + struct param_info { int found; void *params; + struct params *dt_params; + int size; }; static int __init fdt_find_uefi_params(unsigned long node, const char *uname, int depth, void *data) { struct param_info *info = data; + struct params *dt_params = info->dt_params; const void *prop; void *dest; u64 val; - int i, len; + int i, len, offset; - if (depth != 1 || strcmp(uname, "chosen") != 0) - return 0; + if (efi_enabled(EFI_PARAVIRT)) { + if (depth != 1 || strcmp(uname, "hypervisor") != 0) + return 0; - for (i = 0; i < ARRAY_SIZE(dt_params); i++) { + offset = of_get_flat_dt_subnode_by_name(node, "uefi"); + if (offset < 0) + return 0; + node = offset; + } else { + if (depth != 1 || strcmp(uname, "chosen") != 0) + return 0; + } + + for (i = 0; i < info->size; i++) { prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len); if (!prop) return 0; @@ -530,12 +553,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params) info.found = 0; info.params = params; + if (efi_enabled(EFI_PARAVIRT)) { + info.dt_params = xen_fdt_params; + info.size = ARRAY_SIZE(xen_fdt_params); + } else { + info.dt_params = fdt_params; + info.size = ARRAY_SIZE(fdt_params); + } + ret = of_scan_flat_dt(fdt_find_uefi_params, &info); if (!info.found) pr_info("UEFI not found.\n"); else if (!ret) pr_err("Can't find '%s' in device tree!\n", - dt_params[info.found].name); + info.dt_params[info.found].name); return ret; }