Currently, devices are plugged before features are negotiated.
If the backend doesn't support VIRTIO_F_VERSION_1, the transport
need to rewind some settings.
This is the case for both PCI and CCW.
For CCW, a post_plugged callback had been introduced, where
max_rev field is just updated if VIRTIO_F_VERSION_1 is not
supported by the backend.
For PCI, implementing the post_plugged would be much more
complicated, so the current fix consists in checking whether
the backend supports VIRTIO_F_VERSION_1 in the backend.
This patch propose to replace existing solutions with a common
approach by negociating features before ->device_plugged() is
called. A pre_plugged callback is introduced so that the
transports can set their supported features.
Cc: Cornelia Huck <cornelia.h...@de.ibm.com>
Cc: Marcel Apfelbaum <mar...@redhat.com>
Cc: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
---
Cornelia,
This is what I proposed earlier today.
I have tested it successfully with PCI.
Does it sound good to you? Would you have a try with CCW?
Note that this is based on my earlier series which fixes PCI.
Thanks,
Maxime
---
hw/s390x/virtio-ccw.c | 30 +++++++++++++++---------------
hw/virtio/virtio-bus.c | 9 +++++----
hw/virtio/virtio-pci.c | 18 ++++++++++++++----
include/hw/virtio/virtio-bus.h | 10 +++++-----
4 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a554a24..e10a88d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1261,6 +1261,16 @@ static int virtio_ccw_load_config(DeviceState *d,
QEMUFile *f)
return 0;
}
+static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)
+{
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+ VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+
+ if (dev->max_rev >= 1) {
+ virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
+ }
+}
+
/* This is called by virtio-bus just after the device is plugged. */
static void virtio_ccw_device_plugged(DeviceState *d, Error **errp)
{
@@ -1270,6 +1280,10 @@ static void virtio_ccw_device_plugged(DeviceState *d,
Error **errp)
SubchDev *sch = ccw_dev->sch;
int n = virtio_get_num_queues(vdev);
+ if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
+ dev->max_rev = 0;
+ }
+
if (virtio_get_num_queues(vdev) > VIRTIO_CCW_QUEUE_MAX) {
error_setg(errp, "The number of virtqueues %d "
"exceeds ccw limit %d", n,
@@ -1283,25 +1297,11 @@ static void virtio_ccw_device_plugged(DeviceState *d,
Error **errp)
sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus);
- if (dev->max_rev >= 1) {
- virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
- }
css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
d->hotplugged, 1);
}
-static void virtio_ccw_post_plugged(DeviceState *d, Error **errp)
-{
- VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
-
- if (!virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1)) {
- /* A backend didn't support modern virtio. */
- dev->max_rev = 0;
- }
-}
-
static void virtio_ccw_device_unplugged(DeviceState *d)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
@@ -1593,8 +1593,8 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass,
void *data)
k->load_queue = virtio_ccw_load_queue;
k->save_config = virtio_ccw_save_config;
k->load_config = virtio_ccw_load_config;
+ k->pre_plugged = virtio_ccw_pre_plugged;
k->device_plugged = virtio_ccw_device_plugged;
- k->post_plugged = virtio_ccw_post_plugged;
k->device_unplugged = virtio_ccw_device_unplugged;
k->ioeventfd_started = virtio_ccw_ioeventfd_started;
k->ioeventfd_set_started = virtio_ccw_ioeventfd_set_started;
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index a85b7c8..9d2111f 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -49,16 +49,17 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error
**errp)
DPRINTF("%s: plug device.\n", qbus->name);
- if (klass->device_plugged != NULL) {
- klass->device_plugged(qbus->parent, errp);
+ if (klass->pre_plugged != NULL) {
+ klass->pre_plugged(qbus->parent, errp);
}
/* Get the features of the plugged device. */
assert(vdc->get_features != NULL);
vdev->host_features = vdc->get_features(vdev, vdev->host_features,
errp);
- if (klass->post_plugged != NULL) {
- klass->post_plugged(qbus->parent, errp);
+
+ if (klass->device_plugged != NULL) {
+ klass->device_plugged(qbus->parent, errp);
}
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 9e88d7b..07e6c60 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1569,6 +1569,18 @@ static void
virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
®ion->mr);
}
+static void virtio_pci_pre_plugged(DeviceState *d, Error **errp)
+{
+ VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+ if (virtio_pci_modern(proxy)) {
+ virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
+ }
+
+ virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);
+}
+
/* This is called by virtio-bus just after the device is plugged. */
static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
{
@@ -1585,7 +1597,7 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
* Virtio capabilities present without
* VIRTIO_F_VERSION_1 confuses guests
*/
- if (!virtio_test_backend_virtio_1(vdev, errp)) {
+ if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
virtio_pci_disable_modern(proxy);
}
@@ -1637,7 +1649,6 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
struct virtio_pci_cfg_cap *cfg_mask;
- virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
virtio_pci_modern_regions_init(proxy);
virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap);
@@ -1702,8 +1713,6 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
if (!kvm_has_many_ioeventfds()) {
proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
}
-
- virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);
}
static void virtio_pci_device_unplugged(DeviceState *d)
@@ -2459,6 +2468,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass,
void *data)
k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
k->vmstate_change = virtio_pci_vmstate_change;
+ k->pre_plugged = virtio_pci_pre_plugged;
k->device_plugged = virtio_pci_device_plugged;
k->device_unplugged = virtio_pci_device_unplugged;
k->query_nvectors = virtio_pci_query_nvectors;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index f3e5ef3..24caa0a 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -54,16 +54,16 @@ typedef struct VirtioBusClass {
int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
void (*vmstate_change)(DeviceState *d, bool running);
/*
+ * Expose the features the transport layer supports before
+ * the negotiation takes place.
+ */
+ void (*pre_plugged)(DeviceState *d, Error **errp);
+ /*
* transport independent init function.
* This is called by virtio-bus just after the device is plugged.
*/
void (*device_plugged)(DeviceState *d, Error **errp);
/*
- * Re-evaluate setup after feature bits have been validated
- * by the device backend.
- */
- void (*post_plugged)(DeviceState *d, Error **errp);
- /*
* transport independent exit function.
* This is called by virtio-bus just before the device is unplugged.
*/
--
2.7.4
