From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH for-xen-4.5 v4 09/18] x86: move boot_loader_name from mbi to boot_info Date: Fri, 17 Oct 2014 22:05:16 +0100 Message-ID: <5441848C.20502@citrix.com> References: <1413555132-22138-1-git-send-email-daniel.kiper@oracle.com> <1413555132-22138-10-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.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XfEi6-0003Cu-Ee for xen-devel@lists.xenproject.org; Fri, 17 Oct 2014 21:05:18 +0000 In-Reply-To: <1413555132-22138-10-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: > Signed-off-by: Daniel Kiper > --- > v4 - suggestions/fixes: > - change boot_loader_name type to "const char *" > (suggested by Andrew Cooper). > --- > xen/arch/x86/boot_info.c | 12 ++++++------ > xen/arch/x86/efi/efi-boot.h | 3 +-- > xen/arch/x86/setup.c | 13 +++++-------- > xen/common/efi/runtime.c | 1 + > xen/include/asm-x86/boot_info.h | 3 +++ > 5 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c > index c5f2a18..a81e98b 100644 > --- a/xen/arch/x86/boot_info.c > +++ b/xen/arch/x86/boot_info.c > @@ -29,6 +29,7 @@ > static multiboot_info_t __read_mostly mbi; > > static boot_info_t __read_mostly boot_info_mb = { > + .boot_loader_name = "UNKNOWN", This should be lowercase if you wish to match the code removed lower down. > .warn_msg = NULL, > .err_msg = NULL > }; > @@ -39,12 +40,6 @@ unsigned long __init __init_mbi(u32 mbd_pa) > > enable_bsp_exception_support(); > > - if ( mbd->boot_loader_name ) > - { > - mbi.flags = MBI_LOADERNAME; I have only noticed in this patch, but the above line should strictly be an |=, when it is introduced in the earlier patch. > - mbi.boot_loader_name = mbd->boot_loader_name; > - } > - > if ( mbd->cmdline ) > { > mbi.flags |= MBI_CMDLINE; > @@ -77,5 +72,10 @@ unsigned long __init __init_mbi(u32 mbd_pa) > > boot_info_t __init *__init_boot_info(u32 mbd_pa) > { > + mbd_t *mbd = __va(mbd_pa); > + > + if ( mbd->boot_loader_name ) > + boot_info_mb.boot_loader_name = __va(mbd->boot_loader_name); > + > return &boot_info_mb; > } > diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h > index 3da1a2a..19be165 100644 > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -11,7 +11,7 @@ > > static struct file __initdata ucode; > static multiboot_info_t __initdata mbi = { > - .flags = MBI_MODULES | MBI_LOADERNAME > + .flags = MBI_MODULES > }; > static module_t __initdata mb_modules[3]; > > @@ -310,7 +310,6 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name, > * These must not be initialized statically, since the value must > * not get relocated when processing base relocations later. > */ > - mbi.boot_loader_name = (long)"EFI"; > mbi.mods_addr = (long)mb_modules; > } > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 18fa3e4..a9dbc05 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -552,7 +552,7 @@ void __init enable_bsp_exception_support(void) > void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr) > { > char *memmap_type = NULL; > - char *cmdline, *kextra, *loader; > + char *cmdline, *kextra; > unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity; > multiboot_info_t *mbi = (multiboot_info_t *)mbi_p; > module_t *mod = (module_t *)__va(mbi->mods_addr); > @@ -577,13 +577,10 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr > /* Exception support was enabled before __start_xen() call. */ > } > > - loader = (mbi->flags & MBI_LOADERNAME) > - ? (char *)__va(mbi->boot_loader_name) : "unknown"; > - > /* Parse the command-line options. */ > cmdline = cmdline_cook((mbi->flags & MBI_CMDLINE) ? > __va(mbi->cmdline) : NULL, > - loader); > + boot_info->boot_loader_name); > if ( (kextra = strstr(cmdline, " -- ")) != NULL ) > { > /* > @@ -623,7 +620,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr > if ( boot_info->warn_msg ) > printk(boot_info->warn_msg); > > - printk("Bootloader: %s\n", loader); > + printk("Bootloader: %s\n", boot_info->boot_loader_name); > > printk("Command line: %s\n", cmdline); > > @@ -697,7 +694,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr > l3_bootmap[l3_table_offset(BOOTSTRAP_MAP_BASE)] = > l3e_from_paddr(__pa(l2_bootmap), __PAGE_HYPERVISOR); > > - memmap_type = loader; > + memmap_type = boot_info->boot_loader_name; > } > else if ( e820_raw_nr != 0 ) > { > @@ -1379,7 +1376,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr > { > static char __initdata dom0_cmdline[MAX_GUEST_CMDLINE]; > > - cmdline = cmdline_cook(cmdline, loader); > + cmdline = cmdline_cook(cmdline, boot_info->boot_loader_name); > safe_strcpy(dom0_cmdline, cmdline); > > if ( kextra != NULL ) > diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c > index eb0acae..7846b2b 100644 > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -54,6 +54,7 @@ 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 = { > + .boot_loader_name = "EFI", > .warn_msg = NULL, > .err_msg = NULL > }; > diff --git a/xen/include/asm-x86/boot_info.h b/xen/include/asm-x86/boot_info.h > index f0a76b2..9e68447 100644 > --- a/xen/include/asm-x86/boot_info.h > +++ b/xen/include/asm-x86/boot_info.h > @@ -30,6 +30,9 @@ > * are compatible as much as possible with relevant EFI/ACPI types. > */ > typedef struct { > + /* Boot loader name. */ > + const char *boot_loader_name; > + I think this can reasonably be named simply "loader". ~Andrew > /* > * Info about warning occurred during boot_info initialization. > * NULL if everything went OK.