* [PATCH v11 01/10] virtio-gpu: Unrealize GL device
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-13 8:44 ` Akihiko Odaki
2024-05-11 18:22 ` [PATCH v11 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
Even though GL GPU doesn't support hotplugging today, free virgl
resources when GL device is unrealized. For consistency.
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-gl.c | 11 +++++++++++
hw/display/virtio-gpu-virgl.c | 9 +++++++++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfbfc..0c0a8d136954 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
+{
+ VirtIOGPU *g = VIRTIO_GPU(qdev);
+ VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
+
+ if (gl->renderer_inited) {
+ virtio_gpu_virgl_deinit(g);
+ }
+}
+
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
vdc->realize = virtio_gpu_gl_device_realize;
+ vdc->unrealize = virtio_gpu_gl_device_unrealize;
vdc->reset = virtio_gpu_gl_reset;
device_class_set_props(dc, virtio_gpu_gl_properties);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e6619c..b0500eccf8e0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
return capset2_max_ver ? 2 : 1;
}
+
+void virtio_gpu_virgl_deinit(VirtIOGPU *g)
+{
+ if (g->fence_poll) {
+ timer_free(g->fence_poll);
+ }
+
+ virgl_renderer_cleanup(NULL);
+}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ed44cdad6b34..b657187159d9 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -336,6 +336,7 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
+void virtio_gpu_virgl_deinit(VirtIOGPU *g);
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v11 01/10] virtio-gpu: Unrealize GL device
2024-05-11 18:22 ` [PATCH v11 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
@ 2024-05-13 8:44 ` Akihiko Odaki
2024-05-15 16:18 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-13 8:44 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/12 3:22, Dmitry Osipenko wrote:
> Even though GL GPU doesn't support hotplugging today, free virgl
> resources when GL device is unrealized. For consistency.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> hw/display/virtio-gpu-gl.c | 11 +++++++++++
> hw/display/virtio-gpu-virgl.c | 9 +++++++++
> include/hw/virtio/virtio-gpu.h | 1 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index e06be60dfbfc..0c0a8d136954 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
> +{
> + VirtIOGPU *g = VIRTIO_GPU(qdev);
> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
> +
> + if (gl->renderer_inited) {
> + virtio_gpu_virgl_deinit(g);
> + }
> +}
> +
> static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
> vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
>
> vdc->realize = virtio_gpu_gl_device_realize;
> + vdc->unrealize = virtio_gpu_gl_device_unrealize;
> vdc->reset = virtio_gpu_gl_reset;
> device_class_set_props(dc, virtio_gpu_gl_properties);
> }
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9f34d0e6619c..b0500eccf8e0 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>
> return capset2_max_ver ? 2 : 1;
> }
> +
> +void virtio_gpu_virgl_deinit(VirtIOGPU *g)
> +{
> + if (g->fence_poll) {
Isn't g->fence_poll always non-NULL when this function is called?
> + timer_free(g->fence_poll);
> + }
> +
> + virgl_renderer_cleanup(NULL);
> +}
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ed44cdad6b34..b657187159d9 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -336,6 +336,7 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
> void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
> void virtio_gpu_virgl_reset(VirtIOGPU *g);
> int virtio_gpu_virgl_init(VirtIOGPU *g);
> +void virtio_gpu_virgl_deinit(VirtIOGPU *g);
> int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
>
> #endif
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 01/10] virtio-gpu: Unrealize GL device
2024-05-13 8:44 ` Akihiko Odaki
@ 2024-05-15 16:18 ` Dmitry Osipenko
2024-05-15 16:22 ` Akihiko Odaki
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 16:18 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/13/24 11:44, Akihiko Odaki wrote:
> On 2024/05/12 3:22, Dmitry Osipenko wrote:
>> Even though GL GPU doesn't support hotplugging today, free virgl
>> resources when GL device is unrealized. For consistency.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>> hw/display/virtio-gpu-gl.c | 11 +++++++++++
>> hw/display/virtio-gpu-virgl.c | 9 +++++++++
>> include/hw/virtio/virtio-gpu.h | 1 +
>> 3 files changed, 21 insertions(+)
>>
>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>> index e06be60dfbfc..0c0a8d136954 100644
>> --- a/hw/display/virtio-gpu-gl.c
>> +++ b/hw/display/virtio-gpu-gl.c
>> @@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
>> DEFINE_PROP_END_OF_LIST(),
>> };
>> +static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>> +{
>> + VirtIOGPU *g = VIRTIO_GPU(qdev);
>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>> +
>> + if (gl->renderer_inited) {
>> + virtio_gpu_virgl_deinit(g);
>> + }
>> +}
>> +
>> static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass
>> *klass, void *data)
>> vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
>> vdc->realize = virtio_gpu_gl_device_realize;
>> + vdc->unrealize = virtio_gpu_gl_device_unrealize;
>> vdc->reset = virtio_gpu_gl_reset;
>> device_class_set_props(dc, virtio_gpu_gl_properties);
>> }
>> diff --git a/hw/display/virtio-gpu-virgl.c
>> b/hw/display/virtio-gpu-virgl.c
>> index 9f34d0e6619c..b0500eccf8e0 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>> return capset2_max_ver ? 2 : 1;
>> }
>> +
>> +void virtio_gpu_virgl_deinit(VirtIOGPU *g)
>> +{
>> + if (g->fence_poll) {
>
> Isn't g->fence_poll always non-NULL when this function is called?
virtio_gpu_virgl_init() is invoked when first cmd is executed, please
see virtio_gpu_gl_handle_ctrl() that invokes it. Hence g->fence_poll can
be NULL.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 01/10] virtio-gpu: Unrealize GL device
2024-05-15 16:18 ` Dmitry Osipenko
@ 2024-05-15 16:22 ` Akihiko Odaki
2024-05-15 16:47 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-15 16:22 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/16 1:18, Dmitry Osipenko wrote:
> On 5/13/24 11:44, Akihiko Odaki wrote:
>> On 2024/05/12 3:22, Dmitry Osipenko wrote:
>>> Even though GL GPU doesn't support hotplugging today, free virgl
>>> resources when GL device is unrealized. For consistency.
>>>
>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> ---
>>> hw/display/virtio-gpu-gl.c | 11 +++++++++++
>>> hw/display/virtio-gpu-virgl.c | 9 +++++++++
>>> include/hw/virtio/virtio-gpu.h | 1 +
>>> 3 files changed, 21 insertions(+)
>>>
>>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>>> index e06be60dfbfc..0c0a8d136954 100644
>>> --- a/hw/display/virtio-gpu-gl.c
>>> +++ b/hw/display/virtio-gpu-gl.c
>>> @@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>> +static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>> +{
>>> + VirtIOGPU *g = VIRTIO_GPU(qdev);
>>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>>> +
>>> + if (gl->renderer_inited) {
>>> + virtio_gpu_virgl_deinit(g);
>>> + }
>>> +}
>>> +
>>> static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
>>> {
>>> DeviceClass *dc = DEVICE_CLASS(klass);
>>> @@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass
>>> *klass, void *data)
>>> vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
>>> vdc->realize = virtio_gpu_gl_device_realize;
>>> + vdc->unrealize = virtio_gpu_gl_device_unrealize;
>>> vdc->reset = virtio_gpu_gl_reset;
>>> device_class_set_props(dc, virtio_gpu_gl_properties);
>>> }
>>> diff --git a/hw/display/virtio-gpu-virgl.c
>>> b/hw/display/virtio-gpu-virgl.c
>>> index 9f34d0e6619c..b0500eccf8e0 100644
>>> --- a/hw/display/virtio-gpu-virgl.c
>>> +++ b/hw/display/virtio-gpu-virgl.c
>>> @@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>>> return capset2_max_ver ? 2 : 1;
>>> }
>>> +
>>> +void virtio_gpu_virgl_deinit(VirtIOGPU *g)
>>> +{
>>> + if (g->fence_poll) {
>>
>> Isn't g->fence_poll always non-NULL when this function is called?
>
> virtio_gpu_virgl_init() is invoked when first cmd is executed, please
> see virtio_gpu_gl_handle_ctrl() that invokes it. Hence g->fence_poll can
> be NULL.
>
But it already checks renderer_inited, doesn't it? And I think it's
better to utilize one single flag to represent that virgl is enabled
instead of checking several variables (fence_poll and cmdq_resume_bh in
the future).
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 01/10] virtio-gpu: Unrealize GL device
2024-05-15 16:22 ` Akihiko Odaki
@ 2024-05-15 16:47 ` Dmitry Osipenko
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 16:47 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/15/24 19:22, Akihiko Odaki wrote:
> On 2024/05/16 1:18, Dmitry Osipenko wrote:
>> On 5/13/24 11:44, Akihiko Odaki wrote:
>>> On 2024/05/12 3:22, Dmitry Osipenko wrote:
>>>> Even though GL GPU doesn't support hotplugging today, free virgl
>>>> resources when GL device is unrealized. For consistency.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>>> ---
>>>> hw/display/virtio-gpu-gl.c | 11 +++++++++++
>>>> hw/display/virtio-gpu-virgl.c | 9 +++++++++
>>>> include/hw/virtio/virtio-gpu.h | 1 +
>>>> 3 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>>>> index e06be60dfbfc..0c0a8d136954 100644
>>>> --- a/hw/display/virtio-gpu-gl.c
>>>> +++ b/hw/display/virtio-gpu-gl.c
>>>> @@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
>>>> DEFINE_PROP_END_OF_LIST(),
>>>> };
>>>> +static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>>> +{
>>>> + VirtIOGPU *g = VIRTIO_GPU(qdev);
>>>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>>>> +
>>>> + if (gl->renderer_inited) {
>>>> + virtio_gpu_virgl_deinit(g);
>>>> + }
>>>> +}
>>>> +
>>>> static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
>>>> {
>>>> DeviceClass *dc = DEVICE_CLASS(klass);
>>>> @@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass
>>>> *klass, void *data)
>>>> vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
>>>> vdc->realize = virtio_gpu_gl_device_realize;
>>>> + vdc->unrealize = virtio_gpu_gl_device_unrealize;
>>>> vdc->reset = virtio_gpu_gl_reset;
>>>> device_class_set_props(dc, virtio_gpu_gl_properties);
>>>> }
>>>> diff --git a/hw/display/virtio-gpu-virgl.c
>>>> b/hw/display/virtio-gpu-virgl.c
>>>> index 9f34d0e6619c..b0500eccf8e0 100644
>>>> --- a/hw/display/virtio-gpu-virgl.c
>>>> +++ b/hw/display/virtio-gpu-virgl.c
>>>> @@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>>>> return capset2_max_ver ? 2 : 1;
>>>> }
>>>> +
>>>> +void virtio_gpu_virgl_deinit(VirtIOGPU *g)
>>>> +{
>>>> + if (g->fence_poll) {
>>>
>>> Isn't g->fence_poll always non-NULL when this function is called?
>>
>> virtio_gpu_virgl_init() is invoked when first cmd is executed, please
>> see virtio_gpu_gl_handle_ctrl() that invokes it. Hence g->fence_poll can
>> be NULL.
>>
>
> But it already checks renderer_inited, doesn't it? And I think it's
> better to utilize one single flag to represent that virgl is enabled
> instead of checking several variables (fence_poll and cmdq_resume_bh in
> the future).
The virtio_gpu_virgl_init() will have to be changed to do that because
virgl_renderer_init() can fail before fence_poll/cmdq_resume_bh are inited.
Though, the error returned by virtio_gpu_virgl_init() isn't checked by
virtio_gpu_gl_handle_ctrl(), which leads to a further Qemu crash due to
fence_poll=NULL. I'll try to improve it all in v12, thanks.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v11 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
New virglrerenderer features were stabilized with release of v1.0.0.
Presence of symbols in virglrenderer.h doesn't guarantee ABI compatibility
with pre-release development versions of libvirglerender. Use virglrenderer
version to decide reliably which virgl features are available.
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
meson.build | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build
index 83ae4347c7f9..ca2798dbac37 100644
--- a/meson.build
+++ b/meson.build
@@ -2286,11 +2286,8 @@ config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
-if virgl.found()
- config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT',
- cc.has_member('struct virgl_renderer_resource_info_ext', 'd3d_tex2d',
- prefix: '#include <virglrenderer.h>',
- dependencies: virgl))
+if virgl.version().version_compare('>=1.0.0')
+ config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 03/10] virtio-gpu: Support context-init feature with virglrenderer
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled Dmitry Osipenko
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Huang Rui <ray.huang@amd.com>
Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init
feature flags. Expose this feature and support creating virglrenderer
context with flags using context_id if libvirglrenderer is new enough.
Originally-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-gl.c | 4 ++++
hw/display/virtio-gpu-virgl.c | 20 ++++++++++++++++++--
meson.build | 1 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 0c0a8d136954..95806999189e 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -127,6 +127,10 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
virtio_gpu_virgl_get_num_capsets(g);
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+ g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
+#endif
+
virtio_gpu_device_realize(qdev, errp);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b0500eccf8e0..8306961ad502 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -106,8 +106,24 @@ 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) {
+ if (!virtio_gpu_context_init_enabled(g->parent_obj.conf)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: context_init disabled",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+ virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+ return;
+#endif
+ }
+
+ virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
}
static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/meson.build b/meson.build
index ca2798dbac37..5448eb17f39b 100644
--- a/meson.build
+++ b/meson.build
@@ -2288,6 +2288,7 @@ config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
+ config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (2 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
The udmabuf usage is mandatory when virgl is disabled and blobs feature
enabled in the Qemu machine configuration. If virgl and blobs are enabled,
then udmabuf requirement is optional. Since udmabuf isn't widely supported
by a popular Linux distros today, let's relax the udmabuf requirement for
blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is
available to Qemu users without a need to have udmabuf available in the
system.
Reviewed-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ae831b6b3e3e..dac272ecadb1 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1472,6 +1472,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
+ !virtio_gpu_virgl_enabled(g->parent_obj.conf) &&
!virtio_gpu_have_udmabuf()) {
error_setg(errp, "need rutabaga or udmabuf for blob resources");
return;
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 05/10] virtio-gpu: Add virgl resource management
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (3 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Huang Rui <ray.huang@amd.com>
In a preparation to adding host blobs support to virtio-gpu, add virgl
resource management that allows to retrieve resource based on its ID
and virgl resource wrapper on top of simple resource that will be contain
fields specific to virgl.
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-virgl.c | 76 +++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 8306961ad502..c6c63ca1c373 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -22,6 +22,23 @@
#include <virglrenderer.h>
+struct virtio_gpu_virgl_resource {
+ struct virtio_gpu_simple_resource base;
+};
+
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
+{
+ struct virtio_gpu_simple_resource *res;
+
+ res = virtio_gpu_find_resource(g, resource_id);
+ if (!res) {
+ return NULL;
+ }
+
+ return container_of(res, struct virtio_gpu_virgl_resource, base);
+}
+
#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
static void *
virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
@@ -35,11 +52,34 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_2d c2d;
struct virgl_renderer_resource_create_args args;
+ struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c2d);
trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
c2d.width, c2d.height);
+ if (c2d.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_virgl_find_resource(g, c2d.resource_id);
+ if (res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c2d.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = g_new0(struct virtio_gpu_virgl_resource, 1);
+ res->base.width = c2d.width;
+ res->base.height = c2d.height;
+ res->base.format = c2d.format;
+ res->base.resource_id = c2d.resource_id;
+ QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c2d.resource_id;
args.target = 2;
args.format = c2d.format;
@@ -59,11 +99,34 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_3d c3d;
struct virgl_renderer_resource_create_args args;
+ struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c3d);
trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
c3d.width, c3d.height, c3d.depth);
+ if (c3d.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_virgl_find_resource(g, c3d.resource_id);
+ if (res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c3d.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = g_new0(struct virtio_gpu_virgl_resource, 1);
+ res->base.width = c3d.width;
+ res->base.height = c3d.height;
+ res->base.format = c3d.format;
+ res->base.resource_id = c3d.resource_id;
+ QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c3d.resource_id;
args.target = c3d.target;
args.format = c3d.format;
@@ -82,12 +145,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
struct virtio_gpu_resource_unref unref;
+ struct virtio_gpu_virgl_resource *res;
struct iovec *res_iovs = NULL;
int num_iovs = 0;
VIRTIO_GPU_FILL_CMD(unref);
trace_virtio_gpu_cmd_res_unref(unref.resource_id);
+ res = virtio_gpu_virgl_find_resource(g, unref.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, unref.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -95,6 +167,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
}
virgl_renderer_resource_unref(unref.resource_id);
+
+ QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+ g_free(res);
}
static void virgl_cmd_context_create(VirtIOGPU *g,
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 06/10] virtio-gpu: Support blob scanout using dmabuf fd
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (4 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 07/10] virtio-gpu: Support suspension of commands processing Dmitry Osipenko
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Robert Beckett <bob.beckett@collabora.com>
Support displaying blob resources by handling SET_SCANOUT_BLOB
command.
Signed-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-virgl.c | 109 +++++++++++++++++++++++++++++++++
hw/display/virtio-gpu.c | 12 ++--
include/hw/virtio/virtio-gpu.h | 7 +++
meson.build | 1 +
4 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index c6c63ca1c373..524f15220b7f 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,8 @@
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
#include "ui/egl-helpers.h"
@@ -78,6 +80,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
res->base.height = c2d.height;
res->base.format = c2d.format;
res->base.resource_id = c2d.resource_id;
+ res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -125,6 +128,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
res->base.height = c3d.height;
res->base.format = c3d.format;
res->base.resource_id = c3d.resource_id;
+ res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -509,6 +513,106 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
g_free(resp);
}
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_framebuffer fb = { 0 };
+ struct virgl_renderer_resource_info info;
+ struct virtio_gpu_virgl_resource *res;
+ struct virtio_gpu_set_scanout_blob ss;
+ uint64_t fbend;
+
+ VIRTIO_GPU_FILL_CMD(ss);
+ virtio_gpu_scanout_blob_bswap(&ss);
+ trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+ ss.r.width, ss.r.height, ss.r.x,
+ ss.r.y);
+
+ if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+ __func__, ss.scanout_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+ return;
+ }
+
+ if (ss.resource_id == 0) {
+ virtio_gpu_disable_scanout(g, ss.scanout_id);
+ return;
+ }
+
+ if (ss.width < 16 ||
+ ss.height < 16 ||
+ ss.r.x + ss.r.width > ss.width ||
+ ss.r.y + ss.r.height > ss.height) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
+ " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+ __func__, ss.scanout_id, ss.resource_id,
+ ss.r.x, ss.r.y, ss.r.width, ss.r.height,
+ ss.width, ss.height);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
+
+ res = virtio_gpu_virgl_find_resource(g, ss.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+ if (virgl_renderer_resource_get_info(ss.resource_id, &info)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not have info %d\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+ if (res->base.dmabuf_fd < 0) {
+ res->base.dmabuf_fd = info.fd;
+ }
+ if (res->base.dmabuf_fd < 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource not backed by dmabuf %d\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ fb.format = virtio_gpu_get_pixman_format(ss.format);
+ if (!fb.format) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n",
+ __func__, ss.format);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
+
+ fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+ fb.width = ss.width;
+ fb.height = ss.height;
+ fb.stride = ss.strides[0];
+ fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+
+ fbend = fb.offset;
+ fbend += fb.stride * (ss.r.height - 1);
+ fbend += fb.bytes_pp * ss.r.width;
+ if (fbend > res->base.blob_size) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
+
+ g->parent_obj.enable = 1;
+ if (virtio_gpu_update_dmabuf(g, ss.scanout_id, &res->base, &fb, &ss.r)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to update dmabuf\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
+
+ virtio_gpu_update_scanout(g, ss.scanout_id, &res->base, &fb, &ss.r);
+}
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
+
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -575,6 +679,11 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
case VIRTIO_GPU_CMD_GET_EDID:
virtio_gpu_get_edid(g, cmd);
break;
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+ case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
+ virgl_cmd_set_scanout_blob(g, cmd);
+ break;
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
default:
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index dac272ecadb1..1e57a53d346c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -380,7 +380,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
-static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
+void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
{
struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
struct virtio_gpu_simple_resource *res;
@@ -597,11 +597,11 @@ static void virtio_unref_resource(pixman_image_t *image, void *data)
pixman_image_unref(data);
}
-static void virtio_gpu_update_scanout(VirtIOGPU *g,
- uint32_t scanout_id,
- struct virtio_gpu_simple_resource *res,
- struct virtio_gpu_framebuffer *fb,
- struct virtio_gpu_rect *r)
+void virtio_gpu_update_scanout(VirtIOGPU *g,
+ uint32_t scanout_id,
+ struct virtio_gpu_simple_resource *res,
+ struct virtio_gpu_framebuffer *fb,
+ struct virtio_gpu_rect *r)
{
struct virtio_gpu_simple_resource *ores;
struct virtio_gpu_scanout *scanout;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index b657187159d9..ba36497c477f 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -329,6 +329,13 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
struct virtio_gpu_framebuffer *fb,
struct virtio_gpu_rect *r);
+void virtio_gpu_update_scanout(VirtIOGPU *g,
+ uint32_t scanout_id,
+ struct virtio_gpu_simple_resource *res,
+ struct virtio_gpu_framebuffer *fb,
+ struct virtio_gpu_rect *r);
+void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id);
+
/* virtio-gpu-3d.c */
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd);
diff --git a/meson.build b/meson.build
index 5448eb17f39b..0433ea200dd7 100644
--- a/meson.build
+++ b/meson.build
@@ -2289,6 +2289,7 @@ config_host_data.set('CONFIG_VNC_SASL', sasl.found())
if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
+ config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 07/10] virtio-gpu: Support suspension of commands processing
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (5 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
` (2 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
Check whether command processing has been finished; otherwise, stop
processing commands and retry the command again next time. This allows
us to support asynchronous execution of non-fenced commands needed for
unmapping host blobs safely.
Suggested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1e57a53d346c..6c8c7213bafa 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,11 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
/* process command */
vgc->process_cmd(g, cmd);
+ /* command suspended */
+ if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
+ break;
+ }
+
QTAILQ_REMOVE(&g->cmdq, cmd, next);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
g->stats.requests++;
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (6 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 07/10] virtio-gpu: Support suspension of commands processing Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-13 9:18 ` Akihiko Odaki
2024-05-11 18:22 ` [PATCH v11 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
2024-05-11 18:22 ` [PATCH v11 10/10] virtio-gpu: Support Venus context Dmitry Osipenko
9 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Antonio Caggiano <antonio.caggiano@collabora.com>
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-virgl.c | 274 ++++++++++++++++++++++++++++++++-
hw/display/virtio-gpu.c | 4 +-
include/hw/virtio/virtio-gpu.h | 2 +
3 files changed, 277 insertions(+), 3 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 524f15220b7f..3f2e406be3a4 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,7 @@
struct virtio_gpu_virgl_resource {
struct virtio_gpu_simple_resource base;
+ MemoryRegion *mr;
};
static struct virtio_gpu_virgl_resource *
@@ -49,6 +50,117 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
}
#endif
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+struct virtio_gpu_virgl_hostmem_region {
+ MemoryRegion mr;
+ struct VirtIOGPU *g;
+ struct virtio_gpu_virgl_resource *res;
+};
+
+static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
+{
+ VirtIOGPU *g = opaque;
+
+ virtio_gpu_process_cmdq(g);
+}
+
+static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+{
+ MemoryRegion *mr = MEMORY_REGION(obj);
+ struct virtio_gpu_virgl_hostmem_region *vmr;
+ VirtIOGPUBase *b;
+
+ vmr = container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+ vmr->res->mr = NULL;
+
+ b = VIRTIO_GPU_BASE(vmr->g);
+ b->renderer_blocked--;
+
+ /*
+ * memory_region_unref() is executed from RCU thread context, while
+ * virglrenderer works only on the main-loop thread that's holding GL
+ * context.
+ */
+ qemu_bh_schedule(vmr->g->cmdq_resume_bh);
+ g_free(vmr);
+}
+
+static int
+virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res,
+ uint64_t offset)
+{
+ struct virtio_gpu_virgl_hostmem_region *vmr;
+ VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+ MemoryRegion *mr;
+ uint64_t size;
+ void *data;
+ int ret;
+
+ if (!virtio_gpu_hostmem_enabled(b->conf)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
+ return -EOPNOTSUPP;
+ }
+
+ ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n",
+ __func__, strerror(-ret));
+ return ret;
+ }
+
+ vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+ vmr->res = res;
+ vmr->g = g;
+
+ mr = &vmr->mr;
+ memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+ memory_region_add_subregion(&b->hostmem, offset, mr);
+ memory_region_set_enabled(mr, true);
+
+ /*
+ * MR could outlive the resource if MR's reference is held outside of
+ * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+ * and thus, making the data pointer invalid, we will block virtio-gpu
+ * command processing until MR is fully unreferenced and freed.
+ */
+ OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+ res->mr = mr;
+
+ return 0;
+}
+
+static int
+virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res)
+{
+ VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+ MemoryRegion *mr = res->mr;
+ int ret;
+
+ if (mr) {
+ /* render will be unblocked once MR is freed */
+ b->renderer_blocked++;
+
+ /* memory region owns self res->mr object and frees it by itself */
+ memory_region_set_enabled(mr, false);
+ memory_region_del_subregion(&b->hostmem, mr);
+ object_unparent(OBJECT(mr));
+ } else {
+ ret = virgl_renderer_resource_unmap(res->base.resource_id);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: failed to unmap virgl resource: %s\n",
+ __func__, strerror(-ret));
+ return ret;
+ }
+ }
+
+ return 0;
+}
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
+
static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -146,7 +258,8 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
}
static void virgl_cmd_resource_unref(VirtIOGPU *g,
- struct virtio_gpu_ctrl_command *cmd)
+ struct virtio_gpu_ctrl_command *cmd,
+ bool *cmd_suspended)
{
struct virtio_gpu_resource_unref unref;
struct virtio_gpu_virgl_resource *res;
@@ -164,6 +277,12 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
return;
}
+ if (res->mr) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource is mapped\n", __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+
virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -514,6 +633,134 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
}
#ifdef HAVE_VIRGL_RESOURCE_BLOB
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virgl_renderer_resource_create_blob_args virgl_args = { 0 };
+ struct virtio_gpu_resource_create_blob cblob;
+ struct virtio_gpu_virgl_resource *res;
+ int ret;
+
+ if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
+
+ VIRTIO_GPU_FILL_CMD(cblob);
+ virtio_gpu_create_blob_bswap(&cblob);
+ trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+ if (cblob.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_virgl_find_resource(g, cblob.resource_id);
+ if (res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, cblob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = g_new0(struct virtio_gpu_virgl_resource, 1);
+ res->base.resource_id = cblob.resource_id;
+ res->base.blob_size = cblob.size;
+ res->base.dmabuf_fd = -1;
+
+ if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+ cmd, &res->base.addrs,
+ &res->base.iov, &res->base.iov_cnt);
+ if (!ret) {
+ g_free(res);
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+ }
+
+ QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
+ virgl_args.res_handle = cblob.resource_id;
+ virgl_args.ctx_id = cblob.hdr.ctx_id;
+ virgl_args.blob_mem = cblob.blob_mem;
+ virgl_args.blob_id = cblob.blob_id;
+ virgl_args.blob_flags = cblob.blob_flags;
+ virgl_args.size = cblob.size;
+ virgl_args.iovecs = res->base.iov;
+ virgl_args.num_iovs = res->base.iov_cnt;
+
+ ret = virgl_renderer_resource_create_blob(&virgl_args);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
+ __func__, strerror(-ret));
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ }
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_resource_map_blob mblob;
+ struct virtio_gpu_virgl_resource *res;
+ struct virtio_gpu_resp_map_info resp;
+ int ret;
+
+ VIRTIO_GPU_FILL_CMD(mblob);
+ virtio_gpu_map_blob_bswap(&mblob);
+
+ res = virtio_gpu_virgl_find_resource(g, mblob.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, mblob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ ret = virtio_gpu_virgl_map_resource_blob(g, res, mblob.offset);
+ if (ret) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+
+ memset(&resp, 0, sizeof(resp));
+ resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
+ virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info);
+ virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
+}
+
+static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd,
+ bool *cmd_suspended)
+{
+ struct virtio_gpu_resource_unmap_blob ublob;
+ struct virtio_gpu_virgl_resource *res;
+ int ret;
+
+ VIRTIO_GPU_FILL_CMD(ublob);
+ virtio_gpu_unmap_blob_bswap(&ublob);
+
+ res = virtio_gpu_virgl_find_resource(g, ublob.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ublob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ ret = virtio_gpu_virgl_async_unmap_resource_blob(g, res);
+ if (ret) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+
+ if (res->mr) {
+ *cmd_suspended = true;
+ }
+}
+
static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -616,6 +863,8 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
+ bool cmd_suspended = false;
+
VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
virgl_renderer_force_ctx_0();
@@ -657,7 +906,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
virgl_cmd_resource_flush(g, cmd);
break;
case VIRTIO_GPU_CMD_RESOURCE_UNREF:
- virgl_cmd_resource_unref(g, cmd);
+ virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
break;
case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
/* TODO add security */
@@ -680,6 +929,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
virtio_gpu_get_edid(g, cmd);
break;
#ifdef HAVE_VIRGL_RESOURCE_BLOB
+ case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
+ virgl_cmd_resource_create_blob(g, cmd);
+ break;
+ case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
+ virgl_cmd_resource_map_blob(g, cmd);
+ break;
+ case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
+ virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
+ break;
case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
virgl_cmd_set_scanout_blob(g, cmd);
break;
@@ -689,6 +947,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
break;
}
+ if (cmd_suspended) {
+ return;
+ }
if (cmd->finished) {
return;
}
@@ -854,6 +1115,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
virtio_gpu_print_stats, g);
timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
+
+ g->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
+ virtio_gpu_virgl_resume_cmdq_bh,
+ g);
+
return 0;
}
@@ -869,6 +1135,10 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
void virtio_gpu_virgl_deinit(VirtIOGPU *g)
{
+ if (g->cmdq_resume_bh) {
+ qemu_bh_delete(g->cmdq_resume_bh);
+ }
+
if (g->fence_poll) {
timer_free(g->fence_poll);
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6c8c7213bafa..052ab493a00b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1483,10 +1483,12 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
return;
}
+#ifndef HAVE_VIRGL_RESOURCE_BLOB
if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
- error_setg(errp, "blobs and virgl are not compatible (yet)");
+ error_setg(errp, "old virglrenderer, blob resources unsupported");
return;
}
+#endif
}
if (!virtio_gpu_base_device_realize(qdev,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ba36497c477f..c0b0b0eac08b 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -210,6 +210,8 @@ struct VirtIOGPU {
QTAILQ_HEAD(, VGPUDMABuf) bufs;
VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
} dmabuf;
+
+ QEMUBH *cmdq_resume_bh;
};
struct VirtIOGPUClass {
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-11 18:22 ` [PATCH v11 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
@ 2024-05-13 9:18 ` Akihiko Odaki
2024-05-15 16:39 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-13 9:18 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/12 3:22, Dmitry Osipenko wrote:
> From: Antonio Caggiano <antonio.caggiano@collabora.com>
>
> Support BLOB resources creation, mapping and unmapping by calling the
> new stable virglrenderer 0.10 interface. Only enabled when available and
> via the blob config. E.g. -device virtio-vga-gl,blob=true
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> hw/display/virtio-gpu-virgl.c | 274 ++++++++++++++++++++++++++++++++-
> hw/display/virtio-gpu.c | 4 +-
> include/hw/virtio/virtio-gpu.h | 2 +
> 3 files changed, 277 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 524f15220b7f..3f2e406be3a4 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -26,6 +26,7 @@
>
> struct virtio_gpu_virgl_resource {
> struct virtio_gpu_simple_resource base;
> + MemoryRegion *mr;
> };
>
> static struct virtio_gpu_virgl_resource *
> @@ -49,6 +50,117 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
> }
> #endif
>
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> +struct virtio_gpu_virgl_hostmem_region {
> + MemoryRegion mr;
> + struct VirtIOGPU *g;
> + struct virtio_gpu_virgl_resource *res;
> +};
> +
> +static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
> +{
> + VirtIOGPU *g = opaque;
> +
> + virtio_gpu_process_cmdq(g);
> +}
> +
> +static void virtio_gpu_virgl_hostmem_region_free(void *obj)
> +{
> + MemoryRegion *mr = MEMORY_REGION(obj);
> + struct virtio_gpu_virgl_hostmem_region *vmr;
> + VirtIOGPUBase *b;
> +
> + vmr = container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
> + vmr->res->mr = NULL;
> +
> + b = VIRTIO_GPU_BASE(vmr->g);
> + b->renderer_blocked--;
> +
> + /*
> + * memory_region_unref() is executed from RCU thread context, while
> + * virglrenderer works only on the main-loop thread that's holding GL
> + * context.
> + */
> + qemu_bh_schedule(vmr->g->cmdq_resume_bh);
> + g_free(vmr);
> +}
> +
> +static int
> +virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
> + struct virtio_gpu_virgl_resource *res,
> + uint64_t offset)
> +{
> + struct virtio_gpu_virgl_hostmem_region *vmr;
> + VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> + MemoryRegion *mr;
> + uint64_t size;
> + void *data;
> + int ret;
> +
> + if (!virtio_gpu_hostmem_enabled(b->conf)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
> + return -EOPNOTSUPP;
> + }
> +
> + ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
> + if (ret) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n",
> + __func__, strerror(-ret));
> + return ret;
> + }
> +
> + vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
> + vmr->res = res;
> + vmr->g = g;
> +
> + mr = &vmr->mr;
> + memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
> + memory_region_add_subregion(&b->hostmem, offset, mr);
> + memory_region_set_enabled(mr, true);
> +
> + /*
> + * MR could outlive the resource if MR's reference is held outside of
> + * virtio-gpu. In order to prevent unmapping resource while MR is alive,
> + * and thus, making the data pointer invalid, we will block virtio-gpu
> + * command processing until MR is fully unreferenced and freed.
> + */
> + OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
> +
> + res->mr = mr;
> +
> + return 0;
> +}
> +
> +static int
> +virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
> + struct virtio_gpu_virgl_resource *res)
> +{
> + VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> + MemoryRegion *mr = res->mr;
> + int ret;
> +
> + if (mr) {
> + /* render will be unblocked once MR is freed */
> + b->renderer_blocked++;
> +
> + /* memory region owns self res->mr object and frees it by itself */
> + memory_region_set_enabled(mr, false);
> + memory_region_del_subregion(&b->hostmem, mr);
> + object_unparent(OBJECT(mr));
> + } else {
> + ret = virgl_renderer_resource_unmap(res->base.resource_id);
> + if (ret) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: failed to unmap virgl resource: %s\n",
> + __func__, strerror(-ret));
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> +
> static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -146,7 +258,8 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
> }
>
> static void virgl_cmd_resource_unref(VirtIOGPU *g,
> - struct virtio_gpu_ctrl_command *cmd)
> + struct virtio_gpu_ctrl_command *cmd,
> + bool *cmd_suspended)
This parameter is not used as it returns an error if the resource is
still mapped.
It may be better to actually implement unmapping instead of returning an
error for consistency with the iov operation. Apparently crosvm also
unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
> {
> struct virtio_gpu_resource_unref unref;
> struct virtio_gpu_virgl_resource *res;
> @@ -164,6 +277,12 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
> return;
> }
>
> + if (res->mr) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource is mapped\n", __func__);
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + return;
> + }
> +
> virgl_renderer_resource_detach_iov(unref.resource_id,
> &res_iovs,
> &num_iovs);
> @@ -514,6 +633,134 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
> }
>
> #ifdef HAVE_VIRGL_RESOURCE_BLOB
> +static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virgl_renderer_resource_create_blob_args virgl_args = { 0 };
> + struct virtio_gpu_resource_create_blob cblob;
> + struct virtio_gpu_virgl_resource *res;
> + int ret;
> +
> + if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) {
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> + return;
> + }
> +
> + VIRTIO_GPU_FILL_CMD(cblob);
> + virtio_gpu_create_blob_bswap(&cblob);
> + trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
> +
> + if (cblob.resource_id == 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = virtio_gpu_virgl_find_resource(g, cblob.resource_id);
> + if (res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
> + __func__, cblob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = g_new0(struct virtio_gpu_virgl_resource, 1);
> + res->base.resource_id = cblob.resource_id;
> + res->base.blob_size = cblob.size;
> + res->base.dmabuf_fd = -1;
> +
> + if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
> + ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
> + cmd, &res->base.addrs,
> + &res->base.iov, &res->base.iov_cnt);
> + if (!ret) {
> + g_free(res);
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + return;
> + }
> + }
> +
> + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
> +
> + virgl_args.res_handle = cblob.resource_id;
> + virgl_args.ctx_id = cblob.hdr.ctx_id;
> + virgl_args.blob_mem = cblob.blob_mem;
> + virgl_args.blob_id = cblob.blob_id;
> + virgl_args.blob_flags = cblob.blob_flags;
> + virgl_args.size = cblob.size;
> + virgl_args.iovecs = res->base.iov;
> + virgl_args.num_iovs = res->base.iov_cnt;
> +
> + ret = virgl_renderer_resource_create_blob(&virgl_args);
> + if (ret) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
> + __func__, strerror(-ret));
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
reslist keeps the stale res even if an error happens.
> + }
> +}
> +
> +static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_resource_map_blob mblob;
> + struct virtio_gpu_virgl_resource *res;
> + struct virtio_gpu_resp_map_info resp;
> + int ret;
> +
> + VIRTIO_GPU_FILL_CMD(mblob);
> + virtio_gpu_map_blob_bswap(&mblob);
> +
> + res = virtio_gpu_virgl_find_resource(g, mblob.resource_id);
> + if (!res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, mblob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + ret = virtio_gpu_virgl_map_resource_blob(g, res, mblob.offset);
> + if (ret) {
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + return;
> + }
> +
> + memset(&resp, 0, sizeof(resp));
> + resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
> + virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info);
> + virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> +}
> +
> +static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd,
> + bool *cmd_suspended)
> +{
> + struct virtio_gpu_resource_unmap_blob ublob;
> + struct virtio_gpu_virgl_resource *res;
> + int ret;
> +
> + VIRTIO_GPU_FILL_CMD(ublob);
> + virtio_gpu_unmap_blob_bswap(&ublob);
> +
> + res = virtio_gpu_virgl_find_resource(g, ublob.resource_id);
> + if (!res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, ublob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + ret = virtio_gpu_virgl_async_unmap_resource_blob(g, res);
> + if (ret) {
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + return;
> + }
> +
> + if (res->mr) {
> + *cmd_suspended = true;
> + }
> +}
> +
> static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -616,6 +863,8 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
> void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> + bool cmd_suspended = false;
> +
> VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>
> virgl_renderer_force_ctx_0();
> @@ -657,7 +906,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> virgl_cmd_resource_flush(g, cmd);
> break;
> case VIRTIO_GPU_CMD_RESOURCE_UNREF:
> - virgl_cmd_resource_unref(g, cmd);
> + virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
> break;
> case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
> /* TODO add security */
> @@ -680,6 +929,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> virtio_gpu_get_edid(g, cmd);
> break;
> #ifdef HAVE_VIRGL_RESOURCE_BLOB
> + case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
> + virgl_cmd_resource_create_blob(g, cmd);
> + break;
> + case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
> + virgl_cmd_resource_map_blob(g, cmd);
> + break;
> + case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> + virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
> + break;
> case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
> virgl_cmd_set_scanout_blob(g, cmd);
> break;
> @@ -689,6 +947,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> break;
> }
>
> + if (cmd_suspended) {
> + return;
> + }
> if (cmd->finished) {
> return;
> }
Nitpick: I would write cmd_suspended || cmd->finished to save a few lines.
> @@ -854,6 +1115,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> virtio_gpu_print_stats, g);
> timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
> }
> +
> + g->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
> + virtio_gpu_virgl_resume_cmdq_bh,
> + g);
> +
> return 0;
> }
>
> @@ -869,6 +1135,10 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>
> void virtio_gpu_virgl_deinit(VirtIOGPU *g)
> {
> + if (g->cmdq_resume_bh) {
> + qemu_bh_delete(g->cmdq_resume_bh);
> + }
> +
> if (g->fence_poll) {
> timer_free(g->fence_poll);
> }
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 6c8c7213bafa..052ab493a00b 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1483,10 +1483,12 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> return;
> }
>
> +#ifndef HAVE_VIRGL_RESOURCE_BLOB
> if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
> - error_setg(errp, "blobs and virgl are not compatible (yet)");
> + error_setg(errp, "old virglrenderer, blob resources unsupported");
> return;
> }
> +#endif
> }
>
> if (!virtio_gpu_base_device_realize(qdev,
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ba36497c477f..c0b0b0eac08b 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -210,6 +210,8 @@ struct VirtIOGPU {
> QTAILQ_HEAD(, VGPUDMABuf) bufs;
> VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
> } dmabuf;
> +
> + QEMUBH *cmdq_resume_bh;
Move this to VirtIOGPUGL.
> };
>
> struct VirtIOGPUClass {
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-13 9:18 ` Akihiko Odaki
@ 2024-05-15 16:39 ` Dmitry Osipenko
2024-05-15 16:42 ` Akihiko Odaki
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 16:39 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/13/24 12:18, Akihiko Odaki wrote:
>> static void virgl_cmd_resource_unref(VirtIOGPU *g,
>> - struct virtio_gpu_ctrl_command
>> *cmd)
>> + struct virtio_gpu_ctrl_command
>> *cmd,
>> + bool *cmd_suspended)
>
> This parameter is not used as it returns an error if the resource is
> still mapped.
Missed to remove it by accident
> It may be better to actually implement unmapping instead of returning an
> error for consistency with the iov operation. Apparently crosvm also
> unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
Then I'll add back `async_unmap_in_progress` because resource can be
both mapped/unmapped on unref, and we'll need flag to know whether async
unmapping has been finished to do the final unmapping of the resource.
...
>> + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
>> +
>> + virgl_args.res_handle = cblob.resource_id;
>> + virgl_args.ctx_id = cblob.hdr.ctx_id;
>> + virgl_args.blob_mem = cblob.blob_mem;
>> + virgl_args.blob_id = cblob.blob_id;
>> + virgl_args.blob_flags = cblob.blob_flags;
>> + virgl_args.size = cblob.size;
>> + virgl_args.iovecs = res->base.iov;
>> + virgl_args.num_iovs = res->base.iov_cnt;
>> +
>> + ret = virgl_renderer_resource_create_blob(&virgl_args);
>> + if (ret) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error:
>> %s\n",
>> + __func__, strerror(-ret));
>> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>
> reslist keeps the stale res even if an error happens.
Good catch
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-15 16:39 ` Dmitry Osipenko
@ 2024-05-15 16:42 ` Akihiko Odaki
2024-05-15 17:01 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-15 16:42 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/16 1:39, Dmitry Osipenko wrote:
> On 5/13/24 12:18, Akihiko Odaki wrote:
>>> static void virgl_cmd_resource_unref(VirtIOGPU *g,
>>> - struct virtio_gpu_ctrl_command
>>> *cmd)
>>> + struct virtio_gpu_ctrl_command
>>> *cmd,
>>> + bool *cmd_suspended)
>>
>> This parameter is not used as it returns an error if the resource is
>> still mapped.
>
> Missed to remove it by accident
>
>> It may be better to actually implement unmapping instead of returning an
>> error for consistency with the iov operation. Apparently crosvm also
>> unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
>
> Then I'll add back `async_unmap_in_progress` because resource can be
> both mapped/unmapped on unref, and we'll need flag to know whether async
> unmapping has been finished to do the final unmapping of the resource.
Such a situation should be already handled since unmapping in progress
blocks all commands (not just VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB but
literally all, including VIRTIO_GPU_CMD_RESOURCE_UNREF).
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-15 16:42 ` Akihiko Odaki
@ 2024-05-15 17:01 ` Dmitry Osipenko
2024-05-15 17:04 ` Akihiko Odaki
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 17:01 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/15/24 19:42, Akihiko Odaki wrote:
>>> It may be better to actually implement unmapping instead of returning an
>>> error for consistency with the iov operation. Apparently crosvm also
>>> unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
>>
>> Then I'll add back `async_unmap_in_progress` because resource can be
>> both mapped/unmapped on unref, and we'll need flag to know whether async
>> unmapping has been finished to do the final unmapping of the resource.
>
> Such a situation should be already handled since unmapping in progress
> blocks all commands (not just VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB but
> literally all, including VIRTIO_GPU_CMD_RESOURCE_UNREF).
The async unmapping consists of 3 parts:
1. begin async unmapping with memory_region_del_subregion() and suspend
2. wait for res->mr to be freed and resume
3. finish the unmapping with final virgl_renderer_resource_unmap()
Parts 1 and 3 are handled by virtio_gpu_virgl_async_unmap_resource_blob()
The VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB is different because we know that
blob is mapped in the first place. Hence we can safely perform the part
3, assuming that parts 1/2 has been completed.
In case of VIRTIO_GPU_CMD_RESOURCE_UNREF, blob can be unmapped in the
first place and we can't do the part 3 because it will error out for
unmapped resource since parts 1/2 were not performed.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-15 17:01 ` Dmitry Osipenko
@ 2024-05-15 17:04 ` Akihiko Odaki
2024-05-15 17:15 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-15 17:04 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/16 2:01, Dmitry Osipenko wrote:
> On 5/15/24 19:42, Akihiko Odaki wrote:
>>>> It may be better to actually implement unmapping instead of returning an
>>>> error for consistency with the iov operation. Apparently crosvm also
>>>> unmaps blobs with VIRTIO_GPU_CMD_RESOURCE_UNREF.
>>>
>>> Then I'll add back `async_unmap_in_progress` because resource can be
>>> both mapped/unmapped on unref, and we'll need flag to know whether async
>>> unmapping has been finished to do the final unmapping of the resource.
>>
>> Such a situation should be already handled since unmapping in progress
>> blocks all commands (not just VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB but
>> literally all, including VIRTIO_GPU_CMD_RESOURCE_UNREF).
>
> The async unmapping consists of 3 parts:
>
> 1. begin async unmapping with memory_region_del_subregion() and suspend
> 2. wait for res->mr to be freed and resume
> 3. finish the unmapping with final virgl_renderer_resource_unmap()
>
> Parts 1 and 3 are handled by virtio_gpu_virgl_async_unmap_resource_blob()
>
>
> The VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB is different because we know that
> blob is mapped in the first place. Hence we can safely perform the part
> 3, assuming that parts 1/2 has been completed.
>
> In case of VIRTIO_GPU_CMD_RESOURCE_UNREF, blob can be unmapped in the
> first place and we can't do the part 3 because it will error out for
> unmapped resource since parts 1/2 were not performed.
>
VIRTIO_GPU_CMD_RESOURCE_UNREF should also call
virtio_gpu_virgl_async_unmap_resource_blob(). I guess that's the
original intention of having a function for this instead of inlining the
content of this function to virgl_cmd_resource_unmap_blob().
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-15 17:04 ` Akihiko Odaki
@ 2024-05-15 17:15 ` Dmitry Osipenko
2024-05-16 4:57 ` Akihiko Odaki
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 17:15 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/15/24 20:04, Akihiko Odaki wrote:
>>
>
> VIRTIO_GPU_CMD_RESOURCE_UNREF should also call
> virtio_gpu_virgl_async_unmap_resource_blob(). I guess that's the
> original intention of having a function for this instead of inlining the
> content of this function to virgl_cmd_resource_unmap_blob().
Correct, previous patchset versions unmapped resource on unref.
In v11 I dropped unmapping from unref to avoid adding additional
`async_unmap_in_progress` flag because normally map/unmap will be
balanced by guest anyways.
The virtio-gpu spec doesn't tell that resource have to be implicitly
unmapped on unref. In a case of Linux guest, it actually will be a bug
to unref a mapped resource because guest will continue to map and use
the destroyed resource.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 08/10] virtio-gpu: Handle resource blob commands
2024-05-15 17:15 ` Dmitry Osipenko
@ 2024-05-16 4:57 ` Akihiko Odaki
0 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-16 4:57 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/16 2:15, Dmitry Osipenko wrote:
> On 5/15/24 20:04, Akihiko Odaki wrote:
>>>
>>
>> VIRTIO_GPU_CMD_RESOURCE_UNREF should also call
>> virtio_gpu_virgl_async_unmap_resource_blob(). I guess that's the
>> original intention of having a function for this instead of inlining the
>> content of this function to virgl_cmd_resource_unmap_blob().
>
> Correct, previous patchset versions unmapped resource on unref.
>
> In v11 I dropped unmapping from unref to avoid adding additional
> `async_unmap_in_progress` flag because normally map/unmap will be
> balanced by guest anyways.
>
> The virtio-gpu spec doesn't tell that resource have to be implicitly
> unmapped on unref. In a case of Linux guest, it actually will be a bug
> to unref a mapped resource because guest will continue to map and use
> the destroyed resource.
>
Additional `async_unmap_in_progress` flag should not be necessary as
explained earlier.
It is a valid design not to issue UNMAP_BLOB before UNREF if the
automatically performs the unmapping operation. A guest needs to ensure
the blob is not mapped in a guest userspace virtual address space, but
it does not require issuing UNMAP_BLOB, which is to unmap the blob from
the guest physical address space.
In case of Linux, virtio_gpu_vram_free() calls virtio_gpu_cmd_unmap() to
issue UNMAP_BLOB before UNREF, which is actually not necessary. Linux
still needs to ensure that the blob is not mapped in a guest userspace
virtual address space, but that is done before virtio_gpu_vram_free()
gets called, and virtio_gpu_cmd_unmap() has nothing to do with that.
It is still a good practice for a guest to issue UNMAP_BLOB in such a
case because the spec does not say the VMM will automatically unmap the
blob for UNREF, and that's what Linux does. From the VMM perspective,
it's better to perform the unmapping operation for UNREF because the
spec does not say the guest always issue UNMAP_BLOB before UNREF.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v11 09/10] virtio-gpu: Register capsets dynamically
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (7 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
2024-05-13 9:20 ` Akihiko Odaki
2024-05-11 18:22 ` [PATCH v11 10/10] virtio-gpu: Support Venus context Dmitry Osipenko
9 siblings, 1 reply; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-gl.c | 6 ++++--
hw/display/virtio-gpu-virgl.c | 33 +++++++++++++++++++++------------
include/hw/virtio/virtio-gpu.h | 4 +++-
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 95806999189e..431fc2881a00 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -124,8 +124,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
}
g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
- VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
- virtio_gpu_virgl_get_num_capsets(g);
+ g->capset_ids = virtio_gpu_virgl_get_capsets(g);
+ VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len;
#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
@@ -148,6 +148,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
if (gl->renderer_inited) {
virtio_gpu_virgl_deinit(g);
}
+
+ g_array_unref(g->capset_ids);
}
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 3f2e406be3a4..135974431492 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -590,19 +590,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
VIRTIO_GPU_FILL_CMD(info);
memset(&resp, 0, sizeof(resp));
- if (info.capset_index == 0) {
- resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
- virgl_renderer_get_cap_set(resp.capset_id,
- &resp.capset_max_version,
- &resp.capset_max_size);
- } else if (info.capset_index == 1) {
- resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
+
+ if (info.capset_index < g->capset_ids->len) {
+ resp.capset_id = g_array_index(g->capset_ids, uint32_t,
+ info.capset_index);
virgl_renderer_get_cap_set(resp.capset_id,
&resp.capset_max_version,
&resp.capset_max_size);
- } else {
- resp.capset_max_version = 0;
- resp.capset_max_size = 0;
}
resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
@@ -1123,14 +1117,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
return 0;
}
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
+static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
+{
+ g_array_append_val(capset_ids, capset_id);
+}
+
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
uint32_t capset2_max_ver, capset2_max_size;
+ GArray *capset_ids;
+
+ capset_ids = g_array_new(false, false, sizeof(uint32_t));
+
+ /* VIRGL is always supported. */
+ virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
+
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
&capset2_max_ver,
&capset2_max_size);
+ if (capset2_max_ver) {
+ virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
+ }
- return capset2_max_ver ? 2 : 1;
+ return capset_ids;
}
void virtio_gpu_virgl_deinit(VirtIOGPU *g)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index c0b0b0eac08b..7bbc6ef268eb 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -212,6 +212,8 @@ struct VirtIOGPU {
} dmabuf;
QEMUBH *cmdq_resume_bh;
+
+ GArray *capset_ids;
};
struct VirtIOGPUClass {
@@ -346,6 +348,6 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
void virtio_gpu_virgl_deinit(VirtIOGPU *g);
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v11 09/10] virtio-gpu: Register capsets dynamically
2024-05-11 18:22 ` [PATCH v11 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
@ 2024-05-13 9:20 ` Akihiko Odaki
2024-05-15 16:23 ` Dmitry Osipenko
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2024-05-13 9:20 UTC (permalink / raw)
To: Dmitry Osipenko, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 2024/05/12 3:22, Dmitry Osipenko wrote:
> From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>
> virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
> assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
> like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> hw/display/virtio-gpu-gl.c | 6 ++++--
> hw/display/virtio-gpu-virgl.c | 33 +++++++++++++++++++++------------
> include/hw/virtio/virtio-gpu.h | 4 +++-
> 3 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index 95806999189e..431fc2881a00 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -124,8 +124,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
> }
>
> g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
> - VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
> - virtio_gpu_virgl_get_num_capsets(g);
> + g->capset_ids = virtio_gpu_virgl_get_capsets(g);
> + VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len;
>
> #ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
> @@ -148,6 +148,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
> if (gl->renderer_inited) {
> virtio_gpu_virgl_deinit(g);
> }
> +
> + g_array_unref(g->capset_ids);
> }
>
> static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 3f2e406be3a4..135974431492 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -590,19 +590,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
> VIRTIO_GPU_FILL_CMD(info);
>
> memset(&resp, 0, sizeof(resp));
> - if (info.capset_index == 0) {
> - resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
> - virgl_renderer_get_cap_set(resp.capset_id,
> - &resp.capset_max_version,
> - &resp.capset_max_size);
> - } else if (info.capset_index == 1) {
> - resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
> +
> + if (info.capset_index < g->capset_ids->len) {
> + resp.capset_id = g_array_index(g->capset_ids, uint32_t,
> + info.capset_index);
> virgl_renderer_get_cap_set(resp.capset_id,
> &resp.capset_max_version,
> &resp.capset_max_size);
> - } else {
> - resp.capset_max_version = 0;
> - resp.capset_max_size = 0;
> }
> resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
> virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> @@ -1123,14 +1117,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> return 0;
> }
>
> -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
> +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
> +{
> + g_array_append_val(capset_ids, capset_id);
> +}
Is it worthwhile to have a function for this?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v11 09/10] virtio-gpu: Register capsets dynamically
2024-05-13 9:20 ` Akihiko Odaki
@ 2024-05-15 16:23 ` Dmitry Osipenko
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-15 16:23 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
On 5/13/24 12:20, Akihiko Odaki wrote:
...
>> -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>> +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t
>> capset_id)
>> +{
>> + g_array_append_val(capset_ids, capset_id);
>> +}
>
> Is it worthwhile to have a function for this?
It's necessary to have it because g_array_append_val() is actually a macro that takes &capset_id ptr internally.
I.e. g_array_append_val(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2) will fail to compile with:
/usr/include/glib-2.0/glib/garray.h:66:59: error: lvalue required as unary '&' operand
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v11 10/10] virtio-gpu: Support Venus context
2024-05-11 18:22 [PATCH v11 00/10] Support blob memory and venus on qemu Dmitry Osipenko
` (8 preceding siblings ...)
2024-05-11 18:22 ` [PATCH v11 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
@ 2024-05-11 18:22 ` Dmitry Osipenko
9 siblings, 0 replies; 24+ messages in thread
From: Dmitry Osipenko @ 2024-05-11 18:22 UTC (permalink / raw)
To: Akihiko Odaki, Huang Rui, Marc-André Lureau,
Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S . Tsirkin,
Stefano Stabellini, Anthony PERARD, Antonio Caggiano,
Dr . David Alan Gilbert, Robert Beckett, Gert Wollny,
Alex Bennée
Cc: qemu-devel, Gurchetan Singh, ernunes, Alyssa Ross,
Roger Pau Monné, Alex Deucher, Stefano Stabellini,
Christian König, Xenia Ragiadakou,
Pierre-Eric Pelloux-Prayer, Honglei Huang, Julia Zhang,
Chen Jiqian, Yiwei Zhang
From: Antonio Caggiano <antonio.caggiano@collabora.com>
Request Venus when initializing VirGL and if venus=true flag is set for
virtio-gpu-gl device.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
hw/display/virtio-gpu-gl.c | 2 ++
hw/display/virtio-gpu-virgl.c | 22 ++++++++++++++++++----
hw/display/virtio-gpu.c | 13 +++++++++++++
include/hw/virtio/virtio-gpu.h | 3 +++
meson.build | 1 +
5 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 431fc2881a00..1deb3e64b781 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -137,6 +137,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_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
+ VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 135974431492..f1cad005dd8f 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1094,6 +1094,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
}
#endif
+#ifdef VIRGL_RENDERER_VENUS
+ if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+ flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
+ }
+#endif
ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
if (ret != 0) {
@@ -1124,7 +1129,7 @@ static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
- uint32_t capset2_max_ver, capset2_max_size;
+ uint32_t capset_max_ver, capset_max_size;
GArray *capset_ids;
capset_ids = g_array_new(false, false, sizeof(uint32_t));
@@ -1133,12 +1138,21 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
- &capset2_max_ver,
- &capset2_max_size);
- if (capset2_max_ver) {
+ &capset_max_ver,
+ &capset_max_size);
+ if (capset_max_ver) {
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
}
+ if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+ virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+ &capset_max_ver,
+ &capset_max_size);
+ if (capset_max_size) {
+ virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VENUS);
+ }
+ }
+
return capset_ids;
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 052ab493a00b..0518bb858e88 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1491,6 +1491,19 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
#endif
}
+ if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+#ifdef HAVE_VIRGL_VENUS
+ if (!virtio_gpu_blob_enabled(g->parent_obj.conf) ||
+ !virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+ error_setg(errp, "venus requires enabled blob and hostmem options");
+ return;
+ }
+#else
+ error_setg(errp, "old virglrenderer, venus unsupported");
+ return;
+#endif
+ }
+
if (!virtio_gpu_base_device_realize(qdev,
virtio_gpu_handle_ctrl_cb,
virtio_gpu_handle_cursor_cb,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7bbc6ef268eb..9940bbcbaacc 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -99,6 +99,7 @@ enum virtio_gpu_base_conf_flags {
VIRTIO_GPU_FLAG_BLOB_ENABLED,
VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
+ VIRTIO_GPU_FLAG_VENUS_ENABLED,
};
#define virtio_gpu_virgl_enabled(_cfg) \
@@ -117,6 +118,8 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
#define virtio_gpu_hostmem_enabled(_cfg) \
(_cfg.hostmem > 0)
+#define virtio_gpu_venus_enabled(_cfg) \
+ (_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
struct virtio_gpu_base_conf {
uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 0433ea200dd7..20241931730f 100644
--- a/meson.build
+++ b/meson.build
@@ -2290,6 +2290,7 @@ if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB', 1)
+ config_host_data.set('HAVE_VIRGL_VENUS', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
^ permalink raw reply related [flat|nested] 24+ messages in thread