Re: [RFC PATCH 08/13] gfxstream + rutabaga prep: added need defintions, fields, and options

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2023/04/21 10:12, Gurchetan Singh wrote:
> This modifies the common virtio-gpu.h file have the fields and
> defintions needed by gfxstream/rutabaga.  It also modifies VirtioGPUGL
> to have the runtime options needed by rutabaga.  They are:
> 
> - a colon separated list of capset names, defined in the virtio spec
> - a wayland socket path to enable guest Wayland passthrough
> 
> The command to run these would be:
> 
> -device virtio-vga-gl,capset_names=gfxstream:cross-domain, \
>          wayland_socket_path=/run/user/1000/wayland-0,hostmem=8G  \

It will be nice if it automatically determines the socket path according to:
https://wayland.freedesktop.org/docs/html/apb.html#Client-classwl__display_1af048371dfef7577bd39a3c04b78d0374

A documentation to set up cross-domain just like what you can find in 
the crosvm book* will also be helpful.
https://crosvm.dev/book/devices/wayland.html

> 
> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> ---
>   hw/display/virtio-gpu-gl.c     | 2 ++
>   include/hw/virtio/virtio-gpu.h | 8 ++++++++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index 547e697333..15270b0c8a 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -29,6 +29,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>   static Property virtio_gpu_gl_properties[] = {
>       DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
>                       VIRTIO_GPU_FLAG_STATS_ENABLED, false),
> +    DEFINE_PROP_STRING("capset_names", VirtIOGPUGL, capset_names),
> +    DEFINE_PROP_STRING("wayland_socket_path", VirtIOGPUGL, wayland_socket_path),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 421733d751..a35ade3608 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -94,6 +94,7 @@ enum virtio_gpu_base_conf_flags {
>       VIRTIO_GPU_FLAG_DMABUF_ENABLED,
>       VIRTIO_GPU_FLAG_BLOB_ENABLED,
>       VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
> +    VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
>   };
>   
>   #define virtio_gpu_virgl_enabled(_cfg) \
> @@ -106,6 +107,8 @@ enum virtio_gpu_base_conf_flags {
>       (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
>   #define virtio_gpu_blob_enabled(_cfg) \
>       (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
> +#define virtio_gpu_rutabaga_enabled(_cfg) \
> +    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
>   #define virtio_gpu_hostmem_enabled(_cfg) \
>       (_cfg.hostmem > 0)
>   #define virtio_gpu_context_init_enabled(_cfg) \
> @@ -217,6 +220,11 @@ struct VirtIOGPUGL {
>   
>       bool renderer_inited;
>       bool renderer_reset;
> +
> +    char *capset_names;
> +    char *wayland_socket_path;
> +    uint32_t num_capsets;
> +    void *rutabaga;

I prefer to have a line:
struct rutabaga;

In virtio-gpu.h and use it here. Perhaps it may be a bit weird to have 
such a declaration in a renderer-independent file, but it's practically 
harmless, can prevent something rouge from being assigned to the member, 
and allows to use the variable without annoying casts.

>   };
>   
>   struct VhostUserGPU {