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

[Alex Bennée]

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?

> 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?

>
> Stefan
>
> [[End of PGP Signed Part]]


-- 
Alex Bennée