qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 00/10] Support blob memory and venus on qemu
@ 2024-05-04 20:28 Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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

Hello,

This series enables Vulkan Venus context support on virtio-gpu.

All virglrender and almost all Linux kernel prerequisite changes
needed by Venus are already in upstream. For kernel there is a pending
KVM patchset that fixes mapping of compound pages needed for DRM drivers
using TTM [1], othewrwise hostmem blob mapping will fail with a KVM error
from Qemu.

[1] https://lore.kernel.org/kvm/20240229025759.1187910-1-stevensd@google.com/

You'll need to use recent Mesa version containing patch that removes
dependency on cross-device feature from Venus that isn't supported by
Qemu [2].

[2] https://gitlab.freedesktop.org/mesa/mesa/-/commit/087e9a96d13155e26987befae78b6ccbb7ae242b

Example Qemu cmdline that enables Venus for virtio-gpu:

  qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true


Changes from V9 to V10

- Dropped 'async_unmap_in_progress' variable and switched to use
  aio_bh_new() isntead of oneshot variant in the "blob commands" patch.

- Further improved error messages by printing error code when actual error
  occurrs and using ERR_UNSPEC instead of ERR_ENOMEM when we don't really
  know if it was ENOMEM for sure.

- Added vdc->unrealize for the virtio GL device and freed virgl data.

- Dropped UUID and doc/migration patches. UUID feature isn't needed
  anymore, instead we changed Mesa Venus driver to not require UUID.

- Renamed virtio-gpu-gl "vulkan" property name back to "venus".

Changes from V8 to V9

- Added resuming of cmdq processing when hostmem MR is freed,
  as was suggested by Akihiko Odaki.

- Added more error messages, suggested by Akihiko Odaki

- Dropped superfluous 'res->async_unmap_completed', suggested
  by Akihiko Odaki.

- Kept using cmd->suspended flag. Akihiko Odaki suggested to make
  virtio_gpu_virgl_process_cmd() return false if cmd processing is
  suspended, but it's not easy to implement due to ubiquitous
  VIRTIO_GPU_FILL_CMD() macros that returns void, requiring to change
  all the virtio-gpu processing code.

- Added back virtio_gpu_virgl_resource as was requested by Akihiko Odaki,
  though I'm not convinced it's really needed.

- Switched to use GArray, renamed capset2_max_ver/size vars and moved
  "vulkan" property definition to the virtio-gpu-gl device in the Venus
  patch, like was suggested by Akihiko Odaki.

- Moved UUID to virtio_gpu_virgl_resource and dropped UUID save/restore
  since it will require bumping VM version and virgl device isn't miratable
  anyways.

- Fixed exposing UUID feature with Rutabaga

- Dropped linux-headers update patch because headers were already updated
  in Qemu/staging.

- Added patch that updates virtio migration doc with a note about virtio-gpu
  migration specifics, suggested by Akihiko Odaki.

- Addressed coding style issue noticed by Akihiko Odaki

Changes from V7 to V8

- Supported suspension of virtio-gpu commands processing and made
  unmapping of hostmem region asynchronous by blocking/suspending
  cmd processing until region is unmapped. Suggested by Akihiko Odaki.

- Fixed arm64 building of x86 targets using updated linux-headers.
  Corrected the update script. Thanks to Rob Clark for reporting
  the issue.

- Added new patch that makes registration of virgl capsets dynamic.
  Requested by Antonio Caggiano and Pierre-Eric Pelloux-Prayer.

- Venus capset now isn't advertised if Vulkan is disabled with vulkan=false

Changes from V6 to V7

- Used scripts/update-linux-headers.sh to update Qemu headers based
  on Linux v6.8-rc3 that adds Venus capset definition to virtio-gpu
  protocol, was requested by Peter Maydel

- Added r-bs that were given to v6 patches. Corrected missing s-o-bs

- Dropped context_init Qemu's virtio-gpu device configuration flag,
  was suggested by Marc-André Lureau

- Added missing error condition checks spotted by Marc-André Lureau
  and Akihiko Odaki, and few more

- Returned back res->mr referencing to memory_region_init_ram_ptr() like
  was suggested by Akihiko Odaki. Incorporated fix suggested by Pierre-Eric
  to specify the MR name

- Dropped the virgl_gpu_resource wrapper, cleaned up and simplified
  patch that adds blob-cmd support

- Fixed improper blob resource removal from resource list on resource_unref
  that was spotted by Akihiko Odaki

- Change order of the blob patches, was suggested by Akihiko Odaki.
  The cmd_set_scanout_blob support is enabled first

- Factored out patch that adds resource management support to virtio-gpu-gl,
  was requested by Marc-André Lureau

- Simplified and improved the UUID support patch, dropped the hash table
  as we don't need it for now. Moved QemuUUID to virtio_gpu_simple_resource.
  This all was suggested by Akihiko Odaki and Marc-André Lureau

- Dropped console_has_gl() check, suggested by Akihiko Odaki

- Reworked Meson cheking of libvirglrender features, made new features
  available based on virglrender pkgconfig version instead of checking
  symbols in header. This should fix build error using older virglrender
  version, reported by Alex Bennée

- Made enabling of Venus context configrable via new virtio-gpu device
  "vulkan=true" flag, suggested by Marc-André Lureau. The flag is disabled
  by default because it requires blob and hostmem options to be enabled
  and configured

Changes from V5 to V6

- Move macros configurations under virgl.found() and rename
  HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS.

- Handle the case while context_init is disabled.

- Enable context_init by default.

- Move virtio_gpu_virgl_resource_unmap() into
  virgl_cmd_resource_unmap_blob().

- Introduce new struct virgl_gpu_resource to store virgl specific members.

- Remove erro handling of g_new0, because glib will abort() on OOM.

- Set resource uuid as option.

- Implement optional subsection of vmstate_virtio_gpu_resource_uuid_state
  for virtio live migration.

- Use g_int_hash/g_int_equal instead of the default

- Add scanout_blob function for virtio-gpu-virgl

- Resolve the memory leak on virtio-gpu-virgl

- Remove the unstable API flags check because virglrenderer is already 1.0

- Squash the render server flag support into "Initialize Venus"

Changes from V4 (virtio gpu V4) to V5

- Inverted patch 5 and 6 because we should configure
  HAVE_VIRGL_CONTEXT_INIT firstly.

- Validate owner of memory region to avoid slowing down DMA.

- Use memory_region_init_ram_ptr() instead of
  memory_region_init_ram_device_ptr().

- Adjust sequence to allocate gpu resource before virglrender resource
  creation

- Add virtio migration handling for uuid.

- Send kernel patch to define VIRTIO_GPU_CAPSET_VENUS.
  https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.huang@amd.com/

- Add meson check to make sure unstable APIs defined from 0.9.0.

Changes from V1 to V2 (virtio gpu V4)

- Remove unused #include "hw/virtio/virtio-iommu.h"

- Add a local function, called virgl_resource_destroy(), that is used
  to release a vgpu resource on error paths and in resource_unref.

- Remove virtio_gpu_virgl_resource_unmap from
  virtio_gpu_cleanup_mapping(),
  since this function won't be called on blob resources and also because
  blob resources are unmapped via virgl_cmd_resource_unmap_blob().

- In virgl_cmd_resource_create_blob(), do proper cleanup in error paths
  and move QTAILQ_INSERT_HEAD(&g->reslist, res, next) after the resource
  has been fully initialized.

- Memory region has a different life-cycle from virtio gpu resources
  i.e. cannot be released synchronously along with the vgpu resource.
  So, here the field "region" was changed to a pointer and is allocated
  dynamically when the blob is mapped.
  Also, since the pointer can be used to indicate whether the blob
  is mapped, the explicite field "mapped" was removed.

- In virgl_cmd_resource_map_blob(), add check on the value of
  res->region, to prevent beeing called twice on the same resource.

- Add a patch to enable automatic deallocation of memory regions to resolve
  use-after-free memory corruption with a reference.

Antonio Caggiano (2):
  virtio-gpu: Handle resource blob commands
  virtio-gpu: Support Venus context

Dmitry Osipenko (4):
  virtio-gpu: Unrealize GL device
  virtio-gpu: Use pkgconfig version to decide which virgl features are
    available
  virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
  virtio-gpu: Support suspension of commands processing

Huang Rui (2):
  virtio-gpu: Support context-init feature with virglrenderer
  virtio-gpu: Add virgl resource management

Pierre-Eric Pelloux-Prayer (1):
  virtio-gpu: Register capsets dynamically

Robert Beckett (1):
  virtio-gpu: Support blob scanout using dmabuf fd

 hw/display/virtio-gpu-gl.c       |  24 +-
 hw/display/virtio-gpu-rutabaga.c |   1 +
 hw/display/virtio-gpu-virgl.c    | 534 ++++++++++++++++++++++++++++++-
 hw/display/virtio-gpu.c          |  35 +-
 include/hw/virtio/virtio-gpu.h   |  18 +-
 meson.build                      |  10 +-
 6 files changed, 590 insertions(+), 32 deletions(-)

-- 
2.44.0



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v10 01/10] virtio-gpu: Unrealize GL device
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-07 15:16   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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] 18+ messages in thread

* [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-07 15:18   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 5db2dbc12ec7..f4a4d71c1978 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] 18+ messages in thread

* [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-08  6:17   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled Dmitry Osipenko
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 f4a4d71c1978..513cb2ea6d03 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] 18+ messages in thread

* [PATCH v10 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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] 18+ messages in thread

* [PATCH v10 05/10] virtio-gpu: Add virgl resource management
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-08  6:47   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 | 74 +++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 8306961ad502..a040324f5024 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,19 @@ 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) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
     virgl_renderer_resource_detach_iov(unref.resource_id,
                                        &res_iovs,
                                        &num_iovs);
@@ -95,6 +165,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] 18+ messages in thread

* [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-08  6:48   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 07/10] virtio-gpu: Support suspension of commands processing Dmitry Osipenko
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 a040324f5024..2fedccb1fc8d 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;
@@ -507,6 +511,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)
 {
@@ -573,6 +677,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 513cb2ea6d03..9a00e29a80cf 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] 18+ messages in thread

* [PATCH v10 07/10] virtio-gpu: Support suspension of commands processing
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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

Add new "suspended" flag to virtio_gpu_ctrl_command telling cmd
processor that it should stop processing commands and retry again
next time until flag is unset.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-gl.c       | 1 +
 hw/display/virtio-gpu-rutabaga.c | 1 +
 hw/display/virtio-gpu-virgl.c    | 3 +++
 hw/display/virtio-gpu.c          | 5 +++++
 include/hw/virtio/virtio-gpu.h   | 1 +
 5 files changed, 11 insertions(+)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 95806999189e..2a9e549ad2e9 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -79,6 +79,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         cmd->vq = vq;
         cmd->error = 0;
         cmd->finished = false;
+        cmd->suspended = false;
         QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
         cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
     }
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 17bf701a2163..b6e84d436fb2 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1061,6 +1061,7 @@ static void virtio_gpu_rutabaga_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         cmd->vq = vq;
         cmd->error = 0;
         cmd->finished = false;
+        cmd->suspended = false;
         QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
         cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
     }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 2fedccb1fc8d..dc2b5a496630 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -687,6 +687,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         break;
     }
 
+    if (cmd->suspended) {
+        return;
+    }
     if (cmd->finished) {
         return;
     }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1e57a53d346c..a1bd4d6914c4 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
         /* process command */
         vgc->process_cmd(g, cmd);
 
+        if (cmd->suspended) {
+            break;
+        }
+
         QTAILQ_REMOVE(&g->cmdq, cmd, next);
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
             g->stats.requests++;
@@ -1113,6 +1117,7 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         cmd->vq = vq;
         cmd->error = 0;
         cmd->finished = false;
+        cmd->suspended = false;
         QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
         cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
     }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ba36497c477f..0c00c303f41b 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -132,6 +132,7 @@ struct virtio_gpu_ctrl_command {
     struct virtio_gpu_ctrl_hdr cmd_hdr;
     uint32_t error;
     bool finished;
+    bool suspended;
     QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
 };
 
-- 
2.44.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v10 08/10] virtio-gpu: Handle resource blob commands
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 07/10] virtio-gpu: Support suspension of commands processing Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
  2024-05-04 20:28 ` [PATCH v10 10/10] virtio-gpu: Support Venus context Dmitry Osipenko
  9 siblings, 0 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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  | 266 +++++++++++++++++++++++++++++++++
 hw/display/virtio-gpu.c        |   4 +-
 include/hw/virtio/virtio-gpu.h |   2 +
 3 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index dc2b5a496630..d92c58b77865 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,114 @@ 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);
+
+    /*
+     * Potentially, 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
+     * released.
+     */
+    OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+    res->mr = mr;
+
+    return 0;
+}
+
+static void
+virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
+                                           struct virtio_gpu_virgl_resource *res,
+                                           bool *cmd_suspended)
+{
+    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+    MemoryRegion *mr = res->mr;
+
+    if (mr && *cmd_suspended == false) {
+        /* render will be unblocked when 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));
+
+        *cmd_suspended = true;
+    } else if (!mr && *cmd_suspended) {
+        virgl_renderer_resource_unmap(res->base.resource_id);
+
+        *cmd_suspended = false;
+    }
+}
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
+
 static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
                                          struct virtio_gpu_ctrl_command *cmd)
 {
@@ -162,6 +271,13 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
         return;
     }
 
+    if (res->mr || cmd->suspended) {
+        virtio_gpu_virgl_async_unmap_resource_blob(g, res, &cmd->suspended);
+        if (cmd->suspended) {
+            return;
+        }
+    }
+
     virgl_renderer_resource_detach_iov(unref.resource_id,
                                        &res_iovs,
                                        &num_iovs);
@@ -512,6 +628,138 @@ 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;
+    }
+
+    if (res->mr) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already mapped %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)
+{
+    struct virtio_gpu_resource_unmap_blob ublob;
+    struct virtio_gpu_virgl_resource *res;
+
+    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;
+    }
+
+    if (!res->mr && !cmd->suspended) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already unmapped %d\n",
+                      __func__, ublob.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    virtio_gpu_virgl_async_unmap_resource_blob(g, res, &cmd->suspended);
+}
+
 static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
                                        struct virtio_gpu_ctrl_command *cmd)
 {
@@ -678,6 +926,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);
+        break;
     case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
         virgl_cmd_set_scanout_blob(g, cmd);
         break;
@@ -855,6 +1112,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;
 }
 
@@ -870,6 +1132,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 a1bd4d6914c4..45c1f2006712 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 0c00c303f41b..a98847b88087 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -211,6 +211,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] 18+ messages in thread

* [PATCH v10 09/10] virtio-gpu: Register capsets dynamically
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  2024-05-08  6:56   ` Marc-André Lureau
  2024-05-04 20:28 ` [PATCH v10 10/10] virtio-gpu: Support Venus context Dmitry Osipenko
  9 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 2a9e549ad2e9..cd39e0650862 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -125,8 +125,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;
@@ -149,6 +149,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 d92c58b77865..1babda4efad5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -585,19 +585,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));
@@ -1120,14 +1114,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 a98847b88087..105308a36865 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -213,6 +213,8 @@ struct VirtIOGPU {
     } dmabuf;
 
     QEMUBH *cmdq_resume_bh;
+
+    GArray *capset_ids;
 };
 
 struct VirtIOGPUClass {
@@ -347,6 +349,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] 18+ messages in thread

* [PATCH v10 10/10] virtio-gpu: Support Venus context
  2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2024-05-04 20:28 ` [PATCH v10 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
@ 2024-05-04 20:28 ` Dmitry Osipenko
  9 siblings, 0 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2024-05-04 20:28 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 cd39e0650862..f2e4da56d8c4 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -138,6 +138,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 1babda4efad5..a0a5e5b7a3de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1091,6 +1091,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) {
@@ -1121,7 +1126,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));
@@ -1130,12 +1135,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 45c1f2006712..e86326b25a72 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 105308a36865..8b7bbf853fe8 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 9a00e29a80cf..5e59f031b4b7 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] 18+ messages in thread

* Re: [PATCH v10 01/10] virtio-gpu: Unrealize GL device
  2024-05-04 20:28 ` [PATCH v10 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
@ 2024-05-07 15:16   ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-07 15:16 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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

Hi

On Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> 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) {
> +        timer_free(g->fence_poll);

might be worth to set g->fence_poll to NULL here, to be able to call
deinit() multiple times.

> +    }
> +
> +    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
>

otherwise:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available
  2024-05-04 20:28 ` [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
@ 2024-05-07 15:18   ` Marc-André Lureau
  2024-05-07 15:20     ` Marc-André Lureau
  0 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-07 15:18 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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

Hi

On Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> 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 5db2dbc12ec7..f4a4d71c1978 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)

If it checks for version >=1.0, we may as well depend on >=1.0.1 imho,
and then simply use VIRGL_CHECK_VERSION in the code.

>  endif
>  config_host_data.set('CONFIG_VIRTFS', have_virtfs)
>  config_host_data.set('CONFIG_VTE', vte.found())
> --
> 2.44.0
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available
  2024-05-07 15:18   ` Marc-André Lureau
@ 2024-05-07 15:20     ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-07 15:20 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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

Hi

On Tue, May 7, 2024 at 7:18 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
> >
> > 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 5db2dbc12ec7..f4a4d71c1978 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)
>
> If it checks for version >=1.0, we may as well depend on >=1.0.1 imho,
> and then simply use VIRGL_CHECK_VERSION in the code.
>

..but we would need a fallback if VIRGL_CHECK_VERSION isn't defined in
that case (which could simply fail).




-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer
  2024-05-04 20:28 ` [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
@ 2024-05-08  6:17   ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-08  6:17 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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

Hi

On Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> 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

I am wondering if there should be a:

#else
    g_assert_not_reached();
#endif

rather than falling back to a regular context creation.

> +    }
> +
> +    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 f4a4d71c1978..513cb2ea6d03 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
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 05/10] virtio-gpu: Add virgl resource management
  2024-05-04 20:28 ` [PATCH v10 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
@ 2024-05-08  6:47   ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-08  6:47 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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 Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> 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>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/display/virtio-gpu-virgl.c | 74 +++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 8306961ad502..a040324f5024 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,19 @@ 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) {
> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +        return;
> +    }
> +
>      virgl_renderer_resource_detach_iov(unref.resource_id,
>                                         &res_iovs,
>                                         &num_iovs);
> @@ -95,6 +165,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
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd
  2024-05-04 20:28 ` [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
@ 2024-05-08  6:48   ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-08  6:48 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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 Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> 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>

Acked-by: Marc-André Lureau <marcandre.lureau@redhat.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 a040324f5024..2fedccb1fc8d 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;
> @@ -507,6 +511,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)
>  {
> @@ -573,6 +677,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 513cb2ea6d03..9a00e29a80cf 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
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v10 09/10] virtio-gpu: Register capsets dynamically
  2024-05-04 20:28 ` [PATCH v10 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
@ 2024-05-08  6:56   ` Marc-André Lureau
  0 siblings, 0 replies; 18+ messages in thread
From: Marc-André Lureau @ 2024-05-08  6:56 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Akihiko Odaki, Huang Rui, 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, 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 Sun, May 5, 2024 at 12:29 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> 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>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.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 2a9e549ad2e9..cd39e0650862 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -125,8 +125,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;
> @@ -149,6 +149,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 d92c58b77865..1babda4efad5 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -585,19 +585,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));
> @@ -1120,14 +1114,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 a98847b88087..105308a36865 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -213,6 +213,8 @@ struct VirtIOGPU {
>      } dmabuf;
>
>      QEMUBH *cmdq_resume_bh;
> +
> +    GArray *capset_ids;
>  };
>
>  struct VirtIOGPUClass {
> @@ -347,6 +349,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
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-05-08  6:56 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-04 20:28 [PATCH v10 00/10] Support blob memory and venus on qemu Dmitry Osipenko
2024-05-04 20:28 ` [PATCH v10 01/10] virtio-gpu: Unrealize GL device Dmitry Osipenko
2024-05-07 15:16   ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available Dmitry Osipenko
2024-05-07 15:18   ` Marc-André Lureau
2024-05-07 15:20     ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer Dmitry Osipenko
2024-05-08  6:17   ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled Dmitry Osipenko
2024-05-04 20:28 ` [PATCH v10 05/10] virtio-gpu: Add virgl resource management Dmitry Osipenko
2024-05-08  6:47   ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd Dmitry Osipenko
2024-05-08  6:48   ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 07/10] virtio-gpu: Support suspension of commands processing Dmitry Osipenko
2024-05-04 20:28 ` [PATCH v10 08/10] virtio-gpu: Handle resource blob commands Dmitry Osipenko
2024-05-04 20:28 ` [PATCH v10 09/10] virtio-gpu: Register capsets dynamically Dmitry Osipenko
2024-05-08  6:56   ` Marc-André Lureau
2024-05-04 20:28 ` [PATCH v10 10/10] virtio-gpu: Support Venus context Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).