[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus.
From: |
KONRAD Frédéric |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus. |
Date: |
Wed, 21 Nov 2012 15:05:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121029 Thunderbird/16.0.2 |
On 21/11/2012 14:04, Andreas Färber wrote:
Am 16.11.2012 16:35, schrieb address@hidden:
From: KONRAD Frederic <address@hidden>
This patch create a new VirtioBus, which can be added to Virtio transports like
virtio-pci, virtio-mmio,...
One VirtIODevice can be connected to this device, like virtio-blk in the 3rd
patch.
The VirtioBus shares through a VirtioBusInfo structure :
* two callbacks with the transport : init_cb and exit_cb, which must be
called by the VirtIODevice, after the initialization and before the
destruction, to put the right PCI IDs and/or stop the event fd.
* a VirtIOBindings structure.
Signed-off-by: KONRAD Frederic <address@hidden>
---
hw/Makefile.objs | 1 +
hw/virtio-bus.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/virtio-bus.h | 49 ++++++++++++++++++++++++
3 files changed, 161 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 af4ab0c..1b25d77 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,6 +1,7 @@
common-obj-y = usb/ ide/
common-obj-y += loader.o
common-obj-$(CONFIG_VIRTIO) += virtio-console.o
+common-obj-$(CONFIG_VIRTIO) += virtio-bus.o
common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
common-obj-y += fw_cfg.o
common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o
diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c
new file mode 100644
index 0000000..b2e7e9c
--- /dev/null
+++ b/hw/virtio-bus.c
@@ -0,0 +1,111 @@
+/*
+ * VirtioBus
+ *
+ * Copyright (C) 2012 : GreenSocs Ltd
+ * http://www.greensocs.com/ , email: address@hidden
+ *
+ * Developed by :
+ * Frederic Konrad <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
Any chance to use GPLv2 "or (at your option) any later version"? If not,
please mention in the commit message why.
Yes, I made the change.
+ */
+
+#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
We recently had a discussion about bitrotting DPRINTF() statements where
I suggested to use if (0) instead of a no-op macro like this that
doesn't reference fmt and the varargs.
I don't understand what you suggested, can you point me to an example ?
+
+static char *virtio_bus_get_fw_dev_path(DeviceState *dev);
+
+static void virtio_bus_class_init(ObjectClass *klass, void *data)
+{
+ BusClass *k = BUS_CLASS(klass);
+ k->get_fw_dev_path = virtio_bus_get_fw_dev_path;
+}
+
+static const TypeInfo virtio_bus_info = {
+ .name = TYPE_VIRTIO_BUS,
+ .parent = TYPE_BUS,
+ .instance_size = sizeof(VirtioBus),
+ .class_init = virtio_bus_class_init,
+};
+
+/* VirtioBus */
+
+static int next_virtio_bus;
+
+/* Create a virtio bus, and attach to transport. */
+void virtio_bus_new(VirtioBus *bus, DeviceState *host,
+ const VirtioBusInfo *info)
+{
+ /*
+ * Setting name to NULL return always "virtio.0"
+ * as bus name in info qtree.
+ */
+ char *bus_name = g_strdup_printf("%s.%d", BUS_NAME, next_virtio_bus);
+ qbus_create_inplace(&bus->qbus, TYPE_VIRTIO_BUS, host, bus_name);
Accessing the qbus field directly is considered old-style. Please use
the BUS() macro to cast to a new BusState* variable and pass that here...
Ok, I'll look.
+ bus->busnr = next_virtio_bus++;
+ bus->info = info;
+ /* no hotplug for the moment ? */
+ bus->qbus.allow_hotplug = 0;
...and access its fields through it. More cases below.
+ bus->bus_in_use = false;
+ DPRINTF("bus %s created\n", bus_name);
+}
+
+/* Bind the VirtIODevice to the VirtioBus. */
+void virtio_bus_bind_device(VirtioBus *bus)
+{
+ assert(bus != NULL);
+ assert(bus->vdev != NULL);
+ virtio_bind_device(bus->vdev, &(bus->info->virtio_bindings),
+ bus->qbus.parent);
+}
+
+/* This must be called to when the VirtIODevice init */
"called when the ...Device inits" or "called during ...Device init"
oops sorry for that.
+int virtio_bus_init_cb(VirtioBus *bus, VirtIODevice *vdev)
+{
+ if (bus->bus_in_use == true) {
Even if bus_in_use is of bool type, doing an explicit "true" comparison
is not a good idea in C since everything non-zero is to be considered
true, not just the "true" constant.
sure.
+ error_report("%s in use.\n", bus->qbus.name);
+ return -1;
+ }
+ assert(bus->info->init_cb != NULL);
+ /* keep the VirtIODevice in the VirtioBus */
+ bus->vdev = vdev;
+ bus->info->init_cb(bus->qbus.parent);
+
+ bus->bus_in_use = true;
+ return 0;
+}
+
+/* This must be called when the VirtIODevice exit */
Similar grammar issue as above.
+void virtio_bus_exit_cb(VirtioBus *bus)
+{
+ assert(bus->info->exit_cb != NULL);
+ bus->info->exit_cb(bus->qbus.parent);
+ bus->bus_in_use = false;
+}
+
+static char *virtio_bus_get_fw_dev_path(DeviceState *dev)
+{
+ return g_strdup_printf("%s", qdev_fw_name(dev));
+}
+
+
+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..6ea5035
--- /dev/null
+++ b/hw/virtio-bus.h
@@ -0,0 +1,49 @@
+/*
+ * VirtioBus
+ *
+ * Copyright (C) 2012 : GreenSocs Ltd
+ * http://www.greensocs.com/ , email: address@hidden
+ *
+ * Developed by :
+ * Frederic Konrad <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef _VIRTIO_BUS_H_
+#define _VIRTIO_BUS_H_
+
+#include "qdev.h"
+#include "sysemu.h"
+#include "virtio.h"
+
+#define TYPE_VIRTIO_BUS "VIRTIO"
Is there a special reason for all-uppercase here? Is lowercase already
taken?
No, there are no reasons, it was the case for scsi-bus.
You should add QOM cast macros VIRTIO_BUS() et al. to access the
VirtioBus fields.
+#define BUS_NAME "virtio"
+
+typedef struct VirtioBus VirtioBus;
+typedef struct VirtioBusInfo VirtioBusInfo;
+
+struct VirtioBusInfo {
+ void (*init_cb)(DeviceState *dev);
+ void (*exit_cb)(DeviceState *dev);
+ VirtIOBindings virtio_bindings;
+};
+
+struct VirtioBus {
+ BusState qbus;
You could name this field parent_obj to break the old qbus pattern.
+ int busnr;
+ bool bus_in_use;
+ uint16_t pci_device_id;
+ uint16_t pci_class;
pci_* in VirtioBus does not strike me as the best naming. Either this is
specific to the PCI backend and doesn't belong here, or it's not a
pci_device_id but a device_id.
it is specific to the PCI backend, and I removed it.
+ VirtIODevice *vdev;
+ const VirtioBusInfo *info;
+};
+
+void virtio_bus_new(VirtioBus *bus, DeviceState *host,
+ const VirtioBusInfo *info);
+void virtio_bus_bind_device(VirtioBus *bus);
+int virtio_bus_init_cb(VirtioBus *bus, VirtIODevice *dev);
+void virtio_bus_exit_cb(VirtioBus *bus);
+
+#endif
Regards,
Andreas
Thanks,
Fred
- [Qemu-devel] [RFC PATCH 0/3] Virtio refactoring., fred . konrad, 2012/11/16
- [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., fred . konrad, 2012/11/16
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., Peter Maydell, 2012/11/19
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., Cornelia Huck, 2012/11/20
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., KONRAD Frédéric, 2012/11/20
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., Cornelia Huck, 2012/11/20
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., KONRAD Frédéric, 2012/11/20
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., Cornelia Huck, 2012/11/20
- Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., KONRAD Frédéric, 2012/11/21
Re: [Qemu-devel] [RFC PATCH 1/3] virtio-bus : Introduce VirtioBus., Andreas Färber, 2012/11/21
[Qemu-devel] [RFC PATCH 2/3] virtio-pci : Add a virtio-bus interface, fred . konrad, 2012/11/16
[Qemu-devel] [RFC PATCH 3/3] virtio-blk : add the virtio-blk device., fred . konrad, 2012/11/16