Re: What is the correct way to handle the VirtIO config space in vhost-user?

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4845 bytes --]

On Mon, Feb 28, 2022 at 04:16:43PM +0000, Alex Bennée wrote:
> 
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > [[PGP Signed Part:Undecided]]
> > On Fri, Feb 25, 2022 at 05:32:43PM +0000, Alex Bennée wrote:
> >> 
> >> [Apologies to CC list for repost due to fat fingering the mailing list address]
> >> 
> <snip>
> >> 
> >> (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).
> >
> > Class init is a one-time per-class initializer function. It is mostly
> > used for setting up callbacks/overridden methods from the base class.
> >
> > Instance init is like an object constructor in object-oriented
> > programming.
> 
> I phrased my statement poorly. What I meant to say is I sometimes find
> QEMUs approach to using class over instance initialisation inconsistent.
> I think I understand the "policy" as use class init until there is a
> case where you can't (e.g. having individual control of each instance of
> a device).
> 
> > This is not a .get_config() method, it's a VIRTIO configuration change
> > notification handler. The vhost-user-blk device server ("slave") sends
> > this notification to notify the driver that configuration space contents
> > have been updated (e.g. the disk was resized).
> 
> So this should come in the initial vhost-user set of handshake messages
> if the VHOST_USER_PROTOCOL_F_CONFIG is negotiated between the master and
> slave? I guess without this protocol feature vhost-user can't support
> writeable config spaces?

The VHOST_USER_PROTOCOL_F_CONFIG vhost-user protocol feature bit
enables:
1. VHOST_USER_GET_CONFIG - reading configuration space
2. VHOST_USER_SET_CONFIG - writing configuration space
3. VHOST_USER_SLAVE_CONFIG_CHANGE_MSG - change notifications

If the vhost-user server is supposed to participate in configuration
space accesses/notifications, then it needs to implement
VHOST_USER_PROTOCOL_F_CONFIG.

QEMU's vhost-user-blk assumes the vhost-user server supports
VHOST_USER_PROTOCOL_F_CONFIG. It's an optional vhost-user protocol
feature but the virtio-blk device relies on configuration space
(otherwise QEMU's --device vhost-user-blk wouldn't know the capacity of
the disk). vhost_user_blk_realize_connect() sends VHOST_USER_GET_CONFIG
to fetch the configuration space contents when the device is
instantiated.

Some vhost-user device types don't need VHOST_USER_PROTOCOL_F_CONFIG. In
that case QEMU's --device vhost-user-FOO implements .get/set_config()
itself. virtio-net is an example where this is the case.

> > QEMU fetches the new
> > config space contents from the device and then forwards the notification
> > to the guest.
> >
> > The .get_config() method for vhost-user-blk.c is:
> >
> >   static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> >   {
> >       VHostUserBlk *s = VHOST_USER_BLK(vdev);
> >   
> >       /* Our num_queues overrides the device backend */
> >       virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> >   
> >       memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
> >   }
> >
> > vhost_user_blk_update_config() is simple, it copies out s->blkcfg.
> >
> >> 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?
> >
> > VIRTIO allows the driver to write to the config space. This is used to
> > toggle the disk write cache on the virtio-blk device, for example.
> >
> >> So my question is which approach is the correct one? Is one a legacy
> >> approach or is it "depends on what you are doing"?
> >
> > Yes, it depends on whether the device sends configuration space change
> > notifications or not. If not, a traditional .get_config() like
> > vhost-user-gpu can be used. If yes, then caching the configuration space
> > contents like vhost-user-blk is convenient.
> 
> Is there any feature flag for this in the VirtIO spec? I had a look and
> couldn't see an obvious common one. Does it basically come down to the
> verbiage in the Device configure layout section for any given device?

The contents of the configuration space are device-specific, so there is
no generic feature flag. Many devices don't update the configuration
space and therefore don't send change notifications. The details are
documented for each device type (e.g. "if the driver negotiated the
VIRTIO_CONSOLE_F_SIZE feature, a configuration change notification
indicates that the updated size can be read from the configuration
fields").

I just noticed that VIRTIO does not specify that the virtio-blk capacity
field can change. The spec is incomplete and I will send a patch for
that.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]