On 10/31/24 10:44, Akihiko Odaki wrote:
> On 2024/10/25 8:33, Dmitry Osipenko wrote:
>> Support asynchronous fencing feature of virglrenderer. It allows Qemu to
>> handle fence as soon as it's signalled instead of periodically polling
>> the fence status. This feature is required for enabling DRM context
>> support in Qemu because legacy fencing mode isn't supported for DRM
>> contexts in virglrenderer.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipe...@collabora.com>
>> ---
>> hw/display/virtio-gpu-gl.c | 3 +
>> hw/display/virtio-gpu-virgl.c | 138 +++++++++++++++++++++++++++------
>> include/hw/virtio/virtio-gpu.h | 13 ++++
>> 3 files changed, 132 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>> index 7c0e448b4661..53d938f23f20 100644
>> --- a/hw/display/virtio-gpu-gl.c
>> +++ b/hw/display/virtio-gpu-gl.c
>> @@ -170,6 +170,9 @@ static void
>> virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>> if (gl->renderer_state >= RS_INITED) {
>> #if VIRGL_VERSION_MAJOR >= 1
>> qemu_bh_delete(gl->cmdq_resume_bh);
>> +
>> + virtio_gpu_virgl_reset_async_fences(g);
>> + qemu_bh_delete(gl->async_fence_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 3c564683820b..37b40e258398 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -891,6 +891,7 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>> void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>> struct virtio_gpu_ctrl_command
>> *cmd)
>> {
>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>> bool cmd_suspended = false;
>> int ret;
>> @@ -992,34 +993,71 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>> trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd-
>> >cmd_hdr.type);
>> - /*
>> - * Unlike other virglrenderer functions, this one returns a positive
>> - * error code.
>> - */
>> - ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
>> - if (ret)
>> - qemu_log_mask(LOG_GUEST_ERROR,
>> - "%s: virgl_renderer_create_fence error: %s",
>> - __func__, strerror(ret));
>> + if (gl->context_fence_enabled &&
>> + (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX)) {
>> + uint32_t flags = 0;
>> +
>> + ret = virgl_renderer_context_create_fence(cmd-
>> >cmd_hdr.ctx_id, flags,
>> + cmd->cmd_hdr.ring_idx,
>> + cmd-
>> >cmd_hdr.fence_id);
>> + if (ret)
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: virgl_renderer_context_create_fence
>> error: %s",
>> + __func__, strerror(-ret));
>> + } else {
>> + /*
>> + * Unlike other virglrenderer functions, this one returns a
>> positive
>> + * error code.
>> + */
>> + ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
>> + if (ret)
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: virgl_renderer_create_fence error: %s",
>> + __func__, strerror(ret));
>> + }
>> }
>> -static void virgl_write_fence(void *opaque, uint32_t fence)
>> +static void virtio_gpu_virgl_async_fence_bh(void *opaque)
>> {
>> - VirtIOGPU *g = opaque;
>> + QSLIST_HEAD(, virtio_gpu_virgl_context_fence) async_fenceq;
>> struct virtio_gpu_ctrl_command *cmd, *tmp;
>> + struct virtio_gpu_virgl_context_fence *f;
>> + VirtIOGPU *g = opaque;
>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>> - QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
>> - /*
>> - * the guest can end up emitting fences out of order
>> - * so we should check all fenced cmds not just the first one.
>> - */
>> - if (cmd->cmd_hdr.fence_id > fence) {
>> - continue;
>> + QSLIST_MOVE_ATOMIC(&async_fenceq, &gl->async_fenceq);
>> +
>> + while (!QSLIST_EMPTY(&async_fenceq)) {
>> + f = QSLIST_FIRST(&async_fenceq);
>> +
>> + QSLIST_REMOVE_HEAD(&async_fenceq, next);
>> +
>> + QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
>> + /*
>> + * the guest can end up emitting fences out of order
>> + * so we should check all fenced cmds not just the first
>> one.
>> + */
>> + if (cmd->cmd_hdr.fence_id > f->fence_id) {
>> + continue;
>> + }
>> + if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
>> + if (cmd->cmd_hdr.ring_idx != f->ring_idx) {
>> + continue;
>> + }
>> + if (cmd->cmd_hdr.ctx_id != f->ctx_id) {
>> + continue;
>> + }
>> + } else if (f->ring_idx >= 0) {
>> + /* ctx0 GL-query fences don't have ring info */
>> + continue;
>> + }
>> + virtio_gpu_ctrl_response_nodata(g, cmd,
>> VIRTIO_GPU_RESP_OK_NODATA);
>> + QTAILQ_REMOVE(&g->fenceq, cmd, next);
>> + g_free(cmd);
>> }
>> - trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
>> - virtio_gpu_ctrl_response_nodata(g, cmd,
>> VIRTIO_GPU_RESP_OK_NODATA);
>> - QTAILQ_REMOVE(&g->fenceq, cmd, next);
>> - g_free(cmd);
>> +
>> + trace_virtio_gpu_fence_resp(f->fence_id);
>> + g_free(f);
>> g->inflight--;
>> if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
>> trace_virtio_gpu_dec_inflight_fences(g->inflight);
>> @@ -1027,6 +1065,50 @@ static void virgl_write_fence(void *opaque,
>> uint32_t fence)
>> }
>> }
>> +void virtio_gpu_virgl_reset_async_fences(VirtIOGPU *g)
>> +{
>> + struct virtio_gpu_virgl_context_fence *f, *f_tmp;
>> + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>> +
>> + QSLIST_FOREACH_SAFE(f, &gl->async_fenceq, next, f_tmp) {
>> + QSLIST_REMOVE(&gl->async_fenceq, f,
>> virtio_gpu_virgl_context_fence,
>> + next);
>
> Replace QSLIST_FOREACH_SAFE() and QSLIST_REMOVE() with QSLIST_EMPTY(),
> QSLIST_FIRST() and QSLIST_REMOVE_HEAD() as you have done for
> virtio_gpu_virgl_async_fence_bh().
Ack
--
Best regards,
Dmitry
