Re: [PATCH v2 05/15] hw/virtio: add boilerplate for vhost-user-gpio device

[Vincent Whitchurch]

On Tue, May 24, 2022 at 04:40:46PM +0100, Alex Bennée wrote:
> +static int vu_gpio_start(VirtIODevice *vdev)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
> +    int ret, i;
> +
> +    if (!k->set_guest_notifiers) {
> +        error_report("binding does not support guest notifiers");
> +        return -ENOSYS;
> +    }
> +
> +    ret = vhost_dev_enable_notifiers(&gpio->vhost_dev, vdev);
> +    if (ret < 0) {
> +        error_report("Error enabling host notifiers: %d", ret);
> +        return ret;
> +    }
> +
> +    ret = k->set_guest_notifiers(qbus->parent, gpio->vhost_dev.nvqs, true);
> +    if (ret < 0) {
> +        error_report("Error binding guest notifier: %d", ret);
> +        goto err_host_notifiers;
> +    }
> +
> +    /*
> +     * Before we start up we need to ensure we have the final feature
> +     * set needed for the vhost configuration.
> +     */
> +    vhost_ack_features(&gpio->vhost_dev, feature_bits, 
> vdev->backend_features);

This is doing the feature handling differently from the other
vhost-user-* implementations, and it doesn't seem to work for me.
Negotiated features (I noticed it with VIRTIO_RING_F_EVENT_IDX) never
make it to VHOST_USER_SET_FEATURES.

If I change this code to match vhost-user-i2c and the other
implementations like in the patch below, it works.

The guest is Linux v5.18.  The backend uses libvhost-user and is the one
posted here (with a few fixes):

 
https://lore.kernel.org/lkml/20220311162445.346685-3-vincent.whitchurch@axis.com/

8<-------
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index 87e3976880..1dc7af6b03 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -73,7 +73,7 @@ static int vu_gpio_start(VirtIODevice *vdev)
      * Before we start up we need to ensure we have the final feature
      * set needed for the vhost configuration.
      */
-    vhost_ack_features(&gpio->vhost_dev, feature_bits, vdev->backend_features);
+    gpio->vhost_dev.acked_features = vdev->guest_features;
 
     ret = vhost_dev_start(&gpio->vhost_dev, vdev);
     if (ret < 0) {
@@ -156,9 +156,7 @@ static uint64_t vu_gpio_get_features(VirtIODevice *vdev, 
uint64_t features,
     virtio_add_feature(&features, VIRTIO_GPIO_F_IRQ);
     virtio_add_feature(&features, VIRTIO_F_VERSION_1);
 
-    vdev->backend_features = vhost_get_features(&gpio->vhost_dev, feature_bits,
-                                                features);
-    return vdev->backend_features;
+    return vhost_get_features(&gpio->vhost_dev, feature_bits, features);
 }
 
 static void vu_gpio_handle_output(VirtIODevice *vdev, VirtQueue *vq)
-- 
2.28.0