Re: [Qemu-devel] [PATCHv6 3/3] virtio: add features as qdev properties

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/04/2010 10:08 AM, Michael S. Tsirkin wrote:
> Add feature bits as properties to virtio. This makes it possible to e.g. define
> machine without indirect buffer support, which is required for 0.10
> compatibility, or without hardware checksum support, which is required for 0.11
> compatibility.  Since default values for optional features are now set by qdev,
> get_features callback has been modified: it sets non-optional bits, and clears
> bits not supported by host.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> Acked-by: Gerd Hoffmann<kraxel@redhat.com>
>
> ---
>   hw/s390-virtio-bus.c |   12 +++++++++---
>   hw/s390-virtio-bus.h |    1 +
>   hw/syborg_virtio.c   |   12 +++++++-----
>   hw/virtio-balloon.c  |    4 ++--
>   hw/virtio-blk.c      |    6 +-----
>   hw/virtio-blk.h      |    8 ++++++++
>   hw/virtio-console.c  |    4 ++--
>   hw/virtio-net.c      |   39 ++++++++++++++++-----------------------
>   hw/virtio-net.h      |   20 ++++++++++++++++++++
>   hw/virtio-pci.c      |   25 +++++++++++++++++--------
>   hw/virtio.c          |    2 +-
>   hw/virtio.h          |    7 ++++++-
>   12 files changed, 90 insertions(+), 50 deletions(-)
>
> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> index 6c0da11..980e7eb 100644
> --- a/hw/s390-virtio-bus.c
> +++ b/hw/s390-virtio-bus.c
> @@ -101,6 +101,7 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, VirtIODevice *vdev)
>       bus->dev_offs += dev_len;
>
>       virtio_bind_device(vdev,&virtio_s390_bindings, dev);
> +    dev->host_features = vdev->get_features(vdev, dev->host_features);
>       s390_virtio_device_sync(dev);
>
>       return 0;
> @@ -222,9 +223,7 @@ static void s390_virtio_device_sync(VirtIOS390Device *dev)
>       cur_offs += num_vq * VIRTIO_VQCONFIG_LEN;
>
>       /* Sync feature bitmap */
> -    if (dev->vdev->get_features) {
> -        stl_phys(cur_offs, dev->vdev->get_features(dev->vdev));
> -    }
> +    stl_phys(cur_offs, dev->host_features);
>
>       dev->feat_offs = cur_offs + dev->feat_len;
>       cur_offs += dev->feat_len * 2;
> @@ -310,10 +309,17 @@ static void virtio_s390_notify(void *opaque, uint16_t vector)
>       kvm_s390_virtio_irq(s390_cpu_addr2state(0), 0, token);
>   }
>
> +static unsigned virtio_s390_get_features(void *opaque)
> +{
> +    VirtIOS390Device *dev = (VirtIOS390Device*)opaque;
> +    return dev->host_features;
> +}
> +
>   /**************** S390 Virtio Bus Device Descriptions *******************/
>
>   static const VirtIOBindings virtio_s390_bindings = {
>       .notify = virtio_s390_notify,
> +    .get_features = virtio_s390_get_features,
>   };
>
>   static VirtIOS390DeviceInfo s390_virtio_net = {
> diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
> index ef36714..8ae2065 100644
> --- a/hw/s390-virtio-bus.h
> +++ b/hw/s390-virtio-bus.h
> @@ -40,6 +40,7 @@ typedef struct VirtIOS390Device {
>       VirtIODevice *vdev;
>       DriveInfo *dinfo;
>       NICConf nic;
> +    uint32_t host_features;
>   } VirtIOS390Device;
>
>   typedef struct VirtIOS390Bus {
> diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
> index fe6fc23..ca026ee 100644
> --- a/hw/syborg_virtio.c
> +++ b/hw/syborg_virtio.c
> @@ -66,6 +66,7 @@ typedef struct {
>       uint32_t int_enable;
>       uint32_t id;
>       NICConf nic;
> +    uint32_t host_features;
>   } SyborgVirtIOProxy;
>
>   static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
> @@ -86,8 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
>           ret = s->id;
>           break;
>       case SYBORG_VIRTIO_HOST_FEATURES:
> -        ret = vdev->get_features(vdev);
> -        ret |= vdev->binding->get_features(s);
> +        ret = s->host_features;
>           break;
>       case SYBORG_VIRTIO_GUEST_FEATURES:
>           ret = vdev->guest_features;
> @@ -244,9 +244,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector)
>
>   static unsigned syborg_virtio_get_features(void *opaque)
>   {
> -    unsigned ret = 0;
> -    ret |= (1<<  VIRTIO_F_NOTIFY_ON_EMPTY);
> -    return ret;
> +    SyborgVirtIOProxy *proxy = opaque;
> +    return proxy->host_features;
>   }
>
>   static VirtIOBindings syborg_virtio_bindings = {
> @@ -272,6 +271,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev)
>       qemu_register_reset(virtio_reset, vdev);
>
>       virtio_bind_device(vdev,&syborg_virtio_bindings, proxy);
> +    proxy->host_features |= (0x1<<  VIRTIO_F_NOTIFY_ON_EMPTY);
> +    proxy->host_features = vdev->get_features(vdev, proxy->host_features);
>       return 0;
>   }
>
> @@ -292,6 +293,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
>       .qdev.size  = sizeof(SyborgVirtIOProxy),
>       .qdev.props = (Property[]) {
>           DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
> +        DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
>           DEFINE_PROP_END_OF_LIST(),
>       }
>   };
>    

This breaks the arm-softmmu build (syborg_virtio.o).  You probably need 
to include virtio-net.h in this file.

Regards,

Anthony Liguori