Re: [RFC QEMU PATCH 04/18] virtio-gpu: CONTEXT_INIT feature - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: "Huang Rui" <ray.huang@amd.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Antonio Caggiano" <antonio.caggiano@collabora.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Robert Beckett" <bob.beckett@collabora.com>,
	qemu-devel@nongnu.org, xen-devel@lists.xenproject.org
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Stewart Hildebrand" <Stewart.Hildebrand@amd.com>,
	"Xenia Ragiadakou" <burzalodowa@gmail.com>,
	"Honglei Huang" <honglei1.huang@amd.com>,
	"Julia Zhang" <julia.zhang@amd.com>,
	"Chen Jiqian" <Jiqian.Chen@amd.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [RFC QEMU PATCH 04/18] virtio-gpu: CONTEXT_INIT feature
Date: Mon, 13 Mar 2023 10:06:16 +0100	[thread overview]
Message-ID: <32588d0e-a1f2-30c4-5e9f-e6e7c4190b65@linaro.org> (raw)
In-Reply-To: <20230312092244.451465-5-ray.huang@amd.com>

On 12/3/23 10:22, Huang Rui wrote:
> From: Antonio Caggiano <antonio.caggiano@collabora.com>
> 
> Create virgl renderer context with flags using context_id when valid.
> The feature can be enabled via the context_init config option.
> A warning message will be emitted and the feature will not be used
> when linking with virglrenderer versions without context_init support.
> 
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/display/virtio-gpu-base.c   |  3 +++
>   hw/display/virtio-gpu-virgl.c  | 16 ++++++++++++++--
>   hw/display/virtio-gpu.c        |  2 ++
>   include/hw/virtio/virtio-gpu.h |  3 +++
>   meson.build                    |  4 ++++
>   5 files changed, 26 insertions(+), 2 deletions(-)

> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9b5e3dc782..41712b79ee 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -99,8 +99,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
>       trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
>                                       cc.debug_name);
>   
> -    virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
> -                                  cc.debug_name);
> +    if (cc.context_init) {
> +#ifdef HAVE_VIRGL_CONTEXT_INIT
> +        virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
> +                                                 cc.context_init,
> +                                                 cc.nlen,
> +                                                 cc.debug_name);
> +        return;
> +#else
> +        qemu_log_mask(LOG_UNIMP,
> +                      "Linked virglrenderer does not support context-init\n");

This is dubious, almost nobody will notice this log.

Can this happen? If so, this void function doesn't create any context...

> +#endif
> +    }
> +
> +    virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
>   }