[Qemu-devel] [RFC PATCH 0/4] Configurable PCI device addresses

[Qemu-devel] [RFC PATCH 0/4] Configurable PCI device addresses

[Markus Armbruster]

I'm marking this patch series as RFC, because I'm not quite done
testing.  Please review.

Provide addr= in device option arguments for NICs (PCI only) and
virtio-blk-pci.  More devices to follow.

Command line options -net nic and -drive get a new parameter addr,
which is interpreted by device code.

Because only virtio-blk-pci implements -drive parameter addr, -drive
rejects it unless if=virtio.

Only PCI NICs implement -net nic parameter addr, others silently
ignore it.

Monitor command pci_add already provides syntax to select the PCI
address, but it doesn't actually work (see patch 3 for details).
This patch series makes pci_add work without changing its syntax.
addr= in the third argument is caught and rejected because the first
argument already has a mandatory pci_addr=.  See patches 2 and 4 for
how that's done.

Aside: the pci_add syntax is ugly.  It has two name=value,...
arguments, and the first one only accepts the name pci_addr (all other
names are silently ignored).  Similar ugliness in pci_del and
drive_add.

While testing this, I stumbled over a pre-existing pci_add problem: it
uses ordinary device initialization code, which prints to stderr and
exits on error.  That's not approproate for monitor commands.
Example: "pci_add pci_addr=auto nic model=xxx" goes
qemu_pci_hot_add_nic() -> pci_nic_init() ->
qemu_check_nic_model_list() -> exit().  Not fixed.

Testing showed that PCI devices work in some slots, but not in others,
regardless of this patch (to be reported separately).  This patch
makes the problem more visible, but it doesn't cause it.


Markus Armbruster (4):
  Fix do_pci_register_device() to reject devfn already in use
  Support addr=... in option argument of -net nic
  Make first argument of monitor command pci_add work
  Support addr=... in option argument of -drive if=virtio

 hw/mips_malta.c        |   10 +++---
 hw/pc.c                |    7 +++-
 hw/pci-hotplug.c       |   64 ++++++++++++++++++++++++++++-------------------
 hw/pci.c               |   53 ++++++++++++++++++++++++++++-----------
 hw/pci.h               |    6 ++--
 hw/ppc440_bamboo.c     |    6 +++-
 hw/ppc_newworld.c      |    2 +-
 hw/ppc_oldworld.c      |    2 +-
 hw/ppc_prep.c          |    2 +-
 hw/ppce500_mpc8544ds.c |    6 +++-
 hw/r2d.c               |    2 +-
 hw/realview.c          |    2 +-
 hw/versatilepb.c       |    2 +-
 net.c                  |    5 +++-
 net.h                  |    1 +
 qemu-options.hx        |   10 +++++--
 sysemu.h               |    1 +
 vl.c                   |   14 +++++++++-
 18 files changed, 129 insertions(+), 66 deletions(-)

[Qemu-devel] [PATCH 1/4] Fix do_pci_register_device() to reject devfn already in use

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 0a738db..36c92d7 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -251,6 +251,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         }
         return NULL;
     found: ;
+    } else if (bus->devices[devfn]) {
+        return NULL;
     }
     pci_dev->bus = bus;
     pci_dev->devfn = devfn;
-- 
1.6.0.6

[Qemu-devel] [PATCH 2/4] Support addr=... in option argument of -net nic

[Markus Armbruster]

Make net_client_init() accept addr=, put the value into struct
NICinfo.  Use it in pci_nic_init(), and remove arguments bus and
devfn.

Don't support addr= in third argument of monitor command pci_add,
because that clashes with its first argument.  Admittedly unelegant.

Machines "malta" and "r2d" have a default NIC with a well-known PCI
address.  Deal with that the same way as the NIC model: make
pci_nic_init() take an optional default to be used when the user
doesn't specify one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mips_malta.c        |   10 +++++-----
 hw/pc.c                |    2 +-
 hw/pci-hotplug.c       |   11 +++++++----
 hw/pci.c               |   48 +++++++++++++++++++++++++++++++++++++++++++-----
 hw/pci.h               |    4 ++--
 hw/ppc440_bamboo.c     |    2 +-
 hw/ppc_newworld.c      |    2 +-
 hw/ppc_oldworld.c      |    2 +-
 hw/ppc_prep.c          |    2 +-
 hw/ppce500_mpc8544ds.c |    2 +-
 hw/r2d.c               |    2 +-
 hw/realview.c          |    2 +-
 hw/versatilepb.c       |    2 +-
 net.c                  |    5 ++++-
 net.h                  |    1 +
 qemu-options.hx        |    7 ++++---
 16 files changed, 75 insertions(+), 29 deletions(-)

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index ed104f0..e916468 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -474,19 +474,19 @@ static void audio_init (PCIBus *pci_bus)
 #endif
 
 /* Network support */
-static void network_init (PCIBus *pci_bus)
+static void network_init(void)
 {
     int i;
 
     for(i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
-        int devfn = -1;
+        const char *default_devaddr = NULL;
 
         if (i == 0 && (!nd->model || strcmp(nd->model, "pcnet") == 0))
             /* The malta board has a PCNet card using PCI SLOT 11 */
-            devfn = 88;
+            default_devaddr = "0b";
 
-        pci_nic_init(pci_bus, nd, devfn, "pcnet");
+        pci_nic_init(nd, "pcnet", default_devaddr);
     }
 }
 
@@ -943,7 +943,7 @@ void mips_malta_init (ram_addr_t ram_size,
 #endif
 
     /* Network card */
-    network_init(pci_bus);
+    network_init();
 
     /* Optional PCI video card */
     if (cirrus_vga_enabled) {
diff --git a/hw/pc.c b/hw/pc.c
index 143b697..4b918bb 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1063,7 +1063,7 @@ static void pc_init1(ram_addr_t ram_size,
         if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
             pc_init_ne2k_isa(nd, i8259);
         else
-            pci_nic_init(pci_bus, nd, -1, "ne2k_pci");
+            pci_nic_init(nd, "ne2k_pci", NULL);
     }
 
     qemu_system_hot_add_init();
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 4d49c29..e2b14ca 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -33,15 +33,18 @@
 #include "virtio-blk.h"
 
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
-static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, PCIBus *pci_bus,
-                                       const char *opts)
+static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, const char *opts)
 {
     int ret;
 
     ret = net_client_init(mon, "nic", opts);
     if (ret < 0)
         return NULL;
-    return pci_nic_init(pci_bus, &nd_table[ret], -1, "rtl8139");
+    if (nd_table[ret].devaddr) {
+        monitor_printf(mon, "Parameter addr not supported\n");
+        return NULL;
+    }
+    return pci_nic_init(&nd_table[ret], "rtl8139", NULL);
 }
 
 void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
@@ -149,7 +152,7 @@ void pci_device_hot_add(Monitor *mon, const char *pci_addr, const char *type,
     }
 
     if (strcmp(type, "nic") == 0)
-        dev = qemu_pci_hot_add_nic(mon, pci_bus, opts);
+        dev = qemu_pci_hot_add_nic(mon, opts);
     else if (strcmp(type, "storage") == 0)
         dev = qemu_pci_hot_add_storage(mon, pci_bus, opts);
     else
diff --git a/hw/pci.c b/hw/pci.c
index 36c92d7..1064de8 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -238,6 +238,24 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
     return pci_parse_devaddr(devaddr, domp, busp, slotp);
 }
 
+static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
+{
+    int dom, bus;
+    unsigned slot;
+
+    if (!devaddr) {
+        *devfnp = -1;
+        return pci_find_bus(0);
+    }
+
+    if (pci_parse_devaddr(devaddr, &dom, &bus, &slot) < 0) {
+        return NULL;
+    }
+
+    *devfnp = slot << 3;
+    return pci_find_bus(bus);
+}
+
 /* -1 for devfn means auto assign */
 static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          const char *name, int devfn,
@@ -786,6 +804,24 @@ void pci_info(Monitor *mon)
     pci_for_each_device(0, pci_info_device);
 }
 
+static PCIDevice *pci_create(const char *name, const char *devaddr)
+{
+    PCIBus *bus;
+    int devfn;
+    DeviceState *dev;
+
+    bus = pci_get_bus_devfn(&devfn, devaddr);
+    if (!bus) {
+        fprintf(stderr, "Invalid PCI device address %s for device %s\n",
+                devaddr, name);
+        exit(1);
+    }
+
+    dev = qdev_create(&bus->qbus, name);
+    qdev_set_prop_int(dev, "devfn", devfn);
+    return (PCIDevice *)dev;
+}
+
 static const char * const pci_nic_models[] = {
     "ne2k_pci",
     "i82551",
@@ -811,9 +847,11 @@ static const char * const pci_nic_names[] = {
 };
 
 /* Initialize a PCI NIC.  */
-PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
-                  const char *default_model)
+PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
+                        const char *default_devaddr)
 {
+    const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
+    PCIDevice *pci_dev;
     DeviceState *dev;
     int i;
 
@@ -821,12 +859,12 @@ PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
 
     for (i = 0; pci_nic_models[i]; i++) {
         if (strcmp(nd->model, pci_nic_models[i]) == 0) {
-            dev = qdev_create(&bus->qbus, pci_nic_names[i]);
-            qdev_set_prop_int(dev, "devfn", devfn);
+            pci_dev = pci_create(pci_nic_names[i], devaddr);
+            dev = &pci_dev->qdev;
             qdev_set_netdev(dev, nd);
             qdev_init(dev);
             nd->private = dev;
-            return (PCIDevice *)dev;
+            return pci_dev;
         }
     }
 
diff --git a/hw/pci.h b/hw/pci.h
index fcca526..935de17 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -183,8 +183,8 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          qemu_irq *pic, int devfn_min, int nirq);
 
-PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
-                  const char *default_model);
+PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
+                        const char *default_devaddr);
 void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(void *opaque, uint32_t addr, int len);
 int pci_bus_num(PCIBus *s);
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 00aa2c7..312abde 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -125,7 +125,7 @@ static void bamboo_init(ram_addr_t ram_size,
         for (i = 0; i < nb_nics; i++) {
             /* There are no PCI NICs on the Bamboo board, but there are
              * PCI slots, so we can pick whatever default model we want. */
-            pci_nic_init(pcibus, &nd_table[i], -1, "e1000");
+            pci_nic_init(&nd_table[i], "e1000", NULL);
         }
     }
 
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index 22beedb..a1057b4 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -304,7 +304,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
                                serial_hds[0], serial_hds[1], ESCC_CLOCK, 4);
 
     for(i = 0; i < nb_nics; i++)
-        pci_nic_init(pci_bus, &nd_table[i], -1, "ne2k_pci");
+        pci_nic_init(&nd_table[i], "ne2k_pci", NULL);
 
     if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) {
         fprintf(stderr, "qemu: too many IDE bus\n");
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index b2c329b..686e81f 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -315,7 +315,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
                                serial_hds[1], ESCC_CLOCK, 4);
 
     for(i = 0; i < nb_nics; i++)
-        pci_nic_init(pci_bus, &nd_table[i], -1, "ne2k_pci");
+        pci_nic_init(&nd_table[i], "ne2k_pci", NULL);
 
 
     if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) {
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 83f2eca..b6f19a5 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -680,7 +680,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
         if (strcmp(nd_table[i].model, "ne2k_isa") == 0) {
             isa_ne2000_init(ne2000_io[i], i8259[ne2000_irq[i]], &nd_table[i]);
         } else {
-            pci_nic_init(pci_bus, &nd_table[i], -1, "ne2k_pci");
+            pci_nic_init(&nd_table[i], "ne2k_pci", NULL);
         }
     }
 
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index d9ed36c..aff3ae2 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -225,7 +225,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 
         /* Register network interfaces. */
         for (i = 0; i < nb_nics; i++) {
-            pci_nic_init(pci_bus, &nd_table[i], -1, "virtio");
+            pci_nic_init(&nd_table[i], "virtio", NULL);
         }
     }
 
diff --git a/hw/r2d.c b/hw/r2d.c
index a529ab4..8ce6832 100644
--- a/hw/r2d.c
+++ b/hw/r2d.c
@@ -231,7 +231,7 @@ static void r2d_init(ram_addr_t ram_size,
 
     /* NIC: rtl8139 on-board, and 2 slots. */
     for (i = 0; i < nb_nics; i++)
-        pci_nic_init(pci, &nd_table[i], (i==0)? 2<<3: -1, "rtl8139");
+        pci_nic_init(&nd_table[i], "rtl8139", i==0 ? "2" : NULL);
 
     /* Todo: register on board registers */
     if (kernel_filename) {
diff --git a/hw/realview.c b/hw/realview.c
index 62d8bf5..8e176b9 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -125,7 +125,7 @@ static void realview_init(ram_addr_t ram_size,
             smc91c111_init(nd, 0x4e000000, pic[28]);
             done_smc = 1;
         } else {
-            pci_nic_init(pci_bus, nd, -1, "rtl8139");
+            pci_nic_init(nd, "rtl8139", NULL);
         }
     }
 
diff --git a/hw/versatilepb.c b/hw/versatilepb.c
index 1f1b1bc..3371121 100644
--- a/hw/versatilepb.c
+++ b/hw/versatilepb.c
@@ -212,7 +212,7 @@ static void versatile_init(ram_addr_t ram_size,
             smc91c111_init(nd, 0x10010000, sic[25]);
             done_smc = 1;
         } else {
-            pci_nic_init(pci_bus, nd, -1, "rtl8139");
+            pci_nic_init(nd, "rtl8139", NULL);
         }
     }
     if (usb_enabled) {
diff --git a/net.c b/net.c
index af9de73..baa3412 100644
--- a/net.c
+++ b/net.c
@@ -2103,7 +2103,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     }
     if (!strcmp(device, "nic")) {
         static const char * const nic_params[] = {
-            "vlan", "name", "macaddr", "model", NULL
+            "vlan", "name", "macaddr", "model", "addr", NULL
         };
         NICInfo *nd;
         uint8_t *macaddr;
@@ -2138,6 +2138,9 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         if (get_param_value(buf, sizeof(buf), "model", p)) {
             nd->model = strdup(buf);
         }
+        if (get_param_value(buf, sizeof(buf), "addr", p)) {
+            nd->devaddr = strdup(buf);
+        }
         nd->vlan = vlan;
         nd->name = name;
         nd->used = 1;
diff --git a/net.h b/net.h
index 89e7706..763d56f 100644
--- a/net.h
+++ b/net.h
@@ -88,6 +88,7 @@ struct NICInfo {
     uint8_t macaddr[6];
     const char *model;
     const char *name;
+    const char *devaddr;
     VLANState *vlan;
     void *private;
     int used;
diff --git a/qemu-options.hx b/qemu-options.hx
index 9d5e05a..895b248 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -733,7 +733,7 @@ STEXI
 ETEXI
 
 DEF("net", HAS_ARG, QEMU_OPTION_net,
-    "-net nic[,vlan=n][,macaddr=addr][,model=type][,name=str]\n"
+    "-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str]\n"
     "                create a new Network Interface Card and connect it to VLAN 'n'\n"
 #ifdef CONFIG_SLIRP
     "-net user[,vlan=n][,name=str][,hostname=host]\n"
@@ -767,10 +767,11 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "-net none       use it alone to have zero network devices; if no -net option\n"
     "                is provided, the default is '-net nic -net user'\n")
 STEXI
-@item -net nic[,vlan=@var{n}][,macaddr=@var{addr}][,model=@var{type}][,name=@var{name}]
+@item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}][,name=@var{name}][,addr=@var{addr}]
 Create a new Network Interface Card and connect it to VLAN @var{n} (@var{n}
 = 0 is the default). The NIC is an ne2k_pci by default on the PC
-target. Optionally, the MAC address can be changed to @var{addr}
+target. Optionally, the MAC address can be changed to @var{mac}, the
+device address set to @var{addr} (PCI cards only),
 and a @var{name} can be assigned for use in monitor commands. If no
 @option{-net} option is specified, a single NIC is created.
 Qemu can emulate several different models of network card.
-- 
1.6.0.6

[Qemu-devel] [PATCH 3/4] Make first argument of monitor command pci_add work

[Markus Armbruster]

Simply pass the PCI address through qemu_pci_hot_add_nic() to
pci_nic_init() and through qemu_pci_hot_add_storage() to pci_create().

Before, pci_device_hot_add() passed along the PCI bus to use, and
ignored any user-specified slot.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-hotplug.c |   51 ++++++++++++++++++++++++++-------------------------
 hw/pci.c         |   19 +------------------
 hw/pci.h         |    2 +-
 3 files changed, 28 insertions(+), 44 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index e2b14ca..031643e 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -33,7 +33,8 @@
 #include "virtio-blk.h"
 
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
-static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, const char *opts)
+static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
+                                       const char *devaddr, const char *opts)
 {
     int ret;
 
@@ -44,7 +45,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, const char *opts)
         monitor_printf(mon, "Parameter addr not supported\n");
         return NULL;
     }
-    return pci_nic_init(&nd_table[ret], "rtl8139", NULL);
+    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
 }
 
 void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
@@ -89,10 +90,11 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
     return;
 }
 
-static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon, PCIBus *pci_bus,
+static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
+                                           const char *devaddr,
                                            const char *opts)
 {
-    void *opaque = NULL;
+    PCIDevice *dev;
     int type = -1, drive_idx = -1;
     char buf[128];
 
@@ -103,63 +105,62 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon, PCIBus *pci_bus,
             type = IF_VIRTIO;
         } else {
             monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
-            goto out;
+            return NULL;
         }
     } else {
         monitor_printf(mon, "no if= specified\n");
-        goto out;
+        return NULL;
     }
 
     if (get_param_value(buf, sizeof(buf), "file", opts)) {
         drive_idx = add_init_drive(opts);
         if (drive_idx < 0)
-            goto out;
+            return NULL;
     } else if (type == IF_VIRTIO) {
         monitor_printf(mon, "virtio requires a backing file/device.\n");
-        goto out;
+        return NULL;
     }
 
     switch (type) {
     case IF_SCSI:
-        opaque = pci_create_simple(pci_bus, -1, "lsi53c895a");
+        dev = pci_create("lsi53c895a", devaddr);
         break;
     case IF_VIRTIO:
-        opaque = pci_create_simple(pci_bus, -1, "virtio-blk-pci");
+        dev = pci_create("virtio-blk-pci", devaddr);
         break;
+    default:
+        dev = NULL;
     }
-
-out:
-    return opaque;
+    if (dev)
+        qdev_init(&dev->qdev);
+    return dev;
 }
 
 void pci_device_hot_add(Monitor *mon, const char *pci_addr, const char *type,
                         const char *opts)
 {
     PCIDevice *dev = NULL;
-    PCIBus *pci_bus;
-    int dom, bus;
-    unsigned slot;
+    const char *devaddr = NULL;
+    char buf[32];
 
-    if (pci_assign_devaddr(pci_addr, &dom, &bus, &slot)) {
+    if (!get_param_value(buf, sizeof(buf), "pci_addr", pci_addr)) {
         monitor_printf(mon, "Invalid pci address\n");
         return;
     }
 
-    pci_bus = pci_find_bus(bus);
-    if (!pci_bus) {
-        monitor_printf(mon, "Can't find pci_bus %d\n", bus);
-        return;
-    }
+    if (strcmp(buf, "auto"))
+        devaddr = buf;
 
     if (strcmp(type, "nic") == 0)
-        dev = qemu_pci_hot_add_nic(mon, opts);
+        dev = qemu_pci_hot_add_nic(mon, devaddr, opts);
     else if (strcmp(type, "storage") == 0)
-        dev = qemu_pci_hot_add_storage(mon, pci_bus, opts);
+        dev = qemu_pci_hot_add_storage(mon, devaddr, opts);
     else
         monitor_printf(mon, "invalid type: %s\n", type);
 
     if (dev) {
-        qemu_system_device_hot_add(bus, PCI_SLOT(dev->devfn), 1);
+        qemu_system_device_hot_add(pci_bus_num(dev->bus),
+                                   PCI_SLOT(dev->devfn), 1);
         monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
                        0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
                        PCI_FUNC(dev->devfn));
diff --git a/hw/pci.c b/hw/pci.c
index 1064de8..46bbefa 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -221,23 +221,6 @@ int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
     return pci_parse_devaddr(devaddr, domp, busp, slotp);
 }
 
-int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
-{
-    char devaddr[32];
-
-    if (!get_param_value(devaddr, sizeof(devaddr), "pci_addr", addr))
-	return -1;
-
-    if (!strcmp(devaddr, "auto")) {
-        *domp = *busp = 0;
-        *slotp = -1;
-        /* want to support dom/bus auto-assign at some point */
-        return 0;
-    }
-
-    return pci_parse_devaddr(devaddr, domp, busp, slotp);
-}
-
 static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
 {
     int dom, bus;
@@ -804,7 +787,7 @@ void pci_info(Monitor *mon)
     pci_for_each_device(0, pci_info_device);
 }
 
-static PCIDevice *pci_create(const char *name, const char *devaddr)
+PCIDevice *pci_create(const char *name, const char *devaddr)
 {
     PCIBus *bus;
     int devfn;
diff --git a/hw/pci.h b/hw/pci.h
index 935de17..c8625de 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -193,7 +193,6 @@ PCIBus *pci_find_bus(int bus_num);
 PCIDevice *pci_find_device(int bus_num, int slot, int function);
 
 int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
-int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
 
 void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
@@ -220,6 +219,7 @@ pci_config_set_class(uint8_t *pci_config, uint16_t val)
 typedef void (*pci_qdev_initfn)(PCIDevice *dev);
 void pci_qdev_register(const char *name, int size, pci_qdev_initfn init);
 
+PCIDevice *pci_create(const char *name, const char *devaddr);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 
 /* lsi53c895a.c */
-- 
1.6.0.6

[Qemu-devel] [PATCH 4/4] Support addr=... in option argument of -drive if=virtio

[Markus Armbruster]

From: Markus Armbruster <armbru@pond.sub.org>

Make drive_init() accept addr=, put the value into struct DriveInfo.
Use it in all the places that create virtio-blk-pci devices:
pc_init1(), bamboo_init(), mpc8544ds_init().

Don't support addr= in third argument of monitor command pci_add and
second argument of drive_add, because that clashes with their first
arguments.  Admittedly unelegant.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pc.c                |    5 ++++-
 hw/pci-hotplug.c       |    8 ++++++++
 hw/ppc440_bamboo.c     |    4 +++-
 hw/ppce500_mpc8544ds.c |    4 +++-
 qemu-options.hx        |    3 +++
 sysemu.h               |    1 +
 vl.c                   |   14 +++++++++++++-
 7 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 4b918bb..cd3ac82 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -848,6 +848,7 @@ static void pc_init1(ram_addr_t ram_size,
     ram_addr_t below_4g_mem_size, above_4g_mem_size = 0;
     int bios_size, isa_bios_size, oprom_area_size;
     PCIBus *pci_bus;
+    PCIDevice *pci_dev;
     int piix3_devfn = -1;
     CPUState *env;
     qemu_irq *cpu_irq;
@@ -1146,7 +1147,9 @@ static void pc_init1(ram_addr_t ram_size,
         int unit_id = 0;
 
         while ((index = drive_get_index(IF_VIRTIO, 0, unit_id)) != -1) {
-            pci_create_simple(pci_bus, -1, "virtio-blk-pci");
+            pci_dev = pci_create("virtio-blk-pci",
+                                 drives_table[index].devaddr);
+            qdev_init(&pci_dev->qdev);
             unit_id++;
         }
     }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 031643e..952e27a 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -70,6 +70,10 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
     drive_idx = add_init_drive(opts);
     if (drive_idx < 0)
         return;
+    if (drives_table[drive_idx].devaddr) {
+        monitor_printf(mon, "Parameter addr not supported\n");
+        return;
+    }
     type = drives_table[drive_idx].type;
     bus = drive_get_max_bus (type);
 
@@ -116,6 +120,10 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         drive_idx = add_init_drive(opts);
         if (drive_idx < 0)
             return NULL;
+        if (drives_table[drive_idx].devaddr) {
+            monitor_printf(mon, "Parameter addr not supported\n");
+            return NULL;
+        }
     } else if (type == IF_VIRTIO) {
         monitor_printf(mon, "virtio requires a backing file/device.\n");
         return NULL;
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 312abde..3bc04a2 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -90,6 +90,7 @@ static void bamboo_init(ram_addr_t ram_size,
 {
     unsigned int pci_irq_nrs[4] = { 28, 27, 26, 25 };
     PCIBus *pcibus;
+    PCIDevice *pci_dev;
     CPUState *env;
     uint64_t elf_entry;
     uint64_t elf_lowaddr;
@@ -110,7 +111,8 @@ static void bamboo_init(ram_addr_t ram_size,
 
         /* Add virtio block devices. */
         while ((i = drive_get_index(IF_VIRTIO, 0, unit_id)) != -1) {
-            pci_create_simple(pcibus, -1, "virtio-blk-pci");
+            pci_dev = pci_create("virtio-blk-pci", drives_table[i].devaddr);
+            qdev_init(&pci_dev->qdev);
             unit_id++;
         }
 
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index aff3ae2..c0e367d 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -157,6 +157,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
                          const char *cpu_model)
 {
     PCIBus *pci_bus;
+    PCIDevice *pci_dev;
     CPUState *env;
     uint64_t elf_entry;
     uint64_t elf_lowaddr;
@@ -219,7 +220,8 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 
         /* Add virtio block devices. */
         while ((i = drive_get_index(IF_VIRTIO, 0, unit_id)) != -1) {
-            pci_create_simple(pci_bus, -1, "virtio-blk-pci");
+            pci_dev = pci_create("virtio-blk-pci", drives_table[i].devaddr);
+            qdev_init(&pci_dev->qdev);
             unit_id++;
         }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 895b248..fdeda10 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -92,6 +92,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
+    "       [,addr=A]\n"
     "                use 'file' as a drive image\n")
 STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -126,6 +127,8 @@ the format.  Can be used to specifiy format=raw to avoid interpreting
 an untrusted format header.
 @item serial=@var{serial}
 This option specifies the serial number to assign to the device.
+@item addr=@var{addr}
+Specify the controller's PCI address (if=virtio only).
 @end table
 
 By default, writethrough caching is used for all block device.  This means that
diff --git a/sysemu.h b/sysemu.h
index fe24415..e48668c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -156,6 +156,7 @@ typedef enum {
 
 typedef struct DriveInfo {
     BlockDriverState *bdrv;
+    const char *devaddr;
     BlockInterfaceType type;
     int bus;
     int unit;
diff --git a/vl.c b/vl.c
index 3242c23..2e812d3 100644
--- a/vl.c
+++ b/vl.c
@@ -2209,12 +2209,14 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
     int index;
     int cache;
     int bdrv_flags, onerror;
+    const char *devaddr;
     int drives_table_idx;
     char *str = arg->opt;
     static const char * const params[] = { "bus", "unit", "if", "index",
                                            "cyls", "heads", "secs", "trans",
                                            "media", "snapshot", "file",
-                                           "cache", "format", "serial", "werror",
+                                           "cache", "format", "serial",
+                                           "werror", "addr",
                                            NULL };
 
     if (check_params(buf, sizeof(buf), params, str) < 0) {
@@ -2428,6 +2430,15 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
         }
     }
 
+    devaddr = NULL;
+    if (get_param_value(buf, sizeof(buf), "addr", str)) {
+        if (type != IF_VIRTIO) {
+            fprintf(stderr, "addr is not supported by in '%s'\n", str);
+            return -1;
+        }
+        devaddr = strdup(buf);
+    }
+
     /* compute bus and unit according index */
 
     if (index != -1) {
@@ -2489,6 +2500,7 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
     bdrv = bdrv_new(buf);
     drives_table_idx = drive_get_free_idx();
     drives_table[drives_table_idx].bdrv = bdrv;
+    drives_table[drives_table_idx].devaddr = devaddr;
     drives_table[drives_table_idx].type = type;
     drives_table[drives_table_idx].bus = bus_id;
     drives_table[drives_table_idx].unit = unit_id;
-- 
1.6.0.6

Re: [Qemu-devel] [RFC PATCH 0/4] Configurable PCI device addresses

[Richard W.M. Jones]

On Thu, Jun 18, 2009 at 03:14:06PM +0200, Markus Armbruster wrote:
> I'm marking this patch series as RFC, because I'm not quite done
> testing.  Please review.

Does the output of 'info pci' need to be fixed too?

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [RFC PATCH 0/4] Configurable PCI device addresses

[Gerd Hoffmann]

> Markus Armbruster (4):
>    Fix do_pci_register_device() to reject devfn already in use
>    Support addr=... in option argument of -net nic
>    Make first argument of monitor command pci_add work
>    Support addr=... in option argument of -drive if=virtio

Whole series looks sane to me.

cheers,
   Gerd