[Qemu-devel] [PATCH 0/5] s390x: some virtio patches

[Qemu-devel] [PATCH 0/5] s390x: some virtio patches

[Cornelia Huck]

Hi,

here are some virtio-related s390x patches. I'd like to submit them
before sending any further s390x patches, simply to get them out of
the way and to allow the various virtio-related patchsets to proceed
without generating needless conflicts in s390x code.

After applying these patches, both the old s390-virtio transport and
virtio-ccw should be more similar to what everybody else does, so
doing cross-transport reworks should become easier.

As always, branch available at

git://github.com/cohuck/qemu s390-next

I plan to send a pull request later this week, unless there are complaints.

Christian Borntraeger (2):
  s390-virtio: Accommodate guests using virtqueues too early
  s390-virtio: clear {used,avail}_event_idx on reset as well

Cornelia Huck (3):
  s390-virtio: use common features
  virtio-ccw: change realization sequence
  virtio-ccw: implement ->device_plugged

 hw/s390x/s390-virtio-bus.c | 28 +++++++++---------
 hw/s390x/s390-virtio.c     | 10 +++++++
 hw/s390x/virtio-ccw.c      | 72 ++++++++++++++++++++++++----------------------
 3 files changed, 62 insertions(+), 48 deletions(-)

-- 
2.4.0

[Qemu-devel] [PATCH 1/5] s390-virtio: Accommodate guests using virtqueues too early

[Cornelia Huck]

From: Christian Borntraeger <borntraeger@de.ibm.com>

Feature updates are not a synchronuous operation for the legacy
s390-virtio transport. This transport syncs the guest feature bits
(those from finalize) on the set_status hypercall. Before that qemu
thinks that features are zero, which means QEMU will misbehave, e.g.
it will not write the event index, even if the guest asks for it.

Let's detect the case where a kick happens before the driver is ready
and force sync the features.
With this workaround, it is now safe to switch to the common feature
bit handling code as used by all other transports.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 3a1b9ee..59750db 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -77,6 +77,16 @@ static int s390_virtio_hcall_notify(const uint64_t *args)
     if (mem > ram_size) {
         VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus, mem, &i);
         if (dev) {
+            /*
+             * Older kernels will use the virtqueue before setting DRIVER_OK.
+             * In this case the feature bits are not yet up to date, meaning
+             * that several funny things can happen, e.g. the guest thinks
+             * EVENT_IDX is on and QEMU thinks it is off. Let's force a feature
+             * and status sync.
+             */
+            if (!(dev->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+                s390_virtio_device_update_status(dev);
+            }
             virtio_queue_notify(dev->vdev, i);
         } else {
             r = -EINVAL;
-- 
2.4.0

[Qemu-devel] [PATCH 2/5] s390-virtio: use common features

[Cornelia Huck]

We used to avoid enabling event_idx for virtio-blk devices via
s390-virtio, but we now have a workaround in place for guests trying
to use the device before setting DRIVER_OK. Therefore, let's add
DEFINE_VIRTIO_COMMON_FEATURES to the base device so all devices get
those common features - and make s390-virtio use the same mechanism
as the other transports do.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-bus.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index c27f8a5..4a33c6a 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -530,7 +530,6 @@ static unsigned virtio_s390_get_features(DeviceState *d)
 /**************** S390 Virtio Bus Device Descriptions *******************/
 
 static Property s390_virtio_net_properties[] = {
-    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -592,18 +591,12 @@ static const TypeInfo s390_virtio_serial = {
     .class_init    = s390_virtio_serial_class_init,
 };
 
-static Property s390_virtio_rng_properties[] = {
-    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void s390_virtio_rng_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
 
     k->realize = s390_virtio_rng_realize;
-    dc->props = s390_virtio_rng_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }
 
@@ -632,10 +625,16 @@ 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;
@@ -651,7 +650,6 @@ static const TypeInfo virtio_s390_device_info = {
 };
 
 static Property s390_virtio_scsi_properties[] = {
-    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
     DEFINE_VIRTIO_SCSI_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -675,18 +673,12 @@ static const TypeInfo s390_virtio_scsi = {
 };
 
 #ifdef CONFIG_VHOST_SCSI
-static Property s390_vhost_scsi_properties[] = {
-    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void s390_vhost_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
 
     k->realize = s390_vhost_scsi_realize;
-    dc->props = s390_vhost_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
-- 
2.4.0

Re: [Qemu-devel] [PATCH 2/5] s390-virtio: use common features

[Shannon Zhao]

On 2015/5/4 23:28, Cornelia Huck wrote:
> We used to avoid enabling event_idx for virtio-blk devices via
> s390-virtio, but we now have a workaround in place for guests trying
> to use the device before setting DRIVER_OK. Therefore, let's add
> DEFINE_VIRTIO_COMMON_FEATURES to the base device so all devices get
> those common features - and make s390-virtio use the same mechanism
> as the other transports do.
> 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---
>  hw/s390x/s390-virtio-bus.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index c27f8a5..4a33c6a 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -530,7 +530,6 @@ static unsigned virtio_s390_get_features(DeviceState *d)
>  /**************** S390 Virtio Bus Device Descriptions *******************/
>  
>  static Property s390_virtio_net_properties[] = {
> -    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
>      DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> @@ -592,18 +591,12 @@ static const TypeInfo s390_virtio_serial = {
>      .class_init    = s390_virtio_serial_class_init,
>  };
>  
> -static Property s390_virtio_rng_properties[] = {
> -    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void s390_virtio_rng_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
>  
>      k->realize = s390_virtio_rng_realize;
> -    dc->props = s390_virtio_rng_properties;
>      set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>  }
>  
> @@ -632,10 +625,16 @@ 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;
> @@ -651,7 +650,6 @@ static const TypeInfo virtio_s390_device_info = {
>  };
>  
>  static Property s390_virtio_scsi_properties[] = {
> -    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
>      DEFINE_VIRTIO_SCSI_FEATURES(VirtIOS390Device, host_features),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> @@ -675,18 +673,12 @@ static const TypeInfo s390_virtio_scsi = {
>  };
>  
>  #ifdef CONFIG_VHOST_SCSI
> -static Property s390_vhost_scsi_properties[] = {
> -    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void s390_vhost_scsi_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
>  
>      k->realize = s390_vhost_scsi_realize;
> -    dc->props = s390_vhost_scsi_properties;
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  }
>  
> 

-- 
Shannon

[Qemu-devel] [PATCH 3/5] s390-virtio: clear {used, avail}_event_idx on reset as well

[Cornelia Huck]

From: Christian Borntraeger <borntraeger@de.ibm.com>

The old s390-virtio transport clears the vring used/avail indices in
the shared area on reset. When we enabled event_idx for virtio-blk, we
noticed that this is not enough: We also need to clear the published
used/avail event indices, or reboot will fail.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-bus.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 4a33c6a..4f69cbb 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -77,10 +77,18 @@ void s390_virtio_reset_idx(VirtIOS390Device *dev)
             VIRTIO_VRING_AVAIL_IDX_OFFS;
         address_space_stw(&address_space_memory, idx_addr, 0,
                           MEMTXATTRS_UNSPECIFIED, NULL);
+        idx_addr = virtio_queue_get_avail_addr(dev->vdev, i) +
+            virtio_queue_get_avail_size(dev->vdev, i);
+        address_space_stw(&address_space_memory, idx_addr, 0,
+                          MEMTXATTRS_UNSPECIFIED, NULL);
         idx_addr = virtio_queue_get_used_addr(dev->vdev, i) +
             VIRTIO_VRING_USED_IDX_OFFS;
         address_space_stw(&address_space_memory, idx_addr, 0,
                           MEMTXATTRS_UNSPECIFIED, NULL);
+        idx_addr = virtio_queue_get_used_addr(dev->vdev, i) +
+            virtio_queue_get_used_size(dev->vdev, i);
+        address_space_stw(&address_space_memory, idx_addr, 0,
+                          MEMTXATTRS_UNSPECIFIED, NULL);
     }
 }
 
-- 
2.4.0

[Qemu-devel] [PATCH 4/5] virtio-ccw: change realization sequence

[Cornelia Huck]

virtio-ccw has an odd sequence of realizing devices: first the
device-specific relization (net, block, ...), then the generic
realization. It feels less odd to have the generic realization
callback trigger the device-specific realization instead (and this
also matches what virtio-pci does).

One thing to note: We need to defer initializing the cu model in the
sense id data until after the device-specific realization has been
performed, as we need to refer to the virtio device's device_id.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-Id: <1429627016-30656-2-git-send-email-cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index c1d8288..1c2bd9d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -642,8 +642,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
     return ret;
 }
 
-static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
-                                      VirtIODevice *vdev, Error **errp)
+static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 {
     unsigned int cssid = 0;
     unsigned int ssid = 0;
@@ -654,6 +653,9 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
     SubchDev *sch;
     int num;
     DeviceState *parent = DEVICE(dev);
+    Error *err = NULL;
+    VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
+    VirtIODevice *vdev;
 
     sch = g_malloc0(sizeof(SubchDev));
 
@@ -766,6 +768,18 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
     memset(&sch->id, 0, sizeof(SenseId));
     sch->id.reserved = 0xff;
     sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
+
+    if (k->realize) {
+        k->realize(dev, &err);
+    }
+    if (err) {
+        error_propagate(errp, err);
+        css_subch_assign(cssid, ssid, schid, devno, NULL);
+        goto out_err;
+    }
+
+    /* device_id is only set after vdev has been realized */
+    vdev = virtio_ccw_get_vdev(sch);
     sch->id.cu_model = vdev->device_id;
 
     /* Only the first 32 feature bits are used. */
@@ -813,10 +827,7 @@ static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 static void virtio_ccw_net_instance_init(Object *obj)
@@ -839,10 +850,7 @@ static void virtio_ccw_blk_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 static void virtio_ccw_blk_instance_init(Object *obj)
@@ -879,10 +887,7 @@ static void virtio_ccw_serial_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 
@@ -904,10 +909,7 @@ static void virtio_ccw_balloon_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 static void balloon_ccw_stats_get_all(Object *obj, struct Visitor *v,
@@ -972,10 +974,7 @@ static void virtio_ccw_scsi_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 static void virtio_ccw_scsi_instance_init(Object *obj)
@@ -999,10 +998,7 @@ static void vhost_ccw_scsi_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_bool(OBJECT(vdev), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
-        return;
     }
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 static void vhost_ccw_scsi_instance_init(Object *obj)
@@ -1030,8 +1026,6 @@ static void virtio_ccw_rng_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     object_property_set_link(OBJECT(dev),
                              OBJECT(dev->vdev.conf.rng), "rng",
                              NULL);
-
-    virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
 }
 
 /* DeviceState to VirtioCcwDevice. Note: used on datapath,
@@ -1640,10 +1634,9 @@ static const TypeInfo virtio_ccw_rng = {
 static void virtio_ccw_busdev_realize(DeviceState *dev, Error **errp)
 {
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
-    VirtIOCCWDeviceClass *_info = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
 
     virtio_ccw_bus_new(&_dev->bus, sizeof(_dev->bus), _dev);
-    _info->realize(_dev, errp);
+    virtio_ccw_device_realize(_dev, errp);
 }
 
 static int virtio_ccw_busdev_exit(DeviceState *dev)
-- 
2.4.0

[Qemu-devel] [PATCH 5/5] virtio-ccw: implement ->device_plugged

[Cornelia Huck]

Let's move operations that are only valid after the backend has been
realized to a ->device_plugged callback, just as virtio-pci does.
Also reorder setting up the host feature bits to the sequence used
by virtio-pci.

While we're at it, also add a ->device_unplugged callback to stop
ioeventfd, just to be on the safe side.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-Id: <1429627016-30656-3-git-send-email-cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 1c2bd9d..430cc6f 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -652,10 +652,8 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     bool found = false;
     SubchDev *sch;
     int num;
-    DeviceState *parent = DEVICE(dev);
     Error *err = NULL;
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
-    VirtIODevice *vdev;
 
     sch = g_malloc0(sizeof(SubchDev));
 
@@ -778,19 +776,6 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
         goto out_err;
     }
 
-    /* device_id is only set after vdev has been realized */
-    vdev = virtio_ccw_get_vdev(sch);
-    sch->id.cu_model = vdev->device_id;
-
-    /* Only the first 32 feature bits are used. */
-    dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
-                                                         dev->host_features[0]);
-
-    virtio_add_feature(&dev->host_features[0], VIRTIO_F_NOTIFY_ON_EMPTY);
-    virtio_add_feature(&dev->host_features[0], VIRTIO_F_BAD_FEATURE);
-
-    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
-                          parent->hotplugged, 1);
     return;
 
 out_err:
@@ -1428,6 +1413,30 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
     return 0;
 }
 
+/* This is called by virtio-bus just after the device is plugged. */
+static void virtio_ccw_device_plugged(DeviceState *d)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+    SubchDev *sch = dev->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]);
+
+    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
+                          d->hotplugged, 1);
+}
+
+static void virtio_ccw_device_unplugged(DeviceState *d)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+    virtio_ccw_stop_ioeventfd(dev);
+}
 /**************** Virtio-ccw Bus Device Descriptions *******************/
 
 static Property virtio_ccw_net_properties[] = {
@@ -1752,6 +1761,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->device_plugged = virtio_ccw_device_plugged;
+    k->device_unplugged = virtio_ccw_device_unplugged;
 }
 
 static const TypeInfo virtio_ccw_bus_info = {
-- 
2.4.0

Re: [Qemu-devel] [PATCH 5/5] virtio-ccw: implement ->device_plugged

[Shannon Zhao]

On 2015/5/4 23:28, Cornelia Huck wrote:
> Let's move operations that are only valid after the backend has been
> realized to a ->device_plugged callback, just as virtio-pci does.
> Also reorder setting up the host feature bits to the sequence used
> by virtio-pci.
> 
> While we're at it, also add a ->device_unplugged callback to stop
> ioeventfd, just to be on the safe side.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Message-Id: <1429627016-30656-3-git-send-email-cornelia.huck@de.ibm.com>

Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---
>  hw/s390x/virtio-ccw.c | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 1c2bd9d..430cc6f 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -652,10 +652,8 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>      bool found = false;
>      SubchDev *sch;
>      int num;
> -    DeviceState *parent = DEVICE(dev);
>      Error *err = NULL;
>      VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
> -    VirtIODevice *vdev;
>  
>      sch = g_malloc0(sizeof(SubchDev));
>  
> @@ -778,19 +776,6 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>          goto out_err;
>      }
>  
> -    /* device_id is only set after vdev has been realized */
> -    vdev = virtio_ccw_get_vdev(sch);
> -    sch->id.cu_model = vdev->device_id;
> -
> -    /* Only the first 32 feature bits are used. */
> -    dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
> -                                                         dev->host_features[0]);
> -
> -    virtio_add_feature(&dev->host_features[0], VIRTIO_F_NOTIFY_ON_EMPTY);
> -    virtio_add_feature(&dev->host_features[0], VIRTIO_F_BAD_FEATURE);
> -
> -    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> -                          parent->hotplugged, 1);
>      return;
>  
>  out_err:
> @@ -1428,6 +1413,30 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
>      return 0;
>  }
>  
> +/* This is called by virtio-bus just after the device is plugged. */
> +static void virtio_ccw_device_plugged(DeviceState *d)
> +{
> +    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> +    SubchDev *sch = dev->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]);
> +
> +    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> +                          d->hotplugged, 1);
> +}
> +
> +static void virtio_ccw_device_unplugged(DeviceState *d)
> +{
> +    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> +
> +    virtio_ccw_stop_ioeventfd(dev);
> +}
>  /**************** Virtio-ccw Bus Device Descriptions *******************/
>  
>  static Property virtio_ccw_net_properties[] = {
> @@ -1752,6 +1761,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->device_plugged = virtio_ccw_device_plugged;
> +    k->device_unplugged = virtio_ccw_device_unplugged;
>  }
>  
>  static const TypeInfo virtio_ccw_bus_info = {
> 

-- 
Shannon