[PULL 00/17] maintainer updates for May (testing, plugins, virtio-gpu)

[PULL 00/17] maintainer updates for May (testing, plugins, virtio-gpu)

[Alex Bennée]

The following changes since commit 09be8a511a2e278b45729d7b065d30c68dd699d0:

  Merge tag 'pull-qapi-2025-06-03' of https://repo.or.cz/qemu/armbru into staging (2025-06-03 09:19:26 -0400)

are available in the Git repository at:

  https://gitlab.com/stsquad/qemu.git tags/pull-10.1-maintainer-may-2025-050625-1

for you to fetch changes up to 66835968ca7246d385218be9776a80a5136563b7:

  gdbstub: update aarch64-core.xml (2025-06-05 13:47:37 +0100)

----------------------------------------------------------------
Maintainer updates for May 2025:

  - expose ~/.cache/qemu to container builds
  - disable debug info in CI
  - allow boot.S to handle target el mode selection
  - new arguments for ips plugin
  - update virtio-gpu MAINTAINERS
  - re-factoring of blob MemoryRegion handling
  - remove extra draw call causing corruption
  - support Venus fence contexts
  - cleanup assets in size_memop
  - fix include guard in gdbstub
  - introduce qGDBServerVersion gdbstub query
  - update gdb aarch64-core.xml to support bitfields

----------------------------------------------------------------
Alex Bennée (11):
      tests/docker: expose $HOME/.cache/qemu as docker volume
      gitlab: disable debug info on CI builds
      tests/tcg: make aarch64 boot.S handle different starting modes
      contrib/plugins: add a scaling factor to the ips arg
      contrib/plugins: allow setting of instructions per quantum
      MAINTAINERS: add myself to virtio-gpu for Odd Fixes
      MAINTAINERS: add Akihiko and Dmitry as reviewers
      hw/display: re-arrange memory region tracking
      include/exec: fix assert in size_memop
      include/gdbstub: fix include guard in commands.h
      gdbstub: assert earlier in handle_read_all_regs

Dominik 'Disconnect3d' Czarnota (1):
      gdbstub: Implement qGDBServerVersion packet

Dongwon Kim (1):
      ui/gtk-gl-area: Remove extra draw call in refresh

Manos Pitsidianakis (2):
      virtio-gpu: refactor async blob unmapping
      gdbstub: update aarch64-core.xml

Nabih Estefan (1):
      tests/qtest: Avoid unaligned access in IGB test

Yiwei Zhang (1):
      virtio-gpu: support context init multiple timeline

 MAINTAINERS                               |   5 +-
 docs/about/emulation.rst                  |   4 +
 include/exec/memop.h                      |   4 +-
 include/gdbstub/commands.h                |   2 +-
 include/system/memory.h                   |   1 +
 contrib/plugins/ips.c                     |  49 ++++++++-
 gdbstub/gdbstub.c                         |  19 +++-
 hw/display/virtio-gpu-virgl.c             | 102 +++++++++++++-----
 tests/qtest/libqos/igb.c                  |   4 +-
 ui/gtk-gl-area.c                          |   1 -
 .gitlab-ci.d/buildtest-template.yml       |   1 +
 gdb-xml/aarch64-core.xml                  |  52 ++++++++-
 tests/docker/Makefile.include             |  10 +-
 tests/tcg/aarch64/Makefile.softmmu-target |   3 +-
 tests/tcg/aarch64/system/boot.S           | 172 +++++++++++++++++++++++++++++-
 15 files changed, 382 insertions(+), 47 deletions(-)

-- 
2.47.2

[PULL 01/17] tests/docker: expose $HOME/.cache/qemu as docker volume

[Alex Bennée]

If you want to run functional tests we should share .cache/qemu so we
don't force containers to continually re-download images. We also move
ccache to use this shared area.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-2-alex.bennee@linaro.org>

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index fa1cbb6726..3959d8a028 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -185,8 +185,10 @@ docker:
 
 docker-help: docker
 
+# Where QEMU caches build artefacts
+DOCKER_QEMU_CACHE_DIR := $$HOME/.cache/qemu
 # Use a global constant ccache directory to speed up repetitive builds
-DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
+DOCKER_QEMU_CCACHE_DIR := DOCKER_QEMU_CACHE_DIR/docker-ccache
 
 # This rule if for directly running against an arbitrary docker target.
 # It is called by the expanded docker targets (e.g. make
@@ -195,7 +197,7 @@ DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
 # For example: make docker-run TEST="test-quick" IMAGE="debian:arm64" EXECUTABLE=./aarch64-linux-user/qemu-aarch64
 #
 docker-run: docker-qemu-src
-	@mkdir -p "$(DOCKER_CCACHE_DIR)"
+	@mkdir -p "$(DOCKER_QEMU_CCACHE_DIR)"
 	@if test -z "$(IMAGE)" || test -z "$(TEST)"; \
 		then echo "Invalid target $(IMAGE)/$(TEST)"; exit 1; \
 	fi
@@ -222,8 +224,8 @@ docker-run: docker-qemu-src
 			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
 			-e SHOW_ENV=$(SHOW_ENV) 			\
 			$(if $(NOUSER),,				\
-				-e CCACHE_DIR=/var/tmp/ccache 		\
-				-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
+				-v $(DOCKER_QEMU_CACHE_DIR):$(DOCKER_QEMU_CACHE_DIR) 	\
+				-e CCACHE_DIR=$(DOCKER_QEMU_CCACHE_DIR)	\
 			)						\
 			-v $$(readlink -e $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
 			$(IMAGE) 					\
-- 
2.47.2

[PULL 02/17] gitlab: disable debug info on CI builds

[Alex Bennée]

Our default build enables debug info which adds hugely to the size of
the builds as well as the size of cached objects. Disable debug info
across the board to save space and reduce pressure on the CI system.
We still have a number of builds which explicitly enable debug and
related extra asserts like --enable-debug-tcg.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-3-alex.bennee@linaro.org>

diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index 118371e377..19663126ca 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -24,6 +24,7 @@
     - ccache --zero-stats
     - section_start configure "Running configure"
     - ../configure --enable-werror --disable-docs --enable-fdt=system
+          --disable-debug-info
           ${TARGETS:+--target-list="$TARGETS"}
           $CONFIGURE_ARGS ||
       { cat config.log meson-logs/meson-log.txt && exit 1; }
-- 
2.47.2

[PULL 03/17] tests/tcg: make aarch64 boot.S handle different starting modes

[Alex Bennée]

Currently the boot.S code assumes everything starts at EL1. This will
break things like the memory test which will barf on unaligned memory
access when run at a higher level.

Adapt the boot code to do some basic verification of the starting mode
and the minimal configuration to move to the lower exception levels.
With this we can run the memory test with:

  -M virt,secure=on
  -M virt,secure=on,virtualization=on
  -M virt,virtualisation=on

If a test needs to be at a particular EL it can use the semihosting
command line to indicate the level we should execute in.

Cc: Julian Armistead <julian.armistead@linaro.org>
Cc: Jim MacArthur <jim.macarthur@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-4-alex.bennee@linaro.org>

diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target
index 9c52475b7a..f7a7d2b800 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -68,7 +68,8 @@ run-plugin-semiconsole-with-%: semiconsole
 
 # vtimer test needs EL2
 QEMU_EL2_MACHINE=-machine virt,virtualization=on,gic-version=2 -cpu cortex-a57 -smp 4
-run-vtimer: QEMU_OPTS=$(QEMU_EL2_MACHINE) $(QEMU_BASE_ARGS) -kernel
+QEMU_EL2_BASE_ARGS=-semihosting-config enable=on,target=native,chardev=output,arg="2"
+run-vtimer: QEMU_OPTS=$(QEMU_EL2_MACHINE) $(QEMU_EL2_BASE_ARGS) -kernel
 
 # Simple Record/Replay Test
 .PHONY: memory-record
diff --git a/tests/tcg/aarch64/system/boot.S b/tests/tcg/aarch64/system/boot.S
index a5df9c173d..8bfa4e4efc 100644
--- a/tests/tcg/aarch64/system/boot.S
+++ b/tests/tcg/aarch64/system/boot.S
@@ -16,6 +16,7 @@
 #define semihosting_call hlt 0xf000
 #define SYS_WRITEC	0x03	/* character to debug channel */
 #define SYS_WRITE0	0x04	/* string to debug channel */
+#define SYS_GET_CMDLINE 0x15	/* get command line */
 #define SYS_EXIT	0x18
 
 	.align	12
@@ -70,21 +71,172 @@ lower_a32_sync:
 lower_a32_irq:
 lower_a32_fiq:
 lower_a32_serror:
+	adr	x1, .unexp_excp
+exit_msg:
 	mov	x0, SYS_WRITE0
-	adr	x1, .error
 	semihosting_call
 	mov	x0, 1 /* EXIT_FAILURE */
 	bl 	_exit
 	/* never returns */
 
 	.section .rodata
-.error:
-	.string "Terminated by exception.\n"
+.unexp_excp:
+	.string "Unexpected exception.\n"
+.high_el_msg:
+	.string "Started in lower EL than requested.\n"
+.unexp_el0:
+	.string "Started in invalid EL.\n"
+
+	.align 8
+.get_cmd:
+	.quad	cmdline
+	.quad	128
 
 	.text
 	.align 4
 	.global __start
 __start:
+	/*
+         * Initialise the stack for whatever EL we are in before
+	 * anything else, we need it to be able to _exit cleanly.
+	 * It's smaller than the stack we pass to the C code but we
+	 * don't need much.
+	 */
+	adrp	x0, system_stack_end
+	add	x0, x0, :lo12:system_stack_end
+	mov	sp, x0
+
+	/*
+	 * The test can set the semihosting command line to the target
+	 * EL needed for the test. However if no semihosting args are set we will
+	 * end up with -kernel/-append data (see semihosting_arg_fallback).
+	 * Keep the normalised target in w11.
+	 */
+	mov	x0, SYS_GET_CMDLINE
+	adr	x1, .get_cmd
+	semihosting_call
+	adrp	x10, cmdline
+	add	x10, x10, :lo12:cmdline
+	ldrb	w11, [x10]
+
+	/* sanity check, normalise char to EL, clamp to 1 if outside range */
+	subs w11, w11, #'0'
+	b.lt el_default
+	cmp  w11, #3
+	b.gt el_default
+	b 1f
+
+el_high:
+	adr	x1, .high_el_msg
+	b	exit_msg
+
+el_default:
+	mov	w11, #1
+
+1:
+	/* Determine current Exception Level */
+	mrs	x0, CurrentEL
+	lsr	x0, x0, #2	  /* CurrentEL[3:2] contains the current EL */
+
+	/* Are we already in a lower EL than we want? */
+	cmp	w11, w0
+	bgt	el_high
+
+	/* Branch based on current EL */
+	cmp	x0, #3
+	b.eq	setup_el3
+	cmp	x0, #2
+	b.eq	setup_el2
+	cmp	x0, #1
+	b.eq	at_testel	     /* Already at EL1, skip transition */
+
+	/* Should not be at EL0 - error out */
+	adr 	x1, .unexp_el0
+	b	exit_msg
+
+setup_el3:
+	/* Ensure we trap if we get anything wrong */
+	adr	x0, vector_table
+	msr	vbar_el3, x0
+
+	/* Does the test want to be at EL3? */
+	cmp	w11, #3
+	beq	at_testel
+
+	/* Configure EL3 to for lower states (EL2 or EL1) */
+	mrs	x0, scr_el3
+	orr	x0, x0, #(1 << 10)    /* RW = 1: EL2/EL1 execution state is AArch64 */
+	orr	x0, x0, #(1 << 0)     /* NS = 1: Non-secure state */
+	msr	scr_el3, x0
+
+	/*
+	 * We need to check if EL2 is actually enabled via ID_AA64PFR0_EL1,
+	 * otherwise we should just jump straight to EL1.
+	 */
+	mrs	x0, id_aa64pfr0_el1
+	ubfx	x0, x0, #8, #4	      /* Extract EL2 field (bits 11:8) */
+	cbz	x0, el2_not_present   /* If field is 0 no EL2 */
+
+
+	/* Prepare SPSR for exception return to EL2 */
+	mov	x0, #0x3c9	      /* DAIF bits and EL2h mode (9) */
+	msr	spsr_el3, x0
+
+	/* Set EL2 entry point */
+	adr	x0, setup_el2
+	msr	elr_el3, x0
+
+	/* Return to EL2 */
+	eret
+
+el2_not_present:
+	/* Initialize SCTLR_EL1 with reset value */
+	msr	sctlr_el1, xzr
+
+	/* Set EL1 entry point */
+	adr	x0, at_testel
+	msr	elr_el3, x0
+
+	/* Prepare SPSR for exception return to EL1h with interrupts masked */
+	mov	x0, #0x3c5	      /* DAIF bits and EL1h mode (5) */
+	msr	spsr_el3, x0
+
+	isb			      /* Synchronization barrier */
+	eret			      /* Jump to EL1 */
+
+setup_el2:
+	/* Ensure we trap if we get anything wrong */
+	adr	x0, vector_table
+	msr	vbar_el2, x0
+
+	/* Does the test want to be at EL2? */
+	cmp	w11, #2
+	beq	at_testel
+
+	/* Configure EL2 to allow transition to EL1 */
+	mrs	x0, hcr_el2
+	orr	x0, x0, #(1 << 31)    /* RW = 1: EL1 execution state is AArch64 */
+	msr	hcr_el2, x0
+
+	/* Initialize SCTLR_EL1 with reset value */
+	msr	sctlr_el1, xzr
+
+	/* Set EL1 entry point */
+	adr	x0, at_testel
+	msr	elr_el2, x0
+
+	/* Prepare SPSR for exception return to EL1 */
+	mov	x0, #(0x5 << 0)	      /* EL1h (SPx), with interrupts disabled */
+	msr	spsr_el2, x0
+
+	/* Return to EL1 */
+	eret
+
+	/*
+	 * At the target EL for the test, usually EL1. Note we still
+	 * set everything up as if we were at EL1.
+	 */
+at_testel:
 	/* Installs a table of exception vectors to catch and handle all
 	   exceptions by terminating the process with a diagnostic.  */
 	adr	x0, vector_table
@@ -100,7 +252,7 @@ __start:
 	 * maps RAM to the first Gb. The stage2 tables have two 2mb
 	 * translation block entries covering a series of adjacent
 	 * 4k pages.
-	*/
+	 */
 
 	/* Stage 1 entry: indexed by IA[38:30] */
 	adr	x1, .				/* phys address */
@@ -198,7 +350,8 @@ __start:
 	orr	x0, x0, #(3 << 16)
 	msr	cpacr_el1, x0
 
-	/* Setup some stack space and enter the test code.
+	/*
+	 * Setup some stack space before we enter the test code.
 	 * Assume everything except the return value is garbage when we
 	 * return, we won't need it.
 	 */
@@ -233,6 +386,11 @@ __sys_outc:
 	ret
 
 	.data
+
+	.align 8
+cmdline:
+	.space 128, 0
+
 	.align	12
 
 	/* Translation table
@@ -246,6 +404,10 @@ ttb_stage2:
 	.space	4096, 0
 
 	.align	12
+system_stack:
+	.space 4096, 0
+system_stack_end:
+
 stack:
 	.space 65536, 0
 stack_end:
-- 
2.47.2

[PULL 04/17] tests/qtest: Avoid unaligned access in IGB test

[Alex Bennée]

From: Nabih Estefan <nabihestefan@google.com>

  ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment

Instead of straight casting the uint8_t array, we can use ldl_le_p and
lduw_l_p to assure the unaligned access works properly against
uint32_t and uint16_t.

Signed-off-by: Nabih Estefan <nabihestefan@google.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20250429155621.2028198-1-nabihestefan@google.com>
[AJB: fix commit message, remove unneeded casts]
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-5-alex.bennee@linaro.org>

diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
index f40c4ec4cd..ab3ef6f0c3 100644
--- a/tests/qtest/libqos/igb.c
+++ b/tests/qtest/libqos/igb.c
@@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
     e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
     e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
     e1000e_macreg_write(&d->e1000e, E1000_RA,
-                        le32_to_cpu(*(uint32_t *)address));
+                        ldl_le_p(address));
     e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
                         E1000_RAH_AV | E1000_RAH_POOL_1 |
-                        le16_to_cpu(*(uint16_t *)(address + 4)));
+                        lduw_le_p(address + 4));
 
     /* Set supported receive descriptor mode */
     e1000e_macreg_write(&d->e1000e,
-- 
2.47.2

Re: [PULL 04/17] tests/qtest: Avoid unaligned access in IGB test

[Akihiko Odaki]

On 2025/06/06 1:26, Alex Bennée wrote:
> From: Nabih Estefan <nabihestefan@google.com>
> 
>    ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> 
> Instead of straight casting the uint8_t array, we can use ldl_le_p and
> lduw_l_p to assure the unaligned access works properly against
> uint32_t and uint16_t.
> 
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Tested-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20250429155621.2028198-1-nabihestefan@google.com>
> [AJB: fix commit message, remove unneeded casts]
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-5-alex.bennee@linaro.org>

Somehow this patch was sent to my old email address though other patches 
were correctly sent to my new email address.

The patch itself looks good to me, so:
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Regards,
Akihiko Odaki

> 
> diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
> index f40c4ec4cd..ab3ef6f0c3 100644
> --- a/tests/qtest/libqos/igb.c
> +++ b/tests/qtest/libqos/igb.c
> @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
>       e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
>       e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
>       e1000e_macreg_write(&d->e1000e, E1000_RA,
> -                        le32_to_cpu(*(uint32_t *)address));
> +                        ldl_le_p(address));
>       e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
>                           E1000_RAH_AV | E1000_RAH_POOL_1 |
> -                        le16_to_cpu(*(uint16_t *)(address + 4)));
> +                        lduw_le_p(address + 4));
>   
>       /* Set supported receive descriptor mode */
>       e1000e_macreg_write(&d->e1000e,

[PULL 05/17] contrib/plugins: add a scaling factor to the ips arg

[Alex Bennée]

It's easy to get lost in zeros while setting the numbers of
instructions per second. Add a scaling suffix to make things simpler.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-6-alex.bennee@linaro.org>

diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
index e5297dbb01..d067971135 100644
--- a/contrib/plugins/ips.c
+++ b/contrib/plugins/ips.c
@@ -129,6 +129,18 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
     qemu_plugin_scoreboard_free(vcpus);
 }
 
+typedef struct {
+    const char *suffix;
+    unsigned long multipler;
+} ScaleEntry;
+
+/* a bit like units.h but not binary */
+static const ScaleEntry scales[] = {
+    { "k", 1000 },
+    { "m", 1000 * 1000 },
+    { "g", 1000 * 1000 * 1000 },
+};
+
 QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
                                            const qemu_info_t *info, int argc,
                                            char **argv)
@@ -137,12 +149,32 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
         char *opt = argv[i];
         g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
         if (g_strcmp0(tokens[0], "ips") == 0) {
-            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
+            char *endptr = NULL;
+            max_insn_per_second = g_ascii_strtoull(tokens[1], &endptr, 10);
             if (!max_insn_per_second && errno) {
                 fprintf(stderr, "%s: couldn't parse %s (%s)\n",
                         __func__, tokens[1], g_strerror(errno));
                 return -1;
             }
+
+            if (endptr && *endptr != 0) {
+                g_autofree gchar *lower = g_utf8_strdown(endptr, -1);
+                unsigned long scale = 0;
+
+                for (int j = 0; j < G_N_ELEMENTS(scales); j++) {
+                    if (g_strcmp0(lower, scales[j].suffix) == 0) {
+                        scale = scales[j].multipler;
+                        break;
+                    }
+                }
+
+                if (scale) {
+                    max_insn_per_second *= scale;
+                } else {
+                    fprintf(stderr, "bad suffix: %s\n", endptr);
+                    return -1;
+                }
+            }
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
-- 
2.47.2

[PULL 06/17] contrib/plugins: allow setting of instructions per quantum

[Alex Bennée]

The default is we update time every 1/10th of a second or so. However
for some cases we might want to update time more frequently. Allow
this to be set via the command line through the ipq argument.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-7-alex.bennee@linaro.org>

diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index a72591ee4d..456d01d5b0 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -811,6 +811,10 @@ This plugin can limit the number of Instructions Per Second that are executed::
   * - ips=N
     - Maximum number of instructions per cpu that can be executed in one second.
       The plugin will sleep when the given number of instructions is reached.
+  * - ipq=N
+    - Instructions per quantum. How many instructions before we re-calculate time.
+      The lower the number the more accurate time will be, but the less efficient the plugin.
+      Defaults to ips/10
 
 Other emulation features
 ------------------------
diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
index d067971135..f110c565bc 100644
--- a/contrib/plugins/ips.c
+++ b/contrib/plugins/ips.c
@@ -145,6 +145,8 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
                                            const qemu_info_t *info, int argc,
                                            char **argv)
 {
+    bool ipq_set = false;
+
     for (int i = 0; i < argc; i++) {
         char *opt = argv[i];
         g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
@@ -175,6 +177,14 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
                     return -1;
                 }
             }
+        } else if (g_strcmp0(tokens[0], "ipq") == 0) {
+            max_insn_per_quantum = g_ascii_strtoull(tokens[1], NULL, 10);
+
+            if (!max_insn_per_quantum) {
+                fprintf(stderr, "bad ipq value: %s\n", tokens[0]);
+                return -1;
+            }
+            ipq_set = true;
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
@@ -182,7 +192,10 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
     }
 
     vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
-    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
+
+    if (!ipq_set) {
+        max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
+    }
 
     if (max_insn_per_quantum == 0) {
         fprintf(stderr, "minimum of %d instructions per second needed\n",
-- 
2.47.2

[PULL 07/17] MAINTAINERS: add myself to virtio-gpu for Odd Fixes

[Alex Bennée]

Seeing as I've taken a few patches to here now I might as well put
myself forward to maintain virtio-gpu. I've marked it as Odd Fixes as
it is not my core focus. If someone with more GPU experience comes
forward we can always update again.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-8-alex.bennee@linaro.org>

diff --git a/MAINTAINERS b/MAINTAINERS
index 16af37986a..7718199979 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2668,7 +2668,8 @@ F: hw/display/ramfb*.c
 F: include/hw/display/ramfb.h
 
 virtio-gpu
-S: Orphan
+M: Alex Bennée <alex.bennee@linaro.org>
+S: Odd Fixes
 F: hw/display/virtio-gpu*
 F: hw/display/virtio-vga.*
 F: include/hw/virtio/virtio-gpu.h
-- 
2.47.2

[PULL 08/17] MAINTAINERS: add Akihiko and Dmitry as reviewers

[Alex Bennée]

Thanks for volunteering to help.

Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-9-alex.bennee@linaro.org>

diff --git a/MAINTAINERS b/MAINTAINERS
index 7718199979..194695c881 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2669,6 +2669,8 @@ F: include/hw/display/ramfb.h
 
 virtio-gpu
 M: Alex Bennée <alex.bennee@linaro.org>
+R: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
+R: Dmitry Osipenko <dmitry.osipenko@collabora.com>
 S: Odd Fixes
 F: hw/display/virtio-gpu*
 F: hw/display/virtio-vga.*
-- 
2.47.2

[PULL 09/17] hw/display: re-arrange memory region tracking

[Alex Bennée]

QOM objects can be embedded in other QOM objects and managed as part
of their lifetime but this isn't the case for
virtio_gpu_virgl_hostmem_region. However before we can split it out we
need some other way of associating the wider data structure with the
memory region.

Fortunately MemoryRegion has an opaque pointer. This is passed down to
MemoryRegionOps for device type regions but is unused in the
memory_region_init_ram_ptr() case. Use the opaque to carry the
reference and allow the final MemoryRegion object to be reaped when
its reference count is cleared.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
Cc: qemu-stable@nongnu.org
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>

diff --git a/include/system/memory.h b/include/system/memory.h
index fc35a0dcad..90715ff44a 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -784,6 +784,7 @@ struct MemoryRegion {
     DeviceState *dev;
 
     const MemoryRegionOps *ops;
+    /* opaque data, used by backends like @ops */
     void *opaque;
     MemoryRegion *container;
     int mapped_via_alias; /* Mapped via an alias, container might be NULL */
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 145a0b3879..71a7500de9 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -52,17 +52,11 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
 
 #if VIRGL_VERSION_MAJOR >= 1
 struct virtio_gpu_virgl_hostmem_region {
-    MemoryRegion mr;
+    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;
@@ -73,14 +67,12 @@ static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
 static void virtio_gpu_virgl_hostmem_region_free(void *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
-    struct virtio_gpu_virgl_hostmem_region *vmr;
+    struct virtio_gpu_virgl_hostmem_region *vmr = mr->opaque;
     VirtIOGPUBase *b;
     VirtIOGPUGL *gl;
 
-    vmr = to_hostmem_region(mr);
-    vmr->finish_unmapping = true;
-
     b = VIRTIO_GPU_BASE(vmr->g);
+    vmr->finish_unmapping = true;
     b->renderer_blocked--;
 
     /*
@@ -118,8 +110,8 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
 
     vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
     vmr->g = g;
+    mr = g_new0(MemoryRegion, 1);
 
-    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);
@@ -131,7 +123,9 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
      * command processing until MR is fully unreferenced and freed.
      */
     OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+    mr->opaque = vmr;
 
+    vmr->mr = mr;
     res->mr = mr;
 
     return 0;
@@ -142,16 +136,15 @@ 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;
+    struct virtio_gpu_virgl_hostmem_region *vmr;
     int ret;
 
     if (!mr) {
         return 0;
     }
-
-    vmr = to_hostmem_region(res->mr);
+    vmr = mr->opaque;
 
     /*
      * Perform async unmapping in 3 steps:
-- 
2.47.2

Re: [PULL 09/17] hw/display: re-arrange memory region tracking

[Akihiko Odaki]

On 2025/06/06 1:26, Alex Bennée wrote:
> QOM objects can be embedded in other QOM objects and managed as part
> of their lifetime but this isn't the case for
> virtio_gpu_virgl_hostmem_region. However before we can split it out we
> need some other way of associating the wider data structure with the
> memory region.
> 
> Fortunately MemoryRegion has an opaque pointer. This is passed down to
> MemoryRegionOps for device type regions but is unused in the
> memory_region_init_ram_ptr() case. Use the opaque to carry the
> reference and allow the final MemoryRegion object to be reaped when
> its reference count is cleared.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>

I have told you that you should address all comments before sending a 
series again a few times[1][2], but you haven't done that.

For this patch, I raised a concern with [3]:

I pointed out it has no effect (fixing or improving something) other 
than adding a memory allocation, but you didn't make a reply to prove 
otherwise.

I also pointed out it leaks memory and you asked for a test case[4], but 
you made this pull request without giving me 24 hours to reply to it.

The situation of "[PULL 03/17] tests/tcg: make aarch64 boot.S handle 
different starting modes" is also similar. I added a comment about 
symbol naming and you gave a reasoning, but I didn't get time to review 
it either[5]. Besides, I also had a suggestion to make the code shorter 
for the past version, but it is also dismissed.

I also pointed out "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call 
in refresh" has an undressed comment[2][7].

I would like to see improvements in how comments are addressed before a 
series is resent.

Regards,
Akihiko Odaki

[1] 
https://lore.kernel.org/qemu-devel/e6af12bd-1c36-4e50-8bae-d8d80cad13a0@daynix.com
[2] 
https://lore.kernel.org/qemu-devel/e037e38c-dd8d-4f65-b2d5-2629be5f6740@daynix.com
[3] 
https://lore.kernel.org/qemu-devel/1a86b86d-145a-44fc-9f87-2804767fb109@daynix.com/
[4] https://lore.kernel.org/qemu-devel/87o6v2764e.fsf@draig.linaro.org/
[5] https://lore.kernel.org/qemu-devel/874iwu372j.fsf@draig.linaro.org/
[6] 
https://lore.kernel.org/qemu-devel/7a76e746-9022-48cf-8216-775071e6d631@daynix.com/
[7] 
https://lore.kernel.org/qemu-devel/63911dcc-482b-45c5-9468-120ae3df691b@daynix.com/

> 
> diff --git a/include/system/memory.h b/include/system/memory.h
> index fc35a0dcad..90715ff44a 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -784,6 +784,7 @@ struct MemoryRegion {
>       DeviceState *dev;
>   
>       const MemoryRegionOps *ops;
> +    /* opaque data, used by backends like @ops */
>       void *opaque;
>       MemoryRegion *container;
>       int mapped_via_alias; /* Mapped via an alias, container might be NULL */
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 145a0b3879..71a7500de9 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -52,17 +52,11 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
>   
>   #if VIRGL_VERSION_MAJOR >= 1
>   struct virtio_gpu_virgl_hostmem_region {
> -    MemoryRegion mr;
> +    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;
> @@ -73,14 +67,12 @@ static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
>   static void virtio_gpu_virgl_hostmem_region_free(void *obj)
>   {
>       MemoryRegion *mr = MEMORY_REGION(obj);
> -    struct virtio_gpu_virgl_hostmem_region *vmr;
> +    struct virtio_gpu_virgl_hostmem_region *vmr = mr->opaque;
>       VirtIOGPUBase *b;
>       VirtIOGPUGL *gl;
>   
> -    vmr = to_hostmem_region(mr);
> -    vmr->finish_unmapping = true;
> -
>       b = VIRTIO_GPU_BASE(vmr->g);
> +    vmr->finish_unmapping = true;
>       b->renderer_blocked--;
>   
>       /*
> @@ -118,8 +110,8 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>   
>       vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
>       vmr->g = g;
> +    mr = g_new0(MemoryRegion, 1);
>   
> -    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);
> @@ -131,7 +123,9 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>        * command processing until MR is fully unreferenced and freed.
>        */
>       OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
> +    mr->opaque = vmr;
>   
> +    vmr->mr = mr;
>       res->mr = mr;
>   
>       return 0;
> @@ -142,16 +136,15 @@ 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;
> +    struct virtio_gpu_virgl_hostmem_region *vmr;
>       int ret;
>   
>       if (!mr) {
>           return 0;
>       }
> -
> -    vmr = to_hostmem_region(res->mr);
> +    vmr = mr->opaque;
>   
>       /*
>        * Perform async unmapping in 3 steps:

Re: [PULL 09/17] hw/display: re-arrange memory region tracking

[Alex Bennée]

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/06/06 1:26, Alex Bennée wrote:
>> QOM objects can be embedded in other QOM objects and managed as part
>> of their lifetime but this isn't the case for
>> virtio_gpu_virgl_hostmem_region. However before we can split it out we
>> need some other way of associating the wider data structure with the
>> memory region.
>> Fortunately MemoryRegion has an opaque pointer. This is passed down
>> to
>> MemoryRegionOps for device type regions but is unused in the
>> memory_region_init_ram_ptr() case. Use the opaque to carry the
>> reference and allow the final MemoryRegion object to be reaped when
>> its reference count is cleared.
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>
>
> I have told you that you should address all comments before sending a
> series again a few times[1][2], but you haven't done that.

I've given reasons. Thanks for your review but you don't get to veto.

> I pointed out it has no effect (fixing or improving something) other
> than adding a memory allocation, but you didn't make a reply to prove
> otherwise.

I explained the commit cover what it is doing.

>
> I also pointed out it leaks memory and you asked for a test case[4],
> but you made this pull request without giving me 24 hours to reply to
> it.

You keep bringing up theoretical issues. We have passing test cases now
and we have plenty of time to address any bugs we might discover. But
holding onto these patches is slowing down other work getting in and I
don't deem it a risk to merge as is.

>
> The situation of "[PULL 03/17] tests/tcg: make aarch64 boot.S handle
> different starting modes" is also similar. I added a comment about
> symbol naming and you gave a reasoning, but I didn't get time to
> review it either[5]. Besides, I also had a suggestion to make the code
> shorter for the past version, but it is also dismissed.
>
> I also pointed out "[PULL 11/17] ui/gtk-gl-area: Remove extra draw
> call in refresh" has an undressed comment[2][7].
>
> I would like to see improvements in how comments are addressed before
> a series is resent.

No - I'm sorry you don't get to veto a pull request because it doesn't
meet your particular standards.

I'm happy with the other review and level of testing of the patches to
put it in a pull request. I held off the other well tested patch in the
series out of an abundance of caution but will keep it in the
virtio-gpu/next tree and re-post once I've done my next sweep for my
maintainer trees.

<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PULL 09/17] hw/display: re-arrange memory region tracking

[Akihiko Odaki]

On 2025/06/06 18:54, Alex Bennée wrote:
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
> 
>> On 2025/06/06 1:26, Alex Bennée wrote:
>>> QOM objects can be embedded in other QOM objects and managed as part
>>> of their lifetime but this isn't the case for
>>> virtio_gpu_virgl_hostmem_region. However before we can split it out we
>>> need some other way of associating the wider data structure with the
>>> memory region.
>>> Fortunately MemoryRegion has an opaque pointer. This is passed down
>>> to
>>> MemoryRegionOps for device type regions but is unused in the
>>> memory_region_init_ram_ptr() case. Use the opaque to carry the
>>> reference and allow the final MemoryRegion object to be reaped when
>>> its reference count is cleared.
>>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>
>>
>> I have told you that you should address all comments before sending a
>> series again a few times[1][2], but you haven't done that.
> 
> I've given reasons. Thanks for your review but you don't get to veto.
> 
>> I pointed out it has no effect (fixing or improving something) other
>> than adding a memory allocation, but you didn't make a reply to prove
>> otherwise.
> 
> I explained the commit cover what it is doing.

It still doesn't explain the motivation.

For example, "[PULL 01/17] tests/docker: expose $HOME/.cache/qemu as 
docker volume" does explain its motivation. It says:

 > If you want to run functional tests we should share .cache/qemu so we
 > don't force containers to continually re-download images.

So, with the patch, containers will no longer require continually 
re-downloading images, which is nice.

Back to this memory region patch, what does it contribute? What's nice 
with this patch?

> 
>>
>> I also pointed out it leaks memory and you asked for a test case[4],
>> but you made this pull request without giving me 24 hours to reply to
>> it.
> 
> You keep bringing up theoretical issues. We have passing test cases now
> and we have plenty of time to address any bugs we might discover. But
> holding onto these patches is slowing down other work getting in and I
> don't deem it a risk to merge as is.

Things that involve concurrency and memory safety will often become 
theoretical.

For example, I recently changed a concurrency algorithm with the 
following patch:
https://lore.kernel.org/qemu-devel/20250526-event-v4-0-5b784cc8e1de@daynix.com/

All reviews were theoretical and did not provide any test case. These 
reviews still matter and I replied them by providing my theory, which 
eventually materialized as a documentation patch:
https://lore.kernel.org/qemu-devel/b41eb6f4-96b8-47bf-90cf-e4918a613dcf@daynix.com/T/#m2603d613d8d8cbbe87b4dce63fd2663c58d52e55

> 
>>
>> The situation of "[PULL 03/17] tests/tcg: make aarch64 boot.S handle
>> different starting modes" is also similar. I added a comment about
>> symbol naming and you gave a reasoning, but I didn't get time to
>> review it either[5]. Besides, I also had a suggestion to make the code
>> shorter for the past version, but it is also dismissed.
>>
>> I also pointed out "[PULL 11/17] ui/gtk-gl-area: Remove extra draw
>> call in refresh" has an undressed comment[2][7].
>>
>> I would like to see improvements in how comments are addressed before
>> a series is resent.
> 
> No - I'm sorry you don't get to veto a pull request because it doesn't
> meet your particular standards.

The standards are not mine, but documented in:
docs/devel/submitting-a-patch.rst

> 
> I'm happy with the other review and level of testing of the patches to
> put it in a pull request. I held off the other well tested patch in the
> series out of an abundance of caution but will keep it in the
> virtio-gpu/next tree and re-post once I've done my next sweep for my
> maintainer trees.

A reviewer don't have a right to veto but the mentioned documentation 
says they can still get a submitter's counterargument when the submitter 
disagrees with them. Reviews should not be just silently ignored.

Regards,
Akihiko Odaki

Re: [PULL 09/17] hw/display: re-arrange memory region tracking

[Alex Bennée]

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/06/06 18:54, Alex Bennée wrote:
>> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
>> 
>>> On 2025/06/06 1:26, Alex Bennée wrote:
>>>> QOM objects can be embedded in other QOM objects and managed as part
>>>> of their lifetime but this isn't the case for
>>>> virtio_gpu_virgl_hostmem_region. However before we can split it out we
>>>> need some other way of associating the wider data structure with the
>>>> memory region.
>>>> Fortunately MemoryRegion has an opaque pointer. This is passed down
>>>> to
>>>> MemoryRegionOps for device type regions but is unused in the
>>>> memory_region_init_ram_ptr() case. Use the opaque to carry the
>>>> reference and allow the final MemoryRegion object to be reaped when
>>>> its reference count is cleared.
>>>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
>>>> Cc: qemu-stable@nongnu.org
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>
>>>
>>> I have told you that you should address all comments before sending a
>>> series again a few times[1][2], but you haven't done that.
>> I've given reasons. Thanks for your review but you don't get to
>> veto.
>> 
>>> I pointed out it has no effect (fixing or improving something) other
>>> than adding a memory allocation, but you didn't make a reply to prove
>>> otherwise.
>> I explained the commit cover what it is doing.
>
> It still doesn't explain the motivation.
>
<snip>

It fixes the anti-pattern of embedding a QOM object into a non-QOM
container. It enables in the following patches the lifetime of the MR to
be covered controlled purely by its references and not be so tangled up
with virglrenderers internals.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PULL 09/17] hw/display: re-arrange memory region tracking

[Akihiko Odaki]

On 2025/06/06 20:31, Alex Bennée wrote:
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
> 
>> On 2025/06/06 18:54, Alex Bennée wrote:
>>> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
>>>
>>>> On 2025/06/06 1:26, Alex Bennée wrote:
>>>>> QOM objects can be embedded in other QOM objects and managed as part
>>>>> of their lifetime but this isn't the case for
>>>>> virtio_gpu_virgl_hostmem_region. However before we can split it out we
>>>>> need some other way of associating the wider data structure with the
>>>>> memory region.
>>>>> Fortunately MemoryRegion has an opaque pointer. This is passed down
>>>>> to
>>>>> MemoryRegionOps for device type regions but is unused in the
>>>>> memory_region_init_ram_ptr() case. Use the opaque to carry the
>>>>> reference and allow the final MemoryRegion object to be reaped when
>>>>> its reference count is cleared.
>>>>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> Message-Id: <20250410122643.1747913-2-manos.pitsidianakis@linaro.org>
>>>>> Cc: qemu-stable@nongnu.org
>>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>>> Message-ID: <20250603110204.838117-10-alex.bennee@linaro.org>
>>>>
>>>> I have told you that you should address all comments before sending a
>>>> series again a few times[1][2], but you haven't done that.
>>> I've given reasons. Thanks for your review but you don't get to
>>> veto.
>>>
>>>> I pointed out it has no effect (fixing or improving something) other
>>>> than adding a memory allocation, but you didn't make a reply to prove
>>>> otherwise.
>>> I explained the commit cover what it is doing.
>>
>> It still doesn't explain the motivation.
>>
> <snip>
> 
> It fixes the anti-pattern of embedding a QOM object into a non-QOM
> container. It enables in the following patches the lifetime of the MR to
> be covered controlled purely by its references and not be so tangled up
> with virglrenderers internals.

The "container" is just a memory allocation made by g_new0() and this 
patch doesn't change that.

I haven't reviewed "[PULL 10/17] virtio-gpu: refactor async blob 
unmapping" yet, but it apparently it doesn't depend on this patch.

The virglrenderer's internals are irrelevant; both 
virtio_gpu_virgl_hostmem_region and MemoryRegion are QEMU-specific 
structures.

Regards,
Akihiko Odaki

[PULL 10/17] virtio-gpu: refactor async blob unmapping

[Alex Bennée]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Change the 3 part async cleanup of a blob memory mapping to check if the
unmapping has finished already after deleting the subregion; this
condition allows us to skip suspending the command and responding to the
guest right away.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20250410122643.1747913-4-manos.pitsidianakis@linaro.org>
Cc: qemu-stable@nongnu.org
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-11-alex.bennee@linaro.org>

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 71a7500de9..b4aa8abb96 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -155,7 +155,32 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
      *    asynchronously by virtio_gpu_virgl_hostmem_region_free().
      * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
      */
+
+    /* 1. Check if we should start unmapping now */
+    if (!vmr->finish_unmapping) {
+        /* begin async unmapping. render will be unblocked once MR is freed */
+        b->renderer_blocked++;
+
+        memory_region_set_enabled(mr, false);
+        memory_region_del_subregion(&b->hostmem, mr);
+        object_unparent(OBJECT(mr));
+        /*
+         * The unmapping might have already finished at this point if no one
+         * else held a reference to the MR; if yes, we can skip suspending the
+         * command and unmap the resource right away.
+         */
+        *cmd_suspended = !vmr->finish_unmapping;
+    }
+
+    /*
+     * 2. if virtio_gpu_virgl_hostmem_region_free hasn't been executed yet, we
+     * have marked the command to be re-processed later by setting
+     * cmd_suspended to true. The freeing callback will be called from RCU
+     * context later.
+     */
+
     if (vmr->finish_unmapping) {
+        /* 3. MemoryRegion has been freed, so finish unmapping */
         res->mr = NULL;
         g_free(vmr);
 
@@ -166,16 +191,6 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
                           __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;
-- 
2.47.2

Re: [PULL 10/17] virtio-gpu: refactor async blob unmapping

[Akihiko Odaki]

On 2025/06/06 1:26, Alex Bennée wrote:
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> 
> Change the 3 part async cleanup of a blob memory mapping to check if the
> unmapping has finished already after deleting the subregion; this
> condition allows us to skip suspending the command and responding to the
> guest right away.

This patch has no effect because "[PATCH v3 13/20] virtio-gpu: refactor 
async blob unmapping" was dropped:
https://lore.kernel.org/qemu-devel/20250521164250.135776-13-alex.bennee@linaro.org/

This is because dropping the patch prevented the unmapping operation 
from finishing after deleting the subregion. I'll show this by showing 
that the patch broke the reference counting in details first, and then 
by showing that the correct reference counting prevents the unmapping 
operation from finishing.

If !vmr->finish_unmapping when virtio_gpu_virgl_unmap_resource_blob() is 
called, there are the following references:

- owner -> subregion
- container -> owner
- FlatView -> owner

Note that reference 2 and 3 were made by memory_region_ref(), which 
makes references to the owner.

For simplicity, let's assume there are only these references for the 
subregion.

And the following functions are called. Note that the order represents 
the time order and the indention represents the call stack.

The main thread:
  bql_lock()
  virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended)
   memory_region_del_subregion(&b->hostmem, mr)
    memory_region_transaction_commit()
     address_space_set_flatview(as)
      flatview_unref(old_view)
       call_rcu1(&old_view->rcu, flatview_destroy)
        qemu_event_set(&rcu_call_ready_event)
    memory_region_unref(mr)
     object_unref(mr->owner) (a)
   object_unparent(OBJECT(mr))
    object_property_del_child(OBJECT(&b->hostmem), OBJECT(mr))
     object_finalize_child_property(OBJECT(&b->hostmem), name, mr)
      object_unref(mr) (b)

The RCU thread:
  call_rcu_thread(NULL)
   qemu_event_wait(&rcu_call_ready_event)
   bql_lock()
   flatview_destroy(old_view)
     memory_region_unref(mr)
      object_unref(mr->owner) (c)

(c) always happens after (b) due to the BQL. The correspondence between 
the object_unref() call and the removed references are as follows:

(a) container -> owner
(b) owner -> subregion
(c) FlatView -> owner

With "[PATCH v3 13/20] virtio-gpu: refactor async blob unmapping", the 
owner is g (VirtIOGPU). So the correspondence will be interpreted as 
follows:

(a) container -> g
(b) g -> subregion
(c) FlatView -> g

Therefore, the subregion will be freed with (b), incorrectly ignoring 
(c). (b) is made during object_unparent(OBJECT(mr)), so it made the 
unmapping operation finish early.

Without "[PATCH v3 13/20] virtio-gpu: refactor async blob unmapping", 
the owner was the memory region itself. So the correspondence will be 
interpreted as follows:

(a) container -> subregion
(b) subregion -> subregion
(c) FlatView -> subregion

Therefore, the subregion will be freed with (c). So the unmapping 
operation will never finish until the BQL gets unlocked.

Regards,
Akihiko Odaki

>  
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20250410122643.1747913-4-manos.pitsidianakis@linaro.org>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-11-alex.bennee@linaro.org>
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 71a7500de9..b4aa8abb96 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -155,7 +155,32 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>        *    asynchronously by virtio_gpu_virgl_hostmem_region_free().
>        * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
>        */
> +
> +    /* 1. Check if we should start unmapping now */
> +    if (!vmr->finish_unmapping) {
> +        /* begin async unmapping. render will be unblocked once MR is freed */
> +        b->renderer_blocked++;
> +
> +        memory_region_set_enabled(mr, false);
> +        memory_region_del_subregion(&b->hostmem, mr);
> +        object_unparent(OBJECT(mr));
> +        /*
> +         * The unmapping might have already finished at this point if no one
> +         * else held a reference to the MR; if yes, we can skip suspending the
> +         * command and unmap the resource right away.
> +         */
> +        *cmd_suspended = !vmr->finish_unmapping;
> +    }
> +
> +    /*
> +     * 2. if virtio_gpu_virgl_hostmem_region_free hasn't been executed yet, we
> +     * have marked the command to be re-processed later by setting
> +     * cmd_suspended to true. The freeing callback will be called from RCU
> +     * context later.
> +     */
> +
>       if (vmr->finish_unmapping) {
> +        /* 3. MemoryRegion has been freed, so finish unmapping */
>           res->mr = NULL;
>           g_free(vmr);
>   
> @@ -166,16 +191,6 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>                             __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;

[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in refresh

[Alex Bennée]

From: Dongwon Kim <dongwon.kim@intel.com>

This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2
which causes some guest display corruption when gtk-gl-area
is used for GTK rendering (e.g. Wayland Compositor) possibly due to
simulataneous accesses on the guest frame buffer by host compositor
and the guest.

Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle")
Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-12-alex.bennee@linaro.org>

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 8151cc413c..429ba914e0 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -168,7 +168,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
 
     if (vc->gfx.guest_fb.dmabuf &&
         qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
-        gd_gl_area_draw(vc);
         return;
     }
 
-- 
2.47.2

[PULL 12/17] virtio-gpu: support context init multiple timeline

[Alex Bennée]

From: Yiwei Zhang <zzyiwei@gmail.com>

Venus and later native contexts have their own fence context along with
multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
the flags must be dispatched to be created on the target context. Fence
signaling also has to be handled on the specific timeline within that
target context.

Before this change, venus fencing is completely broken if the host
driver doesn't support implicit fencing with external memory objects.
Frames can go backwards along with random artifacts on screen if the
host driver doesn't attach an implicit fence to the render target. The
symptom could be hidden by certain guest wsi backend that waits on a
venus native VkFence object for the actual payload with limited present
modes or under special configs. e.g. x11 mailbox or xwayland.

After this change, everything related to venus fencing starts making
sense. Confirmed this via guest and host side perfetto tracing.

Cc: qemu-stable@nongnu.org
Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
Signed-off-by: Yiwei Zhang <zzyiwei@gmail.com>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Message-Id: <20250518152651.334115-1-zzyiwei@gmail.com>
[AJB: remove version history from commit message]
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-13-alex.bennee@linaro.org>

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b4aa8abb96..cea2e12eb9 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -978,6 +978,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
     }
 
     trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
+#if VIRGL_VERSION_MAJOR >= 1
+    if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
+        virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id,
+                                            VIRGL_RENDERER_FENCE_FLAG_MERGEABLE,
+                                            cmd->cmd_hdr.ring_idx,
+                                            cmd->cmd_hdr.fence_id);
+        return;
+    }
+#endif
     virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
 }
 
@@ -991,6 +1000,11 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
          * the guest can end up emitting fences out of order
          * so we should check all fenced cmds not just the first one.
          */
+#if VIRGL_VERSION_MAJOR >= 1
+        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
+            continue;
+        }
+#endif
         if (cmd->cmd_hdr.fence_id > fence) {
             continue;
         }
@@ -1005,6 +1019,29 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
     }
 }
 
+#if VIRGL_VERSION_MAJOR >= 1
+static void virgl_write_context_fence(void *opaque, uint32_t ctx_id,
+                                      uint32_t ring_idx, uint64_t fence_id) {
+    VirtIOGPU *g = opaque;
+    struct virtio_gpu_ctrl_command *cmd, *tmp;
+
+    QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
+        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX &&
+            cmd->cmd_hdr.ctx_id == ctx_id && cmd->cmd_hdr.ring_idx == ring_idx &&
+            cmd->cmd_hdr.fence_id <= fence_id) {
+            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);
+            g->inflight--;
+            if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+                trace_virtio_gpu_dec_inflight_fences(g->inflight);
+            }
+        }
+    }
+}
+#endif
+
 static virgl_renderer_gl_context
 virgl_create_context(void *opaque, int scanout_idx,
                      struct virgl_renderer_gl_ctx_param *params)
@@ -1039,11 +1076,18 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
 }
 
 static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
+#if VIRGL_VERSION_MAJOR >= 1
+    .version             = 3,
+#else
     .version             = 1,
+#endif
     .write_fence         = virgl_write_fence,
     .create_gl_context   = virgl_create_context,
     .destroy_gl_context  = virgl_destroy_context,
     .make_current        = virgl_make_context_current,
+#if VIRGL_VERSION_MAJOR >= 1
+    .write_context_fence = virgl_write_context_fence,
+#endif
 };
 
 static void virtio_gpu_print_stats(void *opaque)
-- 
2.47.2

Re: [PULL 12/17] virtio-gpu: support context init multiple timeline

[Akihiko Odaki]

On 2025/06/06 1:26, Alex Bennée wrote:
> From: Yiwei Zhang <zzyiwei@gmail.com>
> 
> Venus and later native contexts have their own fence context along with
> multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
> the flags must be dispatched to be created on the target context. Fence
> signaling also has to be handled on the specific timeline within that
> target context.
> 
> Before this change, venus fencing is completely broken if the host
> driver doesn't support implicit fencing with external memory objects.
> Frames can go backwards along with random artifacts on screen if the
> host driver doesn't attach an implicit fence to the render target. The
> symptom could be hidden by certain guest wsi backend that waits on a
> venus native VkFence object for the actual payload with limited present
> modes or under special configs. e.g. x11 mailbox or xwayland.
> 
> After this change, everything related to venus fencing starts making
> sense. Confirmed this via guest and host side perfetto tracing.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")

I suggest moving this in the front of the patch series to ease backporting.

I also wonder if "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in 
refresh" requires Cc: qemu-stable@nongnu.org. Fixing -display gtk,gl=on 
for Wayland sounds as important as this patch.

Regards,
Akihiko Odaki

> Signed-off-by: Yiwei Zhang <zzyiwei@gmail.com>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Message-Id: <20250518152651.334115-1-zzyiwei@gmail.com>
> [AJB: remove version history from commit message]
> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-13-alex.bennee@linaro.org>
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index b4aa8abb96..cea2e12eb9 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -978,6 +978,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>       }
>   
>       trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> +#if VIRGL_VERSION_MAJOR >= 1
> +    if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> +        virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id,
> +                                            VIRGL_RENDERER_FENCE_FLAG_MERGEABLE,
> +                                            cmd->cmd_hdr.ring_idx,
> +                                            cmd->cmd_hdr.fence_id);
> +        return;
> +    }
> +#endif
>       virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
>   }
>   
> @@ -991,6 +1000,11 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
>            * the guest can end up emitting fences out of order
>            * so we should check all fenced cmds not just the first one.
>            */
> +#if VIRGL_VERSION_MAJOR >= 1
> +        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> +            continue;
> +        }
> +#endif
>           if (cmd->cmd_hdr.fence_id > fence) {
>               continue;
>           }
> @@ -1005,6 +1019,29 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
>       }
>   }
>   
> +#if VIRGL_VERSION_MAJOR >= 1
> +static void virgl_write_context_fence(void *opaque, uint32_t ctx_id,
> +                                      uint32_t ring_idx, uint64_t fence_id) {
> +    VirtIOGPU *g = opaque;
> +    struct virtio_gpu_ctrl_command *cmd, *tmp;
> +
> +    QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
> +        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX &&
> +            cmd->cmd_hdr.ctx_id == ctx_id && cmd->cmd_hdr.ring_idx == ring_idx &&
> +            cmd->cmd_hdr.fence_id <= fence_id) {
> +            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);
> +            g->inflight--;
> +            if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
> +                trace_virtio_gpu_dec_inflight_fences(g->inflight);
> +            }
> +        }
> +    }
> +}
> +#endif
> +
>   static virgl_renderer_gl_context
>   virgl_create_context(void *opaque, int scanout_idx,
>                        struct virgl_renderer_gl_ctx_param *params)
> @@ -1039,11 +1076,18 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
>   }
>   
>   static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
> +#if VIRGL_VERSION_MAJOR >= 1
> +    .version             = 3,
> +#else
>       .version             = 1,
> +#endif
>       .write_fence         = virgl_write_fence,
>       .create_gl_context   = virgl_create_context,
>       .destroy_gl_context  = virgl_destroy_context,
>       .make_current        = virgl_make_context_current,
> +#if VIRGL_VERSION_MAJOR >= 1
> +    .write_context_fence = virgl_write_context_fence,
> +#endif
>   };
>   
>   static void virtio_gpu_print_stats(void *opaque)

Re: [PULL 12/17] virtio-gpu: support context init multiple timeline

[Yiwei Zhang]

On Sun, Jun 8, 2025 at 1:24 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> On 2025/06/06 1:26, Alex Bennée wrote:
> > From: Yiwei Zhang <zzyiwei@gmail.com>
> >
> > Venus and later native contexts have their own fence context along with
> > multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
> > the flags must be dispatched to be created on the target context. Fence
> > signaling also has to be handled on the specific timeline within that
> > target context.
> >
> > Before this change, venus fencing is completely broken if the host
> > driver doesn't support implicit fencing with external memory objects.
> > Frames can go backwards along with random artifacts on screen if the
> > host driver doesn't attach an implicit fence to the render target. The
> > symptom could be hidden by certain guest wsi backend that waits on a
> > venus native VkFence object for the actual payload with limited present
> > modes or under special configs. e.g. x11 mailbox or xwayland.
> >
> > After this change, everything related to venus fencing starts making
> > sense. Confirmed this via guest and host side perfetto tracing.
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
>
> I suggest moving this in the front of the patch series to ease backporting.
>
> I also wonder if "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in
> refresh" requires Cc: qemu-stable@nongnu.org. Fixing -display gtk,gl=on
> for Wayland sounds as important as this patch.
>
> Regards,
> Akihiko Odaki

Hi Alex,

+1 for Akihiko's point. I'm also curious when will the venus fix land in-tree?

Best,
Yiwei

>
> > Signed-off-by: Yiwei Zhang <zzyiwei@gmail.com>
> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Message-Id: <20250518152651.334115-1-zzyiwei@gmail.com>
> > [AJB: remove version history from commit message]
> > Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > Message-ID: <20250603110204.838117-13-alex.bennee@linaro.org>
> >
> > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> > index b4aa8abb96..cea2e12eb9 100644
> > --- a/hw/display/virtio-gpu-virgl.c
> > +++ b/hw/display/virtio-gpu-virgl.c
> > @@ -978,6 +978,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> >       }
> >
> >       trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> > +#if VIRGL_VERSION_MAJOR >= 1
> > +    if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> > +        virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id,
> > +                                            VIRGL_RENDERER_FENCE_FLAG_MERGEABLE,
> > +                                            cmd->cmd_hdr.ring_idx,
> > +                                            cmd->cmd_hdr.fence_id);
> > +        return;
> > +    }
> > +#endif
> >       virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> >   }
> >
> > @@ -991,6 +1000,11 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
> >            * the guest can end up emitting fences out of order
> >            * so we should check all fenced cmds not just the first one.
> >            */
> > +#if VIRGL_VERSION_MAJOR >= 1
> > +        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> > +            continue;
> > +        }
> > +#endif
> >           if (cmd->cmd_hdr.fence_id > fence) {
> >               continue;
> >           }
> > @@ -1005,6 +1019,29 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
> >       }
> >   }
> >
> > +#if VIRGL_VERSION_MAJOR >= 1
> > +static void virgl_write_context_fence(void *opaque, uint32_t ctx_id,
> > +                                      uint32_t ring_idx, uint64_t fence_id) {
> > +    VirtIOGPU *g = opaque;
> > +    struct virtio_gpu_ctrl_command *cmd, *tmp;
> > +
> > +    QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
> > +        if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX &&
> > +            cmd->cmd_hdr.ctx_id == ctx_id && cmd->cmd_hdr.ring_idx == ring_idx &&
> > +            cmd->cmd_hdr.fence_id <= fence_id) {
> > +            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);
> > +            g->inflight--;
> > +            if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
> > +                trace_virtio_gpu_dec_inflight_fences(g->inflight);
> > +            }
> > +        }
> > +    }
> > +}
> > +#endif
> > +
> >   static virgl_renderer_gl_context
> >   virgl_create_context(void *opaque, int scanout_idx,
> >                        struct virgl_renderer_gl_ctx_param *params)
> > @@ -1039,11 +1076,18 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
> >   }
> >
> >   static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
> > +#if VIRGL_VERSION_MAJOR >= 1
> > +    .version             = 3,
> > +#else
> >       .version             = 1,
> > +#endif
> >       .write_fence         = virgl_write_fence,
> >       .create_gl_context   = virgl_create_context,
> >       .destroy_gl_context  = virgl_destroy_context,
> >       .make_current        = virgl_make_context_current,
> > +#if VIRGL_VERSION_MAJOR >= 1
> > +    .write_context_fence = virgl_write_context_fence,
> > +#endif
> >   };
> >
> >   static void virtio_gpu_print_stats(void *opaque)
>

Re: [PULL 12/17] virtio-gpu: support context init multiple timeline

[Alex Bennée]

Yiwei Zhang <zzyiwei@gmail.com> writes:

> On Sun, Jun 8, 2025 at 1:24 AM Akihiko Odaki
> <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> On 2025/06/06 1:26, Alex Bennée wrote:
>> > From: Yiwei Zhang <zzyiwei@gmail.com>
>> >
>> > Venus and later native contexts have their own fence context along with
>> > multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
>> > the flags must be dispatched to be created on the target context. Fence
>> > signaling also has to be handled on the specific timeline within that
>> > target context.
>> >
>> > Before this change, venus fencing is completely broken if the host
>> > driver doesn't support implicit fencing with external memory objects.
>> > Frames can go backwards along with random artifacts on screen if the
>> > host driver doesn't attach an implicit fence to the render target. The
>> > symptom could be hidden by certain guest wsi backend that waits on a
>> > venus native VkFence object for the actual payload with limited present
>> > modes or under special configs. e.g. x11 mailbox or xwayland.
>> >
>> > After this change, everything related to venus fencing starts making
>> > sense. Confirmed this via guest and host side perfetto tracing.
>> >
>> > Cc: qemu-stable@nongnu.org
>> > Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
>>
>> I suggest moving this in the front of the patch series to ease backporting.
>>
>> I also wonder if "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in
>> refresh" requires Cc: qemu-stable@nongnu.org. Fixing -display gtk,gl=on
>> for Wayland sounds as important as this patch.
>>
>> Regards,
>> Akihiko Odaki
>
> Hi Alex,
>
> +1 for Akihiko's point. I'm also curious when will the venus fix land
> in-tree?

We have a problem that there are two contradictory bugs - one that shows
up in the x86/kvm case and one in the aarch64/tcg case. Both are caused
by the weird lifetime semantics of the virgl resource vs QEMU memory
region and we haven't found a solution that solves both yet.

I'm currently busy with other stuff and need to do a sweep of my other
maintainer trees for 10.1. Once I've done that I'll have another go at
coming up with a solution unless someone else beats me to it.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PULL 12/17] virtio-gpu: support context init multiple timeline

[Yiwei Zhang]

On Thu, Jun 19, 2025 at 11:45 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Yiwei Zhang <zzyiwei@gmail.com> writes:
>
> > On Sun, Jun 8, 2025 at 1:24 AM Akihiko Odaki
> > <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
> >>
> >> On 2025/06/06 1:26, Alex Bennée wrote:
> >> > From: Yiwei Zhang <zzyiwei@gmail.com>
> >> >
> >> > Venus and later native contexts have their own fence context along with
> >> > multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
> >> > the flags must be dispatched to be created on the target context. Fence
> >> > signaling also has to be handled on the specific timeline within that
> >> > target context.
> >> >
> >> > Before this change, venus fencing is completely broken if the host
> >> > driver doesn't support implicit fencing with external memory objects.
> >> > Frames can go backwards along with random artifacts on screen if the
> >> > host driver doesn't attach an implicit fence to the render target. The
> >> > symptom could be hidden by certain guest wsi backend that waits on a
> >> > venus native VkFence object for the actual payload with limited present
> >> > modes or under special configs. e.g. x11 mailbox or xwayland.
> >> >
> >> > After this change, everything related to venus fencing starts making
> >> > sense. Confirmed this via guest and host side perfetto tracing.
> >> >
> >> > Cc: qemu-stable@nongnu.org
> >> > Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
> >>
> >> I suggest moving this in the front of the patch series to ease backporting.
> >>
> >> I also wonder if "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in
> >> refresh" requires Cc: qemu-stable@nongnu.org. Fixing -display gtk,gl=on
> >> for Wayland sounds as important as this patch.
> >>
> >> Regards,
> >> Akihiko Odaki
> >
> > Hi Alex,
> >
> > +1 for Akihiko's point. I'm also curious when will the venus fix land
> > in-tree?
>
> We have a problem that there are two contradictory bugs - one that shows
> up in the x86/kvm case and one in the aarch64/tcg case. Both are caused
> by the weird lifetime semantics of the virgl resource vs QEMU memory
> region and we haven't found a solution that solves both yet.

Sounds like irrelevant to the venus fix. Might be worth filing a
virglrenderer issue with some details. More eyes would be helpful if
this turns out to be some known kvm issue seen before on other vmms.

>
> I'm currently busy with other stuff and need to do a sweep of my other
> maintainer trees for 10.1. Once I've done that I'll have another go at
> coming up with a solution unless someone else beats me to it.

Understood. Hope there's a chance to move the venus fix up in the
queue to land first when you get to this again. Thanks!

>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro

Re: [PULL 12/17] virtio-gpu: support context init multiple timeline

[Akihiko Odaki]

On 2025/06/21 4:47, Yiwei Zhang wrote:
> On Thu, Jun 19, 2025 at 11:45 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Yiwei Zhang <zzyiwei@gmail.com> writes:
>>
>>> On Sun, Jun 8, 2025 at 1:24 AM Akihiko Odaki
>>> <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>>>
>>>> On 2025/06/06 1:26, Alex Bennée wrote:
>>>>> From: Yiwei Zhang <zzyiwei@gmail.com>
>>>>>
>>>>> Venus and later native contexts have their own fence context along with
>>>>> multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
>>>>> the flags must be dispatched to be created on the target context. Fence
>>>>> signaling also has to be handled on the specific timeline within that
>>>>> target context.
>>>>>
>>>>> Before this change, venus fencing is completely broken if the host
>>>>> driver doesn't support implicit fencing with external memory objects.
>>>>> Frames can go backwards along with random artifacts on screen if the
>>>>> host driver doesn't attach an implicit fence to the render target. The
>>>>> symptom could be hidden by certain guest wsi backend that waits on a
>>>>> venus native VkFence object for the actual payload with limited present
>>>>> modes or under special configs. e.g. x11 mailbox or xwayland.
>>>>>
>>>>> After this change, everything related to venus fencing starts making
>>>>> sense. Confirmed this via guest and host side perfetto tracing.
>>>>>
>>>>> Cc: qemu-stable@nongnu.org
>>>>> Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
>>>>
>>>> I suggest moving this in the front of the patch series to ease backporting.
>>>>
>>>> I also wonder if "[PULL 11/17] ui/gtk-gl-area: Remove extra draw call in
>>>> refresh" requires Cc: qemu-stable@nongnu.org. Fixing -display gtk,gl=on
>>>> for Wayland sounds as important as this patch.
>>>>
>>>> Regards,
>>>> Akihiko Odaki
>>>
>>> Hi Alex,
>>>
>>> +1 for Akihiko's point. I'm also curious when will the venus fix land
>>> in-tree?
>>
>> We have a problem that there are two contradictory bugs - one that shows
>> up in the x86/kvm case and one in the aarch64/tcg case. Both are caused
>> by the weird lifetime semantics of the virgl resource vs QEMU memory
>> region and we haven't found a solution that solves both yet.
> 
> Sounds like irrelevant to the venus fix. Might be worth filing a
> virglrenderer issue with some details. More eyes would be helpful if
> this turns out to be some known kvm issue seen before on other vmms.

This patch itself looks good to me so:

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

It's up to Alex whether to wait until other patches (the blob 
mapping/memory region fix in particular) piles up or not.

Regards,
Akihiko Odaki

[PULL 13/17] include/exec: fix assert in size_memop

[Alex Bennée]

We can handle larger sized memops now, expand the range of the assert.

Fixes: 4b473e0c60 (tcg: Expand MO_SIZE to 3 bits)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-14-alex.bennee@linaro.org>

diff --git a/include/exec/memop.h b/include/exec/memop.h
index 407a47d82c..cf7da3362e 100644
--- a/include/exec/memop.h
+++ b/include/exec/memop.h
@@ -162,8 +162,8 @@ static inline unsigned memop_size(MemOp op)
 static inline MemOp size_memop(unsigned size)
 {
 #ifdef CONFIG_DEBUG_TCG
-    /* Power of 2 up to 8.  */
-    assert((size & (size - 1)) == 0 && size >= 1 && size <= 8);
+    /* Power of 2 up to 1024 */
+    assert(is_power_of_2(size) && size >= 1 && size <= (1 << MO_SIZE));
 #endif
     return (MemOp)ctz32(size);
 }
-- 
2.47.2

Re: [PULL 13/17] include/exec: fix assert in size_memop

[Akihiko Odaki]

On 2025/06/06 1:26, Alex Bennée wrote:
> We can handle larger sized memops now, expand the range of the assert.
> 
> Fixes: 4b473e0c60 (tcg: Expand MO_SIZE to 3 bits)
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-14-alex.bennee@linaro.org>
> 
> diff --git a/include/exec/memop.h b/include/exec/memop.h
> index 407a47d82c..cf7da3362e 100644
> --- a/include/exec/memop.h
> +++ b/include/exec/memop.h
> @@ -162,8 +162,8 @@ static inline unsigned memop_size(MemOp op)
>   static inline MemOp size_memop(unsigned size)
>   {
>   #ifdef CONFIG_DEBUG_TCG
> -    /* Power of 2 up to 8.  */
> -    assert((size & (size - 1)) == 0 && size >= 1 && size <= 8);
> +    /* Power of 2 up to 1024 */

I suggested using is_power_of_2() to remove the comment entirely to 
avoid hardcoding a number. Please refer to:
https://lore.kernel.org/qemu-devel/eec76ce0-c3ca-48ed-befe-e0930d4a39d9@linaro.org/

> +    assert(is_power_of_2(size) && size >= 1 && size <= (1 << MO_SIZE));

size >= 1 is unnecessary because being power of 2 implies that. The 
comment doesn't mention this condition either.

Regards,
Akihiko Odaki

>   #endif
>       return (MemOp)ctz32(size);
>   }

[PULL 14/17] include/gdbstub: fix include guard in commands.h

[Alex Bennée]

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-15-alex.bennee@linaro.org>

diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index 40f0514fe9..bff3674872 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -1,5 +1,5 @@
 #ifndef GDBSTUB_COMMANDS_H
-#define GDBSTUB
+#define GDBSTUB_COMMANDS_H
 
 typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
 
-- 
2.47.2

[PULL 15/17] gdbstub: assert earlier in handle_read_all_regs

[Alex Bennée]

When things go wrong we want to assert on the register that failed to
be able to figure out what went wrong.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-16-alex.bennee@linaro.org>

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 565f6b33a9..6023c80d25 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1343,8 +1343,8 @@ static void handle_read_all_regs(GArray *params, void *user_ctx)
         len += gdb_read_register(gdbserver_state.g_cpu,
                                  gdbserver_state.mem_buf,
                                  reg_id);
+        g_assert(len == gdbserver_state.mem_buf->len);
     }
-    g_assert(len == gdbserver_state.mem_buf->len);
 
     gdb_memtohex(gdbserver_state.str_buf, gdbserver_state.mem_buf->data, len);
     gdb_put_strbuf();
-- 
2.47.2

[PULL 16/17] gdbstub: Implement qGDBServerVersion packet

[Alex Bennée]

From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>

This commit adds support for the `qGDBServerVersion` packet to the qemu
gdbstub  which could be used by clients to detect the QEMU version
(and, e.g., use a workaround for known bugs).

This packet is not documented/standarized by GDB but it was implemented
by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1].

This has been implemented by Patryk, who I included in Co-authored-by
and who asked me to send the patch.

[0] https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion
[1] https://github.com/pwndbg/pwndbg/issues/2648

Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com>
[AJB: fix include, checkpatch linewrap]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-17-alex.bennee@linaro.org>

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6023c80d25..def0b7e877 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -28,6 +28,7 @@
 #include "qemu/cutils.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
+#include "qemu/target-info.h"
 #include "trace.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/commands.h"
@@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx)
     gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
 }
 
+static void handle_query_gdb_server_version(GArray *params, void *user_ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;",
+                    target_name(), QEMU_VERSION);
+#else
+    g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;",
+                    target_name(), QEMU_VERSION);
+#endif
+    gdb_put_strbuf();
+}
+
 static void handle_query_first_threads(GArray *params, void *user_ctx)
 {
     gdbserver_state.query_cpu = gdb_first_attached_cpu();
@@ -1842,6 +1855,10 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
         .handler = handle_query_threads,
         .cmd = "sThreadInfo",
     },
+    {
+        .handler = handle_query_gdb_server_version,
+        .cmd = "GDBServerVersion",
+    },
     {
         .handler = handle_query_first_threads,
         .cmd = "fThreadInfo",
-- 
2.47.2

[PULL 17/17] gdbstub: update aarch64-core.xml

[Alex Bennée]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Update aarch64-core.xml to include field definitions for PSTATE, which
in gdb is modelled in the cpsr (current program status register)
pseudo-register, named after the actual cpsr register in armv7.

Defining the fields layout of the register allows easy inspection of for
example, the current exception level (EL):

For example. Before booting a Linux guest, EL=2, but after booting and
Ctrl-C'ing in gdb, we get EL=0:

  (gdb) info registers $cpsr
  cpsr           0x20402009          [ SP EL=2 BTYPE=0 PAN C ]
  (gdb) cont
  Continuing.
  ^C
  Thread 2 received signal SIGINT, Interrupt.
  0x0000ffffaaff286c in ?? ()
  (gdb) info registers $cpsr
  cpsr           0x20001000          [ EL=0 BTYPE=0 SSBS C ]

The aarch64-core.xml has been updated to match exactly the version
retrieved from upstream gdb, retrieved in 2025-05-19 from HEAD commit
9f4dc0b137c86f6ff2098cb1ab69442c69d6023d.

Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/features/aarch64-core.xml;h=b8046510b9a085d30463d37b3ecc8d435f5fb7a4;hb=HEAD
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Message-Id: <20250519-gdbstub-aarch64-pstate-xml-v1-1-b4dbe87fe7c6@linaro.org>
[AJB: expanded upstream link]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-18-alex.bennee@linaro.org>

diff --git a/gdb-xml/aarch64-core.xml b/gdb-xml/aarch64-core.xml
index e1e9dc3f91..b8046510b9 100644
--- a/gdb-xml/aarch64-core.xml
+++ b/gdb-xml/aarch64-core.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0"?>
-<!-- Copyright (C) 2009-2012 Free Software Foundation, Inc.
+<!-- Copyright (C) 2009-2025 Free Software Foundation, Inc.
      Contributed by ARM Ltd.
 
      Copying and distribution of this file, with or without modification,
@@ -42,5 +42,53 @@
   <reg name="sp" bitsize="64" type="data_ptr"/>
 
   <reg name="pc" bitsize="64" type="code_ptr"/>
-  <reg name="cpsr" bitsize="32"/>
+
+  <flags id="cpsr_flags" size="4">
+    <!-- Stack Pointer.  -->
+    <field name="SP" start="0" end="0"/>
+
+    <!-- Exception Level.  -->
+    <field name="EL" start="2" end="3"/>
+    <!-- Execution state.  -->
+    <field name="nRW" start="4" end="4"/>
+
+    <!-- FIQ interrupt mask.  -->
+    <field name="F" start="6" end="6"/>
+    <!-- IRQ interrupt mask.  -->
+    <field name="I" start="7" end="7"/>
+    <!-- SError interrupt mask.  -->
+    <field name="A" start="8" end="8"/>
+    <!-- Debug exception mask.  -->
+    <field name="D" start="9" end="9"/>
+
+    <!-- ARMv8.5-A: Branch Target Identification BTYPE.  -->
+    <field name="BTYPE" start="10" end="11"/>
+
+    <!-- ARMv8.0-A: Speculative Store Bypass.  -->
+    <field name="SSBS" start="12" end="12"/>
+
+    <!-- Illegal Execution state.  -->
+    <field name="IL" start="20" end="20"/>
+    <!-- Software Step.  -->
+    <field name="SS" start="21" end="21"/>
+    <!-- ARMv8.1-A: Privileged Access Never.  -->
+    <field name="PAN" start="22" end="22"/>
+    <!-- ARMv8.2-A: User Access Override.  -->
+    <field name="UAO" start="23" end="23"/>
+    <!-- ARMv8.4-A: Data Independent Timing.  -->
+    <field name="DIT" start="24" end="24"/>
+    <!-- ARMv8.5-A: Tag Check Override.  -->
+    <field name="TCO" start="25" end="25"/>
+
+    <!-- Overflow Condition flag.  -->
+    <field name="V" start="28" end="28"/>
+    <!-- Carry Condition flag.  -->
+    <field name="C" start="29" end="29"/>
+    <!-- Zero Condition flag.  -->
+    <field name="Z" start="30" end="30"/>
+    <!-- Negative Condition flag.  -->
+    <field name="N" start="31" end="31"/>
+  </flags>
+  <reg name="cpsr" bitsize="32" type="cpsr_flags"/>
+
 </feature>
-- 
2.47.2

Re: [PULL 00/17] maintainer updates for May (testing, plugins, virtio-gpu)

[Stefan Hajnoczi]

On Thu, Jun 5, 2025 at 12:28 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 09be8a511a2e278b45729d7b065d30c68dd699d0:
>
>   Merge tag 'pull-qapi-2025-06-03' of https://repo.or.cz/qemu/armbru into staging (2025-06-03 09:19:26 -0400)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stsquad/qemu.git tags/pull-10.1-maintainer-may-2025-050625-1
>
> for you to fetch changes up to 66835968ca7246d385218be9776a80a5136563b7:
>
>   gdbstub: update aarch64-core.xml (2025-06-05 13:47:37 +0100)

I will wait for the discussion to reach a conclusion before merging
this pull request.

Stefan

>
> ----------------------------------------------------------------
> Maintainer updates for May 2025:
>
>   - expose ~/.cache/qemu to container builds
>   - disable debug info in CI
>   - allow boot.S to handle target el mode selection
>   - new arguments for ips plugin
>   - update virtio-gpu MAINTAINERS
>   - re-factoring of blob MemoryRegion handling
>   - remove extra draw call causing corruption
>   - support Venus fence contexts
>   - cleanup assets in size_memop
>   - fix include guard in gdbstub
>   - introduce qGDBServerVersion gdbstub query
>   - update gdb aarch64-core.xml to support bitfields
>
> ----------------------------------------------------------------
> Alex Bennée (11):
>       tests/docker: expose $HOME/.cache/qemu as docker volume
>       gitlab: disable debug info on CI builds
>       tests/tcg: make aarch64 boot.S handle different starting modes
>       contrib/plugins: add a scaling factor to the ips arg
>       contrib/plugins: allow setting of instructions per quantum
>       MAINTAINERS: add myself to virtio-gpu for Odd Fixes
>       MAINTAINERS: add Akihiko and Dmitry as reviewers
>       hw/display: re-arrange memory region tracking
>       include/exec: fix assert in size_memop
>       include/gdbstub: fix include guard in commands.h
>       gdbstub: assert earlier in handle_read_all_regs
>
> Dominik 'Disconnect3d' Czarnota (1):
>       gdbstub: Implement qGDBServerVersion packet
>
> Dongwon Kim (1):
>       ui/gtk-gl-area: Remove extra draw call in refresh
>
> Manos Pitsidianakis (2):
>       virtio-gpu: refactor async blob unmapping
>       gdbstub: update aarch64-core.xml
>
> Nabih Estefan (1):
>       tests/qtest: Avoid unaligned access in IGB test
>
> Yiwei Zhang (1):
>       virtio-gpu: support context init multiple timeline
>
>  MAINTAINERS                               |   5 +-
>  docs/about/emulation.rst                  |   4 +
>  include/exec/memop.h                      |   4 +-
>  include/gdbstub/commands.h                |   2 +-
>  include/system/memory.h                   |   1 +
>  contrib/plugins/ips.c                     |  49 ++++++++-
>  gdbstub/gdbstub.c                         |  19 +++-
>  hw/display/virtio-gpu-virgl.c             | 102 +++++++++++++-----
>  tests/qtest/libqos/igb.c                  |   4 +-
>  ui/gtk-gl-area.c                          |   1 -
>  .gitlab-ci.d/buildtest-template.yml       |   1 +
>  gdb-xml/aarch64-core.xml                  |  52 ++++++++-
>  tests/docker/Makefile.include             |  10 +-
>  tests/tcg/aarch64/Makefile.softmmu-target |   3 +-
>  tests/tcg/aarch64/system/boot.S           | 172 +++++++++++++++++++++++++++++-
>  15 files changed, 382 insertions(+), 47 deletions(-)
>
> --
> 2.47.2
>
>

Re: [PULL 00/17] maintainer updates for May (testing, plugins, virtio-gpu)

[Alex Bennée]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Thu, Jun 5, 2025 at 12:28 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> The following changes since commit 09be8a511a2e278b45729d7b065d30c68dd699d0:
>>
>>   Merge tag 'pull-qapi-2025-06-03' of https://repo.or.cz/qemu/armbru into staging (2025-06-03 09:19:26 -0400)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/stsquad/qemu.git tags/pull-10.1-maintainer-may-2025-050625-1
>>
>> for you to fetch changes up to 66835968ca7246d385218be9776a80a5136563b7:
>>
>>   gdbstub: update aarch64-core.xml (2025-06-05 13:47:37 +0100)
>
> I will wait for the discussion to reach a conclusion before merging
> this pull request.

I've dropped the virtio-gpu/next bits and sent v2.

>
> Stefan
>
>>
>> ----------------------------------------------------------------
>> Maintainer updates for May 2025:
>>
>>   - expose ~/.cache/qemu to container builds
>>   - disable debug info in CI
>>   - allow boot.S to handle target el mode selection
>>   - new arguments for ips plugin
>>   - update virtio-gpu MAINTAINERS
>>   - re-factoring of blob MemoryRegion handling
>>   - remove extra draw call causing corruption
>>   - support Venus fence contexts
>>   - cleanup assets in size_memop
>>   - fix include guard in gdbstub
>>   - introduce qGDBServerVersion gdbstub query
>>   - update gdb aarch64-core.xml to support bitfields
>>
>> ----------------------------------------------------------------
>> Alex Bennée (11):
>>       tests/docker: expose $HOME/.cache/qemu as docker volume
>>       gitlab: disable debug info on CI builds
>>       tests/tcg: make aarch64 boot.S handle different starting modes
>>       contrib/plugins: add a scaling factor to the ips arg
>>       contrib/plugins: allow setting of instructions per quantum
>>       MAINTAINERS: add myself to virtio-gpu for Odd Fixes
>>       MAINTAINERS: add Akihiko and Dmitry as reviewers
>>       hw/display: re-arrange memory region tracking
>>       include/exec: fix assert in size_memop
>>       include/gdbstub: fix include guard in commands.h
>>       gdbstub: assert earlier in handle_read_all_regs
>>
>> Dominik 'Disconnect3d' Czarnota (1):
>>       gdbstub: Implement qGDBServerVersion packet
>>
>> Dongwon Kim (1):
>>       ui/gtk-gl-area: Remove extra draw call in refresh
>>
>> Manos Pitsidianakis (2):
>>       virtio-gpu: refactor async blob unmapping
>>       gdbstub: update aarch64-core.xml
>>
>> Nabih Estefan (1):
>>       tests/qtest: Avoid unaligned access in IGB test
>>
>> Yiwei Zhang (1):
>>       virtio-gpu: support context init multiple timeline
>>
>>  MAINTAINERS                               |   5 +-
>>  docs/about/emulation.rst                  |   4 +
>>  include/exec/memop.h                      |   4 +-
>>  include/gdbstub/commands.h                |   2 +-
>>  include/system/memory.h                   |   1 +
>>  contrib/plugins/ips.c                     |  49 ++++++++-
>>  gdbstub/gdbstub.c                         |  19 +++-
>>  hw/display/virtio-gpu-virgl.c             | 102 +++++++++++++-----
>>  tests/qtest/libqos/igb.c                  |   4 +-
>>  ui/gtk-gl-area.c                          |   1 -
>>  .gitlab-ci.d/buildtest-template.yml       |   1 +
>>  gdb-xml/aarch64-core.xml                  |  52 ++++++++-
>>  tests/docker/Makefile.include             |  10 +-
>>  tests/tcg/aarch64/Makefile.softmmu-target |   3 +-
>>  tests/tcg/aarch64/system/boot.S           | 172 +++++++++++++++++++++++++++++-
>>  15 files changed, 382 insertions(+), 47 deletions(-)
>>
>> --
>> 2.47.2
>>
>>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro