Re: [Qemu-devel] [PATCH 5/5] Add bootindex parameter to net/block/fd device

[Gleb Natapov]

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<address@hidden>
> >---
> >  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), "address@hidden/address@hidden",
> >+                 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
"address@hidden:00:03.0/address@hidden" since BIOS should not really care what 
nic
it is.

--
                        Gleb.