[Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

You can clone that from here :
git.greensocs.com/home/greensocs/git/qemu_virtio.git virtio_refactoring_v6

The problem with the last RFC v5 was that virtio-blk refactoring broke
virtio-blk-pci device ( SEGFAULT ). So I modify this last step to fix that
issue.

In order to not break anything, I think we have to refactor virtio-pci-blk in a
next step then add a supplementary step which clean virtio-blk 
( eg : fix the cast ).

Does it make sense ?

This patch-set is :
    * Introducing a virtio-bus which extends bus-state.
    * Implementing a virtio-pci-bus which extends virtio-bus.
    * Implementing a virtio-pci device which has a virtio-pci-bus.
    * Implementing virtio-device which extends device-states.
    * Implementing a virtio-blk which extends virtio-device.

The first patch is a modification of qdev-monitor.c, it forces the function
qbus_find_recursive(..) to return a non-full bus, and return an error if the
desired bus ( with "bus=" option ) is full. It add a max_dev field to the
bus_state structure. If max_dev=0 it has no limitation, if not the maximum
amount of device connected to the bus is max_dev.

Changes v5 -> v6:
    * Renamed virtio_common_init_ to virtio_init, modify virtio_common_init to
      allocate and call virtio_init. Drop the unused structure size parameters.
    * Renamed init/exit callback in VirtioBusClass.
    * Renamed virtio_blk_init virtio_blk_common_init.
    * Modified virtio_blk_init to call virtio_blk_common_init.

Changes v4 -> v5:
    * use ERROR_CLASS_GENERIC_ERROR in place of creating a new error type for
      the maximum device limitation. ( Peter )
    * Removed bus_in_use function. We assume that the virtio-bus is not in use,
      when plugin in. ( Peter )
    * Added virtio_bus_destroy_device().
    * Implemented the exit function of virtio-pci.
    * Implemented the init callback for virtio-pci ( must be modified, it still
      access vdev directly. ).
    * Implemented the exit callback for virtio-pci.
    * Started virtio-device refactoring.
    * Started virtio-blk refactoring. 

Changes v3 -> v4:
    * Added virtio-bus.o in Makefile.objs ( accidentally dropped from v3 ).
    * *const* TypeInfo in virtio-bus.
    * Introduced virtio-pci-bus.
    * Reintroduced virtio-pci.
    * Introduced virtio-device.
    * Started virtio-blk refactoring.
    * Added an error type in qerror.h for the "bus full" error.

Changes v2 -> v3:
    * Added VirtioBusClass.
    * Renamed VirtioBus -> VirtioBusState.
    * Renamed qbus -> parent_obj.
    * Plug the device only in a non-full bus.

Changes v1 -> v2:
    * All the little fix you suggest ( License, Debug printf, naming convention,
      ...)
    * Added get_virtio_device_id(), and remove the pci_id* from the VirtioBus
      structure.
    * Added virtio_bus_reset().
    * Added cast macros VIRTIO_BUS.
    * Added virtio_bus_plug_device.
    * Replaced the old-style "bus->qbus" by BUS() macro.

Fred.

KONRAD Frederic (6):
  qdev : add a maximum device allowed field for the bus.
  virtio-bus : Introduce virtio-bus
  virtio-pci-bus : Introduce virtio-pci-bus.
  virtio-pci : Refactor virtio-pci device.
  virtio-device : Refactor virtio-device.
  virtio-blk : Add the virtio-blk device.

 hw/Makefile.objs  |   1 +
 hw/qdev-core.h    |   2 +
 hw/qdev-monitor.c |  11 ++++
 hw/virtio-blk.c   | 101 ++++++++++++++++++++++++++++++++----
 hw/virtio-blk.h   |   4 ++
 hw/virtio-bus.c   | 111 ++++++++++++++++++++++++++++++++++++++++
 hw/virtio-bus.h   |  82 ++++++++++++++++++++++++++++++
 hw/virtio-pci.c   | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-pci.h   |  33 +++++++++++-
 hw/virtio.c       |  50 ++++++++++++++----
 hw/virtio.h       |  28 ++++++++++
 11 files changed, 550 insertions(+), 22 deletions(-)
 create mode 100644 hw/virtio-bus.c
 create mode 100644 hw/virtio-bus.h

-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 1/6] qdev : add a maximum device allowed field for the bus.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Add a max_dev field to BusState to specify the maximum amount of devices allowed
on the bus ( have no effect if max_dev=0 )

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/qdev-core.h    |  2 ++
 hw/qdev-monitor.c | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index fff7f0f..ee4becd 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -113,6 +113,8 @@ struct BusState {
     const char *name;
     int allow_hotplug;
     int max_index;
+    /* maximum devices allowed on the bus, 0 : no limit. */
+    int max_dev;
     QTAILQ_HEAD(ChildrenHead, BusChild) children;
     QLIST_ENTRY(BusState) sibling;
 };
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a1b4d6a..7a9d275 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -292,6 +292,17 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,
     if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) {
         match = 0;
     }
+    if ((bus->max_dev != 0) && (bus->max_dev <= bus->max_index)) {
+        if (name != NULL) {
+            /* bus was explicitly specified : return an error. */
+            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full",
+                          bus->name);
+            return NULL;
+        } else {
+            /* bus was not specified : try to find another one. */
+            match = 0;
+        }
+    }
     if (match) {
         return bus;
     }
-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 2/6] virtio-bus : Introduce virtio-bus

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Introduce virtio-bus. Refactored transport device will create a bus which
extends virtio-bus.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/Makefile.objs |   1 +
 hw/virtio-bus.c  | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-bus.h  |  82 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 194 insertions(+)
 create mode 100644 hw/virtio-bus.c
 create mode 100644 hw/virtio-bus.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index d581d8d..6fa4de4 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -3,6 +3,7 @@ common-obj-y += loader.o
 common-obj-$(CONFIG_VIRTIO) += virtio-console.o
 common-obj-$(CONFIG_VIRTIO) += virtio-rng.o
 common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
+common-obj-$(CONFIG_VIRTIO) += virtio-bus.o
 common-obj-y += fw_cfg.o
 common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o
 common-obj-$(CONFIG_PCI) += msix.o msi.o
diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c
new file mode 100644
index 0000000..d162458
--- /dev/null
+++ b/hw/virtio-bus.c
@@ -0,0 +1,111 @@
+/*
+ * VirtioBus
+ *
+ *  Copyright (C) 2012 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw.h"
+#include "qemu-error.h"
+#include "qdev.h"
+#include "virtio-bus.h"
+#include "virtio.h"
+
+/* #define DEBUG_VIRTIO_BUS */
+
+#ifdef DEBUG_VIRTIO_BUS
+#define DPRINTF(fmt, ...) \
+do { printf("virtio_bus: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do { } while (0)
+#endif
+
+/* Plug the VirtIODevice */
+int virtio_bus_plug_device(VirtIODevice *vdev)
+{
+    DeviceState *qdev = DEVICE(vdev);
+    BusState *qbus = BUS(qdev_get_parent_bus(qdev));
+    VirtioBusState *bus = VIRTIO_BUS(qbus);
+    VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
+    DPRINTF("%s : plug device.\n", qbus->name);
+
+    bus->vdev = vdev;
+
+    if (klass->device_plugged != NULL) {
+        klass->device_plugged(qbus->parent);
+    }
+
+    /*
+     * The lines below will disappear when we drop VirtIOBindings.
+     */
+    bus->bindings.notify = klass->notify;
+    bus->bindings.save_config = klass->save_config;
+    bus->bindings.save_queue = klass->save_queue;
+    bus->bindings.load_config = klass->load_config;
+    bus->bindings.load_queue = klass->load_queue;
+    bus->bindings.load_done = klass->load_done;
+    bus->bindings.get_features = klass->get_features;
+    bus->bindings.query_guest_notifiers = klass->query_guest_notifiers;
+    bus->bindings.set_guest_notifiers = klass->set_guest_notifiers;
+    bus->bindings.set_host_notifier = klass->set_host_notifier;
+    bus->bindings.vmstate_change = klass->vmstate_change;
+    virtio_bind_device(bus->vdev, &(bus->bindings), qbus->parent);
+
+    return 0;
+}
+
+/* Destroy the VirtIODevice */
+void virtio_bus_destroy_device(VirtioBusState *bus)
+{
+    DeviceState *qdev;
+    BusState *qbus = BUS(bus);
+    VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
+    DPRINTF("%s : remove device.\n", qbus->name);
+
+    if (bus->vdev != NULL) {
+        if (klass->device_unplug != NULL) {
+            klass->device_unplug(qbus->parent);
+        }
+        qdev = DEVICE(bus->vdev);
+        qdev_free(qdev);
+        bus->vdev = NULL;
+    }
+}
+
+/* Return the virtio device id of the plugged device. */
+uint16_t get_virtio_device_id(VirtioBusState *bus)
+{
+    return bus->vdev->device_id;
+}
+
+static const TypeInfo virtio_bus_info = {
+    .name = TYPE_VIRTIO_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(VirtioBusState),
+    .abstract = true,
+    .class_size = sizeof(VirtioBusClass),
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_bus_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio-bus.h b/hw/virtio-bus.h
new file mode 100644
index 0000000..6e80f13
--- /dev/null
+++ b/hw/virtio-bus.h
@@ -0,0 +1,82 @@
+/*
+ * VirtioBus
+ *
+ *  Copyright (C) 2012 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef VIRTIO_BUS_H
+#define VIRTIO_BUS_H
+
+#include "qdev.h"
+#include "sysemu.h"
+#include "virtio.h"
+
+#define TYPE_VIRTIO_BUS "virtio-bus"
+#define VIRTIO_BUS_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioBusClass, obj, TYPE_VIRTIO_BUS)
+#define VIRTIO_BUS_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioBusClass, klass, TYPE_VIRTIO_BUS)
+#define VIRTIO_BUS(obj) OBJECT_CHECK(VirtioBusState, (obj), TYPE_VIRTIO_BUS)
+
+typedef struct VirtioBusState VirtioBusState;
+
+typedef struct VirtioBusClass {
+    /* This is what a VirtioBus must implement */
+    BusClass parent;
+    void (*notify)(void *opaque, uint16_t vector);
+    void (*save_config)(void *opaque, QEMUFile *f);
+    void (*save_queue)(void *opaque, int n, QEMUFile *f);
+    int (*load_config)(void *opaque, QEMUFile *f);
+    int (*load_queue)(void *opaque, int n, QEMUFile *f);
+    int (*load_done)(void *opaque, QEMUFile *f);
+    unsigned (*get_features)(void *opaque);
+    bool (*query_guest_notifiers)(void *opaque);
+    int (*set_guest_notifiers)(void *opaque, bool assigned);
+    int (*set_host_notifier)(void *opaque, int n, bool assigned);
+    void (*vmstate_change)(void *opaque, bool running);
+    /*
+     * transport independent init function.
+     * This is called by virtio-bus just after the device is plugged.
+     */
+    void (*device_plugged)(void *opaque);
+    /*
+     * transport independent exit function.
+     * This is called by virtio-bus just before the device is unplugged.
+     */
+    void (*device_unplug)(void *opaque);
+} VirtioBusClass;
+
+struct VirtioBusState {
+    BusState parent_obj;
+    /*
+     * Only one VirtIODevice can be plugged on the bus.
+     */
+    VirtIODevice *vdev;
+    /*
+     * This should be removed when we refactor virtio-device.
+     */
+    VirtIOBindings bindings;
+};
+
+int virtio_bus_plug_device(VirtIODevice *vdev);
+void virtio_bus_destroy_device(VirtioBusState *bus);
+uint16_t get_virtio_device_id(VirtioBusState *bus);
+#endif /* VIRTIO_BUS_H */
-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 3/6] virtio-pci-bus : Introduce virtio-pci-bus.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Introduce virtio-pci-bus, which extends virtio-bus. It is used with virtio-pci
transport device.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-pci.c | 37 +++++++++++++++++++++++++++++++++++++
 hw/virtio-pci.h | 19 +++++++++++++++++--
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 71f4fb5..5ac8d0d 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -32,6 +32,7 @@
 #include "blockdev.h"
 #include "virtio-pci.h"
 #include "range.h"
+#include "virtio-bus.h"
 
 /* from Linux's linux/virtio_pci.h */
 
@@ -1118,6 +1119,41 @@ static TypeInfo virtio_scsi_info = {
     .class_init    = virtio_scsi_class_init,
 };
 
+/* virtio-pci-bus */
+
+VirtioBusState *virtio_pci_bus_new(VirtIOPCIProxy *dev)
+{
+    DeviceState *qdev = DEVICE(dev);
+    BusState *qbus = qbus_create(TYPE_VIRTIO_PCI_BUS, qdev, NULL);
+    VirtioBusState *bus = VIRTIO_BUS(qbus);
+    qbus->allow_hotplug = 0;
+    /* Only one virtio-device allowed for virtio-pci. */
+    qbus->max_dev = 1;
+    return bus;
+}
+
+static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
+{
+    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
+    k->notify = virtio_pci_notify;
+    k->save_config = virtio_pci_save_config;
+    k->load_config = virtio_pci_load_config;
+    k->save_queue = virtio_pci_save_queue;
+    k->load_queue = virtio_pci_load_queue;
+    k->get_features = virtio_pci_get_features;
+    k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
+    k->set_host_notifier = virtio_pci_set_host_notifier;
+    k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
+    k->vmstate_change = virtio_pci_vmstate_change;
+}
+
+static const TypeInfo virtio_pci_bus_info = {
+    .name          = TYPE_VIRTIO_PCI_BUS,
+    .parent        = TYPE_VIRTIO_BUS,
+    .instance_size = sizeof(VirtioBusState),
+    .class_init    = virtio_pci_bus_class_init,
+};
+
 static void virtio_pci_register_types(void)
 {
     type_register_static(&virtio_blk_info);
@@ -1126,6 +1162,7 @@ static void virtio_pci_register_types(void)
     type_register_static(&virtio_balloon_info);
     type_register_static(&virtio_scsi_info);
     type_register_static(&virtio_rng_info);
+    type_register_static(&virtio_pci_bus_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index b58d9a2..0e3288e 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -20,6 +20,21 @@
 #include "virtio-rng.h"
 #include "virtio-serial.h"
 #include "virtio-scsi.h"
+#include "virtio-bus.h"
+
+/* VirtIOPCIProxy will be renammed VirtioPCIState at the end. */
+typedef struct VirtIOPCIProxy VirtIOPCIProxy;
+
+/* virtio-pci-bus */
+#define TYPE_VIRTIO_PCI_BUS "virtio-pci-bus"
+#define VIRTIO_PCI_BUS_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioBusClass, obj, TYPE_VIRTIO_PCI_BUS)
+#define VIRTIO_PCI_BUS_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioBusClass, klass, TYPE_VIRTIO_PCI_BUS)
+#define VIRTIO_PCI_BUS(obj) \
+        OBJECT_CHECK(VirtioBusState, (obj), TYPE_VIRTIO_PCI_BUS)
+
+VirtioBusState *virtio_pci_bus_new(VirtIOPCIProxy *dev);
 
 /* Performance improves when virtqueue kick processing is decoupled from the
  * vcpu thread using ioeventfd for some devices. */
@@ -31,7 +46,7 @@ typedef struct {
     unsigned int users;
 } VirtIOIRQFD;
 
-typedef struct {
+struct VirtIOPCIProxy {
     PCIDevice pci_dev;
     VirtIODevice *vdev;
     MemoryRegion bar;
@@ -51,7 +66,7 @@ typedef struct {
     bool ioeventfd_disabled;
     bool ioeventfd_started;
     VirtIOIRQFD *vector_irqfd;
-} VirtIOPCIProxy;
+};
 
 void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
 void virtio_pci_reset(DeviceState *d);
-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 4/6] virtio-pci : Refactor virtio-pci device.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Create the virtio-pci device. This transport device will create a
virtio-pci-bus, so one VirtIODevice can be connected.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-pci.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-pci.h |  14 +++++++
 2 files changed, 126 insertions(+)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 5ac8d0d..ff0b5f1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1119,6 +1119,115 @@ static TypeInfo virtio_scsi_info = {
     .class_init    = virtio_scsi_class_init,
 };
 
+/*
+ * virtio-pci : This is the PCIDevice which have a virtio-pci-bus.
+ */
+
+/* This is called by virtio-bus just after the device is plugged. */
+static void virtio_pci_device_plugged(void *opaque)
+{
+    VirtIOPCIProxy *proxy = VIRTIO_PCI(opaque);
+    uint8_t *config;
+    uint32_t size;
+
+    /* Put the PCI IDs */
+    switch (get_virtio_device_id(proxy->bus)) {
+
+    case VIRTIO_ID_BLOCK:
+        pci_config_set_device_id(proxy->pci_dev.config,
+                                 PCI_DEVICE_ID_VIRTIO_BLOCK);
+        pci_config_set_class(proxy->pci_dev.config, PCI_CLASS_STORAGE_SCSI);
+    break;
+    default:
+        error_report("unknown device id\n");
+    break;
+
+    }
+
+    /* TODO: vdev should be accessed through virtio-bus functions. */
+    proxy->vdev = proxy->bus->vdev;
+    config = proxy->pci_dev.config;
+
+    if (proxy->class_code) {
+        pci_config_set_class(config, proxy->class_code);
+    }
+    pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
+                 pci_get_word(config + PCI_VENDOR_ID));
+    pci_set_word(config + PCI_SUBSYSTEM_ID, get_virtio_device_id(proxy->bus));
+    config[PCI_INTERRUPT_PIN] = 1;
+
+    if (proxy->bus->vdev->nvectors &&
+        msix_init_exclusive_bar(&proxy->pci_dev, proxy->bus->vdev->nvectors,
+                                1)) {
+        proxy->bus->vdev->nvectors = 0;
+    }
+
+    proxy->pci_dev.config_write = virtio_write_config;
+
+    size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev)
+         + proxy->bus->vdev->config_len;
+    if (size & (size-1)) {
+        size = 1 << qemu_fls(size);
+    }
+
+    memory_region_init_io(&proxy->bar, &virtio_pci_config_ops, proxy,
+                          "virtio-pci", size);
+    pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+                     &proxy->bar);
+
+    if (!kvm_has_many_ioeventfds()) {
+        proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
+    }
+
+    proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
+    proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE;
+    proxy->host_features = proxy->bus->vdev->get_features(proxy->bus->vdev,
+                                                          proxy->host_features);
+}
+
+/* This is called by virtio-bus just before the device is unplugged. */
+static void virtio_pci_device_unplug(void *opaque)
+{
+    VirtIOPCIProxy *dev = VIRTIO_PCI(opaque);
+    virtio_pci_stop_ioeventfd(dev);
+}
+
+static int virtio_pci_init(PCIDevice *pci_dev)
+{
+    VirtIOPCIProxy *dev = VIRTIO_PCI(pci_dev);
+    dev->bus = virtio_pci_bus_new(dev);
+    return 0;
+}
+
+static void virtio_pci_exit(PCIDevice *pci_dev)
+{
+    DeviceState *qdev = DEVICE(pci_dev);
+    BusState *qbus = BUS(qdev_get_parent_bus(qdev));
+    VirtioBusState *bus = VIRTIO_BUS(qbus);
+    virtio_bus_destroy_device(bus);
+    qbus_free(qbus);
+}
+
+static void virtio_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = virtio_pci_init;
+    k->exit = virtio_pci_exit;
+    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    k->revision = VIRTIO_PCI_ABI_VERSION;
+    k->class_id = PCI_CLASS_OTHERS;
+    dc->reset = virtio_pci_reset;
+}
+
+static const TypeInfo virtio_pci_info = {
+    .name          = TYPE_VIRTIO_PCI,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(VirtIOPCIProxy),
+    .class_init    = virtio_pci_class_init,
+};
+
 /* virtio-pci-bus */
 
 VirtioBusState *virtio_pci_bus_new(VirtIOPCIProxy *dev)
@@ -1145,6 +1254,8 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->set_host_notifier = virtio_pci_set_host_notifier;
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
     k->vmstate_change = virtio_pci_vmstate_change;
+    k->device_plugged = virtio_pci_device_plugged;
+    k->device_unplug = virtio_pci_device_unplug;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
@@ -1163,6 +1274,7 @@ static void virtio_pci_register_types(void)
     type_register_static(&virtio_scsi_info);
     type_register_static(&virtio_rng_info);
     type_register_static(&virtio_pci_bus_info);
+    type_register_static(&virtio_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index 0e3288e..2c31b7a 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -46,6 +46,17 @@ typedef struct {
     unsigned int users;
 } VirtIOIRQFD;
 
+/*
+ * virtio-pci : This is the PCIDevice which have a virtio-pci-bus.
+ */
+#define TYPE_VIRTIO_PCI "virtio-pci"
+#define VIRTIO_PCI_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioPCIClass, obj, TYPE_VIRTIO_PCI)
+#define VIRTIO_PCI_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioPCIClass, klass, TYPE_VIRTIO_PCI)
+#define VIRTIO_PCI(obj) \
+        OBJECT_CHECK(VirtIOPCIProxy, (obj), TYPE_VIRTIO_PCI)
+
 struct VirtIOPCIProxy {
     PCIDevice pci_dev;
     VirtIODevice *vdev;
@@ -66,6 +77,9 @@ struct VirtIOPCIProxy {
     bool ioeventfd_disabled;
     bool ioeventfd_started;
     VirtIOIRQFD *vector_irqfd;
+    /* That's the virtio-bus on which VirtioDevice will be connected. */
+    VirtioBusState *bus;
+    /* Nothing more for the moment. */
 };
 
 void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 5/6] virtio-device : Refactor virtio-device.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Create the virtio-device which is abstract. All the virtio-device can extend
this class.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
 hw/virtio.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index f40a8c5..2541ea8 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -16,6 +16,7 @@
 #include "trace.h"
 #include "qemu-error.h"
 #include "virtio.h"
+#include "virtio-bus.h"
 #include "qemu-barrier.h"
 
 /* The alignment to use between consumer and producer parts of vring.
@@ -902,14 +903,10 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
     }
 }
 
-VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
-                                 size_t config_size, size_t struct_size)
+void virtio_init(VirtIODevice *vdev, const char *name,
+                 uint16_t device_id, size_t config_size)
 {
-    VirtIODevice *vdev;
     int i;
-
-    vdev = g_malloc0(struct_size);
-
     vdev->device_id = device_id;
     vdev->status = 0;
     vdev->isr = 0;
@@ -917,20 +914,28 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
     vdev->config_vector = VIRTIO_NO_VECTOR;
     vdev->vq = g_malloc0(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
     vdev->vm_running = runstate_is_running();
-    for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
         vdev->vq[i].vector = VIRTIO_NO_VECTOR;
         vdev->vq[i].vdev = vdev;
     }
 
     vdev->name = name;
     vdev->config_len = config_size;
-    if (vdev->config_len)
+    if (vdev->config_len) {
         vdev->config = g_malloc0(config_size);
-    else
+    } else {
         vdev->config = NULL;
+    }
+    vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change,
+                                                     vdev);
+}
 
-    vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change, vdev);
-
+VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
+                                 size_t config_size, size_t struct_size)
+{
+    VirtIODevice *vdev;
+    vdev = g_malloc0(struct_size);
+    virtio_init(vdev, name, device_id, config_size);
     return vdev;
 }
 
@@ -1056,3 +1061,26 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
 {
     return &vq->host_notifier;
 }
+
+static void virtio_device_class_init(ObjectClass *klass, void *data)
+{
+    /* Set the default value here. */
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->bus_type = TYPE_VIRTIO_BUS;
+}
+
+static const TypeInfo virtio_device_info = {
+    .name = TYPE_VIRTIO_DEVICE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(VirtIODevice),
+    .class_init = virtio_device_class_init,
+    .abstract = true,
+    .class_size = sizeof(VirtioDeviceClass),
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_device_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio.h b/hw/virtio.h
index 7c17f7b..79d2b91 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -108,8 +108,17 @@ typedef struct {
 
 #define VIRTIO_NO_VECTOR 0xffff
 
+#define TYPE_VIRTIO_DEVICE "virtio-device"
+#define VIRTIO_DEVICE_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioDeviceClass, obj, TYPE_VIRTIO_DEVICE)
+#define VIRTIO_DEVICE_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioDeviceClass, klass, TYPE_VIRTIO_DEVICE)
+#define VIRTIO_DEVICE(obj) \
+        OBJECT_CHECK(VirtIODevice, (obj), TYPE_VIRTIO_DEVICE)
+
 struct VirtIODevice
 {
+    DeviceState parent_obj;
     const char *name;
     uint8_t status;
     uint8_t isr;
@@ -119,6 +128,9 @@ struct VirtIODevice
     void *config;
     uint16_t config_vector;
     int nvectors;
+    /*
+     * Must be removed as we have it in VirtioDeviceClass.
+     */
     uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
     uint32_t (*bad_features)(VirtIODevice *vdev);
     void (*set_features)(VirtIODevice *vdev, uint32_t val);
@@ -126,6 +138,7 @@ struct VirtIODevice
     void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
     void (*reset)(VirtIODevice *vdev);
     void (*set_status)(VirtIODevice *vdev, uint8_t val);
+    /***/
     VirtQueue *vq;
     const VirtIOBindings *binding;
     void *binding_opaque;
@@ -134,6 +147,21 @@ struct VirtIODevice
     VMChangeStateEntry *vmstate;
 };
 
+typedef struct {
+    /* This is what a VirtioDevice must implement */
+    DeviceClass parent;
+    uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
+    uint32_t (*bad_features)(VirtIODevice *vdev);
+    void (*set_features)(VirtIODevice *vdev, uint32_t val);
+    void (*get_config)(VirtIODevice *vdev, uint8_t *config);
+    void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
+    void (*reset)(VirtIODevice *vdev);
+    void (*set_status)(VirtIODevice *vdev, uint8_t val);
+} VirtioDeviceClass;
+
+void virtio_init(VirtIODevice *vdev, const char *name,
+                         uint16_t device_id, size_t config_size);
+
 VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
                             void (*handle_output)(VirtIODevice *,
                                                   VirtQueue *));
-- 
1.7.11.7

[Qemu-devel] [RFC PATCH v6 6/6] virtio-blk : Add the virtio-blk device.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Create virtio-blk which extends virtio-device, so it can be connected on
virtio-bus. I suggest one step to refactor virtio-blk-pci, and one more to clean
virtio-blk.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/virtio-blk.h |   4 +++
 2 files changed, 96 insertions(+), 9 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index e25cc96..9b6a8f5 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -21,6 +21,7 @@
 #ifdef __linux__
 # include <scsi/sg.h>
 #endif
+#include "virtio-bus.h"
 
 typedef struct VirtIOBlock
 {
@@ -30,11 +31,14 @@ typedef struct VirtIOBlock
     void *rq;
     QEMUBH *bh;
     BlockConf *conf;
-    VirtIOBlkConf *blk;
+    VirtIOBlkConf blk;
     unsigned short sector_mask;
     DeviceState *qdev;
 } VirtIOBlock;
 
+/*
+ * Moving to QOM later in this series.
+ */
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 {
     return (VirtIOBlock *)vdev;
@@ -164,7 +168,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
      */
     req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
 
-    if (!req->dev->blk->scsi) {
+    if (!req->dev->blk.scsi) {
         status = VIRTIO_BLK_S_UNSUPP;
         goto fail;
     }
@@ -384,7 +388,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
          * terminated by '\0' only when shorter than buffer.
          */
         strncpy(req->elem.in_sg[0].iov_base,
-                s->blk->serial ? s->blk->serial : "",
+                s->blk.serial ? s->blk.serial : "",
                 MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
         g_free(req);
@@ -600,9 +604,10 @@ static const BlockDevOps virtio_block_ops = {
     .resize_cb = virtio_blk_resize,
 };
 
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
+                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
 {
-    VirtIOBlock *s;
+    VirtIOBlock *s = *ps;
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
@@ -619,9 +624,20 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
         return NULL;
     }
 
-    s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                          sizeof(struct virtio_blk_config),
-                                          sizeof(VirtIOBlock));
+    /*
+     * We have two cases here : the old virtio-blk-pci device, and the
+     * refactored virtio-blk.
+     */
+    if (s == NULL) {
+        /* virtio-blk-pci */
+        s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
+                                              sizeof(struct virtio_blk_config),
+                                              sizeof(VirtIOBlock));
+    } else {
+        /* virtio-blk */
+        virtio_init(VIRTIO_DEVICE(s), "virtio-blk", VIRTIO_ID_BLOCK,
+                    sizeof(struct virtio_blk_config));
+    }
 
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.set_config = virtio_blk_set_config;
@@ -630,7 +646,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->vdev.reset = virtio_blk_reset;
     s->bs = blk->conf.bs;
     s->conf = &blk->conf;
-    s->blk = blk;
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
@@ -649,6 +665,12 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     return &s->vdev;
 }
 
+VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+{
+    VirtIOBlock *s = NULL;
+    return virtio_blk_common_init(dev, blk, &s);
+}
+
 void virtio_blk_exit(VirtIODevice *vdev)
 {
     VirtIOBlock *s = to_virtio_blk(vdev);
@@ -656,3 +678,64 @@ void virtio_blk_exit(VirtIODevice *vdev)
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
 }
+
+
+static int virtio_device_init(DeviceState *qdev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIOBlock *s = VIRTIO_BLK(qdev);
+
+    VirtIOBlkConf *blk = &(s->blk);
+
+    virtio_blk_common_init(qdev, blk, &s);
+
+    virtio_bus_plug_device(vdev);
+
+    return 0;
+}
+
+static int virtio_device_exit(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    virtio_blk_exit(vdev);
+    return 0;
+}
+
+static Property virtio_blk_properties[] = {
+    DEFINE_BLOCK_PROPERTIES(VirtIOBlock, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, blk.conf),
+    DEFINE_PROP_STRING("serial", VirtIOBlock, blk.serial),
+#ifdef __linux__
+    DEFINE_PROP_BIT("scsi", VirtIOBlock, blk.scsi, 0, true),
+#endif
+    DEFINE_PROP_BIT("config-wce", VirtIOBlock, blk.config_wce, 0, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_blk_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    dc->init = virtio_device_init;
+    dc->exit = virtio_device_exit;
+    dc->props = virtio_blk_properties;
+    vdc->get_config = virtio_blk_update_config;
+    vdc->set_config = virtio_blk_set_config;
+    vdc->get_features = virtio_blk_get_features;
+    vdc->set_status = virtio_blk_set_status;
+    vdc->reset = virtio_blk_reset;
+}
+
+static const TypeInfo virtio_device_info = {
+    .name = TYPE_VIRTIO_BLK,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOBlock),
+    .class_init = virtio_blk_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_device_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index f0740d0..97adaf2 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -17,6 +17,10 @@
 #include "virtio.h"
 #include "hw/block-common.h"
 
+#define TYPE_VIRTIO_BLK "virtio-blk"
+#define VIRTIO_BLK(obj) \
+        OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
+
 /* from Linux's linux/virtio_blk.h */
 
 /* The ID for virtio_block */
-- 
1.7.11.7

Re: [Qemu-devel] [RFC PATCH v6 6/6] virtio-blk : Add the virtio-blk device.

[Peter Maydell]

On 7 December 2012 13:32,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Create virtio-blk which extends virtio-device, so it can be connected on
> virtio-bus. I suggest one step to refactor virtio-blk-pci, and one more to clean
> virtio-blk.

Yes; I think you need to take this patchseries another step forward.
There doesn't seem to be anything particularly obviously wrong
here but it's hard to tell without seeing what happens to
virtio-blk-pci and what the final virtio-blk looks like.

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Fri, Dec 07, 2012 at 02:32:29PM +0100, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> You can clone that from here :
> git.greensocs.com/home/greensocs/git/qemu_virtio.git virtio_refactoring_v6
> 
> The problem with the last RFC v5 was that virtio-blk refactoring broke
> virtio-blk-pci device ( SEGFAULT ). So I modify this last step to fix that
> issue.
> 
> In order to not break anything, I think we have to refactor virtio-pci-blk in a
> next step then add a supplementary step which clean virtio-blk 
> ( eg : fix the cast ).
> 
> Does it make sense ?

I am yet to go over the patches but I did try to read
previous discussion and I am still puzzled about the motivation. One of
the previous messages mentioned this is to allow virtio-mmio.

Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
another bus, like a pci bus, and another binding, like the virtio-pci
binding?

Is the issue that bindings are not devices?
I'm sending a patchset to use DeviceState as binding pointer -
will this address the issue?

If this was covered but I missed this I'll be thankful for
pointers if any.
Thanks,

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[KONRAD Frédéric]

On 17/12/2012 16:45, Michael S. Tsirkin wrote:
> On Fri, Dec 07, 2012 at 02:32:29PM +0100, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> You can clone that from here :
>> git.greensocs.com/home/greensocs/git/qemu_virtio.git virtio_refactoring_v6
>>
>> The problem with the last RFC v5 was that virtio-blk refactoring broke
>> virtio-blk-pci device ( SEGFAULT ). So I modify this last step to fix that
>> issue.
>>
>> In order to not break anything, I think we have to refactor virtio-pci-blk in a
>> next step then add a supplementary step which clean virtio-blk
>> ( eg : fix the cast ).
>>
>> Does it make sense ?
> I am yet to go over the patches but I did try to read
> previous discussion and I am still puzzled about the motivation. One of
> the previous messages mentioned this is to allow virtio-mmio.
Yes, the main goal is to have the choice of the transport device.

eg :

-device virtio-pci,id=transport1 -device virtio-scsi,bus=transport1

or

-device virtio-mmio,id=transport1 -device virtio-scsi,bus=transport1

That's why anthony suggest to create a virtio-bus to connect transport 
to device.

so all virtio-x devices must be created. And virtio-pci must have a 
virtio-bus.

Then to keep compability with the older version virtio-x-pci must create 
virtio-pci and virtio-x.

>
> Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> another bus, like a pci bus, and another binding, like the virtio-pci
> binding?
Do you mean something like creating all virtio device like virtio-mmio-x ?

>
> Is the issue that bindings are not devices?
> I'm sending a patchset to use DeviceState as binding pointer -
> will this address the issue?
The issue is that all is linked, and here the virtio-blk refactoring breaks
virtio-blk-pci and virtio-blk-s390 devices as they didn't use QOM.
But the newer version ( V7 ) didn't break anything.

>
> If this was covered but I missed this I'll be thankful for
> pointers if any.
> Thanks,
>

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Dec 17, 2012 at 06:13:28PM +0100, KONRAD Frédéric wrote:
> On 17/12/2012 16:45, Michael S. Tsirkin wrote:
> >On Fri, Dec 07, 2012 at 02:32:29PM +0100, fred.konrad@greensocs.com wrote:
> >>From: KONRAD Frederic <fred.konrad@greensocs.com>
> >>
> >>You can clone that from here :
> >>git.greensocs.com/home/greensocs/git/qemu_virtio.git virtio_refactoring_v6
> >>
> >>The problem with the last RFC v5 was that virtio-blk refactoring broke
> >>virtio-blk-pci device ( SEGFAULT ). So I modify this last step to fix that
> >>issue.
> >>
> >>In order to not break anything, I think we have to refactor virtio-pci-blk in a
> >>next step then add a supplementary step which clean virtio-blk
> >>( eg : fix the cast ).
> >>
> >>Does it make sense ?
> >I am yet to go over the patches but I did try to read
> >previous discussion and I am still puzzled about the motivation. One of
> >the previous messages mentioned this is to allow virtio-mmio.
> Yes, the main goal is to have the choice of the transport device.
> 
> eg :
> 
> -device virtio-pci,id=transport1 -device virtio-scsi,bus=transport1
> 
> or
> 
> -device virtio-mmio,id=transport1 -device virtio-scsi,bus=transport1
> That's why anthony suggest to create a virtio-bus to connect
> transport to device.

If it's an implementation detail, fine.  If it's user-visible, I think
it's a bad idea.

> so all virtio-x devices must be created. And virtio-pci must have a
> virtio-bus.
> 
> Then to keep compability with the older version virtio-x-pci must
> create virtio-pci and virtio-x.
> >
> >Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >another bus, like a pci bus, and another binding, like the virtio-pci
> >binding?
> Do you mean something like creating all virtio device like virtio-mmio-x ?

Yes or to be closer to what we do with pci, virtio-x-mmio.

> >
> >Is the issue that bindings are not devices?
> >I'm sending a patchset to use DeviceState as binding pointer -
> >will this address the issue?
> The issue is that all is linked, and here the virtio-blk refactoring breaks
> virtio-blk-pci and virtio-blk-s390 devices as they didn't use QOM.
> But the newer version ( V7 ) didn't break anything.

No, my question was why is all this useful?

> >
> >If this was covered but I missed this I'll be thankful for
> >pointers if any.
> >Thanks,
> >

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> another bus, like a pci bus, and another binding, like the virtio-pci
> binding?

(a) the current code is really not very nice because it's not
actually a proper set of QOM/qdev devices
(b) unlike PCI, you can't create sysbus devices on the
command line, because they don't correspond to a user
pluggable bit of hardware. We don't want users to have to know
an address and IRQ number for each virtio-mmio device (especially
since these are board specific); instead the board can create
and wire up transport devices wherever is suitable, and the
user just creates the backend (which is plugged into the virtio bus).

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> > another bus, like a pci bus, and another binding, like the virtio-pci
> > binding?
> 
> (a) the current code is really not very nice because it's not
> actually a proper set of QOM/qdev devices
> (b) unlike PCI, you can't create sysbus devices on the
> command line, because they don't correspond to a user
> pluggable bit of hardware. We don't want users to have to know
> an address and IRQ number for each virtio-mmio device (especially
> since these are board specific); instead the board can create
> and wire up transport devices wherever is suitable, and the
> user just creates the backend (which is plugged into the virtio bus).
> 
> -- PMM

This is what I am saying: create your own bus and put
your devices there. Allocate resources when you init
a device.

Instead you seem to want to expose a virtio device as two devices to
user - if true this is not reasonable.

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 18 December 2012 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> This is what I am saying: create your own bus and put
> your devices there.

What bus?

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[KONRAD Frédéric]

On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>> On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>>> another bus, like a pci bus, and another binding, like the virtio-pci
>>> binding?
>> (a) the current code is really not very nice because it's not
>> actually a proper set of QOM/qdev devices
>> (b) unlike PCI, you can't create sysbus devices on the
>> command line, because they don't correspond to a user
>> pluggable bit of hardware. We don't want users to have to know
>> an address and IRQ number for each virtio-mmio device (especially
>> since these are board specific); instead the board can create
>> and wire up transport devices wherever is suitable, and the
>> user just creates the backend (which is plugged into the virtio bus).
>>
>> -- PMM
> This is what I am saying: create your own bus and put
> your devices there. Allocate resources when you init
> a device.
>
> Instead you seem to want to expose a virtio device as two devices to
> user - if true this is not reasonable.
>
The modifications will be transparent to the user, as we will keep 
virtio-x-pci devices.

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Paolo Bonzini]

Il 18/12/2012 12:26, Peter Maydell ha scritto:
> On 18 December 2012 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>> This is what I am saying: create your own bus and put
>> your devices there.
> 
> What bus?

A virtio bus like the one in these patches.  But mst is suggesting to
leave virtio.c aside, and only use the virtio bus in the virtio-x-mmio
devices, to connect to the virtio-mmio device provided by the board.

Either way is fine for me.

Paolo

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 18 December 2012 11:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/12/2012 12:26, Peter Maydell ha scritto:
>> On 18 December 2012 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> This is what I am saying: create your own bus and put
>>> your devices there.
>>
>> What bus?
>
> A virtio bus like the one in these patches.  But mst is suggesting to
> leave virtio.c aside, and only use the virtio bus in the virtio-x-mmio
> devices, to connect to the virtio-mmio device provided by the board.

That doesn't make any sense to me -- why would you want to make
mmio be randomly different from the other virtio transports? The code
as it stands is a mess which ought to be cleaned up anyway...

To the extent that it's painful for users to manipulate qdev devices on
the command line, that's a problem we ought to address at some point
anyhow.

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 12:06:39PM +0000, Peter Maydell wrote:
> On 18 December 2012 11:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Il 18/12/2012 12:26, Peter Maydell ha scritto:
> >> On 18 December 2012 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>> This is what I am saying: create your own bus and put
> >>> your devices there.
> >>
> >> What bus?
> >
> > A virtio bus like the one in these patches.  But mst is suggesting to
> > leave virtio.c aside, and only use the virtio bus in the virtio-x-mmio
> > devices, to connect to the virtio-mmio device provided by the board.
> 
> That doesn't make any sense to me -- why would you want to make
> mmio be randomly different from the other virtio transports?

Not different at all. virtio-pci uses the pci bus.
It's you who's saying mmio is different and can not
work with existing bindings.

> The code
> as it stands is a mess which ought to be cleaned up anyway...

My experience is all working code is somewhat messy.  This is working
code.

I'm sure we shouldnot add more ways to create devices,
we have two for each device already, that's too much.
You can't seriously add yet another way, keep the two
existing ones around as legacy and claim it's a cleanup.
You are single-handedly making the testing matrix bigger by 1/3.

And what makes virtio so special anyway? e1000 can be used without
exposing users to internal buses and all kind of nastiness like this.
People just want to install a driver and have a faster IO.

> To the extent that it's painful for users to manipulate qdev devices on
> the command line, that's a problem we ought to address at some point
> anyhow.
> 
> -- PMM

It's pretty painful, yes, but adding yet another way to do it is not
addressing the problem.

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >>>another bus, like a pci bus, and another binding, like the virtio-pci
> >>>binding?
> >>(a) the current code is really not very nice because it's not
> >>actually a proper set of QOM/qdev devices
> >>(b) unlike PCI, you can't create sysbus devices on the
> >>command line, because they don't correspond to a user
> >>pluggable bit of hardware. We don't want users to have to know
> >>an address and IRQ number for each virtio-mmio device (especially
> >>since these are board specific); instead the board can create
> >>and wire up transport devices wherever is suitable, and the
> >>user just creates the backend (which is plugged into the virtio bus).
> >>
> >>-- PMM
> >This is what I am saying: create your own bus and put
> >your devices there. Allocate resources when you init
> >a device.
> >
> >Instead you seem to want to expose a virtio device as two devices to
> >user - if true this is not reasonable.
> >
> The modifications will be transparent to the user, as we will keep
> virtio-x-pci devices.

So there are three ways to add virtio pci devices now.
Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
and the new one with two devices.
If yes it's not transparent, it's user visible.
Or did I misunderstand?

Look we can have a virtio network device on a PCI bus.
A very similar device can be created on XXX bus, and
we can and do share a lot of code.
This makes it two devices? Why not 4?
One for TX one for RX one for control one for PCI.
I hope I'm not giving anyone ideas ...

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 18 December 2012 13:10, Michael S. Tsirkin <mst@redhat.com> wrote:
> And what makes virtio so special anyway? e1000 can be used without
> exposing users to internal buses and all kind of nastiness like this.

Congratulations, you're using an architecture that has a pluggable
discoverable bus implemented by just about all machines using that
architecture. That makes things much easier for you.

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Paolo Bonzini]

Il 18/12/2012 15:00, Peter Maydell ha scritto:
> On 18 December 2012 13:10, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > And what makes virtio so special anyway? e1000 can be used without
>> > exposing users to internal buses and all kind of nastiness like this.
> Congratulations, you're using an architecture that has a pluggable
> discoverable bus implemented by just about all machines using that
> architecture. That makes things much easier for you.

Yes, that's true.  And you're basically using virtio as the pluggable
discoverable bus, which is actually a pretty good idea.

However, what you are doing is very similar to what virtio-s390 does,
and it manages to do it just fine with the existing virtio.c
infrastructure.  The only difference is that you have a 1:1 relationship
between virtio-mmio "slots" described by the board and virtio-mmio
devices added by the user.

True, it is not pure qdev, but it is much simpler and doesn't require
convincing grumpy maintainers. :)

Paolo

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 02:00:11PM +0000, Peter Maydell wrote:
> On 18 December 2012 13:10, Michael S. Tsirkin <mst@redhat.com> wrote:
> > And what makes virtio so special anyway? e1000 can be used without
> > exposing users to internal buses and all kind of nastiness like this.
> 
> Congratulations, you're using an architecture that has a pluggable
> discoverable bus implemented by just about all machines using that
> architecture. That makes things much easier for you.
> 
> -- PMM

That's on the guest side. Yes, drivers are a problem but they are not
qemu problem as such.
But on the user/qemu side I dont' yes see why it's so different.

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 18 December 2012 14:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Yes, that's true.  And you're basically using virtio as the pluggable
> discoverable bus, which is actually a pretty good idea.
>
> However, what you are doing is very similar to what virtio-s390 does,
> and it manages to do it just fine with the existing virtio.c
> infrastructure.  The only difference is that you have a 1:1 relationship
> between virtio-mmio "slots" described by the board and virtio-mmio
> devices added by the user.

Also it looks like the board model and the 'bridge' and the transport
implementation are all collaborating to get the virtio memory sorted
out, rather than it just being "instantiate a bridge here"...

> True, it is not pure qdev, but it is much simpler and doesn't require
> convincing grumpy maintainers. :)

I'm not actually personally all that attached to this design -- it's just
trying to implement a suggestion by Anthony.

It does seem frankly bizarre that adding a new transport requires
knowing about all the backends (notice how s390-virtio-bus.c has
to register types for each backend). The kernel gets the transport
vs backend separation much cleaner and it was much easier to
add the virtio support there.

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Paolo Bonzini]

Il 18/12/2012 15:56, Peter Maydell ha scritto:
> On 18 December 2012 14:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Yes, that's true.  And you're basically using virtio as the pluggable
>> discoverable bus, which is actually a pretty good idea.
>>
>> However, what you are doing is very similar to what virtio-s390 does,
>> and it manages to do it just fine with the existing virtio.c
>> infrastructure.  The only difference is that you have a 1:1 relationship
>> between virtio-mmio "slots" described by the board and virtio-mmio
>> devices added by the user.
> 
> Also it looks like the board model and the 'bridge' and the transport
> implementation are all collaborating to get the virtio memory sorted
> out, rather than it just being "instantiate a bridge here"...

But s390 is weird. :)

>> True, it is not pure qdev, but it is much simpler and doesn't require
>> convincing grumpy maintainers. :)
> 
> I'm not actually personally all that attached to this design -- it's just
> trying to implement a suggestion by Anthony.

Yes, and I agree FWIW.

> It does seem frankly bizarre that adding a new transport requires
> knowing about all the backends (notice how s390-virtio-bus.c has
> to register types for each backend). The kernel gets the transport
> vs backend separation much cleaner and it was much easier to
> add the virtio support there.

Yes, I agree.  However, to some extent it's unavoidable.  For example,
the PCI transport needs to know the class id for each backend.  You may
have a single virtio-pci device types, or separate types for
virtio-blk/net/scsi-pci, but it's true anyway.

Paolo

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 02:56:58PM +0000, Peter Maydell wrote:
> > True, it is not pure qdev, but it is much simpler and doesn't require
> > convincing grumpy maintainers. :)
> 
> I'm not actually personally all that attached to this design -- it's just
> trying to implement a suggestion by Anthony.
> 
> It does seem frankly bizarre that adding a new transport requires
> knowing about all the backends (notice how s390-virtio-bus.c has
> to register types for each backend).

I agree it's not pretty. The issue is that things like e.g.
PCI class need per-device handling. We could invent our
own class system but that seems like overkill:
it's rare enough to add new devices.

> The kernel gets the transport
> vs backend separation much cleaner and it was much easier to
> add the virtio support there.
> 
> -- PMM

By the way even there the separation does confuse users sometimes.
Things like autoloading are broken
and some tools poking at sysfs get confused.

At this point it's probably not worth changing though.
-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 03:59:59PM +0100, Paolo Bonzini wrote:
> > It does seem frankly bizarre that adding a new transport requires
> > knowing about all the backends (notice how s390-virtio-bus.c has
> > to register types for each backend). The kernel gets the transport
> > vs backend separation much cleaner and it was much easier to
> > add the virtio support there.
> 
> Yes, I agree.  However, to some extent it's unavoidable.  For example,
> the PCI transport needs to know the class id for each backend.

PCI device ID too (they don't match virtio IDs).

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >>>another bus, like a pci bus, and another binding, like the virtio-pci
> >>>binding?
> >>(a) the current code is really not very nice because it's not
> >>actually a proper set of QOM/qdev devices
> >>(b) unlike PCI, you can't create sysbus devices on the
> >>command line, because they don't correspond to a user
> >>pluggable bit of hardware. We don't want users to have to know
> >>an address and IRQ number for each virtio-mmio device (especially
> >>since these are board specific); instead the board can create
> >>and wire up transport devices wherever is suitable, and the
> >>user just creates the backend (which is plugged into the virtio bus).
> >>
> >>-- PMM
> >This is what I am saying: create your own bus and put
> >your devices there. Allocate resources when you init
> >a device.
> >
> >Instead you seem to want to expose a virtio device as two devices to
> >user - if true this is not reasonable.
> >
> The modifications will be transparent to the user, as we will keep
> virtio-x-pci devices.

Then what's the point of all this?

-device virtio-pci,id=transport1 -device virtio-net,bus=transport1

or

-device virtio-mmio,id=transport1 -device virtio-net,bus=transport1

Is simply an insane way to create a network device.

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Peter Maydell]

On 7 January 2013 19:58, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> The modifications will be transparent to the user, as we will keep
>> virtio-x-pci devices.
>
> Then what's the point of all this?
>
> -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
>
> or
>
> -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
>
> Is simply an insane way to create a network device.

1. You wouldn't create the virtio-mmio transport on the command line,
the machine model does it (it has to because it's a sysbus device
and it needs the address/irq lines wiring up properly), so it's just
 "-device virtio-net" (and let qemu find the bus automatically)
2. We shouldn't be making command line simplicity drive how we
model devices inside QEMU. If we wanted to do that we should have
stuck with the old -net command line arguments which are rather
more userfriendly IMHO. If commandline confusion is getting to
be a problem with all the -device foo stuff then we should probably
fix that at the UI level.

-- PMM

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
>> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>> >>>another bus, like a pci bus, and another binding, like the virtio-pci
>> >>>binding?
>> >>(a) the current code is really not very nice because it's not
>> >>actually a proper set of QOM/qdev devices
>> >>(b) unlike PCI, you can't create sysbus devices on the
>> >>command line, because they don't correspond to a user
>> >>pluggable bit of hardware. We don't want users to have to know
>> >>an address and IRQ number for each virtio-mmio device (especially
>> >>since these are board specific); instead the board can create
>> >>and wire up transport devices wherever is suitable, and the
>> >>user just creates the backend (which is plugged into the virtio bus).
>> >>
>> >>-- PMM
>> >This is what I am saying: create your own bus and put
>> >your devices there. Allocate resources when you init
>> >a device.
>> >
>> >Instead you seem to want to expose a virtio device as two devices to
>> >user - if true this is not reasonable.
>> >
>> The modifications will be transparent to the user, as we will keep
>> virtio-x-pci devices.
>
> So there are three ways to add virtio pci devices now.
> Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
> and the new one with two devices.
> If yes it's not transparent, it's user visible.
> Or did I misunderstand?
>
> Look we can have a virtio network device on a PCI bus.
> A very similar device can be created on XXX bus, and
> we can and do share a lot of code.
> This makes it two devices? Why not 4?
> One for TX one for RX one for control one for PCI.
> I hope I'm not giving anyone ideas ...

Devices != things users need to worry about.

The documented way to create network devices is completely different
than any possible syntax you can conjure up with -device.

Really, -device is not something users should have to deal with--ever.
It's a low level API, not a UI.

Regards,

Anthony Liguori

>
> -- 
> MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
>> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>> >>>another bus, like a pci bus, and another binding, like the virtio-pci
>> >>>binding?
>> >>(a) the current code is really not very nice because it's not
>> >>actually a proper set of QOM/qdev devices
>> >>(b) unlike PCI, you can't create sysbus devices on the
>> >>command line, because they don't correspond to a user
>> >>pluggable bit of hardware. We don't want users to have to know
>> >>an address and IRQ number for each virtio-mmio device (especially
>> >>since these are board specific); instead the board can create
>> >>and wire up transport devices wherever is suitable, and the
>> >>user just creates the backend (which is plugged into the virtio bus).
>> >>
>> >>-- PMM
>> >This is what I am saying: create your own bus and put
>> >your devices there. Allocate resources when you init
>> >a device.
>> >
>> >Instead you seem to want to expose a virtio device as two devices to
>> >user - if true this is not reasonable.
>> >
>> The modifications will be transparent to the user, as we will keep
>> virtio-x-pci devices.
>
> Then what's the point of all this?
>
> -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
>
> or
>
> -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
>
> Is simply an insane way to create a network device.

virtio-pci is pluggable into a bus, virtio-mmio is not.  There is a
fixed number of them for a specific board and you have the option to add
virtio devices to the ones that already exist.

virtio is a mess in its current state.  Large swaths are not qdev'ified
and we jump through hoops to try to set properties.

I can't imagine how you can look at this cleanup and not understand why
it's nicer...

Regards,

Anthony Liguori

>
> -- 
> MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 08:02:32PM +0000, Peter Maydell wrote:
> On 7 January 2013 19:58, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> >> The modifications will be transparent to the user, as we will keep
> >> virtio-x-pci devices.
> >
> > Then what's the point of all this?
> >
> > -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
> >
> > or
> >
> > -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
> >
> > Is simply an insane way to create a network device.
> 
> 1. You wouldn't create the virtio-mmio transport on the command line,
> the machine model does it (it has to because it's a sysbus device
> and it needs the address/irq lines wiring up properly), so it's just
>  "-device virtio-net" (and let qemu find the bus automatically)

Bus auto-detection sounds good and would be nice for pci too.
We had things like model=virtio originally which is pretty close.
But the issue is, how then do you pass bus specific arguments like pci
slot? This is what caused us to go the virtio-net-pci route
to begin with.

> 2. We shouldn't be making command line simplicity drive how we
> model devices inside QEMU.

Confused.  I was told that enabling
 -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
is the reason we have this patchset.

> If we wanted to do that we should have
> stuck with the old -net command line arguments which are rather
> more userfriendly IMHO.

The main thing that confused people with -net was the vlans
and the need to specify -net twice.
A good UI would have been e.g. -nic model=virtio,net=user.

But one bad UI does not justify another one.

> If commandline confusion is getting to
> be a problem with all the -device foo stuff then we should probably
> fix that at the UI level.
> 
> -- PMM

I'd like to see a proposal about how we are going to do this.

-- 
MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 02:12:23PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> >> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >> >>>another bus, like a pci bus, and another binding, like the virtio-pci
> >> >>>binding?
> >> >>(a) the current code is really not very nice because it's not
> >> >>actually a proper set of QOM/qdev devices
> >> >>(b) unlike PCI, you can't create sysbus devices on the
> >> >>command line, because they don't correspond to a user
> >> >>pluggable bit of hardware. We don't want users to have to know
> >> >>an address and IRQ number for each virtio-mmio device (especially
> >> >>since these are board specific); instead the board can create
> >> >>and wire up transport devices wherever is suitable, and the
> >> >>user just creates the backend (which is plugged into the virtio bus).
> >> >>
> >> >>-- PMM
> >> >This is what I am saying: create your own bus and put
> >> >your devices there. Allocate resources when you init
> >> >a device.
> >> >
> >> >Instead you seem to want to expose a virtio device as two devices to
> >> >user - if true this is not reasonable.
> >> >
> >> The modifications will be transparent to the user, as we will keep
> >> virtio-x-pci devices.
> >
> > So there are three ways to add virtio pci devices now.
> > Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
> > and the new one with two devices.
> > If yes it's not transparent, it's user visible.
> > Or did I misunderstand?
> >
> > Look we can have a virtio network device on a PCI bus.
> > A very similar device can be created on XXX bus, and
> > we can and do share a lot of code.
> > This makes it two devices? Why not 4?
> > One for TX one for RX one for control one for PCI.
> > I hope I'm not giving anyone ideas ...
> 
> Devices != things users need to worry about.
> 
> The documented way to create network devices is completely different
> than any possible syntax you can conjure up with -device.
> 
> Really, -device is not something users should have to deal with--ever.
> It's a low level API, not a UI.
> 
> Regards,
> 
> Anthony Liguori

Interesting.
Let's assume I want to put a device behind a pci bridge
(for example I want more than 32 of these).
It's impossible without -device, isn't it?

> >
> > -- 
> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Jan 07, 2013 at 02:12:23PM -0600, Anthony Liguori wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> >> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
>> >> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>> >> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>> >> >>>another bus, like a pci bus, and another binding, like the virtio-pci
>> >> >>>binding?
>> >> >>(a) the current code is really not very nice because it's not
>> >> >>actually a proper set of QOM/qdev devices
>> >> >>(b) unlike PCI, you can't create sysbus devices on the
>> >> >>command line, because they don't correspond to a user
>> >> >>pluggable bit of hardware. We don't want users to have to know
>> >> >>an address and IRQ number for each virtio-mmio device (especially
>> >> >>since these are board specific); instead the board can create
>> >> >>and wire up transport devices wherever is suitable, and the
>> >> >>user just creates the backend (which is plugged into the virtio bus).
>> >> >>
>> >> >>-- PMM
>> >> >This is what I am saying: create your own bus and put
>> >> >your devices there. Allocate resources when you init
>> >> >a device.
>> >> >
>> >> >Instead you seem to want to expose a virtio device as two devices to
>> >> >user - if true this is not reasonable.
>> >> >
>> >> The modifications will be transparent to the user, as we will keep
>> >> virtio-x-pci devices.
>> >
>> > So there are three ways to add virtio pci devices now.
>> > Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
>> > and the new one with two devices.
>> > If yes it's not transparent, it's user visible.
>> > Or did I misunderstand?
>> >
>> > Look we can have a virtio network device on a PCI bus.
>> > A very similar device can be created on XXX bus, and
>> > we can and do share a lot of code.
>> > This makes it two devices? Why not 4?
>> > One for TX one for RX one for control one for PCI.
>> > I hope I'm not giving anyone ideas ...
>> 
>> Devices != things users need to worry about.
>> 
>> The documented way to create network devices is completely different
>> than any possible syntax you can conjure up with -device.
>> 
>> Really, -device is not something users should have to deal with--ever.
>> It's a low level API, not a UI.
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> Interesting.
> Let's assume I want to put a device behind a pci bridge
> (for example I want more than 32 of these).

You don't want to put a device behind a PCI bridge, you want to have
more than 32 devices.

'-net nic' should do the Right Thing when presented with more than 32
devices.

> It's impossible without -device, isn't it?

Think of -device like an API and -net as our UI.  Management tools want
to use an API, because it provides low level control and generally has
limited side effects.

Users want a UI that makes sense.  Trying to make both things satisfy
both audiences will almost certainly fail.

If a common use case cannot be done without resorting to using our API,
then we ought to improve our UI.

Regards,

Anthony Liguori

>
>> >
>> > -- 
>> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Jan 07, 2013 at 08:02:32PM +0000, Peter Maydell wrote:
>> On 7 January 2013 19:58, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> >> The modifications will be transparent to the user, as we will keep
>> >> virtio-x-pci devices.
>> >
>> > Then what's the point of all this?
>> >
>> > -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
>> >
>> > or
>> >
>> > -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
>> >
>> > Is simply an insane way to create a network device.
>> 
>> 1. You wouldn't create the virtio-mmio transport on the command line,
>> the machine model does it (it has to because it's a sysbus device
>> and it needs the address/irq lines wiring up properly), so it's just
>>  "-device virtio-net" (and let qemu find the bus automatically)
>
> Bus auto-detection sounds good and would be nice for pci too.
> We had things like model=virtio originally which is pretty close.
> But the issue is, how then do you pass bus specific arguments like pci
> slot? This is what caused us to go the virtio-net-pci route
> to begin with.

PCI is not the same as MMIO here.

virtio-mmio devices are not pluggable.  It makes a lot more sense to
have a virtio-net-pci device.  But it doesn't make much sense to have a
virtio-net-mmio device.

>
>> 2. We shouldn't be making command line simplicity drive how we
>> model devices inside QEMU.
>
> Confused.  I was told that enabling
>  -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
> is the reason we have this patchset.

You were misinformed.

>> If we wanted to do that we should have
>> stuck with the old -net command line arguments which are rather
>> more userfriendly IMHO.
>
> The main thing that confused people with -net was the vlans
> and the need to specify -net twice.
> A good UI would have been e.g. -nic model=virtio,net=user.

That was the original UI.  It was even called -nics.  See
7c9d8e if you're curious.  I was never a fan of the -net syntax.

> But one bad UI does not justify another one.
>
>> If commandline confusion is getting to
>> be a problem with all the -device foo stuff then we should probably
>> fix that at the UI level.
>> 
>> -- PMM
>
> I'd like to see a proposal about how we are going to do this.

(1) Stop conflating internal modeling with UI

(2) Add UI interfaces as appropriate 

It's really that simple.

Regards,

Anthony Liguori

>
> -- 
> MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 03:24:14PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Jan 07, 2013 at 02:12:23PM -0600, Anthony Liguori wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> >> >> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >> >> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >> >> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >> >> >>>another bus, like a pci bus, and another binding, like the virtio-pci
> >> >> >>>binding?
> >> >> >>(a) the current code is really not very nice because it's not
> >> >> >>actually a proper set of QOM/qdev devices
> >> >> >>(b) unlike PCI, you can't create sysbus devices on the
> >> >> >>command line, because they don't correspond to a user
> >> >> >>pluggable bit of hardware. We don't want users to have to know
> >> >> >>an address and IRQ number for each virtio-mmio device (especially
> >> >> >>since these are board specific); instead the board can create
> >> >> >>and wire up transport devices wherever is suitable, and the
> >> >> >>user just creates the backend (which is plugged into the virtio bus).
> >> >> >>
> >> >> >>-- PMM
> >> >> >This is what I am saying: create your own bus and put
> >> >> >your devices there. Allocate resources when you init
> >> >> >a device.
> >> >> >
> >> >> >Instead you seem to want to expose a virtio device as two devices to
> >> >> >user - if true this is not reasonable.
> >> >> >
> >> >> The modifications will be transparent to the user, as we will keep
> >> >> virtio-x-pci devices.
> >> >
> >> > So there are three ways to add virtio pci devices now.
> >> > Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
> >> > and the new one with two devices.
> >> > If yes it's not transparent, it's user visible.
> >> > Or did I misunderstand?
> >> >
> >> > Look we can have a virtio network device on a PCI bus.
> >> > A very similar device can be created on XXX bus, and
> >> > we can and do share a lot of code.
> >> > This makes it two devices? Why not 4?
> >> > One for TX one for RX one for control one for PCI.
> >> > I hope I'm not giving anyone ideas ...
> >> 
> >> Devices != things users need to worry about.
> >> 
> >> The documented way to create network devices is completely different
> >> than any possible syntax you can conjure up with -device.
> >> 
> >> Really, -device is not something users should have to deal with--ever.
> >> It's a low level API, not a UI.
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >
> > Interesting.
> > Let's assume I want to put a device behind a pci bridge
> > (for example I want more than 32 of these).
> 
> You don't want to put a device behind a PCI bridge, you want to have
> more than 32 devices.
> 
> '-net nic' should do the Right Thing when presented with more than 32
> devices.
> > It's impossible without -device, isn't it?
> 
> Think of -device like an API and -net as our UI.  Management tools want
> to use an API, because it provides low level control and generally has
> limited side effects.
> 
> Users want a UI that makes sense.  Trying to make both things satisfy
> both audiences will almost certainly fail.
> 
> If a common use case cannot be done without resorting to using our API,
> then we ought to improve our UI.
> 
> Regards,
> 
> Anthony Liguori

I guess you are saying we want to add bus= option to -net nic?

> >
> >> >
> >> > -- 
> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> I guess you are saying we want to add bus= option to -net nic?

I absolutely wouldn't object to that.

But I can think of better solutions too.  Like:

    -virtio-net ...

Regards,

Anthony Liguori

>
>> >
>> >> >
>> >> > -- 
>> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 03:51:04PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > I guess you are saying we want to add bus= option to -net nic?
> 
> I absolutely wouldn't object to that.
> 
> But I can think of better solutions too.  Like:
> 
>     -virtio-net ...
> 
> Regards,
> 
> Anthony Liguori

for most people I think virtio is an implementation detail really. no?

> >
> >> >
> >> >> >
> >> >> > -- 
> >> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 03:24:14PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Jan 07, 2013 at 02:12:23PM -0600, Anthony Liguori wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> >> >> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >> >> >On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >> >> >>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> >>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >> >> >>>another bus, like a pci bus, and another binding, like the virtio-pci
> >> >> >>>binding?
> >> >> >>(a) the current code is really not very nice because it's not
> >> >> >>actually a proper set of QOM/qdev devices
> >> >> >>(b) unlike PCI, you can't create sysbus devices on the
> >> >> >>command line, because they don't correspond to a user
> >> >> >>pluggable bit of hardware. We don't want users to have to know
> >> >> >>an address and IRQ number for each virtio-mmio device (especially
> >> >> >>since these are board specific); instead the board can create
> >> >> >>and wire up transport devices wherever is suitable, and the
> >> >> >>user just creates the backend (which is plugged into the virtio bus).
> >> >> >>
> >> >> >>-- PMM
> >> >> >This is what I am saying: create your own bus and put
> >> >> >your devices there. Allocate resources when you init
> >> >> >a device.
> >> >> >
> >> >> >Instead you seem to want to expose a virtio device as two devices to
> >> >> >user - if true this is not reasonable.
> >> >> >
> >> >> The modifications will be transparent to the user, as we will keep
> >> >> virtio-x-pci devices.
> >> >
> >> > So there are three ways to add virtio pci devices now.
> >> > Legacy -device virtio-net-pci, legacy legacy -net nic.model=virtio
> >> > and the new one with two devices.
> >> > If yes it's not transparent, it's user visible.
> >> > Or did I misunderstand?
> >> >
> >> > Look we can have a virtio network device on a PCI bus.
> >> > A very similar device can be created on XXX bus, and
> >> > we can and do share a lot of code.
> >> > This makes it two devices? Why not 4?
> >> > One for TX one for RX one for control one for PCI.
> >> > I hope I'm not giving anyone ideas ...
> >> 
> >> Devices != things users need to worry about.
> >> 
> >> The documented way to create network devices is completely different
> >> than any possible syntax you can conjure up with -device.
> >> 
> >> Really, -device is not something users should have to deal with--ever.
> >> It's a low level API, not a UI.
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >
> > Interesting.
> > Let's assume I want to put a device behind a pci bridge
> > (for example I want more than 32 of these).
> 
> You don't want to put a device behind a PCI bridge, you want to have
> more than 32 devices.
> 
> '-net nic' should do the Right Thing when presented with more than 32
> devices.
> 
> > It's impossible without -device, isn't it?
> 
> Think of -device like an API and -net as our UI.

Wait a second, -net implies vlans no? And this means no offloads so
performance suffers ...

>  Management tools want
> to use an API, because it provides low level control and generally has
> limited side effects.
> 
> Users want a UI that makes sense.  Trying to make both things satisfy
> both audiences will almost certainly fail.
> 
> If a common use case cannot be done without resorting to using our API,
> then we ought to improve our UI.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >> >
> >> > -- 
> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Anthony Liguori]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Jan 07, 2013 at 03:51:04PM -0600, Anthony Liguori wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > I guess you are saying we want to add bus= option to -net nic?
>> 
>> I absolutely wouldn't object to that.
>> 
>> But I can think of better solutions too.  Like:
>> 
>>     -virtio-net ...
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> for most people I think virtio is an implementation detail really. no?

So what is it that people really want to do?

Is tap an implementation detail?  Should we just do -nics 4 and call it
a day?  I'm not being facetious, I really think we should approach it
like this.

We don't need to design our internal interfaces/abstractions such that
they map 1-1 with command line arguments meant for normal users.

Regards,

Anthony Liguori

>
>> >
>> >> >
>> >> >> >
>> >> >> > -- 
>> >> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 04:50:44PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Jan 07, 2013 at 03:51:04PM -0600, Anthony Liguori wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > I guess you are saying we want to add bus= option to -net nic?
> >> 
> >> I absolutely wouldn't object to that.
> >> 
> >> But I can think of better solutions too.  Like:
> >> 
> >>     -virtio-net ...
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >
> > for most people I think virtio is an implementation detail really. no?
> 
> So what is it that people really want to do?

With virtio?  They want to install a driver and have networking go
faster.

> Is tap an implementation detail?  Should we just do -nics 4 and call it
> a day?  I'm not being facetious, I really think we should approach it
> like this.

How would you make it work well?

> We don't need to design our internal interfaces/abstractions such that
> they map 1-1 with command line arguments meant for normal users.

Of course.


> Regards,
> 
> Anthony Liguori
> 
> >
> >> >
> >> >> >
> >> >> >> >
> >> >> >> > -- 
> >> >> >> > MST

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[KONRAD Frédéric]

On 07/01/2013 20:58, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
>>> On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>>>> On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>> Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>>>>> another bus, like a pci bus, and another binding, like the virtio-pci
>>>>> binding?
>>>> (a) the current code is really not very nice because it's not
>>>> actually a proper set of QOM/qdev devices
>>>> (b) unlike PCI, you can't create sysbus devices on the
>>>> command line, because they don't correspond to a user
>>>> pluggable bit of hardware. We don't want users to have to know
>>>> an address and IRQ number for each virtio-mmio device (especially
>>>> since these are board specific); instead the board can create
>>>> and wire up transport devices wherever is suitable, and the
>>>> user just creates the backend (which is plugged into the virtio bus).
>>>>
>>>> -- PMM
>>> This is what I am saying: create your own bus and put
>>> your devices there. Allocate resources when you init
>>> a device.
>>>
>>> Instead you seem to want to expose a virtio device as two devices to
>>> user - if true this is not reasonable.
>>>
>> The modifications will be transparent to the user, as we will keep
>> virtio-x-pci devices.
> Then what's the point of all this?
>
> -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
>
> or
>
> -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
>
> Is simply an insane way to create a network device.
>
To recap :

The idea is to have a virtio-bus between the transport device
(like pci, mmio,... ).

Then we can have a platform with several virtio-mmio and then virtio-bus 
slot.

At the end user can add a virtio-device in the command line with -device
parameter without recompiling the platform. That is not possible with just
creating the virtio-x-mmio devices. The bus= option can be used to
select the bus slot, but I'm not sure it is usefull.

The series keep the virtio-x-pci devices :
eg : step 11/61 for virtio-blk
So -device virtio-blk-pci or -device virtio-blk-s390 works as before.

Of course -device virtio-pci,id=transport1 -device 
virtio-net,bus=transport1 is
possible but why using this command line when we could simply do :
-device virtio-net-pci ?

Fred

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[Michael S. Tsirkin]

On Tue, Jan 08, 2013 at 10:56:54AM +0100, KONRAD Frédéric wrote:
> On 07/01/2013 20:58, Michael S. Tsirkin wrote:
> >On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
> >>On 18/12/2012 12:01, Michael S. Tsirkin wrote:
> >>>On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
> >>>>On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>>>>Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
> >>>>>another bus, like a pci bus, and another binding, like the virtio-pci
> >>>>>binding?
> >>>>(a) the current code is really not very nice because it's not
> >>>>actually a proper set of QOM/qdev devices
> >>>>(b) unlike PCI, you can't create sysbus devices on the
> >>>>command line, because they don't correspond to a user
> >>>>pluggable bit of hardware. We don't want users to have to know
> >>>>an address and IRQ number for each virtio-mmio device (especially
> >>>>since these are board specific); instead the board can create
> >>>>and wire up transport devices wherever is suitable, and the
> >>>>user just creates the backend (which is plugged into the virtio bus).
> >>>>
> >>>>-- PMM
> >>>This is what I am saying: create your own bus and put
> >>>your devices there. Allocate resources when you init
> >>>a device.
> >>>
> >>>Instead you seem to want to expose a virtio device as two devices to
> >>>user - if true this is not reasonable.
> >>>
> >>The modifications will be transparent to the user, as we will keep
> >>virtio-x-pci devices.
> >Then what's the point of all this?
> >
> >-device virtio-pci,id=transport1 -device virtio-net,bus=transport1
> >
> >or
> >
> >-device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
> >
> >Is simply an insane way to create a network device.
> >
> To recap :
> 
> The idea is to have a virtio-bus between the transport device
> (like pci, mmio,... ).
> 
> Then we can have a platform with several virtio-mmio and then
> virtio-bus slot.
> 
> At the end user can add a virtio-device in the command line with -device
> parameter without recompiling the platform. That is not possible with just
> creating the virtio-x-mmio devices. The bus= option can be used to
> select the bus slot, but I'm not sure it is usefull.

pci uses addr option for this, I am guessing mmio can do the same.

> The series keep the virtio-x-pci devices :
> eg : step 11/61 for virtio-blk
> So -device virtio-blk-pci or -device virtio-blk-s390 works as before.
> 
> Of course -device virtio-pci,id=transport1 -device
> virtio-net,bus=transport1 is
> possible but why using this command line when we could simply do :
> -device virtio-net-pci ?
> 
> Fred

Adding multiple ways to do one thing is a bad idea.
I'm fine with modeling virtio by multiple devices
internally, but exposing this to user is a mistake,
we might rework this even more and command line
has to be supported indefinitely.

Re: [Qemu-devel] [RFC PATCH v6 0/6] Virtio refactoring.

[KONRAD Frédéric]

On 08/01/2013 15:02, Michael S. Tsirkin wrote:
> On Tue, Jan 08, 2013 at 10:56:54AM +0100, KONRAD Frédéric wrote:
>> On 07/01/2013 20:58, Michael S. Tsirkin wrote:
>>> On Tue, Dec 18, 2012 at 12:30:20PM +0100, KONRAD Frédéric wrote:
>>>> On 18/12/2012 12:01, Michael S. Tsirkin wrote:
>>>>> On Tue, Dec 18, 2012 at 10:33:37AM +0000, Peter Maydell wrote:
>>>>>> On 17 December 2012 15:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>>> Is the point to allow virtio-mmio?  Why can't virtio-mmio be just
>>>>>>> another bus, like a pci bus, and another binding, like the virtio-pci
>>>>>>> binding?
>>>>>> (a) the current code is really not very nice because it's not
>>>>>> actually a proper set of QOM/qdev devices
>>>>>> (b) unlike PCI, you can't create sysbus devices on the
>>>>>> command line, because they don't correspond to a user
>>>>>> pluggable bit of hardware. We don't want users to have to know
>>>>>> an address and IRQ number for each virtio-mmio device (especially
>>>>>> since these are board specific); instead the board can create
>>>>>> and wire up transport devices wherever is suitable, and the
>>>>>> user just creates the backend (which is plugged into the virtio bus).
>>>>>>
>>>>>> -- PMM
>>>>> This is what I am saying: create your own bus and put
>>>>> your devices there. Allocate resources when you init
>>>>> a device.
>>>>>
>>>>> Instead you seem to want to expose a virtio device as two devices to
>>>>> user - if true this is not reasonable.
>>>>>
>>>> The modifications will be transparent to the user, as we will keep
>>>> virtio-x-pci devices.
>>> Then what's the point of all this?
>>>
>>> -device virtio-pci,id=transport1 -device virtio-net,bus=transport1
>>>
>>> or
>>>
>>> -device virtio-mmio,id=transport1 -device virtio-net,bus=transport1
>>>
>>> Is simply an insane way to create a network device.
>>>
>> To recap :
>>
>> The idea is to have a virtio-bus between the transport device
>> (like pci, mmio,... ).
>>
>> Then we can have a platform with several virtio-mmio and then
>> virtio-bus slot.
>>
>> At the end user can add a virtio-device in the command line with -device
>> parameter without recompiling the platform. That is not possible with just
>> creating the virtio-x-mmio devices. The bus= option can be used to
>> select the bus slot, but I'm not sure it is usefull.
> pci uses addr option for this, I am guessing mmio can do the same.
>
>> The series keep the virtio-x-pci devices :
>> eg : step 11/61 for virtio-blk
>> So -device virtio-blk-pci or -device virtio-blk-s390 works as before.
>>
>> Of course -device virtio-pci,id=transport1 -device
>> virtio-net,bus=transport1 is
>> possible but why using this command line when we could simply do :
>> -device virtio-net-pci ?
>>
>> Fred
> Adding multiple ways to do one thing is a bad idea.
> I'm fine with modeling virtio by multiple devices
> internally, but exposing this to user is a mistake,
> we might rework this even more and command line
> has to be supported indefinitely.
>
for the moment all virtio-x-pci devices extend virtio-pci.
So why not abstracting virtio-pci ?
Then there is only one way to add a virtio-x-pci device.