Patchwork [5/5] Add bootindex parameter to net/block/fd device

login
register
mail settings
Submitter Gleb Natapov
Date Oct. 26, 2010, 10:48 a.m.
Message ID <1288090091-25874-6-git-send-email-gleb@redhat.com>
Download mbox | patch
Permalink /patch/69227/
State New
Headers show

Comments

Gleb Natapov - Oct. 26, 2010, 10:48 a.m.
If bootindex is specified on command line a string that describes device
in firmware readable way is added into sorted list. Later this list will
be passed into firmware to control boot order.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 block_int.h     |    4 +++-
 hw/fdc.c        |   36 ++++++++++++++++++++++++++++++++++++
 hw/ide/qdev.c   |   24 ++++++++++++++++++++++++
 hw/virtio-blk.c |   20 ++++++++++++++++++++
 hw/virtio-net.c |   20 ++++++++++++++++++++
 net.h           |    4 +++-
 sysemu.h        |    9 +++++++++
 vl.c            |   24 ++++++++++++++++++++++++
 8 files changed, 139 insertions(+), 2 deletions(-)
Anthony Liguori - Oct. 26, 2010, 1:29 p.m.
On 10/26/2010 05:48 AM, Gleb Natapov wrote:
> If bootindex is specified on command line a string that describes device
> in firmware readable way is added into sorted list. Later this list will
> be passed into firmware to control boot order.
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> ---
>   block_int.h     |    4 +++-
>   hw/fdc.c        |   36 ++++++++++++++++++++++++++++++++++++
>   hw/ide/qdev.c   |   24 ++++++++++++++++++++++++
>   hw/virtio-blk.c |   20 ++++++++++++++++++++
>   hw/virtio-net.c |   20 ++++++++++++++++++++
>   net.h           |    4 +++-
>   sysemu.h        |    9 +++++++++
>   vl.c            |   24 ++++++++++++++++++++++++
>   8 files changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/block_int.h b/block_int.h
> index e8e7156..60e7be2 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -225,6 +225,7 @@ typedef struct BlockConf {
>       uint16_t logical_block_size;
>       uint16_t min_io_size;
>       uint32_t opt_io_size;
> +    int32_t bootindex;
>   } BlockConf;
>
>   static inline unsigned int get_physical_block_exp(BlockConf *conf)
> @@ -247,6 +248,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
>       DEFINE_PROP_UINT16("physical_block_size", _state,                   \
>                          _conf.physical_block_size, 512),                 \
>       DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
> -    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
> +    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
> +    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)         \
>
>   #endif /* BLOCK_INT_H */
> diff --git a/hw/fdc.c b/hw/fdc.c
> index 1f38d0d..9d0dff5 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -35,6 +35,7 @@
>   #include "sysbus.h"
>   #include "qdev-addr.h"
>   #include "blockdev.h"
> +#include "sysemu.h"
>
>   /********************************************************/
>   /* debug Floppy devices */
> @@ -523,6 +524,8 @@ typedef struct FDCtrlSysBus {
>   typedef struct FDCtrlISABus {
>       ISADevice busdev;
>       struct FDCtrl state;
> +    int32_t bootindexA;
> +    int32_t bootindexB;
>   } FDCtrlISABus;
>
>   static uint32_t fdctrl_read (void *opaque, uint32_t reg)
> @@ -1974,6 +1977,7 @@ static int isabus_fdc_init1(ISADevice *dev)
>       int isairq = 6;
>       int dma_chann = 2;
>       int ret;
> +    char devpath[30], *bus_name;
>
>       register_ioport_read(iobase + 0x01, 5, 1,
>                            &fdctrl_read_port, fdctrl);
> @@ -1992,6 +1996,36 @@ static int isabus_fdc_init1(ISADevice *dev)
>       qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
>       ret = fdctrl_init_common(fdctrl);
>
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    if (isa->bootindexA<  0&&  isa->bootindexB<  0) {
> +        return 0;
> +    }
> +
> +    if (!dev->qdev.parent_bus->info->get_dev_path) {
> +        fprintf(stderr, "Can't create device path for floppy\n");
> +        return 0;
> +    }
> +
> +    bus_name = dev->qdev.parent_bus->info->get_dev_path(&dev->qdev);
> +
> +    if (isa->bootindexA>= 0) {
> +        snprintf(devpath, sizeof(devpath), "%s@%s/fd@a",
> +                 dev->qdev.parent_bus->info->name, bus_name);
> +
> +        add_boot_device_path(isa->bootindexA, strdup(devpath));
> +    }
>    

Even if we're passing a string to SeaBIOS, we should probably keep the 
components structured.  Looks like there's four pieces of data: 1) a bus 
name 2) a bus path 3) a device name 4) a device path.

We derive (1) from the qdev structure, why don't we derive (3) too?

Regards,

Anthony Liguori
Gleb Natapov - Oct. 26, 2010, 2:16 p.m.
On Tue, Oct 26, 2010 at 08:29:21AM -0500, Anthony Liguori wrote:
> On 10/26/2010 05:48 AM, Gleb Natapov wrote:
> >If bootindex is specified on command line a string that describes device
> >in firmware readable way is added into sorted list. Later this list will
> >be passed into firmware to control boot order.
> >
> >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >---
> >  block_int.h     |    4 +++-
> >  hw/fdc.c        |   36 ++++++++++++++++++++++++++++++++++++
> >  hw/ide/qdev.c   |   24 ++++++++++++++++++++++++
> >  hw/virtio-blk.c |   20 ++++++++++++++++++++
> >  hw/virtio-net.c |   20 ++++++++++++++++++++
> >  net.h           |    4 +++-
> >  sysemu.h        |    9 +++++++++
> >  vl.c            |   24 ++++++++++++++++++++++++
> >  8 files changed, 139 insertions(+), 2 deletions(-)
> >
> >diff --git a/block_int.h b/block_int.h
> >index e8e7156..60e7be2 100644
> >--- a/block_int.h
> >+++ b/block_int.h
> >@@ -225,6 +225,7 @@ typedef struct BlockConf {
> >      uint16_t logical_block_size;
> >      uint16_t min_io_size;
> >      uint32_t opt_io_size;
> >+    int32_t bootindex;
> >  } BlockConf;
> >
> >  static inline unsigned int get_physical_block_exp(BlockConf *conf)
> >@@ -247,6 +248,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
> >      DEFINE_PROP_UINT16("physical_block_size", _state,                   \
> >                         _conf.physical_block_size, 512),                 \
> >      DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
> >-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
> >+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
> >+    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)         \
> >
> >  #endif /* BLOCK_INT_H */
> >diff --git a/hw/fdc.c b/hw/fdc.c
> >index 1f38d0d..9d0dff5 100644
> >--- a/hw/fdc.c
> >+++ b/hw/fdc.c
> >@@ -35,6 +35,7 @@
> >  #include "sysbus.h"
> >  #include "qdev-addr.h"
> >  #include "blockdev.h"
> >+#include "sysemu.h"
> >
> >  /********************************************************/
> >  /* debug Floppy devices */
> >@@ -523,6 +524,8 @@ typedef struct FDCtrlSysBus {
> >  typedef struct FDCtrlISABus {
> >      ISADevice busdev;
> >      struct FDCtrl state;
> >+    int32_t bootindexA;
> >+    int32_t bootindexB;
> >  } FDCtrlISABus;
> >
> >  static uint32_t fdctrl_read (void *opaque, uint32_t reg)
> >@@ -1974,6 +1977,7 @@ static int isabus_fdc_init1(ISADevice *dev)
> >      int isairq = 6;
> >      int dma_chann = 2;
> >      int ret;
> >+    char devpath[30], *bus_name;
> >
> >      register_ioport_read(iobase + 0x01, 5, 1,
> >                           &fdctrl_read_port, fdctrl);
> >@@ -1992,6 +1996,36 @@ static int isabus_fdc_init1(ISADevice *dev)
> >      qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
> >      ret = fdctrl_init_common(fdctrl);
> >
> >+    if (ret) {
> >+        return ret;
> >+    }
> >+
> >+    if (isa->bootindexA<  0&&  isa->bootindexB<  0) {
> >+        return 0;
> >+    }
> >+
> >+    if (!dev->qdev.parent_bus->info->get_dev_path) {
> >+        fprintf(stderr, "Can't create device path for floppy\n");
> >+        return 0;
> >+    }
> >+
> >+    bus_name = dev->qdev.parent_bus->info->get_dev_path(&dev->qdev);
> >+
> >+    if (isa->bootindexA>= 0) {
> >+        snprintf(devpath, sizeof(devpath), "%s@%s/fd@a",
> >+                 dev->qdev.parent_bus->info->name, bus_name);
> >+
> >+        add_boot_device_path(isa->bootindexA, strdup(devpath));
> >+    }
> 
> Even if we're passing a string to SeaBIOS, we should probably keep
> the components structured.  Looks like there's four pieces of data:
> 1) a bus name 2) a bus path 3) a device name 4) a device path.
> 
This is pretty much device dependant. For ide it is 1) grandparent bus
name 2) grandparent bus path 3) parent bus name 4) parent bus path.

For floppy it is 1) parent bus name 2) parent bus path 3) drive name
(since one device have two disk attached to boot from).

> We derive (1) from the qdev structure, why don't we derive (3) too?
> 
We can if they are stable. They are a little bit redundant though. For
instance floppy will have isa-fdc there, but we already know that we
are on isa bus from previous path element. The same with virtio. We will
have virtio-net-pci/virtio-blk-pci there. Looks like QDEVism that leaks
into ABI. I actually consider all nic devices to be like
"PCI@0000:00:03.0/net@0" since BIOS should not really care what nic
it is.

--
			Gleb.

Patch

diff --git a/block_int.h b/block_int.h
index e8e7156..60e7be2 100644
--- a/block_int.h
+++ b/block_int.h
@@ -225,6 +225,7 @@  typedef struct BlockConf {
     uint16_t logical_block_size;
     uint16_t min_io_size;
     uint32_t opt_io_size;
+    int32_t bootindex;
 } BlockConf;
 
 static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -247,6 +248,7 @@  static inline unsigned int get_physical_block_exp(BlockConf *conf)
     DEFINE_PROP_UINT16("physical_block_size", _state,                   \
                        _conf.physical_block_size, 512),                 \
     DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
+    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)         \
 
 #endif /* BLOCK_INT_H */
diff --git a/hw/fdc.c b/hw/fdc.c
index 1f38d0d..9d0dff5 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -35,6 +35,7 @@ 
 #include "sysbus.h"
 #include "qdev-addr.h"
 #include "blockdev.h"
+#include "sysemu.h"
 
 /********************************************************/
 /* debug Floppy devices */
@@ -523,6 +524,8 @@  typedef struct FDCtrlSysBus {
 typedef struct FDCtrlISABus {
     ISADevice busdev;
     struct FDCtrl state;
+    int32_t bootindexA;
+    int32_t bootindexB;
 } FDCtrlISABus;
 
 static uint32_t fdctrl_read (void *opaque, uint32_t reg)
@@ -1974,6 +1977,7 @@  static int isabus_fdc_init1(ISADevice *dev)
     int isairq = 6;
     int dma_chann = 2;
     int ret;
+    char devpath[30], *bus_name;
 
     register_ioport_read(iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
@@ -1992,6 +1996,36 @@  static int isabus_fdc_init1(ISADevice *dev)
     qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
     ret = fdctrl_init_common(fdctrl);
 
+    if (ret) {
+        return ret;
+    }
+
+    if (isa->bootindexA < 0 && isa->bootindexB < 0) {
+        return 0;
+    }
+
+    if (!dev->qdev.parent_bus->info->get_dev_path) {
+        fprintf(stderr, "Can't create device path for floppy\n");
+        return 0;
+    }
+
+    bus_name = dev->qdev.parent_bus->info->get_dev_path(&dev->qdev);
+
+    if (isa->bootindexA >= 0) {
+        snprintf(devpath, sizeof(devpath), "%s@%s/fd@a",
+                 dev->qdev.parent_bus->info->name, bus_name);
+
+        add_boot_device_path(isa->bootindexA, strdup(devpath));
+    }
+
+    if (isa->bootindexB >= 0) {
+        snprintf(devpath, sizeof(devpath), "%s@%s/fd@b",
+                 dev->qdev.parent_bus->info->name, bus_name);
+
+        add_boot_device_path(isa->bootindexB, strdup(devpath));
+    }
+
+    qemu_free(bus_name);
     return ret;
 }
 
@@ -2050,6 +2084,8 @@  static ISADeviceInfo isa_fdc_info = {
     .qdev.props = (Property[]) {
         DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
         DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
+        DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
+        DEFINE_PROP_INT32("bootindexB", FDCtrlISABus, bootindexB, -1),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index ab72a85..f4b559b 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -21,6 +21,7 @@ 
 #include "qemu-error.h"
 #include <hw/ide/internal.h>
 #include "blockdev.h"
+#include "sysemu.h"
 
 /* --------------------------------- */
 
@@ -123,6 +124,8 @@  static int ide_drive_initfn(IDEDevice *dev)
     IDEState *s = bus->ifs + dev->unit;
     const char *serial;
     DriveInfo *dinfo;
+    char *ide_bus, *pbus, devpath[100];
+    DeviceState *pdev = bus->qbus.parent;
 
     serial = dev->serial;
     if (!serial) {
@@ -143,6 +146,27 @@  static int ide_drive_initfn(IDEDevice *dev)
     if (!dev->serial) {
         dev->serial = qemu_strdup(s->drive_serial_str);
     }
+
+    if (dev->conf.bootindex < 0) {
+        return 0;
+    }
+
+    if (!dev->qdev.parent_bus->info->get_dev_path ||
+        !pdev->parent_bus->info->get_dev_path) {
+        fprintf(stderr, "Can't create device path for IDE disk\n");
+        return 0;
+    }
+
+    ide_bus = dev->qdev.parent_bus->info->get_dev_path(&dev->qdev);
+    pbus = pdev->parent_bus->info->get_dev_path(pdev);
+
+    snprintf(devpath, sizeof(devpath), "%s@%s/%s@%s",
+             pdev->parent_bus->info->name, pbus, bus->qbus.info->name, ide_bus);
+
+    add_boot_device_path(dev->conf.bootindex, strdup(devpath));
+
+    qemu_free(ide_bus);
+    qemu_free(pbus);
     return 0;
 }
 
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a1df26d..581ca56 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -504,6 +504,7 @@  VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
     int cylinders, heads, secs;
     static int virtio_blk_id;
     DriveInfo *dinfo;
+    char *bus_name, devpath[100];
 
     if (!conf->bs) {
         error_report("virtio-blk-pci: drive property not set");
@@ -542,6 +543,25 @@  VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
     bdrv_set_removable(s->bs, 0);
     s->bs->buffer_alignment = conf->logical_block_size;
 
+    if (conf->bootindex < 0) {
+        goto out;
+    }
+
+    if (!dev->parent_bus->info->get_dev_path) {
+        fprintf(stderr, "Can't create device path for virtio disk\n");
+        goto out;
+    }
+
+    bus_name = dev->parent_bus->info->get_dev_path(dev);
+
+    snprintf(devpath, sizeof(devpath), "%s@%s/virtio-blk@0",
+             dev->parent_bus->info->name, bus_name);
+
+    add_boot_device_path(conf->bootindex, strdup(devpath));
+
+    qemu_free(bus_name);
+
+out:
     return &s->vdev;
 }
 
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 7e1688c..bc8a7d1 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -966,6 +966,7 @@  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
                               virtio_net_conf *net)
 {
     VirtIONet *n;
+    char devpath[100], *bus_name;
 
     n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
                                         sizeof(struct virtio_net_config),
@@ -1018,6 +1019,25 @@  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
                     virtio_net_save, virtio_net_load, n);
     n->vmstate = qemu_add_vm_change_state_handler(virtio_net_vmstate_change, n);
 
+    if (conf->bootindex < 0) {
+        goto out;
+    }
+
+    if (!dev->parent_bus->info->get_dev_path) {
+        fprintf(stderr, "Can't create device path for virtio net\n");
+        goto out;
+    }
+
+    bus_name = dev->parent_bus->info->get_dev_path(dev);
+
+    snprintf(devpath, sizeof(devpath), "%s@%s/virtio-net@0",
+             dev->parent_bus->info->name, bus_name);
+
+    add_boot_device_path(conf->bootindex, strdup(devpath));
+
+    qemu_free(bus_name);
+
+out:
     return &n->vdev;
 }
 
diff --git a/net.h b/net.h
index 44c31a9..6ceca50 100644
--- a/net.h
+++ b/net.h
@@ -17,12 +17,14 @@  typedef struct NICConf {
     MACAddr macaddr;
     VLANState *vlan;
     VLANClientState *peer;
+    int32_t bootindex;
 } NICConf;
 
 #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
     DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr),                \
     DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan),                   \
-    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer)
+    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer),                   \
+    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)
 
 /* VLANs support */
 
diff --git a/sysemu.h b/sysemu.h
index 849dc8c..cc35731 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -193,4 +193,13 @@  void rtc_change_mon_event(struct tm *tm);
 
 void register_devices(void);
 
+typedef struct FWBootEntry FWBootEntry;
+
+struct FWBootEntry {
+    QTAILQ_ENTRY(FWBootEntry) link;
+    int32_t bootindex;
+    char *fw_device_path;
+};
+
+void add_boot_device_path(int32_t bootindex, char *fw_device_path);
 #endif
diff --git a/vl.c b/vl.c
index 42617c2..25d5767 100644
--- a/vl.c
+++ b/vl.c
@@ -234,6 +234,8 @@  const char *prom_envs[MAX_PROM_ENVS];
 const char *nvram = NULL;
 int boot_menu;
 
+QTAILQ_HEAD(, FWBootEntry) fw_boot_order = QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+
 int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 uint64_t node_cpumask[MAX_NODES];
@@ -722,6 +724,28 @@  static void restore_boot_devices(void *opaque)
     qemu_free(standard_boot_devices);
 }
 
+void add_boot_device_path(int32_t bootindex, char *fw_device_path)
+{
+    FWBootEntry *node = qemu_mallocz(sizeof(FWBootEntry)), *i;
+
+    node->bootindex = bootindex;
+    node->fw_device_path = fw_device_path;
+
+    printf("adding '%s' at index %d\n", fw_device_path, bootindex);
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        if (i->bootindex == bootindex) {
+            fprintf(stderr, "Two devices with same boot index %d (%s %s)\n",
+                    bootindex, i->fw_device_path, fw_device_path);
+            exit(1);
+        } else if (i->bootindex < bootindex) {
+            continue;
+        }
+        QTAILQ_INSERT_BEFORE(i, node, link);
+        return;
+    }
+    QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
+}
+
 static void numa_add(const char *optarg)
 {
     char option[128];