Re: [PATCH 0/2] vhost-user-gpu get_edid feature

[Erico Nunes <ernunes@redhat.com>]

On 24/05/2023 13:23, Marc-André Lureau wrote:
> Hi Erico
> 
> On Wed, May 17, 2023 at 8:09 PM Erico Nunes <ernunes@redhat.com
> <mailto:ernunes@redhat.com>> wrote:
> 
>     On 15/05/2023 13:38, Marc-André Lureau wrote:
>     > However, I worry about using the new backend (calling GET_EDID)
>     with an
>     > older front-end/QEMU. It may just hang, since
>     > vhost_user_gpu_handle_display() won't reply to unknown messages.
>     That's
>     > what PROTOCOL_FEATURES were meant for, iirc. Can you check? thanks
> 
>     Indeed as you say, there is a hang with older qemu.
> 
>     From what I see there are the generic protocol_features and also a
>     vhost-user-gpu message for them. I assume it is so that we don't have to
>     add vhost-user-gpu specific features to the generic set?
>     In any case, the current vhost-user-gpu specific protocol_features
>     negotiation happens too late to enable or disable virtio-gpu features
>     (triggered by VHOST_USER_GPU_SET_SOCKET). I suppose we could move it
>     earlier to the time the generic protocol_features are negotiated,
>     through the callback hooks that already exist in the vhost-user layer
>     (not implemented so far by vhost-user-gpu though).
>     So I guess we could remove the protocol_features negotiation that is
>     currently triggered by VHOST_USER_GPU_SET_SOCKET and use that to prevent
>     exposing VIRTIO_GPU_F_EDID at all. Does that make sense?
> 
> 
> Wouldn't this work?
> 
> If VIRTIO_GPU_F_EDID is set and during protocol_features the GET_EDID
> feature is not negotiated, exit the gpu backend with an error.

I sent v2 implementing it this way.
It seems that qemu always requests the virtio feature though (even if
you do pass edid=off to the device, it just doesn't expose it to the
user but does request the feature bit). So given we can't change past
qemu builds I'm not sure if there is a way to make an older qemu with an
updated vhost-user-gpu work at all. The new implementation does print an
error message in that case though.

Thanks

Erico