On Thu, Dec 11, 2014 at 02:25:20PM +0100, Cornelia Huck wrote:
> Devices may support different sets of feature bits depending on which
> revision they're operating at. Let's give the transport a way to
> re-query the device about its features when the revision has been
> changed.
>
> Signed-off-by: Cornelia Huck <cornelia.huck@xxxxxxxxxx>
So now we have both get_features and get_features_rev, and
it's never clear which revision does host_features refer to.
IMHO that's just too messy.
Let's add get_legacy_features and host_legacy_features instead?
> ---
> hw/s390x/virtio-ccw.c | 12 ++++++++++--
> hw/virtio/virtio-bus.c | 14 ++++++++++++--
> include/hw/virtio/virtio-bus.h | 3 +++
> include/hw/virtio/virtio.h | 3 +++
> 4 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index a55e851..8b6b2ab 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -699,6 +699,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> }
> ret = 0;
> dev->revision = revinfo.revision;
> + /* Re-evaluate which features the device wants to offer. */
> + dev->host_features =
> + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features,
> + dev->revision >= 1 ? 1 : 0);
> break;
> default:
> ret = -ENOSYS;
> @@ -712,6 +716,9 @@ static void virtio_sch_disable_cb(SubchDev *sch)
> VirtioCcwDevice *dev = sch->driver_data;
>
> dev->revision = -1;
> + /* Reset the device's features to legacy. */
> + dev->host_features =
> + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0);
> }
>
> static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> @@ -853,8 +860,9 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
> virtio_add_feature(&dev->host_features, VIRTIO_F_BAD_FEATURE);
>
> - dev->host_features = virtio_bus_get_vdev_features(&dev->bus,
> - dev->host_features);
> + /* All devices start in legacy mode. */
> + dev->host_features =
> + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0);
>
> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> parent->hotplugged, 1);
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 32e3fab..a30826c 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -97,18 +97,28 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus)
> }
>
> /* Get the features of the plugged device. */
> -uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus,
> - uint64_t requested_features)
> +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus,
> + uint64_t requested_features,
> + unsigned int revision)
> {
> VirtIODevice *vdev = virtio_bus_get_device(bus);
> VirtioDeviceClass *k;
>
> assert(vdev != NULL);
> k = VIRTIO_DEVICE_GET_CLASS(vdev);
> + if (revision > 0 && k->get_features_rev) {
> + return k->get_features_rev(vdev, requested_features, revision);
> + }
> assert(k->get_features != NULL);
> return k->get_features(vdev, requested_features);
> }
>
> +uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus,
> + uint64_t requested_features)
> +{
> + return virtio_bus_get_vdev_features_rev(bus, requested_features, 0);
> +}
> +
> /* Get bad features of the plugged device. */
> uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
> {
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 0a4dde1..f0916ef 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -84,6 +84,9 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus);
> /* Get the features of the plugged device. */
> uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus,
> uint64_t requested_features);
> +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus,
> + uint64_t requested_features,
> + unsigned int revision);
> /* Get bad features of the plugged device. */
> uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus);
> /* Get config of the plugged device. */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 068211e..1338540 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -147,6 +147,9 @@ typedef struct VirtioDeviceClass {
> DeviceRealize realize;
> DeviceUnrealize unrealize;
> uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features);
> + uint64_t (*get_features_rev)(VirtIODevice *vdev,
> + uint64_t requested_features,
> + unsigned int revision);
> uint64_t (*bad_features)(VirtIODevice *vdev);
> void (*set_features)(VirtIODevice *vdev, uint64_t val);
> int (*validate_features)(VirtIODevice *vdev);
> --
> 1.7.9.5
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
