Patchwork [V3,2/7] virtio-bus: introduce virtio-bus

login
register
mail settings
Submitter fred.konrad@greensocs.com
Date Jan. 14, 2013, 11:08 p.m.
Message ID <1358204886-5688-3-git-send-email-fred.konrad@greensocs.com>
Download mbox | patch
Permalink /patch/211942/
State New
Headers show

Comments

fred.konrad@greensocs.com - Jan. 14, 2013, 11:08 p.m.
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>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/Makefile.objs |   1 +
 hw/virtio-bus.c  | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-bus.h  |  87 +++++++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)
 create mode 100644 hw/virtio-bus.c
 create mode 100644 hw/virtio-bus.h
Peter Maydell - Jan. 21, 2013, 3:48 p.m.
On 14 January 2013 23:08,  <fred.konrad@greensocs.com> wrote:
> +#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)

'obj' and 'klass' need brackets round them, because they're
macro arguments.

-- PMM
Andreas Färber - Jan. 21, 2013, 3:53 p.m.
Am 21.01.2013 16:48, schrieb Peter Maydell:
> On 14 January 2013 23:08,  <fred.konrad@greensocs.com> wrote:
>> +#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)
> 
> 'obj' and 'klass' need brackets round them, because they're
> macro arguments.

BTW since these are macro arguments and not C code, I believe "class"
should be perfectly valid here if you wanted.

Andreas
Peter Maydell - Jan. 21, 2013, 5:39 p.m.
On 21 January 2013 15:53, Andreas Färber <afaerber@suse.de> wrote:
> Am 21.01.2013 16:48, schrieb Peter Maydell:
>> 'obj' and 'klass' need brackets round them, because they're
>> macro arguments.
>
> BTW since these are macro arguments and not C code, I believe "class"
> should be perfectly valid here if you wanted.

Possibly, but we should be consistent with existing style in
other equivalent macros for other QOM types, which means 'klass'.

-- PMM
Eric Blake - Jan. 21, 2013, 5:56 p.m.
On 01/21/2013 08:48 AM, Peter Maydell wrote:
> On 14 January 2013 23:08,  <fred.konrad@greensocs.com> wrote:
>> +#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)
> 
> 'obj' and 'klass' need brackets round them, because they're
> macro arguments.

Brackets are only necessary if the C parser could misinterpret things
without the brackets; but in this particular case, there is no ambiguity
(unless OBJECT_GET_CLASS() and OBJECT_CLASS_CHECK() are improperly
under-parenthesized).
Peter Maydell - Jan. 21, 2013, 6:05 p.m.
On 21 January 2013 17:56, Eric Blake <eblake@redhat.com> wrote:
> On 01/21/2013 08:48 AM, Peter Maydell wrote:
>> On 14 January 2013 23:08,  <fred.konrad@greensocs.com> wrote:
>>> +#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)
>>
>> 'obj' and 'klass' need brackets round them, because they're
>> macro arguments.
>
> Brackets are only necessary if the C parser could misinterpret things
> without the brackets; but in this particular case, there is no ambiguity
> (unless OBJECT_GET_CLASS() and OBJECT_CLASS_CHECK() are improperly
> under-parenthesized).

Yes, but "we can get away without parens here because we know
that other macro over there has parens in it" is unnecessarily
tricky in my opinion.

-- PMM
Eric Blake - Jan. 21, 2013, 6:40 p.m.
On 01/21/2013 11:05 AM, Peter Maydell wrote:
> On 21 January 2013 17:56, Eric Blake <eblake@redhat.com> wrote:
>> On 01/21/2013 08:48 AM, Peter Maydell wrote:
>>> On 14 January 2013 23:08,  <fred.konrad@greensocs.com> wrote:
>>>> +#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)
>>>
>>> 'obj' and 'klass' need brackets round them, because they're
>>> macro arguments.
>>
>> Brackets are only necessary if the C parser could misinterpret things
>> without the brackets; but in this particular case, there is no ambiguity
>> (unless OBJECT_GET_CLASS() and OBJECT_CLASS_CHECK() are improperly
>> under-parenthesized).
> 
> Yes, but "we can get away without parens here because we know
> that other macro over there has parens in it" is unnecessarily
> tricky in my opinion.

The rule of thumb for when to use parentheses in macros should generally
be: can the caller ever pass a series of tokens which would cause a
different parse than a single token; likewise, can the macro expand to
more than one token where the expansion no longer behaves like a single
function call.  If so, then we are burdening the caller with the need to
use parenthesis, and the macro is a potential bug.  If not, then the
parenthesis are redundant.

For something like '#define foo(x) x++', we can demonstrate that passing
*a' leads to '*a++', which has a different meaning than '(*a)++'.  Thus,
that macro has a bug unless written '#define foo(x) ((x)++)'.

But for argument lists, there is nothing the user can pass that can
cause the parser to get confused on the end of the argument list; even
if the user passes more than one token to the parameter 'obj', there is
no way that VIRTIO_BUS_GET_CLASS(*function_to_give_obj()) expanding to
OBJECT_GET_CLASS(VirtioBusClass, *function_to_give_obj(),
TYPE_VIRTIO_BUS) can be parsed differently than
OBJECT_GET_CLASS(VirtioBusClass, (*function_to_give_obj()),
TYPE_VIRTIO_BUS), so the extra parentheses would be redundant in the
definition of VIRTIO_BUS_GET_CLASS.

But if you insist on a rule of thumb that all macro arguments are always
parenthesized, even when not strictly necessary, because it makes for
less thinking when writing the macro, I'm not going to complain.

Patch

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 74b07a7..23ac249 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -8,6 +8,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_bridge_dev.o
 common-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c
new file mode 100644
index 0000000..b8f656e
--- /dev/null
+++ b/hw/virtio-bus.c
@@ -0,0 +1,129 @@ 
+/*
+ * 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-report.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;
+
+    /*
+     * The lines below will disappear when we drop VirtIOBindings, at the end
+     * of the series.
+     */
+    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);
+
+    if (klass->device_plugged != NULL) {
+        klass->device_plugged(qbus->parent);
+    }
+
+    return 0;
+}
+
+/* Reset the virtio_bus */
+void virtio_bus_reset(VirtioBusState *bus)
+{
+    DPRINTF("%s: reset device.\n", qbus->name);
+    if (bus->vdev != NULL) {
+        virtio_reset(bus->vdev);
+    }
+}
+
+/* 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;
+    }
+}
+
+/* Get the device id of the plugged device. */
+uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus)
+{
+    assert(bus->vdev != NULL);
+    return bus->vdev->device_id;
+}
+
+/* Get the config_len field of the plugged device. */
+size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus)
+{
+    assert(bus->vdev != NULL);
+    return bus->vdev->config_len;
+}
+
+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..f378897
--- /dev/null
+++ b/hw/virtio-bus.h
@@ -0,0 +1,87 @@ 
+/*
+ * 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/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)(DeviceState *d, uint16_t vector);
+    void (*save_config)(DeviceState *d, QEMUFile *f);
+    void (*save_queue)(DeviceState *d, int n, QEMUFile *f);
+    int (*load_config)(DeviceState *d, QEMUFile *f);
+    int (*load_queue)(DeviceState *d, int n, QEMUFile *f);
+    int (*load_done)(DeviceState *d, QEMUFile *f);
+    unsigned (*get_features)(DeviceState *d);
+    bool (*query_guest_notifiers)(DeviceState *d);
+    int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
+    int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
+    void (*vmstate_change)(DeviceState *d, bool running);
+    /*
+     * transport independent init function.
+     * This is called by virtio-bus just after the device is plugged.
+     */
+    void (*device_plugged)(DeviceState *d);
+    /*
+     * transport independent exit function.
+     * This is called by virtio-bus just before the device is unplugged.
+     */
+    void (*device_unplug)(DeviceState *d);
+} VirtioBusClass;
+
+struct VirtioBusState {
+    BusState parent_obj;
+    /*
+     * Only one VirtIODevice can be plugged on the bus.
+     */
+    VirtIODevice *vdev;
+    /*
+     * This will be removed at the end of the series.
+     */
+    VirtIOBindings bindings;
+};
+
+int virtio_bus_plug_device(VirtIODevice *vdev);
+void virtio_bus_reset(VirtioBusState *bus);
+void virtio_bus_destroy_device(VirtioBusState *bus);
+/* Get the device id of the plugged device. */
+uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus);
+/* Get the config_len field of the plugged device. */
+size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus);
+
+#endif /* VIRTIO_BUS_H */