Re: [Qemu-devel] [PATCH] virtio: set virtio-mmio transport features for various backends

[Mario Smarduch <m.smarduch@samsung.com>]

Anthony, Michael, KONRAD -
   I would really appreciate if you (or someone else I should include)
can look over this patch. Currently as things are independent transport and
backend interoperability is broken, since the 'proxy' 'host_features' and
for other transport/backend combinations additional fields are not set
to anything, like they are for legacy combined transport/backend.

With this patch the approach is for every transport to have associated
backend properties defined and use the backend type as index into 
its properties and apply them to the transport. Only virtio-mmio
transport is addressed in this patch with few backends to see if
this is acceptable by virtio maintainers.

One issue on applying properties this late is they can't be added to the 
object - none of property accessor functions work at this point. This
is something I'm not sure how to handle.

Thanks,
  Mario


On 02/26/2014 05:22 PM, Mario Smarduch wrote:
> virtio:  set virtio-mmio transport features for various backends
> 
> Hi Peter, i.e others -
> 
> This is a second attempt at setting transport backend features, for
> back-ends that are instantiated separately from transport and dynamically
> plugged in. virtio-mmio transport is addressed by this patch for virtio-net
> and virtio-blk back-ends.
> 
> This patch declares features for all back-ends in the transport file,
> and based on the type of back-end applies the back-end features. A hook
> is added in VirtioBusClass that each transport can override
> to apply specific features for a back-end. The hook is called with the
> back-end device when it's plugged in.
> 
> Thanks,
>  Mario.
> 
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
>  hw/virtio/virtio-bus.c         |    5 +++++
>  hw/virtio/virtio-mmio.c        |   46 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-bus.h |    4 ++++
>  3 files changed, 55 insertions(+)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index eb77019..5eb9ba1 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -44,8 +44,13 @@ int virtio_bus_device_plugged(VirtIODevice *vdev)
>      BusState *qbus = BUS(qdev_get_parent_bus(qdev));
>      VirtioBusState *bus = VIRTIO_BUS(qbus);
>      VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(qbus->parent);
>      DPRINTF("%s: plug device.\n", qbus->name);
> 
> +    if (klass->set_transport_features != NULL) {
> +        klass->set_transport_features(proxy);
> +    }
> +
>      if (klass->device_plugged != NULL) {
>          klass->device_plugged(qbus->parent);
>      }
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 8829eb0..f4e67a2 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -23,6 +23,8 @@
>  #include "hw/virtio/virtio.h"
>  #include "qemu/host-utils.h"
>  #include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-net.h"
> +#include "hw/virtio/virtio-blk.h"
> 
>  /* #define DEBUG_VIRTIO_MMIO */
> 
> @@ -89,6 +91,49 @@ typedef struct {
>      VirtioBusState bus;
>  } VirtIOMMIOProxy;
> 
> +/* supported features for virtio-net backend */
> +static Property virtio_net_transport_properties[] = {
> +    DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +/* supported features for virtio-blk backend */
> +static Property virtio_blk_transport_properties[] = {
> +    DEFINE_VIRTIO_BLK_FEATURES(VirtIOMMIOProxy, host_features),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static struct {
> +    const char *backend_name;
> +    const Property *props;
> +} mmio_transport_backends[] = {
> +    { TYPE_VIRTIO_NET, virtio_net_transport_properties },
> +    { TYPE_VIRTIO_BLK, virtio_blk_transport_properties },
> +    { NULL, NULL },
> +};
> +
> +
> +static void virtio_mmio_set_transport_features(DeviceState *d)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    int i;
> +
> +    for (i = 0; mmio_transport_backends[i].backend_name != NULL; i++) {
> +        if (object_dynamic_cast(OBJECT(vdev),
> +                                 mmio_transport_backends[i].backend_name)) {
> +            const Property *props =  mmio_transport_backends[i].props;
> +            for (; props->qtype == QTYPE_QBOOL && props->qtype != QTYPE_NONE;
> +                   props++) {
> +                if (props->defval == true) {
> +                    proxy->host_features |= (1 << props->bitnr);
> +                } else {
> +                    proxy->host_features &= ~(1 << props->bitnr);
> +                }
> +            }
> +        }
> +    }
> +}
>  static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
>                                  VirtIOMMIOProxy *dev);
> 
> @@ -408,6 +453,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
>      k->load_config = virtio_mmio_load_config;
>      k->get_features = virtio_mmio_get_features;
>      k->device_plugged = virtio_mmio_device_plugged;
> +    k->set_transport_features = virtio_mmio_set_transport_features;
>      k->has_variable_vring_alignment = true;
>      bus_class->max_dev = 1;
>  }
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 0756545..3136fc6 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -62,6 +62,10 @@ typedef struct VirtioBusClass {
>       * This is called by virtio-bus just before the device is unplugged.
>       */
>      void (*device_unplugged)(DeviceState *d);
> +    /* Call before plugging a backend into transport to set features supported
> +     * by the backend.
> +     */
> +    void (*set_transport_features)(DeviceState *d);
>      /*
>       * Does the transport have variable vring alignment?
>       * (ie can it ever call virtio_queue_set_align()?)
>