Re: [Qemu-devel] [PATCH] Add bootloader name to multiboot implementation

[Adam Lackorzynski <adam@os.inf.tu-dresden.de>]

On Wed Oct 29, 2014 at 23:52:03 -0600, Drew DeVault wrote:
> The name is set to "qemu".
> 
> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
> For the future, it may be useful to add a command line flag for setting this to
> some user-specified value. I also considered naming it "qemu-system-i386" or
> "qemu-system-x86_64" (as appropriate), but couldn't find an easy way to get
> those strings and decided it didn't matter.
> 
>  hw/i386/multiboot.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 985ca1e..f86d351 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -54,6 +54,7 @@ enum {
>      MBI_MODS_COUNT  = 20,
>      MBI_MODS_ADDR   = 24,
>      MBI_MMAP_ADDR   = 48,
> +    MBI_BOOTLOADER  = 64,
>  
>      MBI_SIZE        = 88,
>  
> @@ -74,6 +75,7 @@ enum {
>      MULTIBOOT_FLAGS_CMDLINE     = 1 << 2,
>      MULTIBOOT_FLAGS_MODULES     = 1 << 3,
>      MULTIBOOT_FLAGS_MMAP        = 1 << 6,
> +    MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
>  };
>  
>  typedef struct {
> @@ -87,6 +89,8 @@ typedef struct {
>      hwaddr offset_mbinfo;
>      /* offset in buffer for cmdlines in bytes */
>      hwaddr offset_cmdlines;
> +    /* offset in buffer for bootloader name in bytes */
> +    hwaddr offset_bootloader;
>      /* offset of modules in bytes */
>      hwaddr offset_mods;
>      /* available slots for mb modules infos */
> @@ -95,6 +99,8 @@ typedef struct {
>      int mb_mods_count;
>  } MultibootState;
>  
> +const char *bootloader_name = "qemu";
> +
>  static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
>  {
>      hwaddr p = s->offset_cmdlines;
> @@ -105,6 +111,16 @@ static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
>      return s->mb_buf_phys + p;
>  }
>  
> +static uint32_t mb_add_bootloader(MultibootState *s, const char *bootloader)
> +{
> +    hwaddr p = s->offset_bootloader;
> +    char *b = (char *)s->mb_buf + p;
> +
> +    memcpy(b, bootloader, strlen(bootloader) + 1);
> +    s->offset_bootloader += strlen(b) + 1;
> +    return s->mb_buf_phys + p;
> +}
> +
>  static void mb_add_mod(MultibootState *s,
>                         hwaddr start, hwaddr end,
>                         hwaddr cmdline_phys)
> @@ -241,9 +257,10 @@ int load_multiboot(FWCfgState *fw_cfg,
>      mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_kernel_size);
>      mbs.offset_mbinfo = mbs.mb_buf_size;
>  
> -    /* Calculate space for cmdlines and mb_mods */
> +    /* Calculate space for cmdlines, bootloader name, and mb_mods */
>      mbs.mb_buf_size += strlen(kernel_filename) + 1;
>      mbs.mb_buf_size += strlen(kernel_cmdline) + 1;
> +    mbs.mb_buf_size += strlen(bootloader_name) + 1;
>      if (initrd_filename) {
>          const char *r = initrd_filename;
>          mbs.mb_buf_size += strlen(r) + 1;
> @@ -257,9 +274,11 @@ int load_multiboot(FWCfgState *fw_cfg,
>  
>      mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
>  
> -    /* enlarge mb_buf to hold cmdlines and mb-info structs */
> -    mbs.mb_buf          = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
> -    mbs.offset_cmdlines = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
> +    /* enlarge mb_buf to hold cmdlines, bootloader, mb-info structs */
> +    mbs.mb_buf            = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
> +    mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
> +    mbs.offset_bootloader = mbs.offset_cmdlines + strlen(kernel_filename) + 1 
> +                            + strlen(kernel_cmdline) + 1;
>  
>      if (initrd_filename) {
>          char *next_initrd, not_last;
> @@ -306,6 +325,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>               kernel_filename, kernel_cmdline);
>      stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline));
>  
> +    stl_p(bootinfo + MBI_BOOTLOADER, mb_add_bootloader(&mbs, bootloader_name));
> +
>      stl_p(bootinfo + MBI_MODS_ADDR,  mbs.mb_buf_phys + mbs.offset_mbinfo);
>      stl_p(bootinfo + MBI_MODS_COUNT, mbs.mb_mods_count); /* mods_count */
>  
> @@ -314,7 +335,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>                                  | MULTIBOOT_FLAGS_BOOT_DEVICE
>                                  | MULTIBOOT_FLAGS_CMDLINE
>                                  | MULTIBOOT_FLAGS_MODULES
> -                                | MULTIBOOT_FLAGS_MMAP);
> +                                | MULTIBOOT_FLAGS_MMAP
> +                                | MULTIBOOT_FLAGS_BOOTLOADER);
>      stl_p(bootinfo + MBI_BOOT_DEVICE, 0x8000ffff); /* XXX: use the -boot switch? */
>      stl_p(bootinfo + MBI_MMAP_ADDR,   ADDR_E820_MAP);
>  

Looks good to me and no negative impact on a simple test.

Reviewed-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>



Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/