[PATCH v16 00/13] Support blob memory and venus on qemu

[PATCH v16 00/13] Support blob memory and venus on qemu

[Dmitry Osipenko]

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:

  qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \
      -machine q35,accel=kvm,memory-backend=mem1 \
      -object memory-backend-memfd,id=mem1,size=8G -m 8G


Changes from V15 to V16

- Fixed resource_get_info() change made in v15 that was squshed into a
  wrong patch. Squashed set_scanout_blob patch into the blob-commands patch,
  otherwise we'll need to revert back ordering of blob patches to older v7
  variant.

- Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.

- Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
  with the doc addition. Suggested by Akihiko Odaki.

Changes from V14 to V15

- Dropped hostmem mapping state that got unused in v14, suggested by
  Akihiko Odaki.

- Moved resource_get_info() from set_scanout_blob() to create_blob(),
  suggested by Akihiko Odaki.

- Fixed unitilized variable in create_blob(), spotted by Alex Bennée.

Changes from V13 to V14

- Fixed erronous fall-through in renderer_state's switch-case that was
  spotted by Marc-André Lureau.

- Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
  Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.

- Made use of g_autofree in virgl_cmd_resource_create_blob() as was
  suggested by Akihiko Odaki.

- Removed virtio_gpu_virgl_deinit() and moved all deinit code to
  virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.

- Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
  check as was suggested by Marc-André Lureau.

- Added trace event for cmd-suspension as was suggested by Marc-André Lureau.

- Added patch to replace in-flight printf's with trace events as was
  suggested by Marc-André Lureau

Changes from V12 to V13

- Replaced `res->async_unmap_in_progress` flag with a mapping state,
  moved it to the virtio_gpu_virgl_hostmem_region like was suggested
  by Akihiko Odaki.

- Renamed blob_unmap function and added back cmd_suspended argument
  to it. Suggested by Akihiko Odaki.

- Reordered VirtIOGPUGL refactoring patches to minimize code changes
  like was suggested by Akihiko Odaki.

- Replaced gl->renderer_inited with gl->renderer_state, like was suggested
  by Alex Bennée.

- Added gl->renderer state resetting to gl_device_unrealize(), for
  consistency. Suggested by Alex Bennée.

- Added rb's from Alex and Manos.

- Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.

Changes from V11 to V12

- Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
  corrupt resource list and releases resource properly on error. Thanks
  to Akihiko Odaki for spotting the bug.

- Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
  fixing Qemu crash. Besides fixing the crash, it allows to implement
  a cleaner virtio_gpu_virgl_deinit().

- virtio_gpu_virgl_deinit() now assumes that previously virgl was
  initialized successfully when it was inited at all. Suggested by
  Akihiko Odaki.

- Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()

- Added back blob unmapping or RESOURCE_UNREF that was requested
  by Akihiko Odaki. Added comment to the code explaining how
  async unmapping works. Added back `res->async_unmap_in_progress`
  flag and added comment telling why it's needed.

- Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
  suggested by Akihiko Odaki.

- Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
  for consistency with cmdq_resume_bh.

Changes from V10 to V11

- Replaced cmd_resume bool in struct ctrl_command with
  "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
  by Akihiko Odaki.

- Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
  the 'async_unmap_in_progress' flag that was dropped in v9:

    1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
       resource was previously mapped and lets virglrenderer to do the
       checking.

    2. error returned by virgl_renderer_resource_unmap() is now handled
       and reported properly, previously the error wasn't checked. The
       virgl_renderer_resource_unmap() fails if resource wasn't mapped.

    3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
       that is mapped, it's a error condition if guest didn't unmap resource
       before doing the unref. Previously unref was implicitly unmapping
       resource.

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 (1):
  virtio-gpu: Support Venus context

Dmitry Osipenko (8):
  virtio-gpu: Use trace events for tracking number of in-flight fences
  virtio-gpu: Move fence_poll timer to VirtIOGPUGL
  virtio-gpu: Move print_stats timer to VirtIOGPUGL
  virtio-gpu: Handle virtio_gpu_virgl_init() failure
  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: Handle resource blob commands

 docs/system/devices/virtio-gpu.rst |  11 +
 hw/display/trace-events            |   3 +
 hw/display/virtio-gpu-gl.c         |  62 ++-
 hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
 hw/display/virtio-gpu.c            |  44 ++-
 include/hw/virtio/virtio-gpu.h     |  32 +-
 meson.build                        |   5 +-
 7 files changed, 685 insertions(+), 57 deletions(-)

-- 
2.45.2

[PATCH v16 01/13] virtio-gpu: Use trace events for tracking number of in-flight fences

[Dmitry Osipenko]

Replace printf's used for tracking of in-flight fence inc/dec events
with tracing, for consistency with the rest of virtio-gpu code that
uses tracing.

Suggested-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/trace-events       | 2 ++
 hw/display/virtio-gpu-virgl.c | 2 +-
 hw/display/virtio-gpu.c       | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 781f8a33203b..e212710284ae 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -53,6 +53,8 @@ virtio_gpu_cmd_ctx_submit(uint32_t ctx, uint32_t size) "ctx 0x%x, size %d"
 virtio_gpu_update_cursor(uint32_t scanout, uint32_t x, uint32_t y, const char *type, uint32_t res) "scanout %d, x %d, y %d, %s, res 0x%x"
 virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type 0x%x"
 virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
+virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u"
+virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u"
 
 # qxl.c
 disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e6619c..14091b191ec0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -525,7 +525,7 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
         g_free(cmd);
         g->inflight--;
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-            fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+            trace_virtio_gpu_dec_inflight_fences(g->inflight);
         }
     }
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d60b1b2973af..602952a7041b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1066,7 +1066,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
                 if (g->stats.max_inflight < g->inflight) {
                     g->stats.max_inflight = g->inflight;
                 }
-                fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
+                trace_virtio_gpu_inc_inflight_fences(g->inflight);
             }
         } else {
             g_free(cmd);
@@ -1086,7 +1086,7 @@ static void virtio_gpu_process_fenceq(VirtIOGPU *g)
         g_free(cmd);
         g->inflight--;
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-            fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+            trace_virtio_gpu_dec_inflight_fences(g->inflight);
         }
     }
 }
-- 
2.45.2

[PATCH v16 02/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL

[Dmitry Osipenko]

Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh
that are used only by GL device.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c  | 8 +++++---
 include/hw/virtio/virtio-gpu.h | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 14091b191ec0..91dce90f9176 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -594,11 +594,12 @@ static void virtio_gpu_print_stats(void *opaque)
 static void virtio_gpu_fence_poll(void *opaque)
 {
     VirtIOGPU *g = opaque;
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 
     virgl_renderer_poll();
     virtio_gpu_process_cmdq(g);
     if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
-        timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
+        timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
     }
 }
 
@@ -626,6 +627,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 {
     int ret;
     uint32_t flags = 0;
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 
 #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
     if (qemu_egl_display) {
@@ -645,8 +647,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
         return ret;
     }
 
-    g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
-                                 virtio_gpu_fence_poll, g);
+    gl->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+                                  virtio_gpu_fence_poll, g);
 
     if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
         g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7a59379f5a7a..bc69fd78a440 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -196,7 +196,6 @@ struct VirtIOGPU {
     uint64_t hostmem;
 
     bool processing_cmdq;
-    QEMUTimer *fence_poll;
     QEMUTimer *print_stats;
 
     uint32_t inflight;
@@ -231,6 +230,8 @@ struct VirtIOGPUGL {
 
     bool renderer_inited;
     bool renderer_reset;
+
+    QEMUTimer *fence_poll;
 };
 
 struct VhostUserGPU {
-- 
2.45.2

[PATCH v16 03/13] virtio-gpu: Move print_stats timer to VirtIOGPUGL

[Dmitry Osipenko]

Move print_stats timer to VirtIOGPUGL for consistency with
cmdq_resume_bh and fence_poll that are used only by GL device.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c  | 10 ++++++----
 include/hw/virtio/virtio-gpu.h |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 91dce90f9176..a63d1f540f04 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -574,6 +574,7 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
 static void virtio_gpu_print_stats(void *opaque)
 {
     VirtIOGPU *g = opaque;
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 
     if (g->stats.requests) {
         fprintf(stderr, "stats: vq req %4d, %3d -- 3D %4d (%5d)\n",
@@ -588,7 +589,7 @@ static void virtio_gpu_print_stats(void *opaque)
     } else {
         fprintf(stderr, "stats: idle\r");
     }
-    timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
+    timer_mod(gl->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
 }
 
 static void virtio_gpu_fence_poll(void *opaque)
@@ -651,9 +652,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
                                   virtio_gpu_fence_poll, g);
 
     if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-        g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
-                                      virtio_gpu_print_stats, g);
-        timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
+        gl->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+                                       virtio_gpu_print_stats, g);
+        timer_mod(gl->print_stats,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
     }
     return 0;
 }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index bc69fd78a440..7ff989a45a5c 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -196,7 +196,6 @@ struct VirtIOGPU {
     uint64_t hostmem;
 
     bool processing_cmdq;
-    QEMUTimer *print_stats;
 
     uint32_t inflight;
     struct {
@@ -232,6 +231,7 @@ struct VirtIOGPUGL {
     bool renderer_reset;
 
     QEMUTimer *fence_poll;
+    QEMUTimer *print_stats;
 };
 
 struct VhostUserGPU {
-- 
2.45.2

[PATCH v16 04/13] virtio-gpu: Handle virtio_gpu_virgl_init() failure

[Dmitry Osipenko]

virtio_gpu_virgl_init() may fail, leading to a further Qemu crash
because Qemu assumes it never fails. Check virtio_gpu_virgl_init()
return code and don't execute virtio commands on error. Failed
virtio_gpu_virgl_init() will result in a timed out virtio commands
for a guest OS.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-gl.c     | 30 ++++++++++++++++++++++--------
 include/hw/virtio/virtio-gpu.h | 11 +++++++++--
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfbfc..21a1e9a05c5d 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -29,9 +29,14 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
                                              struct virtio_gpu_scanout *s,
                                              uint32_t resource_id)
 {
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
     uint32_t width, height;
     uint32_t pixels, *data;
 
+    if (gl->renderer_state != RS_INITED) {
+        return;
+    }
+
     data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
     if (!data) {
         return;
@@ -65,13 +70,22 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         return;
     }
 
-    if (!gl->renderer_inited) {
-        virtio_gpu_virgl_init(g);
-        gl->renderer_inited = true;
-    }
-    if (gl->renderer_reset) {
-        gl->renderer_reset = false;
+    switch (gl->renderer_state) {
+    case RS_RESET:
         virtio_gpu_virgl_reset(g);
+        /* fallthrough */
+    case RS_START:
+        if (virtio_gpu_virgl_init(g)) {
+            gl->renderer_state = RS_INIT_FAILED;
+            return;
+        }
+
+        gl->renderer_state = RS_INITED;
+        break;
+    case RS_INIT_FAILED:
+        return;
+    case RS_INITED:
+        break;
     }
 
     cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
@@ -98,9 +112,9 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
      * GL functions must be called with the associated GL context in main
      * thread, and when the renderer is unblocked.
      */
-    if (gl->renderer_inited && !gl->renderer_reset) {
+    if (gl->renderer_state == RS_INITED) {
         virtio_gpu_virgl_reset_scanout(g);
-        gl->renderer_reset = true;
+        gl->renderer_state = RS_RESET;
     }
 }
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7ff989a45a5c..6e71d799e5da 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -224,11 +224,18 @@ struct VirtIOGPUClass {
                              Error **errp);
 };
 
+/* VirtIOGPUGL renderer states */
+typedef enum {
+    RS_START,       /* starting state */
+    RS_INIT_FAILED, /* failed initialisation */
+    RS_INITED,      /* initialised and working */
+    RS_RESET,       /* inited and reset pending, moves to start after reset */
+} RenderState;
+
 struct VirtIOGPUGL {
     struct VirtIOGPU parent_obj;
 
-    bool renderer_inited;
-    bool renderer_reset;
+    RenderState renderer_state;
 
     QEMUTimer *fence_poll;
     QEMUTimer *print_stats;
-- 
2.45.2

[PATCH v16 05/13] virtio-gpu: Unrealize GL device

[Dmitry Osipenko]

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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 21a1e9a05c5d..0109244276fc 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -150,6 +150,22 @@ 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_state >= RS_INITED) {
+        if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+            timer_free(gl->print_stats);
+        }
+        timer_free(gl->fence_poll);
+        virgl_renderer_cleanup(NULL);
+    }
+
+    gl->renderer_state = RS_START;
+}
+
 static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -163,6 +179,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);
 }
-- 
2.45.2

[PATCH v16 06/13] virtio-gpu: Use pkgconfig version to decide which virgl features are available

[Dmitry Osipenko]

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.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 2 +-
 meson.build                   | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index a63d1f540f04..ca6f4d6cbb58 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -171,7 +171,7 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
         struct virgl_renderer_resource_info info;
         void *d3d_tex2d = NULL;
 
-#ifdef HAVE_VIRGL_D3D_INFO_EXT
+#if VIRGL_VERSION_MAJOR >= 1
         struct virgl_renderer_resource_info_ext ext;
         memset(&ext, 0, sizeof(ext));
         ret = virgl_renderer_resource_get_info_ext(ss.resource_id, &ext);
diff --git a/meson.build b/meson.build
index 97e00d6f59b8..838d08ef0f9b 100644
--- a/meson.build
+++ b/meson.build
@@ -2329,10 +2329,7 @@ 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))
+  config_host_data.set('VIRGL_VERSION_MAJOR', virgl.version().split('.')[0])
 endif
 config_host_data.set('CONFIG_VIRTFS', have_virtfs)
 config_host_data.set('CONFIG_VTE', vte.found())
-- 
2.45.2

[PATCH v16 07/13] virtio-gpu: Support context-init feature with virglrenderer

[Dmitry Osipenko]

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 ++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 0109244276fc..4fe9e6a0c21c 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -141,6 +141,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);
 
+#if VIRGL_VERSION_MAJOR >= 1
+    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 ca6f4d6cbb58..b3aa444bcfa5 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;
+        }
+
+#if VIRGL_VERSION_MAJOR >= 1
+        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,
-- 
2.45.2

[PATCH v16 08/13] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled

[Dmitry Osipenko]

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 602952a7041b..40a9d089710c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1485,6 +1485,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.45.2

[PATCH v16 09/13] virtio-gpu: Add virgl resource management

[Dmitry Osipenko]

From: Huang Rui <ray.huang@amd.com>

In a preparation to adding host blobs support to virtio-gpu, add virgl
resource management that allows to retrieve resource based on its ID
and virgl resource wrapper on top of simple resource that will be contain
fields specific to virgl.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Antonio Caggiano <quic_acaggian@quicinc.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 76 +++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b3aa444bcfa5..3ffea478e723 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -22,6 +22,23 @@
 
 #include <virglrenderer.h>
 
+struct virtio_gpu_virgl_resource {
+    struct virtio_gpu_simple_resource base;
+};
+
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
+{
+    struct virtio_gpu_simple_resource *res;
+
+    res = virtio_gpu_find_resource(g, resource_id);
+    if (!res) {
+        return NULL;
+    }
+
+    return container_of(res, struct virtio_gpu_virgl_resource, base);
+}
+
 #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
 static void *
 virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
@@ -35,11 +52,34 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 {
     struct virtio_gpu_resource_create_2d c2d;
     struct virgl_renderer_resource_create_args args;
+    struct virtio_gpu_virgl_resource *res;
 
     VIRTIO_GPU_FILL_CMD(c2d);
     trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
                                        c2d.width, c2d.height);
 
+    if (c2d.resource_id == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+                      __func__);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = virtio_gpu_virgl_find_resource(g, c2d.resource_id);
+    if (res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+                      __func__, c2d.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = g_new0(struct virtio_gpu_virgl_resource, 1);
+    res->base.width = c2d.width;
+    res->base.height = c2d.height;
+    res->base.format = c2d.format;
+    res->base.resource_id = c2d.resource_id;
+    QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
     args.handle = c2d.resource_id;
     args.target = 2;
     args.format = c2d.format;
@@ -59,11 +99,34 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 {
     struct virtio_gpu_resource_create_3d c3d;
     struct virgl_renderer_resource_create_args args;
+    struct virtio_gpu_virgl_resource *res;
 
     VIRTIO_GPU_FILL_CMD(c3d);
     trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
                                        c3d.width, c3d.height, c3d.depth);
 
+    if (c3d.resource_id == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+                      __func__);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = virtio_gpu_virgl_find_resource(g, c3d.resource_id);
+    if (res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+                      __func__, c3d.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = g_new0(struct virtio_gpu_virgl_resource, 1);
+    res->base.width = c3d.width;
+    res->base.height = c3d.height;
+    res->base.format = c3d.format;
+    res->base.resource_id = c3d.resource_id;
+    QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
     args.handle = c3d.resource_id;
     args.target = c3d.target;
     args.format = c3d.format;
@@ -82,12 +145,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
                                      struct virtio_gpu_ctrl_command *cmd)
 {
     struct virtio_gpu_resource_unref unref;
+    struct virtio_gpu_virgl_resource *res;
     struct iovec *res_iovs = NULL;
     int num_iovs = 0;
 
     VIRTIO_GPU_FILL_CMD(unref);
     trace_virtio_gpu_cmd_res_unref(unref.resource_id);
 
+    res = virtio_gpu_virgl_find_resource(g, unref.resource_id);
+    if (!res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+                      __func__, unref.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
     virgl_renderer_resource_detach_iov(unref.resource_id,
                                        &res_iovs,
                                        &num_iovs);
@@ -95,6 +167,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
         virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
     }
     virgl_renderer_resource_unref(unref.resource_id);
+
+    QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+    g_free(res);
 }
 
 static void virgl_cmd_context_create(VirtIOGPU *g,
-- 
2.45.2

[PATCH v16 10/13] virtio-gpu: Support suspension of commands processing

[Dmitry Osipenko]

Check whether command processing has been finished; otherwise, stop
processing commands and retry the command again next time. This allows
us to support asynchronous execution of non-fenced commands needed for
unmapping host blobs safely.

Suggested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/trace-events | 1 +
 hw/display/virtio-gpu.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index e212710284ae..d26d663f9638 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -55,6 +55,7 @@ virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type
 virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
 virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u"
 virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u"
+virtio_gpu_cmd_suspended(uint32_t cmd) "cmd 0x%x"
 
 # qxl.c
 disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 40a9d089710c..b348805fbaee 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,12 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
         /* process command */
         vgc->process_cmd(g, cmd);
 
+        /* command suspended */
+        if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
+            trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
+            break;
+        }
+
         QTAILQ_REMOVE(&g->cmdq, cmd, next);
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
             g->stats.requests++;
-- 
2.45.2

[PATCH v16 11/13] virtio-gpu: Handle resource blob commands

[Dmitry Osipenko]

From: Robert Beckett <bob.beckett@collabora.com>

Support BLOB resources creation, mapping, unmapping and set-scanout 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: Robert Beckett <bob.beckett@collabora.com> # added set_scanout_blob
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-gl.c     |   3 +
 hw/display/virtio-gpu-virgl.c  | 414 ++++++++++++++++++++++++++++++++-
 hw/display/virtio-gpu.c        |  18 +-
 include/hw/virtio/virtio-gpu.h |   9 +
 4 files changed, 434 insertions(+), 10 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 4fe9e6a0c21c..5f27568d3ec8 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -160,6 +160,9 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
     VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
 
     if (gl->renderer_state >= RS_INITED) {
+#if VIRGL_VERSION_MAJOR >= 1
+        qemu_bh_delete(gl->cmdq_resume_bh);
+#endif
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
             timer_free(gl->print_stats);
         }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 3ffea478e723..b2f4e215a7ad 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"
 
@@ -24,6 +26,7 @@
 
 struct virtio_gpu_virgl_resource {
     struct virtio_gpu_simple_resource base;
+    MemoryRegion *mr;
 };
 
 static struct virtio_gpu_virgl_resource *
@@ -47,6 +50,145 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
 }
 #endif
 
+#if VIRGL_VERSION_MAJOR >= 1
+struct virtio_gpu_virgl_hostmem_region {
+    MemoryRegion mr;
+    struct VirtIOGPU *g;
+    bool finish_unmapping;
+};
+
+static struct virtio_gpu_virgl_hostmem_region *
+to_hostmem_region(MemoryRegion *mr)
+{
+    return container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+}
+
+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;
+    VirtIOGPUGL *gl;
+
+    vmr = to_hostmem_region(mr);
+    vmr->finish_unmapping = true;
+
+    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.
+     */
+    gl = VIRTIO_GPU_GL(vmr->g);
+    qemu_bh_schedule(gl->cmdq_resume_bh);
+}
+
+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->g = g;
+
+    mr = &vmr->mr;
+    memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+    memory_region_add_subregion(&b->hostmem, offset, mr);
+    memory_region_set_enabled(mr, true);
+
+    /*
+     * MR could outlive the resource if MR's reference is held outside of
+     * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+     * and thus, making the data pointer invalid, we will block virtio-gpu
+     * command processing until MR is fully unreferenced and freed.
+     */
+    OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+    res->mr = mr;
+
+    return 0;
+}
+
+static int
+virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
+                                     struct virtio_gpu_virgl_resource *res,
+                                     bool *cmd_suspended)
+{
+    struct virtio_gpu_virgl_hostmem_region *vmr;
+    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+    MemoryRegion *mr = res->mr;
+    int ret;
+
+    if (!mr) {
+        return 0;
+    }
+
+    vmr = to_hostmem_region(res->mr);
+
+    /*
+     * Perform async unmapping in 3 steps:
+     *
+     * 1. Begin async unmapping with memory_region_del_subregion()
+     *    and suspend/block cmd processing.
+     * 2. Wait for res->mr to be freed and cmd processing resumed
+     *    asynchronously by virtio_gpu_virgl_hostmem_region_free().
+     * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
+     */
+    if (vmr->finish_unmapping) {
+        res->mr = NULL;
+        g_free(vmr);
+
+        ret = virgl_renderer_resource_unmap(res->base.resource_id);
+        if (ret) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: failed to unmap virgl resource: %s\n",
+                          __func__, strerror(-ret));
+            return ret;
+        }
+    } else {
+        *cmd_suspended = true;
+
+        /* render will be unblocked once MR is freed */
+        b->renderer_blocked++;
+
+        /* memory region owns self res->mr object and frees it by itself */
+        memory_region_set_enabled(mr, false);
+        memory_region_del_subregion(&b->hostmem, mr);
+        object_unparent(OBJECT(mr));
+    }
+
+    return 0;
+}
+#endif
+
 static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
                                          struct virtio_gpu_ctrl_command *cmd)
 {
@@ -78,6 +220,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 +268,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;
@@ -142,7 +286,8 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 }
 
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
-                                     struct virtio_gpu_ctrl_command *cmd)
+                                     struct virtio_gpu_ctrl_command *cmd,
+                                     bool *cmd_suspended)
 {
     struct virtio_gpu_resource_unref unref;
     struct virtio_gpu_virgl_resource *res;
@@ -160,6 +305,16 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
         return;
     }
 
+#if VIRGL_VERSION_MAJOR >= 1
+    if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended)) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+    if (*cmd_suspended) {
+        return;
+    }
+#endif
+
     virgl_renderer_resource_detach_iov(unref.resource_id,
                                        &res_iovs,
                                        &num_iovs);
@@ -509,9 +664,241 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
     g_free(resp);
 }
 
+#if VIRGL_VERSION_MAJOR >= 1
+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 };
+    g_autofree struct virtio_gpu_virgl_resource *res = NULL;
+    struct virtio_gpu_resource_create_blob cblob;
+    struct virgl_renderer_resource_info info;
+    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) {
+            cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+            return;
+        }
+    }
+
+    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;
+        virtio_gpu_cleanup_mapping(g, &res->base);
+        return;
+    }
+
+    ret = virgl_renderer_resource_get_info(cblob.resource_id, &info);
+    if (ret) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: resource does not have info %d: %s\n",
+                      __func__, cblob.resource_id, strerror(-ret));
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        virtio_gpu_cleanup_mapping(g, &res->base);
+        virgl_renderer_resource_unref(cblob.resource_id);
+        return;
+    }
+
+    res->base.dmabuf_fd = info.fd;
+
+    QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+    res = NULL;
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+                                        struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_resource_map_blob mblob;
+    struct virtio_gpu_virgl_resource *res;
+    struct virtio_gpu_resp_map_info resp;
+    int ret;
+
+    VIRTIO_GPU_FILL_CMD(mblob);
+    virtio_gpu_map_blob_bswap(&mblob);
+
+    res = virtio_gpu_virgl_find_resource(g, mblob.resource_id);
+    if (!res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+                      __func__, mblob.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    ret = virtio_gpu_virgl_map_resource_blob(g, res, mblob.offset);
+    if (ret) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+
+    memset(&resp, 0, sizeof(resp));
+    resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
+    virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info);
+    virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
+}
+
+static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
+                                          struct virtio_gpu_ctrl_command *cmd,
+                                          bool *cmd_suspended)
+{
+    struct virtio_gpu_resource_unmap_blob ublob;
+    struct virtio_gpu_virgl_resource *res;
+    int ret;
+
+    VIRTIO_GPU_FILL_CMD(ublob);
+    virtio_gpu_unmap_blob_bswap(&ublob);
+
+    res = virtio_gpu_virgl_find_resource(g, ublob.resource_id);
+    if (!res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+                      __func__, ublob.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
+    if (ret) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+}
+
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+                                       struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_framebuffer fb = { 0 };
+    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 (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_UNSPEC;
+        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
+
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
                                       struct virtio_gpu_ctrl_command *cmd)
 {
+    bool cmd_suspended = false;
+
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
     virgl_renderer_force_ctx_0();
@@ -553,7 +940,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         virgl_cmd_resource_flush(g, cmd);
         break;
     case VIRTIO_GPU_CMD_RESOURCE_UNREF:
-        virgl_cmd_resource_unref(g, cmd);
+        virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
         break;
     case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
         /* TODO add security */
@@ -575,12 +962,26 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
     case VIRTIO_GPU_CMD_GET_EDID:
         virtio_gpu_get_edid(g, cmd);
         break;
+#if VIRGL_VERSION_MAJOR >= 1
+    case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
+        virgl_cmd_resource_create_blob(g, cmd);
+        break;
+    case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
+        virgl_cmd_resource_map_blob(g, cmd);
+        break;
+    case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
+        virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
+        break;
+    case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
+        virgl_cmd_set_scanout_blob(g, cmd);
+        break;
+#endif
     default:
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         break;
     }
 
-    if (cmd->finished) {
+    if (cmd_suspended || cmd->finished) {
         return;
     }
     if (cmd->error) {
@@ -749,6 +1150,13 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
         timer_mod(gl->print_stats,
                   qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
     }
+
+#if VIRGL_VERSION_MAJOR >= 1
+    gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
+                                    virtio_gpu_virgl_resume_cmdq_bh,
+                                    g);
+#endif
+
     return 0;
 }
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index b348805fbaee..a5db2256a4bb 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;
@@ -1497,10 +1497,14 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
             return;
         }
 
+#ifdef VIRGL_VERSION_MAJOR
+    #if VIRGL_VERSION_MAJOR < 1
         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
+#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 6e71d799e5da..775005abb337 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -239,6 +239,8 @@ struct VirtIOGPUGL {
 
     QEMUTimer *fence_poll;
     QEMUTimer *print_stats;
+
+    QEMUBH *cmdq_resume_bh;
 };
 
 struct VhostUserGPU {
@@ -338,6 +340,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);
-- 
2.45.2

[PATCH v16 12/13] virtio-gpu: Register capsets dynamically

[Dmitry Osipenko]

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.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 5f27568d3ec8..20a7c316bb23 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -138,8 +138,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;
 
 #if VIRGL_VERSION_MAJOR >= 1
     g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
@@ -171,6 +171,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
     }
 
     gl->renderer_state = RS_START;
+
+    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 b2f4e215a7ad..5a881c58a11d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -622,19 +622,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));
@@ -1160,12 +1154,27 @@ 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;
 }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 775005abb337..83232f4b4bfa 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -209,6 +209,8 @@ struct VirtIOGPU {
         QTAILQ_HEAD(, VGPUDMABuf) bufs;
         VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
     } dmabuf;
+
+    GArray *capset_ids;
 };
 
 struct VirtIOGPUClass {
@@ -354,6 +356,6 @@ 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);
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
 
 #endif
-- 
2.45.2

[PATCH v16 13/13] virtio-gpu: Support Venus context

[Dmitry Osipenko]

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>
---
 docs/system/devices/virtio-gpu.rst | 11 +++++++++++
 hw/display/virtio-gpu-gl.c         |  2 ++
 hw/display/virtio-gpu-virgl.c      | 22 ++++++++++++++++++----
 hw/display/virtio-gpu.c            | 15 +++++++++++++++
 include/hw/virtio/virtio-gpu.h     |  3 +++
 5 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/docs/system/devices/virtio-gpu.rst b/docs/system/devices/virtio-gpu.rst
index cb73dd799858..b7eb0fc0e727 100644
--- a/docs/system/devices/virtio-gpu.rst
+++ b/docs/system/devices/virtio-gpu.rst
@@ -71,6 +71,17 @@ representation back to OpenGL API calls.
 .. _Gallium3D: https://www.freedesktop.org/wiki/Software/gallium/
 .. _virglrenderer: https://gitlab.freedesktop.org/virgl/virglrenderer/
 
+Translation of Vulkan API calls is supported since release of `virglrenderer`_
+v1.0.0 using `venus`_ protocol. ``Venus`` virtio-gpu capability set ("capset")
+requires host blob support (``hostmem`` and ``blob`` fields) and should
+be enabled using ``venus`` field. The ``hostmem`` field specifies the size
+of virtio-gpu host memory window. This is typically between 256M and 8G.
+
+.. parsed-literal::
+    -device virtio-gpu-gl,hostmem=8G,blob=true,venus=true
+
+.. _venus: https://gitlab.freedesktop.org/virgl/venus-protocol/
+
 virtio-gpu rutabaga
 -------------------
 
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 20a7c316bb23..9be452547322 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -151,6 +151,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 5a881c58a11d..eedae7357f1a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1128,6 +1128,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
         flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
     }
 #endif
+#if VIRGL_VERSION_MAJOR >= 1
+    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) {
@@ -1161,7 +1166,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));
@@ -1170,11 +1175,20 @@ 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 a5db2256a4bb..50b5634af13f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1507,6 +1507,21 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 #endif
     }
 
+    if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+#ifdef VIRGL_VERSION_MAJOR
+    #if VIRGL_VERSION_MAJOR >= 1
+        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
+#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 83232f4b4bfa..230fa0c4ee0a 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;
-- 
2.45.2

Re: [PATCH v16 01/13] virtio-gpu: Use trace events for tracking number of in-flight fences

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> Replace printf's used for tracking of in-flight fence inc/dec events
> with tracing, for consistency with the rest of virtio-gpu code that
> uses tracing.
> 
> Suggested-by: Marc-André Lureau <marcandre.lureau@gmail.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 02/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh
> that are used only by GL device.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 03/13] virtio-gpu: Move print_stats timer to VirtIOGPUGL

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> Move print_stats timer to VirtIOGPUGL for consistency with
> cmdq_resume_bh and fence_poll that are used only by GL device.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 04/13] virtio-gpu: Handle virtio_gpu_virgl_init() failure

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> virtio_gpu_virgl_init() may fail, leading to a further Qemu crash
> because Qemu assumes it never fails. Check virtio_gpu_virgl_init()
> return code and don't execute virtio commands on error. Failed
> virtio_gpu_virgl_init() will result in a timed out virtio commands
> for a guest OS.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 05/13] virtio-gpu: Unrealize GL device

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> Even though GL GPU doesn't support hotplugging today, free virgl
> resources when GL device is unrealized. For consistency.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com

Re: [PATCH v16 06/13] virtio-gpu: Use pkgconfig version to decide which virgl features are available

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko 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.
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 07/13] virtio-gpu: Support context-init feature with virglrenderer

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko 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>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 08/13] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> 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>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 09/13] virtio-gpu: Add virgl resource management

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko 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: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 10/13] virtio-gpu: Support suspension of commands processing

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> Check whether command processing has been finished; otherwise, stop
> processing commands and retry the command again next time. This allows
> us to support asynchronous execution of non-fenced commands needed for
> unmapping host blobs safely.
> 
> Suggested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 11/13] virtio-gpu: Handle resource blob commands

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> From: Robert Beckett <bob.beckett@collabora.com>
> 
> Support BLOB resources creation, mapping, unmapping and set-scanout 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: Robert Beckett <bob.beckett@collabora.com> # added set_scanout_blob
> 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>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 12/13] virtio-gpu: Register capsets dynamically

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> 
> virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
> assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
> like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 13/13] virtio-gpu: Support Venus context

[Akihiko Odaki]

On 2024/06/24 0:23, Dmitry Osipenko wrote:
> 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>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 14836 bytes --]

Hi,

All R-b now, it looks good to merge. Thanks for all the effort!

Michael, are you taking it through your tree?

On Sun, Jun 23, 2024 at 7:23 PM Dmitry Osipenko <
dmitry.osipenko@collabora.com> wrote:

> 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:
>
>   qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true
> \
>       -machine q35,accel=kvm,memory-backend=mem1 \
>       -object memory-backend-memfd,id=mem1,size=8G -m 8G
>
>
> Changes from V15 to V16
>
> - Fixed resource_get_info() change made in v15 that was squshed into a
>   wrong patch. Squashed set_scanout_blob patch into the blob-commands
> patch,
>   otherwise we'll need to revert back ordering of blob patches to older v7
>   variant.
>
> - Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.
>
> - Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
>   with the doc addition. Suggested by Akihiko Odaki.
>
> Changes from V14 to V15
>
> - Dropped hostmem mapping state that got unused in v14, suggested by
>   Akihiko Odaki.
>
> - Moved resource_get_info() from set_scanout_blob() to create_blob(),
>   suggested by Akihiko Odaki.
>
> - Fixed unitilized variable in create_blob(), spotted by Alex Bennée.
>
> Changes from V13 to V14
>
> - Fixed erronous fall-through in renderer_state's switch-case that was
>   spotted by Marc-André Lureau.
>
> - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
>   Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.
>
> - Made use of g_autofree in virgl_cmd_resource_create_blob() as was
>   suggested by Akihiko Odaki.
>
> - Removed virtio_gpu_virgl_deinit() and moved all deinit code to
>   virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.
>
> - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
>   check as was suggested by Marc-André Lureau.
>
> - Added trace event for cmd-suspension as was suggested by Marc-André
> Lureau.
>
> - Added patch to replace in-flight printf's with trace events as was
>   suggested by Marc-André Lureau
>
> Changes from V12 to V13
>
> - Replaced `res->async_unmap_in_progress` flag with a mapping state,
>   moved it to the virtio_gpu_virgl_hostmem_region like was suggested
>   by Akihiko Odaki.
>
> - Renamed blob_unmap function and added back cmd_suspended argument
>   to it. Suggested by Akihiko Odaki.
>
> - Reordered VirtIOGPUGL refactoring patches to minimize code changes
>   like was suggested by Akihiko Odaki.
>
> - Replaced gl->renderer_inited with gl->renderer_state, like was suggested
>   by Alex Bennée.
>
> - Added gl->renderer state resetting to gl_device_unrealize(), for
>   consistency. Suggested by Alex Bennée.
>
> - Added rb's from Alex and Manos.
>
> - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.
>
> Changes from V11 to V12
>
> - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
>   corrupt resource list and releases resource properly on error. Thanks
>   to Akihiko Odaki for spotting the bug.
>
> - Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
>   fixing Qemu crash. Besides fixing the crash, it allows to implement
>   a cleaner virtio_gpu_virgl_deinit().
>
> - virtio_gpu_virgl_deinit() now assumes that previously virgl was
>   initialized successfully when it was inited at all. Suggested by
>   Akihiko Odaki.
>
> - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()
>
> - Added back blob unmapping or RESOURCE_UNREF that was requested
>   by Akihiko Odaki. Added comment to the code explaining how
>   async unmapping works. Added back `res->async_unmap_in_progress`
>   flag and added comment telling why it's needed.
>
> - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
>   suggested by Akihiko Odaki.
>
> - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
>   for consistency with cmdq_resume_bh.
>
> Changes from V10 to V11
>
> - Replaced cmd_resume bool in struct ctrl_command with
>   "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
>   by Akihiko Odaki.
>
> - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
>   the 'async_unmap_in_progress' flag that was dropped in v9:
>
>     1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
>        resource was previously mapped and lets virglrenderer to do the
>        checking.
>
>     2. error returned by virgl_renderer_resource_unmap() is now handled
>        and reported properly, previously the error wasn't checked. The
>        virgl_renderer_resource_unmap() fails if resource wasn't mapped.
>
>     3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
>        that is mapped, it's a error condition if guest didn't unmap
> resource
>        before doing the unref. Previously unref was implicitly unmapping
>        resource.
>
> 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 (1):
>   virtio-gpu: Support Venus context
>
> Dmitry Osipenko (8):
>   virtio-gpu: Use trace events for tracking number of in-flight fences
>   virtio-gpu: Move fence_poll timer to VirtIOGPUGL
>   virtio-gpu: Move print_stats timer to VirtIOGPUGL
>   virtio-gpu: Handle virtio_gpu_virgl_init() failure
>   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: Handle resource blob commands
>
>  docs/system/devices/virtio-gpu.rst |  11 +
>  hw/display/trace-events            |   3 +
>  hw/display/virtio-gpu-gl.c         |  62 ++-
>  hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
>  hw/display/virtio-gpu.c            |  44 ++-
>  include/hw/virtio/virtio-gpu.h     |  32 +-
>  meson.build                        |   5 +-
>  7 files changed, 685 insertions(+), 57 deletions(-)
>
> --
> 2.45.2
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 16982 bytes --]

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Michael S. Tsirkin]

On Mon, Jul 01, 2024 at 02:48:58PM +0400, Marc-André Lureau wrote:
> Hi,
> 
> All R-b now, it looks good to merge. Thanks for all the effort! 
> 
> Michael, are you taking it through your tree?


I could but did you intend to give your Reviewed-by?


> On Sun, Jun 23, 2024 at 7:23 PM Dmitry Osipenko <dmitry.osipenko@collabora.com>
> wrote:
> 
>     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:
> 
>       qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true
>     \
>           -machine q35,accel=kvm,memory-backend=mem1 \
>           -object memory-backend-memfd,id=mem1,size=8G -m 8G
> 
> 
>     Changes from V15 to V16
> 
>     - Fixed resource_get_info() change made in v15 that was squshed into a
>       wrong patch. Squashed set_scanout_blob patch into the blob-commands
>     patch,
>       otherwise we'll need to revert back ordering of blob patches to older v7
>       variant.
> 
>     - Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.
> 
>     - Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
>       with the doc addition. Suggested by Akihiko Odaki.
> 
>     Changes from V14 to V15
> 
>     - Dropped hostmem mapping state that got unused in v14, suggested by
>       Akihiko Odaki.
> 
>     - Moved resource_get_info() from set_scanout_blob() to create_blob(),
>       suggested by Akihiko Odaki.
> 
>     - Fixed unitilized variable in create_blob(), spotted by Alex Bennée.
> 
>     Changes from V13 to V14
> 
>     - Fixed erronous fall-through in renderer_state's switch-case that was
>       spotted by Marc-André Lureau.
> 
>     - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
>       Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.
> 
>     - Made use of g_autofree in virgl_cmd_resource_create_blob() as was
>       suggested by Akihiko Odaki.
> 
>     - Removed virtio_gpu_virgl_deinit() and moved all deinit code to
>       virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.
> 
>     - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
>       check as was suggested by Marc-André Lureau.
> 
>     - Added trace event for cmd-suspension as was suggested by Marc-André
>     Lureau.
> 
>     - Added patch to replace in-flight printf's with trace events as was
>       suggested by Marc-André Lureau
> 
>     Changes from V12 to V13
> 
>     - Replaced `res->async_unmap_in_progress` flag with a mapping state,
>       moved it to the virtio_gpu_virgl_hostmem_region like was suggested
>       by Akihiko Odaki.
> 
>     - Renamed blob_unmap function and added back cmd_suspended argument
>       to it. Suggested by Akihiko Odaki.
> 
>     - Reordered VirtIOGPUGL refactoring patches to minimize code changes
>       like was suggested by Akihiko Odaki.
> 
>     - Replaced gl->renderer_inited with gl->renderer_state, like was suggested
>       by Alex Bennée.
> 
>     - Added gl->renderer state resetting to gl_device_unrealize(), for
>       consistency. Suggested by Alex Bennée.
> 
>     - Added rb's from Alex and Manos.
> 
>     - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.
> 
>     Changes from V11 to V12
> 
>     - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
>       corrupt resource list and releases resource properly on error. Thanks
>       to Akihiko Odaki for spotting the bug.
> 
>     - Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
>       fixing Qemu crash. Besides fixing the crash, it allows to implement
>       a cleaner virtio_gpu_virgl_deinit().
> 
>     - virtio_gpu_virgl_deinit() now assumes that previously virgl was
>       initialized successfully when it was inited at all. Suggested by
>       Akihiko Odaki.
> 
>     - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()
> 
>     - Added back blob unmapping or RESOURCE_UNREF that was requested
>       by Akihiko Odaki. Added comment to the code explaining how
>       async unmapping works. Added back `res->async_unmap_in_progress`
>       flag and added comment telling why it's needed.
> 
>     - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
>       suggested by Akihiko Odaki.
> 
>     - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
>       for consistency with cmdq_resume_bh.
> 
>     Changes from V10 to V11
> 
>     - Replaced cmd_resume bool in struct ctrl_command with
>       "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
>       by Akihiko Odaki.
> 
>     - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
>       the 'async_unmap_in_progress' flag that was dropped in v9:
> 
>         1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
>            resource was previously mapped and lets virglrenderer to do the
>            checking.
> 
>         2. error returned by virgl_renderer_resource_unmap() is now handled
>            and reported properly, previously the error wasn't checked. The
>            virgl_renderer_resource_unmap() fails if resource wasn't mapped.
> 
>         3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
>            that is mapped, it's a error condition if guest didn't unmap
>     resource
>            before doing the unref. Previously unref was implicitly unmapping
>            resource.
> 
>     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 (1):
>       virtio-gpu: Support Venus context
> 
>     Dmitry Osipenko (8):
>       virtio-gpu: Use trace events for tracking number of in-flight fences
>       virtio-gpu: Move fence_poll timer to VirtIOGPUGL
>       virtio-gpu: Move print_stats timer to VirtIOGPUGL
>       virtio-gpu: Handle virtio_gpu_virgl_init() failure
>       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: Handle resource blob commands
> 
>      docs/system/devices/virtio-gpu.rst |  11 +
>      hw/display/trace-events            |   3 +
>      hw/display/virtio-gpu-gl.c         |  62 ++-
>      hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
>      hw/display/virtio-gpu.c            |  44 ++-
>      include/hw/virtio/virtio-gpu.h     |  32 +-
>      meson.build                        |   5 +-
>      7 files changed, 685 insertions(+), 57 deletions(-)
> 
>     --
>     2.45.2
> 
> 
> 
> 
> --
> Marc-André Lureau

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Michael S. Tsirkin]

On Sun, Jun 23, 2024 at 06:23:30PM +0300, Dmitry Osipenko wrote:
> 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.


I'd like to see a Tested-by from Alex Bennée for this -
he's been testing v14 last as far as I could see.


> [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:
> 
>   qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \
>       -machine q35,accel=kvm,memory-backend=mem1 \
>       -object memory-backend-memfd,id=mem1,size=8G -m 8G
> 
> 
> Changes from V15 to V16
> 
> - Fixed resource_get_info() change made in v15 that was squshed into a
>   wrong patch. Squashed set_scanout_blob patch into the blob-commands patch,
>   otherwise we'll need to revert back ordering of blob patches to older v7
>   variant.
> 
> - Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.
> 
> - Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
>   with the doc addition. Suggested by Akihiko Odaki.
> 
> Changes from V14 to V15
> 
> - Dropped hostmem mapping state that got unused in v14, suggested by
>   Akihiko Odaki.
> 
> - Moved resource_get_info() from set_scanout_blob() to create_blob(),
>   suggested by Akihiko Odaki.
> 
> - Fixed unitilized variable in create_blob(), spotted by Alex Bennée.
> 
> Changes from V13 to V14
> 
> - Fixed erronous fall-through in renderer_state's switch-case that was
>   spotted by Marc-André Lureau.
> 
> - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
>   Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.
> 
> - Made use of g_autofree in virgl_cmd_resource_create_blob() as was
>   suggested by Akihiko Odaki.
> 
> - Removed virtio_gpu_virgl_deinit() and moved all deinit code to
>   virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.
> 
> - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
>   check as was suggested by Marc-André Lureau.
> 
> - Added trace event for cmd-suspension as was suggested by Marc-André Lureau.
> 
> - Added patch to replace in-flight printf's with trace events as was
>   suggested by Marc-André Lureau
> 
> Changes from V12 to V13
> 
> - Replaced `res->async_unmap_in_progress` flag with a mapping state,
>   moved it to the virtio_gpu_virgl_hostmem_region like was suggested
>   by Akihiko Odaki.
> 
> - Renamed blob_unmap function and added back cmd_suspended argument
>   to it. Suggested by Akihiko Odaki.
> 
> - Reordered VirtIOGPUGL refactoring patches to minimize code changes
>   like was suggested by Akihiko Odaki.
> 
> - Replaced gl->renderer_inited with gl->renderer_state, like was suggested
>   by Alex Bennée.
> 
> - Added gl->renderer state resetting to gl_device_unrealize(), for
>   consistency. Suggested by Alex Bennée.
> 
> - Added rb's from Alex and Manos.
> 
> - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.
> 
> Changes from V11 to V12
> 
> - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
>   corrupt resource list and releases resource properly on error. Thanks
>   to Akihiko Odaki for spotting the bug.
> 
> - Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
>   fixing Qemu crash. Besides fixing the crash, it allows to implement
>   a cleaner virtio_gpu_virgl_deinit().
> 
> - virtio_gpu_virgl_deinit() now assumes that previously virgl was
>   initialized successfully when it was inited at all. Suggested by
>   Akihiko Odaki.
> 
> - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()
> 
> - Added back blob unmapping or RESOURCE_UNREF that was requested
>   by Akihiko Odaki. Added comment to the code explaining how
>   async unmapping works. Added back `res->async_unmap_in_progress`
>   flag and added comment telling why it's needed.
> 
> - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
>   suggested by Akihiko Odaki.
> 
> - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
>   for consistency with cmdq_resume_bh.
> 
> Changes from V10 to V11
> 
> - Replaced cmd_resume bool in struct ctrl_command with
>   "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
>   by Akihiko Odaki.
> 
> - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
>   the 'async_unmap_in_progress' flag that was dropped in v9:
> 
>     1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
>        resource was previously mapped and lets virglrenderer to do the
>        checking.
> 
>     2. error returned by virgl_renderer_resource_unmap() is now handled
>        and reported properly, previously the error wasn't checked. The
>        virgl_renderer_resource_unmap() fails if resource wasn't mapped.
> 
>     3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
>        that is mapped, it's a error condition if guest didn't unmap resource
>        before doing the unref. Previously unref was implicitly unmapping
>        resource.
> 
> 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 (1):
>   virtio-gpu: Support Venus context
> 
> Dmitry Osipenko (8):
>   virtio-gpu: Use trace events for tracking number of in-flight fences
>   virtio-gpu: Move fence_poll timer to VirtIOGPUGL
>   virtio-gpu: Move print_stats timer to VirtIOGPUGL
>   virtio-gpu: Handle virtio_gpu_virgl_init() failure
>   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: Handle resource blob commands
> 
>  docs/system/devices/virtio-gpu.rst |  11 +
>  hw/display/trace-events            |   3 +
>  hw/display/virtio-gpu-gl.c         |  62 ++-
>  hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
>  hw/display/virtio-gpu.c            |  44 ++-
>  include/hw/virtio/virtio-gpu.h     |  32 +-
>  meson.build                        |   5 +-
>  7 files changed, 685 insertions(+), 57 deletions(-)
> 
> -- 
> 2.45.2

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Alex Bennée]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Sun, Jun 23, 2024 at 06:23:30PM +0300, Dmitry Osipenko wrote:
>> 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.
>
>
> I'd like to see a Tested-by from Alex Bennée for this -
> he's been testing v14 last as far as I could see.

For the record I have been testing v16 but so far I'm seeing the same
problems. Broadly:

Guest: Debian Trixie (aarch64 and x86_64) + custom build mesa
Host: Debian Bookworm (x86 with Intel graphics) or Trixie (aarch64 with
AMD graphics) with a small patch to use a vendored virglrenderer:

  Message-Id: <20240605133527.529950-1-alex.bennee@linaro.org>
  Date: Wed,  5 Jun 2024 14:35:27 +0100
  Subject: [RFC PATCH] subprojects: add a wrapper for libvirglrenderer
  From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee@linaro.org>  

which is currently full of debug fprintf's as I've tried to isolate
stuff.

Running vkcube will draw a frame (aarch64 guest) or a black square
(x86_64 guest) and then hang, reporting "stuck in fence wait with iter
at.." in the terminal once vkcube is killed.

The testing on my Aarch64 guests has been run with a manually booted
weston from the framebuffer console. The x86 case was run inside a gnome
session (which was using system mesa) as I was unable to get the
framebuffer console working.

My x86 host is running 6.9.x but currently doesn't have the TTM patch
(as I didn't think it was needed for Intel) however I will apply it and
test next week when I'm back next to the physical machine.

I've got a fairly scratch branch based on v16 with a bunch of additional
debug printfs which showed some funkiness with the mappings:

  virgl_gbm_gpu_import_required: a -> import required
  virgl_gbm_gpu_import_required: a -> import required
  virgl_gbm_gpu_import_required: a -> import required
  virgl_gbm_gpu_import_required: 1 -> import required
  vrend_renderer_resource_unmap: invalid bits 0x4b
  virgl_renderer_resource_unmap: unexpected ret = -22, pipe:0x56265f58d0e0 fd_type:0
  virtio_gpu_virgl_unmap_resource_blob: failed to unmap virgl resource: Invalid argument
  virtio_gpu_virgl_process_cmd: ctrl 0x209, error 0x1200
  [  159.298765] [drm:virtio_gpu_dequeue_ctrl_func] *ERROR* response 0x1200 (command 0x209)
  vrend_renderer_resource_unmap: invalid bits 0x4b
  virgl_renderer_resource_unmap: unexpected ret = -22, pipe:0x56265cfce470 fd_type:0
  virtio_gpu_virgl_unmap_resource_blob: failed to unmap virgl resource: Invalid argument
  virtio_gpu_virgl_process_cmd: ctrl 0x209, error 0x1200
  [  159.400565] [drm:virtio_gpu_dequeue_ctrl_func] *ERROR* response 0x1200 (command 0x209)
  vrend_renderer_resource_unmap: invalid bits 0x4b
  virgl_renderer_resource_unmap: unexpected ret = -22, pipe:0x56265dc596b0 fd_type:0
  virtio_gpu_virgl_unmap_resource_blob: failed to unmap virgl resource: Invalid argument
  virtio_gpu_virgl_process_cmd: ctrl 0x209, error 0x1200
  [  159.502414] [drm:virtio_gpu_dequeue_ctrl_func] *ERROR* response 0x1200 (command 0x209)
  vrend_renderer_resource_unmap: invalid bits 0x4b
  virgl_renderer_resource_unmap: unexpected ret = -22, pipe:0x56265f2256c0 fd_type:0
  virtio_gpu_virgl_unmap_resource_blob: failed to unmap virgl resource: Invalid argument
  virtio_gpu_virgl_process_cmd: ctrl 0x209, error 0x1200
  [  159.603818] [drm:virtio_gpu_dequeue_ctrl_func] *ERROR* response 0x1200 (command 0x209)
  vrend_renderer_resource_unmap: invalid bits 0x4b
  virgl_renderer_resource_unmap: unexpected ret = -22, pipe:0x56265dc594d0 fd_type:0
  virtio_gpu_virgl_unmap_resource_blob: failed to unmap virgl resource: Invalid argument
  virtio_gpu_virgl_process_cmd: ctrl 0x209, error 0x1200
  virgl_gbm_gpu_import_required: a -> import required
  virgl_gbm_gpu_import_required: 2 -> import required
  [  159.705386] [drm:virtio_gpu_dequeue_ctrl_func] *ERROR* response 0x1200 (command 0x209)

but so far I wasn't able to track where the resources were originally
mapped. I did a mini-debug session with Dimity last week but we weren't
able to get anything useful. I won't be able to get back to testing this
until next week.

I also intend to test with vhost-user-gpu and the currently brewing
vhost-device branch:

  https://github.com/rust-vmm/vhost-device/pull/668

to see if I can isolate these as virglrender issues or QEMU memory
mapping issues.

>
>
>> [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:
>> 
>>   qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \
>>       -machine q35,accel=kvm,memory-backend=mem1 \
>>       -object memory-backend-memfd,id=mem1,size=8G -m 8G
>> 
>> 
>> Changes from V15 to V16
>> 
>> - Fixed resource_get_info() change made in v15 that was squshed into a
>>   wrong patch. Squashed set_scanout_blob patch into the blob-commands patch,
>>   otherwise we'll need to revert back ordering of blob patches to older v7
>>   variant.
>> 
>> - Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.
>> 
>> - Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
>>   with the doc addition. Suggested by Akihiko Odaki.
>> 
>> Changes from V14 to V15
>> 
>> - Dropped hostmem mapping state that got unused in v14, suggested by
>>   Akihiko Odaki.
>> 
>> - Moved resource_get_info() from set_scanout_blob() to create_blob(),
>>   suggested by Akihiko Odaki.
>> 
>> - Fixed unitilized variable in create_blob(), spotted by Alex Bennée.
>> 
>> Changes from V13 to V14
>> 
>> - Fixed erronous fall-through in renderer_state's switch-case that was
>>   spotted by Marc-André Lureau.
>> 
>> - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
>>   Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.
>> 
>> - Made use of g_autofree in virgl_cmd_resource_create_blob() as was
>>   suggested by Akihiko Odaki.
>> 
>> - Removed virtio_gpu_virgl_deinit() and moved all deinit code to
>>   virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.
>> 
>> - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
>>   check as was suggested by Marc-André Lureau.
>> 
>> - Added trace event for cmd-suspension as was suggested by Marc-André Lureau.
>> 
>> - Added patch to replace in-flight printf's with trace events as was
>>   suggested by Marc-André Lureau
>> 
>> Changes from V12 to V13
>> 
>> - Replaced `res->async_unmap_in_progress` flag with a mapping state,
>>   moved it to the virtio_gpu_virgl_hostmem_region like was suggested
>>   by Akihiko Odaki.
>> 
>> - Renamed blob_unmap function and added back cmd_suspended argument
>>   to it. Suggested by Akihiko Odaki.
>> 
>> - Reordered VirtIOGPUGL refactoring patches to minimize code changes
>>   like was suggested by Akihiko Odaki.
>> 
>> - Replaced gl->renderer_inited with gl->renderer_state, like was suggested
>>   by Alex Bennée.
>> 
>> - Added gl->renderer state resetting to gl_device_unrealize(), for
>>   consistency. Suggested by Alex Bennée.
>> 
>> - Added rb's from Alex and Manos.
>> 
>> - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.
>> 
>> Changes from V11 to V12
>> 
>> - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
>>   corrupt resource list and releases resource properly on error. Thanks
>>   to Akihiko Odaki for spotting the bug.
>> 
>> - Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
>>   fixing Qemu crash. Besides fixing the crash, it allows to implement
>>   a cleaner virtio_gpu_virgl_deinit().
>> 
>> - virtio_gpu_virgl_deinit() now assumes that previously virgl was
>>   initialized successfully when it was inited at all. Suggested by
>>   Akihiko Odaki.
>> 
>> - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()
>> 
>> - Added back blob unmapping or RESOURCE_UNREF that was requested
>>   by Akihiko Odaki. Added comment to the code explaining how
>>   async unmapping works. Added back `res->async_unmap_in_progress`
>>   flag and added comment telling why it's needed.
>> 
>> - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
>>   suggested by Akihiko Odaki.
>> 
>> - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
>>   for consistency with cmdq_resume_bh.
>> 
>> Changes from V10 to V11
>> 
>> - Replaced cmd_resume bool in struct ctrl_command with
>>   "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
>>   by Akihiko Odaki.
>> 
>> - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
>>   the 'async_unmap_in_progress' flag that was dropped in v9:
>> 
>>     1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
>>        resource was previously mapped and lets virglrenderer to do the
>>        checking.
>> 
>>     2. error returned by virgl_renderer_resource_unmap() is now handled
>>        and reported properly, previously the error wasn't checked. The
>>        virgl_renderer_resource_unmap() fails if resource wasn't mapped.
>> 
>>     3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
>>        that is mapped, it's a error condition if guest didn't unmap resource
>>        before doing the unref. Previously unref was implicitly unmapping
>>        resource.
>> 
>> 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 (1):
>>   virtio-gpu: Support Venus context
>> 
>> Dmitry Osipenko (8):
>>   virtio-gpu: Use trace events for tracking number of in-flight fences
>>   virtio-gpu: Move fence_poll timer to VirtIOGPUGL
>>   virtio-gpu: Move print_stats timer to VirtIOGPUGL
>>   virtio-gpu: Handle virtio_gpu_virgl_init() failure
>>   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: Handle resource blob commands
>> 
>>  docs/system/devices/virtio-gpu.rst |  11 +
>>  hw/display/trace-events            |   3 +
>>  hw/display/virtio-gpu-gl.c         |  62 ++-
>>  hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
>>  hw/display/virtio-gpu.c            |  44 ++-
>>  include/hw/virtio/virtio-gpu.h     |  32 +-
>>  meson.build                        |   5 +-
>>  7 files changed, 685 insertions(+), 57 deletions(-)
>> 
>> -- 
>> 2.45.2

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v16 01/13] virtio-gpu: Use trace events for tracking number of in-flight fences

[Alex Bennée]

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> Replace printf's used for tracking of in-flight fence inc/dec events
> with tracing, for consistency with the rest of virtio-gpu code that
> uses tracing.
>
> Suggested-by: Marc-André Lureau <marcandre.lureau@gmail.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v16 02/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL

[Alex Bennée]

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh
> that are used only by GL device.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Alex Bennée]

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> 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:
>
>   qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \
>       -machine q35,accel=kvm,memory-backend=mem1 \
>       -object memory-backend-memfd,id=mem1,size=8G -m 8G

For the following profiles:

Host Setup:

  x86 host
  Intel Corporation Raptor Lake-S GT1 [UHD Graphics 770]
  Debian Bookworm
  Kernel 6.11.0-rc1-ajb-00146-gf690c27fbc92 (basically https://lore.kernel.org/lkml/20240726235234.228822-1-seanjc@google.com/ + UDMABUF enabled)
  Hand built Virglrenderer (main/4fc19d919f/v1.0.1)
  Hand built Mesa with Venus support (mesa-23.3.6)

x86 Guest Setup

  Current Trixie guest (as off 19/8/24)
  Kernel 6.11.0-rc1-ajb-00146-gf690c27fbc92
  Distro installed:
    weston
    vkcube-wayland
    vkmark

  QEMU: KVM guest with -device virtio-vga-gl,hostmem=4G,blob=on,venus=on

Aarch64 Guest Setup

  Current Trixie guest (as off 19/8/24)
  Kernel 6.11.0-rc1-ajb-00146-gf690c27fbc92
  Distro installed:
    weston
    vkcube-wayland
    vkmark

  + Hand built Mesa (012323a1d, enabled with meson devenv)

  QEMU: TCG Guest with -device virtio-gpu-gl-pci,hostmem=4G,venus=on,blob=on

Have a:

  Tested-by: Alex Bennée <alex.bennee@linaro.org>
  Acked-by: Alex Bennée <alex.bennee@linaro.org>

Next steps:

  - test on Arm AVA (x86 and Arm guests)
  - build some buildroot images with all the right deps for testing

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Alex Bennée]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Sun, Jun 23, 2024 at 06:23:30PM +0300, Dmitry Osipenko wrote:
>> 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.
>
>
> I'd like to see a Tested-by from Alex Bennée for this -
> he's been testing v14 last as far as I could see.

I've posted it on the top. Broadly x86 and Arm work on my x86 host. So
far my Arm host only runs the x86 image with the AMD card OK.

I've updated my test images so you can try them:

  https://fileserver.linaro.org/s/ce5jXBFinPxtEdx

See the notes on the readme on how to run.

>
>
>> [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:
>> 
>>   qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \
>>       -machine q35,accel=kvm,memory-backend=mem1 \
>>       -object memory-backend-memfd,id=mem1,size=8G -m 8G
>> 
>> 
>> Changes from V15 to V16
>> 
>> - Fixed resource_get_info() change made in v15 that was squshed into a
>>   wrong patch. Squashed set_scanout_blob patch into the blob-commands patch,
>>   otherwise we'll need to revert back ordering of blob patches to older v7
>>   variant.
>> 
>> - Changed hostmem mapping state to boolean, suggested by Akihiko Odaki.
>> 
>> - Documented Venus in virtio-gpu doc. Amended "Support Venus context" patch
>>   with the doc addition. Suggested by Akihiko Odaki.
>> 
>> Changes from V14 to V15
>> 
>> - Dropped hostmem mapping state that got unused in v14, suggested by
>>   Akihiko Odaki.
>> 
>> - Moved resource_get_info() from set_scanout_blob() to create_blob(),
>>   suggested by Akihiko Odaki.
>> 
>> - Fixed unitilized variable in create_blob(), spotted by Alex Bennée.
>> 
>> Changes from V13 to V14
>> 
>> - Fixed erronous fall-through in renderer_state's switch-case that was
>>   spotted by Marc-André Lureau.
>> 
>> - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by
>>   Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED.
>> 
>> - Made use of g_autofree in virgl_cmd_resource_create_blob() as was
>>   suggested by Akihiko Odaki.
>> 
>> - Removed virtio_gpu_virgl_deinit() and moved all deinit code to
>>   virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau.
>> 
>> - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR
>>   check as was suggested by Marc-André Lureau.
>> 
>> - Added trace event for cmd-suspension as was suggested by Marc-André Lureau.
>> 
>> - Added patch to replace in-flight printf's with trace events as was
>>   suggested by Marc-André Lureau
>> 
>> Changes from V12 to V13
>> 
>> - Replaced `res->async_unmap_in_progress` flag with a mapping state,
>>   moved it to the virtio_gpu_virgl_hostmem_region like was suggested
>>   by Akihiko Odaki.
>> 
>> - Renamed blob_unmap function and added back cmd_suspended argument
>>   to it. Suggested by Akihiko Odaki.
>> 
>> - Reordered VirtIOGPUGL refactoring patches to minimize code changes
>>   like was suggested by Akihiko Odaki.
>> 
>> - Replaced gl->renderer_inited with gl->renderer_state, like was suggested
>>   by Alex Bennée.
>> 
>> - Added gl->renderer state resetting to gl_device_unrealize(), for
>>   consistency. Suggested by Alex Bennée.
>> 
>> - Added rb's from Alex and Manos.
>> 
>> - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB.
>> 
>> Changes from V11 to V12
>> 
>> - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't
>>   corrupt resource list and releases resource properly on error. Thanks
>>   to Akihiko Odaki for spotting the bug.
>> 
>> - Added new patch that handles virtio_gpu_virgl_init() failure gracefully,
>>   fixing Qemu crash. Besides fixing the crash, it allows to implement
>>   a cleaner virtio_gpu_virgl_deinit().
>> 
>> - virtio_gpu_virgl_deinit() now assumes that previously virgl was
>>   initialized successfully when it was inited at all. Suggested by
>>   Akihiko Odaki.
>> 
>> - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit()
>> 
>> - Added back blob unmapping or RESOURCE_UNREF that was requested
>>   by Akihiko Odaki. Added comment to the code explaining how
>>   async unmapping works. Added back `res->async_unmap_in_progress`
>>   flag and added comment telling why it's needed.
>> 
>> - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes
>>   suggested by Akihiko Odaki.
>> 
>> - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL
>>   for consistency with cmdq_resume_bh.
>> 
>> Changes from V10 to V11
>> 
>> - Replaced cmd_resume bool in struct ctrl_command with
>>   "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested
>>   by Akihiko Odaki.
>> 
>> - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding
>>   the 'async_unmap_in_progress' flag that was dropped in v9:
>> 
>>     1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether
>>        resource was previously mapped and lets virglrenderer to do the
>>        checking.
>> 
>>     2. error returned by virgl_renderer_resource_unmap() is now handled
>>        and reported properly, previously the error wasn't checked. The
>>        virgl_renderer_resource_unmap() fails if resource wasn't mapped.
>> 
>>     3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource
>>        that is mapped, it's a error condition if guest didn't unmap resource
>>        before doing the unref. Previously unref was implicitly unmapping
>>        resource.
>> 
>> 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 (1):
>>   virtio-gpu: Support Venus context
>> 
>> Dmitry Osipenko (8):
>>   virtio-gpu: Use trace events for tracking number of in-flight fences
>>   virtio-gpu: Move fence_poll timer to VirtIOGPUGL
>>   virtio-gpu: Move print_stats timer to VirtIOGPUGL
>>   virtio-gpu: Handle virtio_gpu_virgl_init() failure
>>   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: Handle resource blob commands
>> 
>>  docs/system/devices/virtio-gpu.rst |  11 +
>>  hw/display/trace-events            |   3 +
>>  hw/display/virtio-gpu-gl.c         |  62 ++-
>>  hw/display/virtio-gpu-virgl.c      | 585 +++++++++++++++++++++++++++--
>>  hw/display/virtio-gpu.c            |  44 ++-
>>  include/hw/virtio/virtio-gpu.h     |  32 +-
>>  meson.build                        |   5 +-
>>  7 files changed, 685 insertions(+), 57 deletions(-)
>> 
>> -- 
>> 2.45.2

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v16 00/13] Support blob memory and venus on qemu

[Dmitry Osipenko]

On 8/22/24 16:13, Alex Bennée wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
>> On Sun, Jun 23, 2024 at 06:23:30PM +0300, Dmitry Osipenko wrote:
>>> 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.
>>
>>
>> I'd like to see a Tested-by from Alex Bennée for this -
>> he's been testing v14 last as far as I could see.
> 
> I've posted it on the top. Broadly x86 and Arm work on my x86 host. So
> far my Arm host only runs the x86 image with the AMD card OK.
> 
> I've updated my test images so you can try them:
> 
>   https://fileserver.linaro.org/s/ce5jXBFinPxtEdx
> 
> See the notes on the readme on how to run.

Thanks, Alex! I'll re-post v17 with all new acks/r-b's soon, will also
link your images in the cover letter.

-- 
Best regards,
Dmitry