From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TupOQ-0002Sc-7Z for qemu-devel@nongnu.org; Mon, 14 Jan 2013 14:08:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TupOI-0003Fs-NX for qemu-devel@nongnu.org; Mon, 14 Jan 2013 14:08:22 -0500 Received: from e39.co.us.ibm.com ([32.97.110.160]:49604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TupOI-0003FZ-Dk for qemu-devel@nongnu.org; Mon, 14 Jan 2013 14:08:14 -0500 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 Jan 2013 12:08:13 -0700 Received: from d03relay01.boulder.ibm.com (d03relay01.boulder.ibm.com [9.17.195.226]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 2BA7919D8053 for ; Mon, 14 Jan 2013 12:08:05 -0700 (MST) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay01.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r0EJ7oLw251264 for ; Mon, 14 Jan 2013 12:07:51 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r0EJ73qJ032457 for ; Mon, 14 Jan 2013 12:07:04 -0700 From: Anthony Liguori In-Reply-To: <1357747019-20580-4-git-send-email-fred.konrad@greensocs.com> References: <1357747019-20580-1-git-send-email-fred.konrad@greensocs.com> <1357747019-20580-4-git-send-email-fred.konrad@greensocs.com> Date: Mon, 14 Jan 2013 13:06:52 -0600 Message-ID: <87ehhnwpmb.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH V2 3/7] virtio-device: refactor virtio-device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: fred.konrad@greensocs.com, qemu-devel@nongnu.org Cc: kwolf@redhat.com, peter.maydell@linaro.org, e.voevodin@samsung.com, mst@redhat.com, mark.burton@greensocs.com, agraf@suse.de, cornelia.huck@de.ibm.com, aneesh.kumar@linux.vnet.ibm.com, stefanha@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, afaerber@suse.de fred.konrad@greensocs.com writes: > From: KONRAD Frederic > > Create the virtio-device which is abstract. All the virtio-device can extend > this class. It also add some functions to virtio-bus. > > Signed-off-by: KONRAD Frederic > --- > hw/virtio-bus.c | 35 +++++++++++++++++++++++++++++ > hw/virtio-bus.h | 7 ++++++ > hw/virtio.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++---------- > hw/virtio.h | 30 +++++++++++++++++++++++++ > 4 files changed, 130 insertions(+), 12 deletions(-) > > diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c > index 7813a89..ec43185 100644 > --- a/hw/virtio-bus.c > +++ b/hw/virtio-bus.c > @@ -127,6 +127,41 @@ size_t virtio_device_get_config_len(VirtioBusState *bus) > return bus->vdev->config_len; > } > > +/* Get the features of the plugged device. */ > +uint32_t virtio_device_get_features(VirtioBusState *bus, > + uint32_t requested_features) > +{ > + VirtioDeviceClass *k; > + assert(bus->vdev != NULL); > + k = VIRTIO_DEVICE_GET_CLASS(bus->vdev); > + assert(k->get_features != NULL); > + return k->get_features(bus->vdev, requested_features); > +} > + > +/* Get bad features of the plugged device. */ > +uint32_t virtio_device_get_bad_features(VirtioBusState *bus) > +{ > + VirtioDeviceClass *k; > + assert(bus->vdev != NULL); > + k = VIRTIO_DEVICE_GET_CLASS(bus->vdev); > + if (k->bad_features != NULL) { > + return k->bad_features(bus->vdev); > + } else { > + return 0; > + } > +} > + > +/* Get config of the plugged device. */ > +void virtio_device_get_config(VirtioBusState *bus, uint8_t *config) > +{ > + VirtioDeviceClass *k; > + assert(bus->vdev != NULL); > + k = VIRTIO_DEVICE_GET_CLASS(bus->vdev); > + if (k->get_config != NULL) { > + k->get_config(bus->vdev, config); > + } > +} > + If the prefix is "virtio_device" the first argument should be a VirtIODevice not a virtio_bus. You probably should introduce a virtio_bus_get_vdev() and then call virtio_device(virtio_bus_get_vdev(bus), ...) Or alternatively, rename these to: virtio_bus_get_vdev_* > static const TypeInfo virtio_bus_info = { > .name = TYPE_VIRTIO_BUS, > .parent = TYPE_BUS, > diff --git a/hw/virtio-bus.h b/hw/virtio-bus.h > index 8aa71b2..7bea64a 100644 > --- a/hw/virtio-bus.h > +++ b/hw/virtio-bus.h > @@ -87,5 +87,12 @@ int virtio_device_get_nvectors(VirtioBusState *bus); > void virtio_device_set_nvectors(VirtioBusState *bus, int nvectors); > /* Get the config_len field of the plugged device. */ > size_t virtio_device_get_config_len(VirtioBusState *bus); > +/* Get the features of the plugged device. */ > +uint32_t virtio_device_get_features(VirtioBusState *bus, > + uint32_t requested_features); > +/* Get bad features of the plugged device. */ > +uint32_t virtio_device_get_bad_features(VirtioBusState *bus); > +/* Get config of the plugged device. */ > +void virtio_device_get_config(VirtioBusState *bus, uint8_t *config); > > #endif /* VIRTIO_BUS_H */ > diff --git a/hw/virtio.c b/hw/virtio.c > index 77b53a9..ca170c3 100644 > --- a/hw/virtio.c > +++ b/hw/virtio.c > @@ -17,6 +17,7 @@ > #include "qemu/error-report.h" > #include "virtio.h" > #include "qemu/atomic.h" > +#include "virtio-bus.h" > > /* The alignment to use between consumer and producer parts of vring. > * x86 pagesize again. */ > @@ -875,11 +876,16 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) > return 0; > } > > -void virtio_cleanup(VirtIODevice *vdev) > +void virtio_common_cleanup(VirtIODevice *vdev) > { > qemu_del_vm_change_state_handler(vdev->vmstate); > g_free(vdev->config); > g_free(vdev->vq); > +} > + > +void virtio_cleanup(VirtIODevice *vdev) > +{ > + virtio_common_cleanup(vdev); > g_free(vdev); > } > > @@ -902,14 +908,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 +919,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 +1066,39 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq) > { > return &vq->host_notifier; > } > + > +static int virtio_device_init(DeviceState *qdev) > +{ > + VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > + assert(k->init != NULL); > + if (k->init(vdev) < 0) { > + return -1; > + } > + virtio_bus_plug_device(vdev); > + return 0; > +} > + > +static void virtio_device_class_init(ObjectClass *klass, void *data) > +{ > + /* Set the default value here. */ > + DeviceClass *dc = DEVICE_CLASS(klass); > + dc->init = virtio_device_init; > + 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 1dec9dc..a321fb2 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,10 @@ struct VirtIODevice > void *config; > uint16_t config_vector; > int nvectors; > + /* > + * Function pointers will be removed at the end of the series as they are 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); > @@ -134,6 +147,23 @@ struct VirtIODevice > VMChangeStateEntry *vmstate; > }; > > +typedef struct { It's good form to not make this an anonymous struct. Regards, Anthony Liguori > + /* This is what a VirtioDevice must implement */ > + DeviceClass parent; > + int (*init)(VirtIODevice *vdev); > + 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); > +void virtio_common_cleanup(VirtIODevice *vdev); > + > VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, > void (*handle_output)(VirtIODevice *, > VirtQueue *)); > -- > 1.7.11.7