Patchwork [PATCHv3,14/14] Pass boot device list to firmware.

login
register
mail settings
Submitter Gleb Natapov
Date Nov. 10, 2010, 5:14 p.m.
Message ID <1289409261-5418-15-git-send-email-gleb@redhat.com>
Download mbox | patch
Permalink /patch/70665/
State New
Headers show

Comments

Gleb Natapov - Nov. 10, 2010, 5:14 p.m.
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/fw_cfg.h |    1 +
 hw/pc.c     |   19 +++++++++++++++++++
 sysemu.h    |    1 +
 vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 0 deletions(-)
Blue Swirl - Nov. 10, 2010, 6:50 p.m.
On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote:
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  hw/fw_cfg.h |    1 +
>  hw/pc.c     |   19 +++++++++++++++++++
>  sysemu.h    |    1 +
>  vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 4d13a4f..bfbf1f9 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -31,6 +31,7 @@
>  #define FW_CFG_FILE_FIRST       0x20
>  #define FW_CFG_FILE_SLOTS       0x10
>  #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
> +#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)

You should bump FW_CFG_MAX_ENTRY instead.

>  #define FW_CFG_WRITE_CHANNEL    0x4000
>  #define FW_CFG_ARCH_LOCAL       0x8000
> diff --git a/hw/pc.c b/hw/pc.c
> index 3bf3862..e51f8ba 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -883,6 +883,21 @@ void pc_cpus_init(const char *cpu_model)
>     }
>  }
>
> +static struct machine_ready_notify {
> +    Notifier n;
> +    void *fw_cfg;
> +} machine_ready;
> +
> +
> +static void pc_fw_register_bootindex(struct Notifier* n)
> +{
> +    uint32_t len;
> +    char *bootindex = get_boot_devices_list(&len);
> +
> +    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
> +                     (uint8_t*)bootindex, len);
> +}
> +
>  void pc_memory_init(ram_addr_t ram_size,
>                     const char *kernel_filename,
>                     const char *kernel_cmdline,
> @@ -982,6 +997,10 @@ void pc_memory_init(ram_addr_t ram_size,
>     for (i = 0; i < nb_option_roms; i++) {
>         rom_add_option(option_rom[i]);
>     }
> +
> +    machine_ready.n.notify = pc_fw_register_bootindex;
> +    machine_ready.fw_cfg = fw_cfg;
> +    qemu_add_machine_init_done_notifier(&machine_ready.n);
>  }
>
>  qemu_irq *pc_allocate_cpu_irq(void)
> diff --git a/sysemu.h b/sysemu.h
> index 46a588c..bac2df1 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -197,4 +197,5 @@ void register_devices(void);
>
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>                           const char *suffix);
> +char *get_boot_devices_list(uint32_t *size);
>  #endif
> diff --git a/vl.c b/vl.c
> index 00ab2b4..4b1a406 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -734,6 +734,46 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>     QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
>  }
>
> +char *get_boot_devices_list(uint32_t *size)
> +{
> +    FWBootEntry *i;
> +    uint32_t total = 1, c = 0;
> +    char *list = qemu_malloc(1);
> +
> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +        char *devpath = NULL, *bootpath;
> +        int len;
> +
> +        if (i->dev) {
> +            devpath = qdev_get_fw_dev_path(i->dev);
> +            assert(devpath);
> +        }
> +
> +        if (i->suffix && devpath) {
> +            bootpath = qemu_malloc(strlen(devpath) + strlen(i->suffix) + 2);
> +            sprintf(bootpath, "%s/%s", devpath, i->suffix);
> +            qemu_free(devpath);
> +        } else if (devpath) {
> +            bootpath = devpath;
> +        } else {
> +            bootpath = strdup(i->suffix);
> +        }
> +
> +        len = strlen(bootpath);
> +        list = qemu_realloc(list, total + len + 1);
> +        list[total++] = len;
> +        memcpy(&list[total], bootpath, len);
> +        total += len;
> +        c++;
> +        qemu_free(bootpath);
> +    }
> +
> +    list[0] = c;
> +    *size = total;
> +
> +    return list;
> +}

The format of the list should be documented, it wasn't immediately
obvious where '1' comes from in qemu_malloc(1).
Blue Swirl - Nov. 10, 2010, 7:11 p.m.
On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote:
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  hw/fw_cfg.h |    1 +
>  hw/pc.c     |   19 +++++++++++++++++++
>  sysemu.h    |    1 +
>  vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 4d13a4f..bfbf1f9 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -31,6 +31,7 @@
>  #define FW_CFG_FILE_FIRST       0x20
>  #define FW_CFG_FILE_SLOTS       0x10
>  #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
> +#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)
>
>  #define FW_CFG_WRITE_CHANNEL    0x4000
>  #define FW_CFG_ARCH_LOCAL       0x8000
> diff --git a/hw/pc.c b/hw/pc.c
> index 3bf3862..e51f8ba 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -883,6 +883,21 @@ void pc_cpus_init(const char *cpu_model)
>     }
>  }
>
> +static struct machine_ready_notify {
> +    Notifier n;
> +    void *fw_cfg;
> +} machine_ready;
> +
> +
> +static void pc_fw_register_bootindex(struct Notifier* n)
> +{
> +    uint32_t len;
> +    char *bootindex = get_boot_devices_list(&len);
> +
> +    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
> +                     (uint8_t*)bootindex, len);
> +}
> +
>  void pc_memory_init(ram_addr_t ram_size,
>                     const char *kernel_filename,
>                     const char *kernel_cmdline,
> @@ -982,6 +997,10 @@ void pc_memory_init(ram_addr_t ram_size,
>     for (i = 0; i < nb_option_roms; i++) {
>         rom_add_option(option_rom[i]);
>     }
> +
> +    machine_ready.n.notify = pc_fw_register_bootindex;
> +    machine_ready.fw_cfg = fw_cfg;
> +    qemu_add_machine_init_done_notifier(&machine_ready.n);

Actually this and the above should be moved to generic code, like
vl.c. I doubt this would be any different for other platforms.
Gleb Natapov - Nov. 10, 2010, 7:16 p.m.
On Wed, Nov 10, 2010 at 07:11:54PM +0000, Blue Swirl wrote:
> On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> >  hw/fw_cfg.h |    1 +
> >  hw/pc.c     |   19 +++++++++++++++++++
> >  sysemu.h    |    1 +
> >  vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 61 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> > index 4d13a4f..bfbf1f9 100644
> > --- a/hw/fw_cfg.h
> > +++ b/hw/fw_cfg.h
> > @@ -31,6 +31,7 @@
> >  #define FW_CFG_FILE_FIRST       0x20
> >  #define FW_CFG_FILE_SLOTS       0x10
> >  #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
> > +#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)
> >
> >  #define FW_CFG_WRITE_CHANNEL    0x4000
> >  #define FW_CFG_ARCH_LOCAL       0x8000
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 3bf3862..e51f8ba 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -883,6 +883,21 @@ void pc_cpus_init(const char *cpu_model)
> >     }
> >  }
> >
> > +static struct machine_ready_notify {
> > +    Notifier n;
> > +    void *fw_cfg;
> > +} machine_ready;
> > +
> > +
> > +static void pc_fw_register_bootindex(struct Notifier* n)
> > +{
> > +    uint32_t len;
> > +    char *bootindex = get_boot_devices_list(&len);
> > +
> > +    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
> > +                     (uint8_t*)bootindex, len);
> > +}
> > +
> >  void pc_memory_init(ram_addr_t ram_size,
> >                     const char *kernel_filename,
> >                     const char *kernel_cmdline,
> > @@ -982,6 +997,10 @@ void pc_memory_init(ram_addr_t ram_size,
> >     for (i = 0; i < nb_option_roms; i++) {
> >         rom_add_option(option_rom[i]);
> >     }
> > +
> > +    machine_ready.n.notify = pc_fw_register_bootindex;
> > +    machine_ready.fw_cfg = fw_cfg;
> > +    qemu_add_machine_init_done_notifier(&machine_ready.n);
> 
> Actually this and the above should be moved to generic code, like
> vl.c. I doubt this would be any different for other platforms.
Agree, but we do not have reference to fw_cfg in generic code.

--
			Gleb.
Blue Swirl - Nov. 10, 2010, 7:20 p.m.
2010/11/10 Gleb Natapov <gleb@redhat.com>:
> On Wed, Nov 10, 2010 at 07:11:54PM +0000, Blue Swirl wrote:
>> On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> >
>> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
>> > ---
>> >  hw/fw_cfg.h |    1 +
>> >  hw/pc.c     |   19 +++++++++++++++++++
>> >  sysemu.h    |    1 +
>> >  vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
>> >  4 files changed, 61 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
>> > index 4d13a4f..bfbf1f9 100644
>> > --- a/hw/fw_cfg.h
>> > +++ b/hw/fw_cfg.h
>> > @@ -31,6 +31,7 @@
>> >  #define FW_CFG_FILE_FIRST       0x20
>> >  #define FW_CFG_FILE_SLOTS       0x10
>> >  #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
>> > +#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)
>> >
>> >  #define FW_CFG_WRITE_CHANNEL    0x4000
>> >  #define FW_CFG_ARCH_LOCAL       0x8000
>> > diff --git a/hw/pc.c b/hw/pc.c
>> > index 3bf3862..e51f8ba 100644
>> > --- a/hw/pc.c
>> > +++ b/hw/pc.c
>> > @@ -883,6 +883,21 @@ void pc_cpus_init(const char *cpu_model)
>> >     }
>> >  }
>> >
>> > +static struct machine_ready_notify {
>> > +    Notifier n;
>> > +    void *fw_cfg;
>> > +} machine_ready;
>> > +
>> > +
>> > +static void pc_fw_register_bootindex(struct Notifier* n)
>> > +{
>> > +    uint32_t len;
>> > +    char *bootindex = get_boot_devices_list(&len);
>> > +
>> > +    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
>> > +                     (uint8_t*)bootindex, len);
>> > +}
>> > +
>> >  void pc_memory_init(ram_addr_t ram_size,
>> >                     const char *kernel_filename,
>> >                     const char *kernel_cmdline,
>> > @@ -982,6 +997,10 @@ void pc_memory_init(ram_addr_t ram_size,
>> >     for (i = 0; i < nb_option_roms; i++) {
>> >         rom_add_option(option_rom[i]);
>> >     }
>> > +
>> > +    machine_ready.n.notify = pc_fw_register_bootindex;
>> > +    machine_ready.fw_cfg = fw_cfg;
>> > +    qemu_add_machine_init_done_notifier(&machine_ready.n);
>>
>> Actually this and the above should be moved to generic code, like
>> vl.c. I doubt this would be any different for other platforms.
> Agree, but we do not have reference to fw_cfg in generic code.

How about inside fw_cfg.c then?
Gleb Natapov - Nov. 11, 2010, 7:03 a.m.
On Wed, Nov 10, 2010 at 07:20:51PM +0000, Blue Swirl wrote:
> 2010/11/10 Gleb Natapov <gleb@redhat.com>:
> > On Wed, Nov 10, 2010 at 07:11:54PM +0000, Blue Swirl wrote:
> >> On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >> >
> >> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> >> > ---
> >> >  hw/fw_cfg.h |    1 +
> >> >  hw/pc.c     |   19 +++++++++++++++++++
> >> >  sysemu.h    |    1 +
> >> >  vl.c        |   40 ++++++++++++++++++++++++++++++++++++++++
> >> >  4 files changed, 61 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> >> > index 4d13a4f..bfbf1f9 100644
> >> > --- a/hw/fw_cfg.h
> >> > +++ b/hw/fw_cfg.h
> >> > @@ -31,6 +31,7 @@
> >> >  #define FW_CFG_FILE_FIRST       0x20
> >> >  #define FW_CFG_FILE_SLOTS       0x10
> >> >  #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
> >> > +#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)
> >> >
> >> >  #define FW_CFG_WRITE_CHANNEL    0x4000
> >> >  #define FW_CFG_ARCH_LOCAL       0x8000
> >> > diff --git a/hw/pc.c b/hw/pc.c
> >> > index 3bf3862..e51f8ba 100644
> >> > --- a/hw/pc.c
> >> > +++ b/hw/pc.c
> >> > @@ -883,6 +883,21 @@ void pc_cpus_init(const char *cpu_model)
> >> >     }
> >> >  }
> >> >
> >> > +static struct machine_ready_notify {
> >> > +    Notifier n;
> >> > +    void *fw_cfg;
> >> > +} machine_ready;
> >> > +
> >> > +
> >> > +static void pc_fw_register_bootindex(struct Notifier* n)
> >> > +{
> >> > +    uint32_t len;
> >> > +    char *bootindex = get_boot_devices_list(&len);
> >> > +
> >> > +    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
> >> > +                     (uint8_t*)bootindex, len);
> >> > +}
> >> > +
> >> >  void pc_memory_init(ram_addr_t ram_size,
> >> >                     const char *kernel_filename,
> >> >                     const char *kernel_cmdline,
> >> > @@ -982,6 +997,10 @@ void pc_memory_init(ram_addr_t ram_size,
> >> >     for (i = 0; i < nb_option_roms; i++) {
> >> >         rom_add_option(option_rom[i]);
> >> >     }
> >> > +
> >> > +    machine_ready.n.notify = pc_fw_register_bootindex;
> >> > +    machine_ready.fw_cfg = fw_cfg;
> >> > +    qemu_add_machine_init_done_notifier(&machine_ready.n);
> >>
> >> Actually this and the above should be moved to generic code, like
> >> vl.c. I doubt this would be any different for other platforms.
> > Agree, but we do not have reference to fw_cfg in generic code.
> 
> How about inside fw_cfg.c then?
Will move it there.

--
			Gleb.

Patch

diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 4d13a4f..bfbf1f9 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -31,6 +31,7 @@ 
 #define FW_CFG_FILE_FIRST       0x20
 #define FW_CFG_FILE_SLOTS       0x10
 #define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_BOOTINDEX        (FW_CFG_MAX_ENTRY + 1)
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
 #define FW_CFG_ARCH_LOCAL       0x8000
diff --git a/hw/pc.c b/hw/pc.c
index 3bf3862..e51f8ba 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -883,6 +883,21 @@  void pc_cpus_init(const char *cpu_model)
     }
 }
 
+static struct machine_ready_notify {
+    Notifier n;
+    void *fw_cfg;
+} machine_ready;
+
+
+static void pc_fw_register_bootindex(struct Notifier* n)
+{
+    uint32_t len;
+    char *bootindex = get_boot_devices_list(&len);
+
+    fw_cfg_add_bytes(machine_ready.fw_cfg, FW_CFG_BOOTINDEX,
+                     (uint8_t*)bootindex, len);
+}
+
 void pc_memory_init(ram_addr_t ram_size,
                     const char *kernel_filename,
                     const char *kernel_cmdline,
@@ -982,6 +997,10 @@  void pc_memory_init(ram_addr_t ram_size,
     for (i = 0; i < nb_option_roms; i++) {
         rom_add_option(option_rom[i]);
     }
+
+    machine_ready.n.notify = pc_fw_register_bootindex;
+    machine_ready.fw_cfg = fw_cfg;
+    qemu_add_machine_init_done_notifier(&machine_ready.n);
 }
 
 qemu_irq *pc_allocate_cpu_irq(void)
diff --git a/sysemu.h b/sysemu.h
index 46a588c..bac2df1 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -197,4 +197,5 @@  void register_devices(void);
 
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
+char *get_boot_devices_list(uint32_t *size);
 #endif
diff --git a/vl.c b/vl.c
index 00ab2b4..4b1a406 100644
--- a/vl.c
+++ b/vl.c
@@ -734,6 +734,46 @@  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
     QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
 }
 
+char *get_boot_devices_list(uint32_t *size)
+{
+    FWBootEntry *i;
+    uint32_t total = 1, c = 0;
+    char *list = qemu_malloc(1);
+
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        char *devpath = NULL, *bootpath;
+        int len;
+
+        if (i->dev) {
+            devpath = qdev_get_fw_dev_path(i->dev);
+            assert(devpath);
+        }
+
+        if (i->suffix && devpath) {
+            bootpath = qemu_malloc(strlen(devpath) + strlen(i->suffix) + 2);
+            sprintf(bootpath, "%s/%s", devpath, i->suffix);
+            qemu_free(devpath);
+        } else if (devpath) {
+            bootpath = devpath;
+        } else {
+            bootpath = strdup(i->suffix);
+        }
+
+        len = strlen(bootpath);
+        list = qemu_realloc(list, total + len + 1);
+        list[total++] = len;
+        memcpy(&list[total], bootpath, len);
+        total += len;
+        c++;
+        qemu_free(bootpath);
+    }
+
+    list[0] = c;
+    *size = total;
+
+    return list;
+}
+
 static void numa_add(const char *optarg)
 {
     char option[128];