From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH for-xen-4.5 v4 08/18] x86: introduce boot_info structure Date: Fri, 17 Oct 2014 21:55:47 +0100 Message-ID: <54418253.3040601@citrix.com> References: <1413555132-22138-1-git-send-email-daniel.kiper@oracle.com> <1413555132-22138-9-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XfEYy-0002yq-G3 for xen-devel@lists.xenproject.org; Fri, 17 Oct 2014 20:55:52 +0000 In-Reply-To: <1413555132-22138-9-git-send-email-daniel.kiper@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel Kiper , xen-devel@lists.xenproject.org Cc: jgross@suse.com, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ross.philipson@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, jbeulich@suse.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org On 17/10/2014 15:12, Daniel Kiper wrote: > This patch introduces boot_info structure. Subsequent patches will move > step by step all boot related data to above mentioned struct. At the end > of this process multiboot (v1) protocol dependency will be broken. It means > that most of Xen code (excluding preloader) could be bootloader agnostic > and does not need almost any knowledge about boot protocol. Additionally, > it will be possible to pass all boot data to __start_xen() in one bucket > without any side channels. I do not mention that we are also able to easily > identify boot data in Xen code. > > Here is boot data flow for legacy BIOS platform: > > BIOS -> GRUB -> multiboot[12]* -> reloc() -> MBD -->--\ > / > ------<------<------<------<------<------<----- > \ > \ > ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info > / > BIOS ->-/ > > * multiboot2 is not implemented yet. Look for it in later patches. > > Here is boot data flow for EFI platform: > > EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info > > Signed-off-by: Daniel Kiper > --- > v4 - suggestions/fixes: > - change some variables "char *" type to "const char *" type > (suggested by Andrew Cooper), > - do ARM test build > (suggested by Ian Campbell and Stefano Stabellini). > > v3 - suggestions/fixes: > - further patch split rearrangement > (suggested by Andrew Cooper). > > v2 - suggestions/fixes: > - rename XBI to boot_info > (suggested by Andrew Cooper), > - use more meaningful types in boot_info structure > (suggested by Andrew Cooper, Jan Beulich and Stefano Stabellini), > - improve boot_info structure comment > (suggested by Andrew Cooper and Jan Beulich), > - do data shuffling after exception support initialization > (suggested by Andrew Cooper), > - patch split rearrangement > (suggested by Andrew Cooper and Jan Beulich). > --- > xen/arch/x86/boot/x86_64.S | 10 +++++++-- > xen/arch/x86/boot_info.c | 11 +++++++++ > xen/arch/x86/efi/efi-boot.h | 1 + > xen/arch/x86/setup.c | 13 ++++++++++- > xen/common/efi/efi.h | 7 ++++++ > xen/common/efi/runtime.c | 10 +++++++++ > xen/include/asm-x86/boot_info.h | 47 +++++++++++++++++++++++++++++++++++++++ > 7 files changed, 96 insertions(+), 3 deletions(-) > create mode 100644 xen/include/asm-x86/boot_info.h > > diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S > index fc7ce74..2305b56 100644 > --- a/xen/arch/x86/boot/x86_64.S > +++ b/xen/arch/x86/boot/x86_64.S > @@ -32,9 +32,15 @@ > /* Initialize the Multiboot info struct. */ > mov mbd_pa(%rip),%edi > call __init_mbi > + pushq %rax > + > + /* Initialize the boot_info. */ > + mov mbd_pa(%rip),%edi > + call __init_boot_info > > - /* Pass off the Multiboot info struct to C land. */ > - movq %rax,%rdi > + /* Pass off the Multiboot info struct and the boot_info to C land. */ > + popq %rdi > + movq %rax,%rsi > call __start_xen > ud2 /* Force a panic (invalid opcode). */ > > diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c > index 64bc4d9..c5f2a18 100644 > --- a/xen/arch/x86/boot_info.c > +++ b/xen/arch/x86/boot_info.c > @@ -21,12 +21,18 @@ > #include > #include > > +#include > #include > #include > #include > > static multiboot_info_t __read_mostly mbi; > > +static boot_info_t __read_mostly boot_info_mb = { > + .warn_msg = NULL, > + .err_msg = NULL Almost all of these fields are 0. It would be neater to only initialise the ones which need to be non-0. > +}; > + > unsigned long __init __init_mbi(u32 mbd_pa) > { > mbd_t *mbd = __va(mbd_pa); > @@ -68,3 +74,8 @@ unsigned long __init __init_mbi(u32 mbd_pa) > > return (unsigned long)&mbi; > } > + > +boot_info_t __init *__init_boot_info(u32 mbd_pa) No need for leading underscores on the function name. > +{ > + return &boot_info_mb; > +} > diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h > index 9458e7f..3da1a2a 100644 > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -255,6 +255,7 @@ static void __init noreturn efi_arch_post_exit_boot(void) > [ds] "r" (__HYPERVISOR_DS), > [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)), > "D" (__va(&mbi)) > + "S" (__va(&boot_info_efi)) > : "memory" ); > for( ; ; ); /* not reached */ > } > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index eacb9b2..18fa3e4 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -49,6 +49,7 @@ > #include > #include > #include > +#include > > /* opt_nosmp: If true, secondary processors are ignored. */ > static bool_t __initdata opt_nosmp; > @@ -92,6 +93,8 @@ unsigned long __initdata highmem_start; > size_param("highmem-start", highmem_start); > #endif > > +boot_info_t *boot_info; > + > cpumask_t __read_mostly cpu_present_map; > > unsigned long __read_mostly xen_phys_start; > @@ -546,7 +549,7 @@ void __init enable_bsp_exception_support(void) > sort_exception_tables(); > } > > -void __init noreturn __start_xen(unsigned long mbi_p) > +void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr) > { > char *memmap_type = NULL; > char *cmdline, *kextra, *loader; > @@ -563,6 +566,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) > .stop_bits = 1 > }; > > + boot_info = boot_info_ptr; > + > if ( efi_enabled ) > { > enable_bsp_exception_support(); > @@ -612,6 +617,12 @@ void __init noreturn __start_xen(unsigned long mbi_p) > ehci_dbgp_init(); > console_init_preirq(); > > + if ( boot_info->err_msg ) > + panic(boot_info->err_msg); panic("Fatal error from early boot: %s", boot_info->err_msg); although I suspect any fatal error from boot will prevent us from reaching this point. > + > + if ( boot_info->warn_msg ) > + printk(boot_info->warn_msg); printk(XENLOG_WARNING "Warning from early boot: %s\n", boot_info->warn_msg); ~Andrew > + > printk("Bootloader: %s\n", loader); > > printk("Command line: %s\n", cmdline); > diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h > index bee3b77..526f57c 100644 > --- a/xen/common/efi/efi.h > +++ b/xen/common/efi/efi.h > @@ -8,6 +8,9 @@ > #include > #include > #include > +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */ > +#include > +#endif > > struct efi_pci_rom { > const struct efi_pci_rom *next; > @@ -25,6 +28,10 @@ extern const CHAR16 *efi_fw_vendor; > > extern EFI_RUNTIME_SERVICES *efi_rs; > > +#ifndef CONFIG_ARM /* TODO - boot_info is not implemented on ARM yet */ > +extern boot_info_t boot_info_efi; > +#endif > + > extern UINTN efi_memmap_size, efi_mdesc_size; > extern void *efi_memmap; > > diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c > index 1c43d10..eb0acae 100644 > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -5,6 +5,9 @@ > #include > #include > #include > +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */ > +#include > +#endif > > DEFINE_XEN_GUEST_HANDLE(CHAR16); > > @@ -50,6 +53,13 @@ struct efi __read_mostly efi = { > const struct efi_pci_rom *__read_mostly efi_pci_roms; > > #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */ > +boot_info_t __read_mostly boot_info_efi = { > + .warn_msg = NULL, > + .err_msg = NULL > +}; > +#endif > + > +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */ > unsigned long efi_rs_enter(void) > { > static const u16 fcw = FCW_DEFAULT; > diff --git a/xen/include/asm-x86/boot_info.h b/xen/include/asm-x86/boot_info.h > new file mode 100644 > index 0000000..f0a76b2 > --- /dev/null > +++ b/xen/include/asm-x86/boot_info.h > @@ -0,0 +1,47 @@ > +/* > + * Copyright (c) 2013, 2014 Oracle Corp. > + * Author: Daniel Kiper > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program. If not, see . > + */ > + > +#ifndef __BOOT_INFO_H__ > +#define __BOOT_INFO_H__ > + > +/* > + * Define boot_info type. It will be used to define variable which in turn > + * will store data collected by bootloader and preloader. This way it will > + * be possible to make most of Xen code bootloader agnostic. > + * > + * Some members should have relevant EFI/ACPI types. However, due to type > + * conflicts among ACPI and EFI headers it is not possible to use required > + * EFI/ACPI types here. Instead of them there are simple types in use which > + * are compatible as much as possible with relevant EFI/ACPI types. > + */ > +typedef struct { > + /* > + * Info about warning occurred during boot_info initialization. > + * NULL if everything went OK. > + */ > + const char *warn_msg; > + > + /* > + * Info about error occurred during boot_info initialization. NULL if everything > + * went OK. Otherwise boot_info is not fully/properly initialized. > + */ > + const char *err_msg; > +} boot_info_t; > + > +extern boot_info_t *boot_info; > +#endif /* __BOOT_INFO_H__ */