Re: [Xen-devel] [PATCH for-xen-4.5 v3 06/16] x86: Introduce boot_info structure

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/10/14 18:52, 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
>
> WARNING: ARM build has not been tested yet.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
> 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     |    3 ++-
>  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 |   46 
> +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 96 insertions(+), 4 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 ef8c735..647c33b 100644
> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -32,9 +32,15 @@
>          /* Init mbi. */
>          mov     mbd_pa(%rip),%edi
>          call    __init_mbi
> +        pushq   %rax
> +
> +        /* Init boot_info. */
> +        mov     mbd_pa(%rip),%edi
> +        call    __init_boot_info
>  
> -        /* Pass off the mbi to C land. */
> -        movq    %rax,%rdi
> +        /* Pass off the mbi and 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 a2799aa..83bd255 100644
> --- a/xen/arch/x86/boot_info.c
> +++ b/xen/arch/x86/boot_info.c
> @@ -20,11 +20,17 @@
>  #include <xen/init.h>
>  #include <xen/multiboot.h>
>  
> +#include <asm/boot_info.h>
>  #include <asm/mbd.h>
>  #include <asm/page.h>
>  
>  static multiboot_info_t __read_mostly mbi;
>  
> +static boot_info_t __read_mostly boot_info_mb = {
> +    .warn_msg = NULL,
> +    .err_msg = NULL
> +};

You don't need to explicitly 0 things like this with structure
initialisation.

> +
>  extern void enable_exception_support(void);
>  
>  unsigned long __init __init_mbi(u32 mbd_pa)
> @@ -57,3 +63,8 @@ unsigned long __init __init_mbi(u32 mbd_pa)
>  
>      return __pa(&mbi);
>  }
> +
> +paddr_t __init __init_boot_info(u32 mbd_pa)
> +{
> +    return __pa(&boot_info_mb);
> +}
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 71030b0..6d7c222 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -254,7 +254,8 @@ static void __init noreturn efi_arch_post_exit_boot(void)
>                       [cs] "ir" (__HYPERVISOR_CS),
>                       [ds] "r" (__HYPERVISOR_DS),
>                       [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)),
> -                     "D" (&mbi)
> +                     "D" (&mbi),
> +                     "S" (&boot_info_efi)
>                     : "memory" );
>      for( ; ; ); /* not reached */
>  }
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 24e1be3..d2a1450 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -49,6 +49,7 @@
>  #include <xen/cpu.h>
>  #include <asm/nmi.h>
>  #include <asm/alternative.h>
> +#include <asm/boot_info.h>
>  
>  /* 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;
> @@ -548,7 +551,7 @@ void __init enable_exception_support(void)
>      /* Full exception support from here on in. */
>  }
>  
> -void __init noreturn __start_xen(unsigned long mbi_p)
> +void __init noreturn __start_xen(unsigned long mbi_p, paddr_t boot_info_pa)
>  {
>      char *memmap_type = NULL;
>      char *cmdline, *kextra, *loader;
> @@ -565,6 +568,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>          .stop_bits = 1
>      };
>  
> +    boot_info = __va(boot_info_pa);
> +
>      if ( efi_enabled ) {
>          enable_exception_support();
>      }
> @@ -613,6 +618,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);
> +
> +    if ( boot_info->warn_msg )
> +        printk(boot_info->warn_msg);
> +
>      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 <xen/efi.h>
>  #include <xen/spinlock.h>
>  #include <asm/page.h>
> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> +#include <asm/boot_info.h>
> +#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 <xen/guest_access.h>
>  #include <xen/irq.h>
>  #include <xen/time.h>
> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> +#include <asm/boot_info.h>
> +#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..9ff3c0f
> --- /dev/null
> +++ b/xen/include/asm-x86/boot_info.h
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright (c) 2013, 2014 Oracle Co., 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 <http://www.gnu.org/licenses/>.
> + */
> +
> +#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.
> +     */
> +    char *warn_msg;

const char *

~Andrew

> +
> +    /*
> +     * Info about error occurred during boot_info initialization. NULL if 
> everything
> +     * went OK. Otherwise boot_info is not fully/properly initialized.
> +     */
> +    char *err_msg;
> +} boot_info_t;
> +
> +extern boot_info_t *boot_info;
> +#endif /* __BOOT_INFO_H__ */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel