* [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev
@ 2015-05-26 14:34 Cornelia Huck
2015-05-26 14:34 ` [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features Cornelia Huck
2015-05-28 20:48 ` [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Michael S. Tsirkin
0 siblings, 2 replies; 4+ messages in thread
From: Cornelia Huck @ 2015-05-26 14:34 UTC (permalink / raw)
To: mst; +Cc: Cornelia Huck, qemu-devel, shannon.zhao
Next version of the patch moving the host features from the transports
into the vdev. I have not yet tested migration (probably ok, though),
but the rest should be fine.
I'll try to see how the feature bit changes for virtio-1 work out on
top of this.
v1->v2: remove host features from virtio-pci and s390-virtio transport
proxies [Shannon Zhao]
Cornelia Huck (1):
virtio: move host_features
hw/s390x/s390-virtio-bus.c | 18 ++----------------
hw/s390x/s390-virtio-bus.h | 1 -
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-pci.h | 1 -
hw/virtio/virtio.c | 17 +++++++++--------
include/hw/virtio/virtio-bus.h | 1 -
include/hw/virtio/virtio.h | 1 +
11 files changed, 30 insertions(+), 99 deletions(-)
--
2.3.7
^ permalink raw reply [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features 2015-05-26 14:34 [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Cornelia Huck @ 2015-05-26 14:34 ` Cornelia Huck 2015-05-28 20:54 ` Michael S. Tsirkin 2015-05-28 20:48 ` [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Michael S. Tsirkin 1 sibling, 1 reply; 4+ messages in thread From: Cornelia Huck @ 2015-05-26 14:34 UTC (permalink / raw) To: mst; +Cc: Cornelia Huck, qemu-devel, shannon.zhao 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. Tested-by: Shannon Zhao <shannon.zhao@linaro.org> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- hw/s390x/s390-virtio-bus.c | 18 ++---------------- hw/s390x/s390-virtio-bus.h | 1 - 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-pci.h | 1 - hw/virtio/virtio.c | 17 +++++++++-------- include/hw/virtio/virtio-bus.h | 1 - include/hw/virtio/virtio.h | 1 + 11 files changed, 30 insertions(+), 99 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/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h index 96b1890..7ad295e 100644 --- a/hw/s390x/s390-virtio-bus.h +++ b/hw/s390x/s390-virtio-bus.h @@ -92,7 +92,6 @@ struct VirtIOS390Device { ram_addr_t feat_offs; uint8_t feat_len; VirtIODevice *vdev; - uint32_t host_features; VirtioBusState bus; }; 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; /* 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-pci.h b/hw/virtio/virtio-pci.h index 3bac016..de39468 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -91,7 +91,6 @@ struct VirtIOPCIProxy { uint32_t flags; uint32_t class_code; uint32_t nvectors; - uint32_t host_features; bool ioeventfd_disabled; bool ioeventfd_started; VirtIOIRQFD *vector_irqfd; 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; -- 2.3.7 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features 2015-05-26 14:34 ` [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features Cornelia Huck @ 2015-05-28 20:54 ` Michael S. Tsirkin 0 siblings, 0 replies; 4+ messages in thread From: Michael S. Tsirkin @ 2015-05-28 20:54 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, shannon.zhao On Tue, May 26, 2015 at 04:34:47PM +0200, 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. > > Tested-by: Shannon Zhao <shannon.zhao@linaro.org> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Some comments below but I think they are best addressed by patches on top. > --- > hw/s390x/s390-virtio-bus.c | 18 ++---------------- > hw/s390x/s390-virtio-bus.h | 1 - > 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-pci.h | 1 - > hw/virtio/virtio.c | 17 +++++++++-------- > include/hw/virtio/virtio-bus.h | 1 - > include/hw/virtio/virtio.h | 1 + > 11 files changed, 30 insertions(+), 99 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/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h > index 96b1890..7ad295e 100644 > --- a/hw/s390x/s390-virtio-bus.h > +++ b/hw/s390x/s390-virtio-bus.h > @@ -92,7 +92,6 @@ struct VirtIOS390Device { > ram_addr_t feat_offs; > uint8_t feat_len; > VirtIODevice *vdev; > - uint32_t host_features; > VirtioBusState bus; > }; > > 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); > I think we can drop VIRTIO_F_BAD_FEATURE from ccw: the reason pci has it is because it actually tests it in transport code. ccw doesn't so there's no reason to expose this feature. And I think VIRTIO_F_NOTIFY_ON_EMPTY is safe to move to DEFINE_VIRTIO_COMMON_FEATURES: it's not set on s390 ATM but there's no real reason for this. In fact if you really want the truth, we only want it for virtio net, but we can't change it for old machine types. > 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; > /* 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-pci.h b/hw/virtio/virtio-pci.h > index 3bac016..de39468 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -91,7 +91,6 @@ struct VirtIOPCIProxy { > uint32_t flags; > uint32_t class_code; > uint32_t nvectors; > - uint32_t host_features; > bool ioeventfd_disabled; > bool ioeventfd_started; > VirtIOIRQFD *vector_irqfd; > 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; > -- > 2.3.7 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev 2015-05-26 14:34 [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Cornelia Huck 2015-05-26 14:34 ` [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features Cornelia Huck @ 2015-05-28 20:48 ` Michael S. Tsirkin 1 sibling, 0 replies; 4+ messages in thread From: Michael S. Tsirkin @ 2015-05-28 20:48 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, shannon.zhao On Tue, May 26, 2015 at 04:34:46PM +0200, Cornelia Huck wrote: > Next version of the patch moving the host features from the transports > into the vdev. I have not yet tested migration (probably ok, though), > but the rest should be fine. > > I'll try to see how the feature bit changes for virtio-1 work out on > top of this. > > v1->v2: remove host features from virtio-pci and s390-virtio transport > proxies [Shannon Zhao] > > Cornelia Huck (1): > virtio: move host_features > > hw/s390x/s390-virtio-bus.c | 18 ++---------------- > hw/s390x/s390-virtio-bus.h | 1 - > 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-pci.h | 1 - > hw/virtio/virtio.c | 17 +++++++++-------- > include/hw/virtio/virtio-bus.h | 1 - > include/hw/virtio/virtio.h | 1 + > 11 files changed, 30 insertions(+), 99 deletions(-) I like the diffstat! Don't see anything wrong here, though a couple more cleanups are possible on top. > -- > 2.3.7 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-28 20:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-26 14:34 [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Cornelia Huck 2015-05-26 14:34 ` [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features Cornelia Huck 2015-05-28 20:54 ` Michael S. Tsirkin 2015-05-28 20:48 ` [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).