Re: [PATCH v3 10/20] hw/virtio: add config support to vhost-user-device

[Michael S. Tsirkin]

On Mon, Jul 10, 2023 at 04:35:12PM +0100, Alex Bennée wrote:
> To use the generic device the user will need to provide the config
> region size via the command line. We also add a notifier so the guest
> can be pinged if the remote daemon updates the config.
> 
> With these changes:
> 
>   -device vhost-user-device-pci,virtio-id=41,num_vqs=2,config_size=8
> 
> is equivalent to:
> 
>   -device vhost-user-gpio-pci
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


This one I think it's best to defer until we get a better
handle on how we want the configuration to look.


> ---
>  include/hw/virtio/vhost-user-device.h |  1 +
>  hw/virtio/vhost-user-device.c         | 58 ++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/virtio/vhost-user-device.h 
> b/include/hw/virtio/vhost-user-device.h
> index 9105011e25..3ddf88a146 100644
> --- a/include/hw/virtio/vhost-user-device.h
> +++ b/include/hw/virtio/vhost-user-device.h
> @@ -22,6 +22,7 @@ struct VHostUserBase {
>      CharBackend chardev;
>      uint16_t virtio_id;
>      uint32_t num_vqs;
> +    uint32_t config_size;
>      /* State tracking */
>      VhostUserState vhost_user;
>      struct vhost_virtqueue *vhost_vq;
> diff --git a/hw/virtio/vhost-user-device.c b/hw/virtio/vhost-user-device.c
> index b0239fa033..2b028cae08 100644
> --- a/hw/virtio/vhost-user-device.c
> +++ b/hw/virtio/vhost-user-device.c
> @@ -117,6 +117,42 @@ static uint64_t vub_get_features(VirtIODevice *vdev,
>      return vub->vhost_dev.features & ~(1ULL << 
> VHOST_USER_F_PROTOCOL_FEATURES);
>  }
>  
> +/*
> + * To handle VirtIO config we need to know the size of the config
> + * space. We don't cache the config but re-fetch it from the guest
> + * every time in case something has changed.
> + */
> +static void vub_get_config(VirtIODevice *vdev, uint8_t *config)
> +{
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    Error *local_err = NULL;
> +
> +    /*
> +     * There will have been a warning during vhost_dev_init, but lets
> +     * assert here as nothing will go right now.
> +     */
> +    g_assert(vub->config_size && vub->vhost_user.supports_config == true);
> +
> +    if (vhost_dev_get_config(&vub->vhost_dev, config,
> +                             vub->config_size, &local_err)) {
> +        error_report_err(local_err);
> +    }
> +}
> +
> +/*
> + * When the daemon signals an update to the config we just need to
> + * signal the guest as we re-read the config on demand above.
> + */
> +static int vub_config_notifier(struct vhost_dev *dev)
> +{
> +    virtio_notify_config(dev->vdev);
> +    return 0;
> +}
> +
> +const VhostDevConfigOps vub_config_ops = {
> +    .vhost_dev_config_notifier = vub_config_notifier,
> +};
> +
>  static void vub_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      /*
> @@ -141,12 +177,21 @@ static int vub_connect(DeviceState *dev)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    struct vhost_dev *vhost_dev = &vub->vhost_dev;
>  
>      if (vub->connected) {
>          return 0;
>      }
>      vub->connected = true;
>  
> +    /*
> +     * If we support VHOST_USER_GET_CONFIG we must enable the notifier
> +     * so we can ping the guest when it updates.
> +     */
> +    if (vub->vhost_user.supports_config) {
> +        vhost_dev_set_config_notifier(vhost_dev, &vub_config_ops);
> +    }
> +
>      /* restore vhost state */
>      if (virtio_device_started(vdev, vdev->status)) {
>          vub_start(vdev);
> @@ -214,11 +259,20 @@ static void vub_device_realize(DeviceState *dev, Error 
> **errp)
>          vub->num_vqs = 1; /* reasonable default? */
>      }
>  
> +    /*
> +     * We can't handle config requests unless we know the size of the
> +     * config region, specialisations of the vhost-user-device will be
> +     * able to set this.
> +     */
> +    if (vub->config_size) {
> +        vub->vhost_user.supports_config = true;
> +    }
> +
>      if (!vhost_user_init(&vub->vhost_user, &vub->chardev, errp)) {
>          return;
>      }
>  
> -    virtio_init(vdev, vub->virtio_id, 0);
> +    virtio_init(vdev, vub->virtio_id, vub->config_size);
>  
>      /*
>       * Disable guest notifiers, by default all notifications will be via the
> @@ -268,6 +322,7 @@ static void vub_class_init(ObjectClass *klass, void *data)
>      vdc->realize = vub_device_realize;
>      vdc->unrealize = vub_device_unrealize;
>      vdc->get_features = vub_get_features;
> +    vdc->get_config = vub_get_config;
>      vdc->set_status = vub_set_status;
>  }
>  
> @@ -295,6 +350,7 @@ static Property vud_properties[] = {
>      DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
>      DEFINE_PROP_UINT16("virtio-id", VHostUserBase, virtio_id, 0),
>      DEFINE_PROP_UINT32("num_vqs", VHostUserBase, num_vqs, 1),
> +    DEFINE_PROP_UINT32("config_size", VHostUserBase, config_size, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 2.39.2