[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 1/1] virtio: move host_features
|
From: |
Shannon Zhao |
|
Subject: |
Re: [Qemu-devel] [PATCH RFC 1/1] virtio: move host_features |
|
Date: |
Tue, 26 May 2015 14:58:02 +0800 |
|
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 2015/5/22 20:11, Cornelia Huck wrote:
> Move host_features from the individual transport proxies into
> the virtio device. Transports may continue to add feature bits
> during device plugging.
>
> This should it make easier to offer different sets of host features
> for virtio-1/transitional support.
>
> Signed-off-by: Cornelia Huck <address@hidden>
Test virtio-mmio and virtio-pci. The host_features are same with/without
this patch. There is a small comment on this below.
Tested-by: Shannon Zhao <address@hidden>
> ---
> hw/s390x/s390-virtio-bus.c | 18 ++----------------
> hw/s390x/virtio-ccw.c | 29 ++++++-----------------------
> hw/s390x/virtio-ccw.h | 4 ----
> hw/virtio/virtio-bus.c | 18 +++++-------------
> hw/virtio/virtio-mmio.c | 22 +++-------------------
> hw/virtio/virtio-pci.c | 17 ++++-------------
> hw/virtio/virtio.c | 17 +++++++++--------
> include/hw/virtio/virtio-bus.h | 1 -
> include/hw/virtio/virtio.h | 1 +
> 9 files changed, 30 insertions(+), 97 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 0e35ac9..74e27e8 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -139,8 +139,6 @@ static void s390_virtio_device_init(VirtIOS390Device *dev,
>
> bus->dev_offs += dev_len;
>
> - dev->host_features = virtio_bus_get_vdev_features(&dev->bus,
> - dev->host_features);
> s390_virtio_device_sync(dev);
> s390_virtio_reset_idx(dev);
> if (dev->qdev.hotplugged) {
> @@ -433,7 +431,8 @@ void s390_virtio_device_sync(VirtIOS390Device *dev)
> cur_offs += num_vq * VIRTIO_VQCONFIG_LEN;
>
> /* Sync feature bitmap */
> - address_space_stl_le(&address_space_memory, cur_offs, dev->host_features,
> + address_space_stl_le(&address_space_memory, cur_offs,
> + dev->vdev->host_features,
> MEMTXATTRS_UNSPECIFIED, NULL);
>
> dev->feat_offs = cur_offs + dev->feat_len;
> @@ -528,12 +527,6 @@ static void virtio_s390_notify(DeviceState *d, uint16_t
> vector)
> s390_virtio_irq(0, token);
> }
>
> -static unsigned virtio_s390_get_features(DeviceState *d)
> -{
> - VirtIOS390Device *dev = to_virtio_s390_device(d);
> - return dev->host_features;
> -}
> -
> /**************** S390 Virtio Bus Device Descriptions *******************/
>
> static void s390_virtio_net_class_init(ObjectClass *klass, void *data)
> @@ -626,16 +619,10 @@ static void s390_virtio_busdev_reset(DeviceState *dev)
> virtio_reset(_dev->vdev);
> }
>
> -static Property virtio_s390_properties[] = {
> - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->props = virtio_s390_properties;
> dc->realize = s390_virtio_busdev_realize;
> dc->bus_type = TYPE_S390_VIRTIO_BUS;
> dc->reset = s390_virtio_busdev_reset;
> @@ -733,7 +720,6 @@ static void virtio_s390_bus_class_init(ObjectClass
> *klass, void *data)
> BusClass *bus_class = BUS_CLASS(klass);
> bus_class->max_dev = 1;
> k->notify = virtio_s390_notify;
> - k->get_features = virtio_s390_get_features;
> }
>
> static const TypeInfo virtio_s390_bus_info = {
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index c96101a..0a35595 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -381,8 +381,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> + sizeof(features.features),
> MEMTXATTRS_UNSPECIFIED,
> NULL);
> - if (features.index < ARRAY_SIZE(dev->host_features)) {
> - features.features = dev->host_features[features.index];
> + if (features.index == 0) {
> + features.features = vdev->host_features;
> } else {
> /* Return zeroes if the guest supports more feature bits. */
> features.features = 0;
> @@ -417,7 +417,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ccw.cda,
> MEMTXATTRS_UNSPECIFIED,
> NULL);
> - if (features.index < ARRAY_SIZE(dev->host_features)) {
> + if (features.index == 0) {
> virtio_set_features(vdev, features.features);
> } else {
> /*
> @@ -1098,14 +1098,6 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t
> vector)
> }
> }
>
> -static unsigned virtio_ccw_get_features(DeviceState *d)
> -{
> - VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> -
> - /* Only the first 32 feature bits are used. */
> - return dev->host_features[0];
> -}
> -
> static void virtio_ccw_reset(DeviceState *d)
> {
> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> @@ -1417,14 +1409,12 @@ static void virtio_ccw_device_plugged(DeviceState *d)
> {
> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> SubchDev *sch = dev->sch;
> + VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
>
> sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus);
>
> - /* Only the first 32 feature bits are used. */
> - virtio_add_feature(&dev->host_features[0], VIRTIO_F_NOTIFY_ON_EMPTY);
> - virtio_add_feature(&dev->host_features[0], VIRTIO_F_BAD_FEATURE);
> - dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
> -
> dev->host_features[0]);
> + virtio_add_feature(&vdev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> + virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);
>
> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> d->hotplugged, 1);
> @@ -1675,16 +1665,10 @@ static void virtio_ccw_busdev_unplug(HotplugHandler
> *hotplug_dev,
> object_unparent(OBJECT(dev));
> }
>
> -static Property virtio_ccw_properties[] = {
> - DEFINE_VIRTIO_COMMON_FEATURES(VirtioCcwDevice, host_features[0]),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void virtio_ccw_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->props = virtio_ccw_properties;
> dc->realize = virtio_ccw_busdev_realize;
> dc->exit = virtio_ccw_busdev_exit;
> dc->bus_type = TYPE_VIRTUAL_CSS_BUS;
> @@ -1749,7 +1733,6 @@ static void virtio_ccw_bus_class_init(ObjectClass
> *klass, void *data)
>
> bus_class->max_dev = 1;
> k->notify = virtio_ccw_notify;
> - k->get_features = virtio_ccw_get_features;
> k->vmstate_change = virtio_ccw_vmstate_change;
> k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
> k->set_host_notifier = virtio_ccw_set_host_notifier;
> diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
> index 4fceda7..ad3af76 100644
> --- a/hw/s390x/virtio-ccw.h
> +++ b/hw/s390x/virtio-ccw.h
> @@ -68,9 +68,6 @@ typedef struct VirtIOCCWDeviceClass {
> int (*exit)(VirtioCcwDevice *dev);
> } VirtIOCCWDeviceClass;
>
> -/* Change here if we want to support more feature bits. */
> -#define VIRTIO_CCW_FEATURE_SIZE 1
> -
> /* Performance improves when virtqueue kick processing is decoupled from the
> * vcpu thread using ioeventfd for some devices. */
> #define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1
> @@ -88,7 +85,6 @@ struct VirtioCcwDevice {
> DeviceState parent_obj;
> SubchDev *sch;
> char *bus_id;
> - uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
> VirtioBusState bus;
> bool ioeventfd_started;
> bool ioeventfd_disabled;
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index be886e7..fac452b 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -44,12 +44,17 @@ 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);
> + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> +
> DPRINTF("%s: plug device.\n", qbus->name);
>
> if (klass->device_plugged != NULL) {
> klass->device_plugged(qbus->parent);
> }
>
> + /* Get the features of the plugged device. */
> + assert(vdc->get_features != NULL);
> + vdev->host_features = vdc->get_features(vdev, vdev->host_features);
> return 0;
> }
>
> @@ -96,19 +101,6 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus)
> return vdev->config_len;
> }
>
> -/* Get the features of the plugged device. */
> -uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus,
> - uint32_t requested_features)
> -{
> - VirtIODevice *vdev = virtio_bus_get_device(bus);
> - VirtioDeviceClass *k;
> -
> - assert(vdev != NULL);
> - k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - assert(k->get_features != NULL);
> - return k->get_features(vdev, requested_features);
> -}
> -
> /* Get bad features of the plugged device. */
> uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
> {
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 10123f3..1817a07 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -80,7 +80,6 @@ typedef struct {
> SysBusDevice parent_obj;
> MemoryRegion iomem;
> qemu_irq irq;
> - uint32_t host_features;
Maybe we should delete host_features in PCI and s390 as well.
> /* Guest accessible state needing migration and reset */
> uint32_t host_features_sel;
> uint32_t guest_features_sel;
> @@ -147,7 +146,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr
> offset, unsigned size)
> if (proxy->host_features_sel) {
> return 0;
> }
> - return proxy->host_features;
> + return vdev->host_features;
> case VIRTIO_MMIO_QUEUENUMMAX:
> if (!virtio_queue_get_num(vdev, vdev->queue_sel)) {
> return 0;
> @@ -306,13 +305,6 @@ static void virtio_mmio_update_irq(DeviceState *opaque,
> uint16_t vector)
> qemu_set_irq(proxy->irq, level);
> }
>
> -static unsigned int virtio_mmio_get_features(DeviceState *opaque)
> -{
> - VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> -
> - return proxy->host_features;
> -}
> -
> static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f)
> {
> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> @@ -348,10 +340,9 @@ static void virtio_mmio_reset(DeviceState *d)
> static void virtio_mmio_device_plugged(DeviceState *opaque)
> {
> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>
> - virtio_add_feature(&proxy->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> - proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus,
> -
> proxy->host_features);
> + virtio_add_feature(&vdev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> }
>
> static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
> @@ -367,16 +358,10 @@ static void virtio_mmio_realizefn(DeviceState *d, Error
> **errp)
> sysbus_init_mmio(sbd, &proxy->iomem);
> }
>
> -static Property virtio_mmio_properties[] = {
> - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void virtio_mmio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->props = virtio_mmio_properties;
> dc->realize = virtio_mmio_realizefn;
> dc->reset = virtio_mmio_reset;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> @@ -399,7 +384,6 @@ static void virtio_mmio_bus_class_init(ObjectClass
> *klass, void *data)
> k->notify = virtio_mmio_update_irq;
> k->save_config = virtio_mmio_save_config;
> k->load_config = virtio_mmio_load_config;
> - k->get_features = virtio_mmio_get_features;
> k->device_plugged = virtio_mmio_device_plugged;
> k->has_variable_vring_alignment = true;
> bus_class->max_dev = 1;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 867c9d1..a8383ac 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -306,7 +306,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy,
> uint32_t addr)
>
> switch (addr) {
> case VIRTIO_PCI_HOST_FEATURES:
> - ret = proxy->host_features;
> + ret = vdev->host_features;
> break;
> case VIRTIO_PCI_GUEST_FEATURES:
> ret = vdev->guest_features;
> @@ -434,12 +434,6 @@ static void virtio_write_config(PCIDevice *pci_dev,
> uint32_t address,
> }
> }
>
> -static unsigned virtio_pci_get_features(DeviceState *d)
> -{
> - VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> - return proxy->host_features;
> -}
> -
> static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> unsigned int queue_no,
> unsigned int vector,
> @@ -924,6 +918,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
> VirtioBusState *bus = &proxy->bus;
> uint8_t *config;
> uint32_t size;
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>
> config = proxy->pci_dev.config;
> if (proxy->class_code) {
> @@ -958,10 +953,8 @@ static void virtio_pci_device_plugged(DeviceState *d)
> proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
> }
>
> - virtio_add_feature(&proxy->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> - virtio_add_feature(&proxy->host_features, VIRTIO_F_BAD_FEATURE);
> - proxy->host_features = virtio_bus_get_vdev_features(bus,
> - proxy->host_features);
> + virtio_add_feature(&vdev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> + virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);
> }
>
> static void virtio_pci_device_unplugged(DeviceState *d)
> @@ -999,7 +992,6 @@ static void virtio_pci_reset(DeviceState *qdev)
> static Property virtio_pci_properties[] = {
> DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", VirtIOPCIProxy,
> flags,
> VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false),
> - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -1497,7 +1489,6 @@ static void virtio_pci_bus_class_init(ObjectClass
> *klass, void *data)
> k->load_config = virtio_pci_load_config;
> k->save_queue = virtio_pci_save_queue;
> k->load_queue = virtio_pci_load_queue;
> - k->get_features = virtio_pci_get_features;
> k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
> k->set_host_notifier = virtio_pci_set_host_notifier;
> k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 6985e76..96d9c6a 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -970,13 +970,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>
> int virtio_set_features(VirtIODevice *vdev, uint32_t val)
> {
> - BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> - VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(qbus);
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - uint32_t supported_features = vbusk->get_features(qbus->parent);
> - bool bad = (val & ~supported_features) != 0;
> + bool bad = (val & ~(vdev->host_features)) != 0;
>
> - val &= supported_features;
> + val &= vdev->host_features;
> if (k->set_features) {
> k->set_features(vdev, val);
> }
> @@ -990,7 +987,6 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> int32_t config_len;
> uint32_t num;
> uint32_t features;
> - uint32_t supported_features;
> BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> @@ -1016,9 +1012,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> qemu_get_be32s(f, &features);
>
> if (virtio_set_features(vdev, features) < 0) {
> - supported_features = k->get_features(qbus->parent);
> error_report("Features 0x%x unsupported. Allowed features: 0x%x",
> - features, supported_features);
> + features, vdev->host_features);
> return -1;
> }
> config_len = qemu_get_be32(f);
> @@ -1351,6 +1346,11 @@ static void virtio_device_unrealize(DeviceState *dev,
> Error **errp)
> vdev->bus_name = NULL;
> }
>
> +static Property virtio_properties[] = {
> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void virtio_device_class_init(ObjectClass *klass, void *data)
> {
> /* Set the default value here. */
> @@ -1359,6 +1359,7 @@ static void virtio_device_class_init(ObjectClass
> *klass, void *data)
> dc->realize = virtio_device_realize;
> dc->unrealize = virtio_device_unrealize;
> dc->bus_type = TYPE_VIRTIO_BUS;
> + dc->props = virtio_properties;
> }
>
> static const TypeInfo virtio_device_info = {
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index a4588ca..d4ccdf2 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -47,7 +47,6 @@ typedef struct VirtioBusClass {
> 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);
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 8210cb3..b620da5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -74,6 +74,7 @@ struct VirtIODevice
> uint8_t isr;
> uint16_t queue_sel;
> uint32_t guest_features;
> + uint32_t host_features;
> size_t config_len;
> void *config;
> uint16_t config_vector;
>
--
Shannon