[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
What is the correct way to handle the VirtIO config space in vhost-user?
From: |
Alex Bennée |
Subject: |
What is the correct way to handle the VirtIO config space in vhost-user? |
Date: |
Fri, 25 Feb 2022 17:32:43 +0000 |
User-agent: |
mu4e 1.7.9; emacs 28.0.91 |
[Apologies to CC list for repost due to fat fingering the mailing list address]
Hi Michael,
I was updating my vhost-user-rpmb implementation when I realised it
wasn't handling config space access properly. However when I went to
implement it I realised there seems to be two ways of doing this. For
example in vhost-user-gpu we have:
static void
vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t *config_data)
{
VhostUserGPU *g = VHOST_USER_GPU(vdev);
VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev);
struct virtio_gpu_config *vgconfig =
(struct virtio_gpu_config *)config_data;
Error *local_err = NULL;
int ret;
memset(config_data, 0, sizeof(struct virtio_gpu_config));
ret = vhost_dev_get_config(&g->vhost->dev,
config_data, sizeof(struct virtio_gpu_config),
&local_err);
if (ret) {
error_report_err(local_err);
return;
}
/* those fields are managed by qemu */
vgconfig->num_scanouts = b->virtio_config.num_scanouts;
vgconfig->events_read = b->virtio_config.events_read;
vgconfig->events_clear = b->virtio_config.events_clear;
}
which is setup with .get_config and .set_config functions that poke the
appropriate vhost communication. However to do this needs an instance
init to create a vhost just so it can jump the g->vhost->dev indirection:
static void
vhost_user_gpu_instance_init(Object *obj)
{
VhostUserGPU *g = VHOST_USER_GPU(obj);
g->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND));
object_property_add_alias(obj, "chardev",
OBJECT(g->vhost), "chardev");
}
(aside: this continues my QOM confusion about when things should be in a
class or instance init, up until this point I hadn't needed it in my
stub).
However when grepping around I found some vhost-user devices do it
differently, for example vhost-user-blk has:
static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
{
int ret;
struct virtio_blk_config blkcfg;
VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
Error *local_err = NULL;
ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
sizeof(struct virtio_blk_config),
&local_err);
if (ret < 0) {
error_report_err(local_err);
return ret;
}
/* valid for resize only */
if (blkcfg.capacity != s->blkcfg.capacity) {
s->blkcfg.capacity = blkcfg.capacity;
memcpy(dev->vdev->config, &s->blkcfg, sizeof(struct
virtio_blk_config));
virtio_notify_config(dev->vdev);
}
return 0;
}
Although this seems to miss the ability to "set" a config - although
that seems confusing anyway, surely the guest only ever reads the config
space?
So my question is which approach is the correct one? Is one a legacy
approach or is it "depends on what you are doing"?
Ultimately I guess this points to the need for a bit more API
documentation to make it clear when certain methods should be used.
--
Alex Bennée
- What is the correct way to handle the VirtIO config space in vhost-user?,
Alex Bennée <=