[PATCH v2 00/12] Fixes for 11.1

[PATCH v2 00/12] Fixes for 11.1

[Marc-André Lureau]

Hi

Here are some assorted patches worth considering for the 11.1 freeze
bug-fix period.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
Changes in v2, after Akihiko comments:
- split the "Fix many -Werror=maybe-uninitialized" patch
- qdev-monitor: add drain_rcu parameter to avoid deadlock from virtio-net
- rewrite WITH_QEMU_LOCK_GUARD/WITH_RCU_READ_LOCK_GUARD macros to suppress
  GCC false-positive warnings (new patch)
- block/blkio: use QEMU_LOCK_GUARD instead of WITH_QEMU_LOCK_GUARD in
  getlength() to avoid false-positive (new patch)
- hw/cxl: split invalid-free fix into its own patch
- hw/i3c: fix default return value of d2_i3c_send() (new patch)
- hw/ppc: fix dangling machine->fdt reference after spapr fdt updates (new patch)
- Link to v1: https://lore.kernel.org/qemu-devel/20260725-fix2-v1-0-5c9a6ce7b96a@redhat.com

---
Marc-André Lureau (12):
      hw/display/virtio-gpu-rutabaga: zero-init capset info response
      block/blkio: fix error return value on getlength()
      block/blkio: fix compiler false-positive warning
      include/qemu: adjust LOCK_GUARD macros to avoid potential warning
      hw/cxl: fix invalid free on early return
      hw/i3c: fix default return value of d2_i3c_send()
      Fix some -Werror=maybe-uninitialized
      hw/hexagon: fix machine->fdt leak in qom-test
      hw/ppc: fix dangling fdt reference
      hw/core/machine: free machine->fdt in machine_finalize()
      migration/multifd: fix Error leak in multifd_recv_terminate_threads()
      qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict

 include/monitor/qdev.h           |  3 ++-
 include/qemu/lockable.h          | 19 +++++++++++-------
 include/qemu/rcu.h               | 17 ++++++++++------
 block/blkio.c                    |  7 +++----
 hw/audio/intel-hda.c             |  2 +-
 hw/core/machine.c                |  1 +
 hw/cxl/cxl-host.c                |  2 +-
 hw/display/virtio-gpu-rutabaga.c |  2 ++
 hw/hexagon/virt.c                |  9 +--------
 hw/i3c/dw-i3c.c                  |  2 +-
 hw/net/virtio-net.c              |  2 +-
 hw/ppc/spapr.c                   |  1 +
 hw/ppc/spapr_hcall.c             |  1 +
 migration/multifd.c              |  1 +
 system/qdev-monitor.c            | 42 +++++++++++++++-------------------------
 target/i386/cpu.c                |  3 ++-
 target/i386/emulate/x86_mmu.c    |  4 ++--
 17 files changed, 59 insertions(+), 59 deletions(-)
---
base-commit: 300438ffbb8d9430cac2fcc15cba6f482b2c0587
change-id: 20260725-fix2-6e51d231794b

Best regards,
--  
Marc-André Lureau <marcandre.lureau@redhat.com>

[PATCH v2 01/12] hw/display/virtio-gpu-rutabaga: zero-init capset info response

[Marc-André Lureau]

rutabaga_cmd_get_capset_info() only fills in capset_id,
capset_max_version and capset_max_size before sending the response to
the guest. The remaining fields of struct virtio_gpu_resp_capset_info,
including hdr.fence_id, hdr.ctx_id and hdr.ring_idx, are left with
stack garbage and leaked to the guest, including host pointers useful
for an ASLR bypass.

Zero the response first, matching virgl_cmd_get_capset_info().

Not a real risk thanks to -ftrivial-auto-var-init=zero, but only with
gcc >= 12 or clang >= 16.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3609
Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
Reported-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/virtio-gpu-rutabaga.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 4d7d7b245929..95a19eed7eda 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -555,6 +555,8 @@ rutabaga_cmd_get_capset_info(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
 
     VIRTIO_GPU_FILL_CMD(info);
 
+    memset(&resp, 0, sizeof(resp));
+
     result = rutabaga_get_capset_info(vr->rutabaga, info.capset_index,
                                       &resp.capset_id, &resp.capset_max_version,
                                       &resp.capset_max_size);

-- 
2.55.0

[PATCH v2 02/12] block/blkio: fix error return value on getlength()

[Marc-André Lureau]

Negative values should be returned for errors.

Fixes: fd66dbd424 ("blkio: add libblkio block driver")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/blkio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blkio.c b/block/blkio.c
index fb8bec27d71a..52a8be220d1c 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -941,7 +941,7 @@ static int64_t coroutine_fn blkio_co_getlength(BlockDriverState *bs)
         ret = blkio_get_uint64(s->blkio, "capacity", &capacity);
     }
     if (ret < 0) {
-        return -ret;
+        return ret;
     }
 
     return capacity;

-- 
2.55.0

[PATCH v2 03/12] block/blkio: fix compiler false-positive warning

[Marc-André Lureau]

Under some optimization, gcc produces a false-positive:
    ../block/blkio.c: In function ‘blkio_co_getlength’:
    ../block/blkio.c:943:8: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
      943 |     if (ret < 0) {
          |        ^

Replace WITH_QEMU_LOCK_GUARD with the simpler QEMU_LOCK_GUARD.

Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/blkio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index 52a8be220d1c..4801339f98b2 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -937,9 +937,8 @@ static int64_t coroutine_fn blkio_co_getlength(BlockDriverState *bs)
     uint64_t capacity;
     int ret;
 
-    WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-        ret = blkio_get_uint64(s->blkio, "capacity", &capacity);
-    }
+    QEMU_LOCK_GUARD(&s->blkio_lock);
+    ret = blkio_get_uint64(s->blkio, "capacity", &capacity);
     if (ret < 0) {
         return ret;
     }

-- 
2.55.0

[PATCH v2 04/12] include/qemu: adjust LOCK_GUARD macros to avoid potential warning

[Marc-André Lureau]

Use the same trick used by Linux (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fcc22ac5baf06dd17193de44b60dbcee):
use the compound statement expression with "goto" jumping past the
"loop", effectively leaving only the subscope part of the loop
semantics.

The old loop had to handle auto_unlock to set the var to NULL, but now
it can rely on autoptr.

Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/lockable.h | 19 ++++++++++++-------
 include/qemu/rcu.h      | 17 +++++++++++------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 66713bd42921..2704a1ec077d 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -130,11 +130,15 @@ static inline void qemu_lockable_auto_unlock(QemuLockable *x)
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
 
-#define WITH_QEMU_LOCK_GUARD_(x, var) \
-    for (g_autoptr(QemuLockable) var = \
-                qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE_NONNULL((x))); \
-         var; \
-         qemu_lockable_auto_unlock(var), var = NULL)
+#define WITH_QEMU_LOCK_GUARD_(x, var, label)                        \
+    for (g_autoptr(QemuLockable) var =                              \
+            qemu_lockable_auto_lock(                                \
+                QEMU_MAKE_LOCKABLE_NONNULL((x)));                   \
+         ; ({ goto label; }))                                       \
+        if (0) {                                                    \
+        label:                                                      \
+            break;                                                  \
+        } else
 
 /**
  * WITH_QEMU_LOCK_GUARD - Lock a lock object for scope
@@ -158,8 +162,9 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
  *       ...
  *   }
  */
-#define WITH_QEMU_LOCK_GUARD(x) \
-    WITH_QEMU_LOCK_GUARD_((x), glue(qemu_lockable_auto, __COUNTER__))
+#define WITH_QEMU_LOCK_GUARD(x)                                     \
+    WITH_QEMU_LOCK_GUARD_((x), glue(qemu_lockable_auto, __COUNTER__), \
+                           glue(qemu_lockable_label_, __COUNTER__))
 
 /**
  * QEMU_LOCK_GUARD - Lock an object until the end of the scope
diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 020dbe4d8b77..41d02255ceae 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -178,12 +178,17 @@ static inline void rcu_read_auto_unlock(RCUReadAuto *r)
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(RCUReadAuto, rcu_read_auto_unlock)
 
-#define WITH_RCU_READ_LOCK_GUARD() \
-    WITH_RCU_READ_LOCK_GUARD_(glue(_rcu_read_auto, __COUNTER__))
-
-#define WITH_RCU_READ_LOCK_GUARD_(var) \
-    for (g_autoptr(RCUReadAuto) var = rcu_read_auto_lock(); \
-        (var); rcu_read_auto_unlock(var), (var) = NULL)
+#define WITH_RCU_READ_LOCK_GUARD()                                  \
+    WITH_RCU_READ_LOCK_GUARD_(glue(_rcu_read_auto, __COUNTER__),    \
+                              glue(_rcu_read_label_, __COUNTER__))
+
+#define WITH_RCU_READ_LOCK_GUARD_(var, label)                       \
+    for (g_autoptr(RCUReadAuto) var = rcu_read_auto_lock();         \
+         ; ({ goto label; }))                                       \
+        if (0) {                                                    \
+        label:                                                      \
+            break;                                                  \
+        } else
 
 #define RCU_READ_LOCK_GUARD() \
     g_autoptr(RCUReadAuto) _rcu_read_auto __attribute__((unused)) = rcu_read_auto_lock()

-- 
2.55.0

[PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Marc-André Lureau]

Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path for the non interleaved cases")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/cxl/cxl-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 7e744312f1d8..eba13c9e7cba 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -279,7 +279,7 @@ static void cxl_fmws_direct_passthrough_setup(CXLDirectPTState *state,
     MemoryRegion *mr = NULL;
     uint64_t vmr_size = 0, pmr_size = 0, offset = 0;
     MemoryRegion *direct_mr;
-    g_autofree char *direct_mr_name;
+    g_autofree char *direct_mr_name = NULL;
     unsigned int idx = state->hdm_decoder_idx;
 
     if (ct3d->hostvmem) {

-- 
2.55.0

[PATCH v2 06/12] hw/i3c: fix default return value of d2_i3c_send()

[Marc-André Lureau]

Fixes: 208772a189f9 ("hw/i3c/dw-i3c: Add data TX and RX")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/i3c/dw-i3c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
index 6386499dd9fc..7b887fbd60cb 100644
--- a/hw/i3c/dw-i3c.c
+++ b/hw/i3c/dw-i3c.c
@@ -430,7 +430,7 @@ static int dw_i3c_send_start(DWI3C *s, uint8_t addr, bool is_recv, bool is_i2c)
 static int dw_i3c_send(DWI3C *s, const uint8_t *data, uint32_t num_to_send,
                        uint32_t *num_sent, bool is_i2c)
 {
-    int ret;
+    int ret = 0;
     uint32_t i;
 
     *num_sent = 0;

-- 
2.55.0

[PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Marc-André Lureau]

When compiled with -Og, gcc produces many false-positives
gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).

We already use auto-var-init=zero, but better be explicit.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 target/i386/cpu.c             | 3 ++-
 target/i386/emulate/x86_mmu.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5805d33ab92d..28e4435df23f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object *obj, Visitor *v,
 static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
                                          Error **errp)
 {
-    const AVX10VersionDefinition *def;
+    const AVX10VersionDefinition *def = NULL;
     CPUX86State *env = &cpu->env;
 
     if (!version) {
@@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
             break;
         }
     }
+    assert(def != NULL);
 
     if (def->version < version) {
         error_setg(errp, "avx10-version can be at most %d", def->version);
diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c
index 8d4371467fd7..65bcccd4751e 100644
--- a/target/i386/emulate/x86_mmu.c
+++ b/target/i386/emulate/x86_mmu.c
@@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, target_ulong addr, MMUTranslat
     int largeness = 0;
     target_ulong cr3 = x86_read_cr(cpu, 3);
     uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
-    MMUTranslateResult res;
-    
+    MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED;
+
     memset(pt, 0, sizeof(*pt));
     top_level = gpt_top_level(cpu, pae);
 

-- 
2.55.0

[PATCH v2 08/12] hw/hexagon: fix machine->fdt leak in qom-test

[Marc-André Lureau]

virt_instance_init() built the FDT unconditionally at QOM
instance-init time, so simply instantiating the object (e.g. via
qom-test's introspection, without ever realizing the machine) leaked
the 1MB FDT blob: machine_finalize() does not free machine->fdt.

Other boards (arm/virt, riscv/virt, ...) build the FDT lazily from
their MachineClass::init callback, which only runs when the machine
is actually selected to boot. Do the same here by moving create_fdt()
into virt_init().

Fixes: 88a8bc7f43ff ("hw/hexagon: Define hexagon "virt" machine")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/hexagon/virt.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/hexagon/virt.c b/hw/hexagon/virt.c
index a3638998b873..b75047250260 100644
--- a/hw/hexagon/virt.c
+++ b/hw/hexagon/virt.c
@@ -175,13 +175,6 @@ static void fdt_add_cpu_nodes(const HexagonVirtMachineState *vms)
 
 
 
-static void virt_instance_init(Object *obj)
-{
-    HexagonVirtMachineState *vms = HEXAGON_VIRT_MACHINE(obj);
-
-    create_fdt(vms);
-}
-
 void hexagon_load_fdt(const HexagonVirtMachineState *vms)
 {
     MachineState *ms = MACHINE(vms);
@@ -242,6 +235,7 @@ static void virt_init(MachineState *ms)
     DeviceState *cpu0;
     int32_t clk_phandle;
 
+    create_fdt(vms);
     qemu_fdt_setprop_string(ms->fdt, "/chosen", "bootargs", ms->kernel_cmdline);
 
     vms->sys = get_system_memory();
@@ -341,7 +335,6 @@ static const TypeInfo virt_machine_types[] = { {
     .parent = TYPE_HEXAGON_COMMON_MACHINE,
     .instance_size = sizeof(HexagonVirtMachineState),
     .class_init = virt_class_init,
-    .instance_init = virt_instance_init,
 } };
 
 DEFINE_TYPES(virt_machine_types)

-- 
2.55.0

[PATCH v2 09/12] hw/ppc: fix dangling fdt reference

[Marc-André Lureau]

machine->fdt is aliasing fdt_blob. We need to make sure it is cleared
or updated when we free fdt_blob.

Fixes: d890f2fa9f1e ("hw/ppc: set machine->fdt in spapr machine")
Reported-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/ppc/spapr.c       | 1 +
 hw/ppc/spapr_hcall.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b79828b4e90c..2fa4db3d493a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2137,6 +2137,7 @@ static int spapr_dtb_pre_load(void *opaque)
     g_free(spapr->fdt_blob);
     spapr->fdt_blob = NULL;
     spapr->fdt_size = 0;
+    MACHINE(spapr)->fdt = NULL;
 
     return 0;
 }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 23bcd788daf6..2ed063279200 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1511,6 +1511,7 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
     g_free(spapr->fdt_blob);
     spapr->fdt_size = cb;
     spapr->fdt_blob = fdt;
+    MACHINE(spapr)->fdt = fdt;
     trace_spapr_update_dt(cb);
 
     return H_SUCCESS;

-- 
2.55.0

[PATCH v2 10/12] hw/core/machine: free machine->fdt in machine_finalize()

[Marc-André Lureau]

machine->fdt is a plain g_malloc0() buffer (see create_device_tree())
that boards populate during MachineClass::init, but machine_finalize()
never freed it, unlike the other boot-time buffers (dtb, dumpdtb,
kernel_filename, ...). Free it for consistency.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/core/machine.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 805148678d57..2b24d711d76f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1301,6 +1301,7 @@ static void machine_finalize(Object *obj)
     g_free(ms->kernel_cmdline);
     g_free(ms->dtb);
     g_free(ms->dumpdtb);
+    g_free(ms->fdt);
     g_free(ms->dt_compatible);
     g_free(ms->firmware);
     g_free(ms->device_memory);

-- 
2.55.0

[PATCH v2 11/12] migration/multifd: fix Error leak in multifd_recv_terminate_threads()

[Marc-André Lureau]

If err != NULL, free it.

Fixes: 11dd7be57524 ("migration/multifd: Remove p->quit from recv side")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 migration/multifd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index dbad525d2a28..503014f76ba5 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1056,6 +1056,7 @@ static void multifd_recv_terminate_threads(Error *err)
     trace_multifd_recv_terminate_threads(err != NULL);
 
     if (qatomic_xchg(&multifd_recv_state->exiting, 1)) {
+        error_free(err);
         return;
     }
 

-- 
2.55.0

[PATCH v2 12/12] qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict

[Marc-André Lureau]

qmp_device_add() and hmp_device_add() drain pending RCU callbacks after
a failed device_add, since some bus teardown (e.g. bus_remove_child())
is deferred to call_rcu(). -device on the command line reaches the same
qdev_device_add_from_qdict() with errp pointing at error_fatal, whose
ERRP_GUARD() exits before control returns to the caller, so the caller's
drain_call_rcu() never runs.

Move the drain into qdev_device_add_from_qdict()'s own err_del_dev path
and drop the now-redundant calls in qmp_device_add() and
hmp_device_add(). Gate it behind a new drain_rcu parameter, because
virtio-net failover can reach qdev_device_add_from_qdict() from an MMIO
write dispatched under address_space_write()'s RCU read lock.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/monitor/qdev.h |  3 ++-
 hw/audio/intel-hda.c   |  2 +-
 hw/net/virtio-net.c    |  2 +-
 system/qdev-monitor.c  | 42 ++++++++++++++++--------------------------
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
index f85f25738d58..72d2fa4654d3 100644
--- a/include/monitor/qdev.h
+++ b/include/monitor/qdev.h
@@ -12,7 +12,8 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp);
 int qdev_device_help(QemuOpts *opts);
 DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
 DeviceState *qdev_device_add_from_qdict(const QDict *opts,
-                                        bool from_json, Error **errp);
+                                        bool from_json, bool drain_rcu,
+                                        Error **errp);
 BusState *qdev_find_default_bus(DeviceClass *dc, Error **errp);
 
 /**
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 3d361a4976c6..128c5ee612ce 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1313,7 +1313,7 @@ static void intel_hda_and_codec_init(const char *audiodev)
     BusState *hdabus;
 
     qdict_put_str(props, "driver", "intel-hda");
-    intel_hda = qdev_device_add_from_qdict(props, false, &error_fatal);
+    intel_hda = qdev_device_add_from_qdict(props, false, true, &error_fatal);
     hdabus = QLIST_FIRST(&intel_hda->child_bus);
 
     codec = qdev_new("hda-duplex");
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f0e3beb29032..0c5da8cb71af 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -916,7 +916,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
 
     dev = qdev_device_add_from_qdict(n->primary_opts,
                                      n->primary_opts_from_json,
-                                     &err);
+                                     false, &err);
     if (err) {
         qobject_unref(n->primary_opts);
         n->primary_opts = NULL;
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 00fed791cce1..77508bb1c3f1 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -650,7 +650,8 @@ BusState *qdev_find_default_bus(DeviceClass *dc, Error **errp)
 }
 
 DeviceState *qdev_device_add_from_qdict(const QDict *opts,
-                                        bool from_json, Error **errp)
+                                        bool from_json, bool drain_rcu,
+                                        Error **errp)
 {
     ERRP_GUARD();
     DeviceClass *dc;
@@ -746,6 +747,18 @@ err_del_dev:
     object_unparent(OBJECT(dev));
     object_unref(OBJECT(dev));
 
+    /*
+     * Some bus teardown (e.g. bus_remove_child()) is deferred via
+     * call_rcu(). When safe, drain those callbacks so the failed
+     * device is fully torn down before we return. Callers that are
+     * already inside an RCU read-side critical section (e.g. MMIO
+     * dispatch during virtio-net failover) must pass drain_rcu=false
+     * to avoid deadlocking on the grace period.
+     */
+    if (drain_rcu) {
+        drain_call_rcu();
+    }
+
     return NULL;
 }
 
@@ -755,7 +768,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
     QDict *qdict = qemu_opts_to_qdict(opts, NULL);
     DeviceState *ret;
 
-    ret = qdev_device_add_from_qdict(qdict, false, errp);
+    ret = qdev_device_add_from_qdict(qdict, false, true, errp);
     if (ret) {
         qemu_opts_del(opts);
     }
@@ -869,19 +882,7 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
 {
     DeviceState *dev;
 
-    dev = qdev_device_add_from_qdict(qdict, true, errp);
-    if (!dev) {
-        /*
-         * Drain all pending RCU callbacks. This is done because
-         * some bus related operations can delay a device removal
-         * (in this case this can happen if device is added and then
-         * removed due to a configuration error)
-         * to a RCU callback, but user might expect that this interface
-         * will finish its job completely once qmp command returns result
-         * to the user
-         */
-        drain_call_rcu();
-    }
+    dev = qdev_device_add_from_qdict(qdict, true, true, errp);
     object_unref(OBJECT(dev));
 }
 
@@ -1017,17 +1018,6 @@ void hmp_device_add(Monitor *mon, const QDict *qdict)
     }
     dev = qdev_device_add(opts, &err);
     if (!dev) {
-        /*
-         * Drain all pending RCU callbacks. This is done because
-         * some bus related operations can delay a device removal
-         * (in this case this can happen if device is added and then
-         * removed due to a configuration error)
-         * to a RCU callback, but user might expect that this interface
-         * will finish its job completely once qmp command returns result
-         * to the user
-         */
-        drain_call_rcu();
-
         qemu_opts_del(opts);
     }
     object_unref(dev);

-- 
2.55.0

Re: [PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 12:44:04AM +0400, Marc-André Lureau wrote:
> Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path for the non interleaved cases")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Not commit log really?

> ---
>  hw/cxl/cxl-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index 7e744312f1d8..eba13c9e7cba 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> @@ -279,7 +279,7 @@ static void cxl_fmws_direct_passthrough_setup(CXLDirectPTState *state,
>      MemoryRegion *mr = NULL;
>      uint64_t vmr_size = 0, pmr_size = 0, offset = 0;
>      MemoryRegion *direct_mr;
> -    g_autofree char *direct_mr_name;
> +    g_autofree char *direct_mr_name = NULL;
>      unsigned int idx = state->hdm_decoder_idx;
>  
>      if (ct3d->hostvmem) {
> 
> -- 
> 2.55.0

Re: [PATCH v2 00/12] Fixes for 11.1

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 12:43:59AM +0400, Marc-André Lureau wrote:
> Hi
> 
> Here are some assorted patches worth considering for the 11.1 freeze
> bug-fix period.

why are these in a single patchset? who is expected to merge it?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> Changes in v2, after Akihiko comments:
> - split the "Fix many -Werror=maybe-uninitialized" patch
> - qdev-monitor: add drain_rcu parameter to avoid deadlock from virtio-net
> - rewrite WITH_QEMU_LOCK_GUARD/WITH_RCU_READ_LOCK_GUARD macros to suppress
>   GCC false-positive warnings (new patch)
> - block/blkio: use QEMU_LOCK_GUARD instead of WITH_QEMU_LOCK_GUARD in
>   getlength() to avoid false-positive (new patch)
> - hw/cxl: split invalid-free fix into its own patch
> - hw/i3c: fix default return value of d2_i3c_send() (new patch)
> - hw/ppc: fix dangling machine->fdt reference after spapr fdt updates (new patch)
> - Link to v1: https://lore.kernel.org/qemu-devel/20260725-fix2-v1-0-5c9a6ce7b96a@redhat.com
> 
> ---
> Marc-André Lureau (12):
>       hw/display/virtio-gpu-rutabaga: zero-init capset info response
>       block/blkio: fix error return value on getlength()
>       block/blkio: fix compiler false-positive warning
>       include/qemu: adjust LOCK_GUARD macros to avoid potential warning
>       hw/cxl: fix invalid free on early return
>       hw/i3c: fix default return value of d2_i3c_send()
>       Fix some -Werror=maybe-uninitialized
>       hw/hexagon: fix machine->fdt leak in qom-test
>       hw/ppc: fix dangling fdt reference
>       hw/core/machine: free machine->fdt in machine_finalize()
>       migration/multifd: fix Error leak in multifd_recv_terminate_threads()
>       qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict
> 
>  include/monitor/qdev.h           |  3 ++-
>  include/qemu/lockable.h          | 19 +++++++++++-------
>  include/qemu/rcu.h               | 17 ++++++++++------
>  block/blkio.c                    |  7 +++----
>  hw/audio/intel-hda.c             |  2 +-
>  hw/core/machine.c                |  1 +
>  hw/cxl/cxl-host.c                |  2 +-
>  hw/display/virtio-gpu-rutabaga.c |  2 ++
>  hw/hexagon/virt.c                |  9 +--------
>  hw/i3c/dw-i3c.c                  |  2 +-
>  hw/net/virtio-net.c              |  2 +-
>  hw/ppc/spapr.c                   |  1 +
>  hw/ppc/spapr_hcall.c             |  1 +
>  migration/multifd.c              |  1 +
>  system/qdev-monitor.c            | 42 +++++++++++++++-------------------------
>  target/i386/cpu.c                |  3 ++-
>  target/i386/emulate/x86_mmu.c    |  4 ++--
>  17 files changed, 59 insertions(+), 59 deletions(-)
> ---
> base-commit: 300438ffbb8d9430cac2fcc15cba6f482b2c0587
> change-id: 20260725-fix2-6e51d231794b
> 
> Best regards,
> --  
> Marc-André Lureau <marcandre.lureau@redhat.com>

Re: [PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote:
> When compiled with -Og, gcc produces many false-positives
> gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).

it hurts if you do it? so don't do it then?

> We already use auto-var-init=zero, but better be explicit.

explicit about false positives?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  target/i386/cpu.c             | 3 ++-
>  target/i386/emulate/x86_mmu.c | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 5805d33ab92d..28e4435df23f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object *obj, Visitor *v,
>  static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
>                                           Error **errp)
>  {
> -    const AVX10VersionDefinition *def;
> +    const AVX10VersionDefinition *def = NULL;
>      CPUX86State *env = &cpu->env;
>  
>      if (!version) {
> @@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
>              break;
>          }
>      }
> +    assert(def != NULL);
>  
>      if (def->version < version) {
>          error_setg(errp, "avx10-version can be at most %d", def->version);


what does assert have to do with gcc warnings?

> diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c
> index 8d4371467fd7..65bcccd4751e 100644
> --- a/target/i386/emulate/x86_mmu.c
> +++ b/target/i386/emulate/x86_mmu.c
> @@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, target_ulong addr, MMUTranslat
>      int largeness = 0;
>      target_ulong cr3 = x86_read_cr(cpu, 3);
>      uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
> -    MMUTranslateResult res;
> -    
> +    MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED;
> +


This is not 0 as the commit log implies.



>      memset(pt, 0, sizeof(*pt));
>      top_level = gpt_top_level(cpu, pae);
>  
> 
> -- 
> 2.55.0

Re: [PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Marc-André Lureau]

Hi

On Mon, Jul 27, 2026 at 12:53 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote:
> > When compiled with -Og, gcc produces many false-positives
> > gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).
>
> it hurts if you do it? so don't do it then?

We are not far from getting it working, we can accommodate a bit of
code while making it a bit clearer for the reader too.

>
> > We already use auto-var-init=zero, but better be explicit.
>
> explicit about false positives?

Explicit initialization

>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  target/i386/cpu.c             | 3 ++-
> >  target/i386/emulate/x86_mmu.c | 4 ++--
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 5805d33ab92d..28e4435df23f 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object *obj, Visitor *v,
> >  static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
> >                                           Error **errp)
> >  {
> > -    const AVX10VersionDefinition *def;
> > +    const AVX10VersionDefinition *def = NULL;
> >      CPUX86State *env = &cpu->env;
> >
> >      if (!version) {
> > @@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
> >              break;
> >          }
> >      }
> > +    assert(def != NULL);
> >
> >      if (def->version < version) {
> >          error_setg(errp, "avx10-version can be at most %d", def->version);
>
>
> what does assert have to do with gcc warnings?

That's what is expected at this point, unfortunately the compiler
doesn't care we help it here. I can drop it

> > diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c
> > index 8d4371467fd7..65bcccd4751e 100644
> > --- a/target/i386/emulate/x86_mmu.c
> > +++ b/target/i386/emulate/x86_mmu.c
> > @@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, target_ulong addr, MMUTranslat
> >      int largeness = 0;
> >      target_ulong cr3 = x86_read_cr(cpu, 3);
> >      uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
> > -    MMUTranslateResult res;
> > -
> > +    MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED;
> > +
>
>
> This is not 0 as the commit log implies.

Initializing with 0 implies defaulting to MMU_TRANSLATE_SUCCESS - not
the best choice here.

Re: [PATCH v2 00/12] Fixes for 11.1

[Marc-André Lureau]

Hi

On Mon, Jul 27, 2026 at 12:50 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jul 27, 2026 at 12:43:59AM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > Here are some assorted patches worth considering for the 11.1 freeze
> > bug-fix period.
>
> why are these in a single patchset? who is expected to merge it?

Given that they are kind of general coding fixes, I think I can send a
PR once the patch are reviewed.

>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > Changes in v2, after Akihiko comments:
> > - split the "Fix many -Werror=maybe-uninitialized" patch
> > - qdev-monitor: add drain_rcu parameter to avoid deadlock from virtio-net
> > - rewrite WITH_QEMU_LOCK_GUARD/WITH_RCU_READ_LOCK_GUARD macros to suppress
> >   GCC false-positive warnings (new patch)
> > - block/blkio: use QEMU_LOCK_GUARD instead of WITH_QEMU_LOCK_GUARD in
> >   getlength() to avoid false-positive (new patch)
> > - hw/cxl: split invalid-free fix into its own patch
> > - hw/i3c: fix default return value of d2_i3c_send() (new patch)
> > - hw/ppc: fix dangling machine->fdt reference after spapr fdt updates (new patch)
> > - Link to v1: https://lore.kernel.org/qemu-devel/20260725-fix2-v1-0-5c9a6ce7b96a@redhat.com
> >
> > ---
> > Marc-André Lureau (12):
> >       hw/display/virtio-gpu-rutabaga: zero-init capset info response
> >       block/blkio: fix error return value on getlength()
> >       block/blkio: fix compiler false-positive warning
> >       include/qemu: adjust LOCK_GUARD macros to avoid potential warning
> >       hw/cxl: fix invalid free on early return
> >       hw/i3c: fix default return value of d2_i3c_send()
> >       Fix some -Werror=maybe-uninitialized
> >       hw/hexagon: fix machine->fdt leak in qom-test
> >       hw/ppc: fix dangling fdt reference
> >       hw/core/machine: free machine->fdt in machine_finalize()
> >       migration/multifd: fix Error leak in multifd_recv_terminate_threads()
> >       qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict
> >
> >  include/monitor/qdev.h           |  3 ++-
> >  include/qemu/lockable.h          | 19 +++++++++++-------
> >  include/qemu/rcu.h               | 17 ++++++++++------
> >  block/blkio.c                    |  7 +++----
> >  hw/audio/intel-hda.c             |  2 +-
> >  hw/core/machine.c                |  1 +
> >  hw/cxl/cxl-host.c                |  2 +-
> >  hw/display/virtio-gpu-rutabaga.c |  2 ++
> >  hw/hexagon/virt.c                |  9 +--------
> >  hw/i3c/dw-i3c.c                  |  2 +-
> >  hw/net/virtio-net.c              |  2 +-
> >  hw/ppc/spapr.c                   |  1 +
> >  hw/ppc/spapr_hcall.c             |  1 +
> >  migration/multifd.c              |  1 +
> >  system/qdev-monitor.c            | 42 +++++++++++++++-------------------------
> >  target/i386/cpu.c                |  3 ++-
> >  target/i386/emulate/x86_mmu.c    |  4 ++--
> >  17 files changed, 59 insertions(+), 59 deletions(-)
> > ---
> > base-commit: 300438ffbb8d9430cac2fcc15cba6f482b2c0587
> > change-id: 20260725-fix2-6e51d231794b
> >
> > Best regards,
> > --
> > Marc-André Lureau <marcandre.lureau@redhat.com>
>
>

Re: [PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Marc-André Lureau]

On Mon, Jul 27, 2026 at 12:49 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jul 27, 2026 at 12:44:04AM +0400, Marc-André Lureau wrote:
> > Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path for the non interleaved cases")
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Not commit log really?

SSIA, but I can make it longer if it helps.

>
> > ---
> >  hw/cxl/cxl-host.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> > index 7e744312f1d8..eba13c9e7cba 100644
> > --- a/hw/cxl/cxl-host.c
> > +++ b/hw/cxl/cxl-host.c
> > @@ -279,7 +279,7 @@ static void cxl_fmws_direct_passthrough_setup(CXLDirectPTState *state,
> >      MemoryRegion *mr = NULL;
> >      uint64_t vmr_size = 0, pmr_size = 0, offset = 0;
> >      MemoryRegion *direct_mr;
> > -    g_autofree char *direct_mr_name;
> > +    g_autofree char *direct_mr_name = NULL;
> >      unsigned int idx = state->hdm_decoder_idx;
> >
> >      if (ct3d->hostvmem) {
> >
> > --
> > 2.55.0
>
>

Re: [PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Akihiko Odaki]

On 2026/07/27 6:09, Marc-André Lureau wrote:
> On Mon, Jul 27, 2026 at 12:49 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Mon, Jul 27, 2026 at 12:44:04AM +0400, Marc-André Lureau wrote:
>>> Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path for the non interleaved cases")
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Not commit log really?
> 
> SSIA, but I can make it longer if it helps.

I agree the subject is sufficient so:

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

That said, we can quote docs/devel/style.rst if desired:

 > * Variables declared with g_auto* MUST always be initialized,
 >   otherwise the cleanup function will use uninitialized stack memory

Regards,
Akihiko Odaki

> 
>>
>>> ---
>>>   hw/cxl/cxl-host.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
>>> index 7e744312f1d8..eba13c9e7cba 100644
>>> --- a/hw/cxl/cxl-host.c
>>> +++ b/hw/cxl/cxl-host.c
>>> @@ -279,7 +279,7 @@ static void cxl_fmws_direct_passthrough_setup(CXLDirectPTState *state,
>>>       MemoryRegion *mr = NULL;
>>>       uint64_t vmr_size = 0, pmr_size = 0, offset = 0;
>>>       MemoryRegion *direct_mr;
>>> -    g_autofree char *direct_mr_name;
>>> +    g_autofree char *direct_mr_name = NULL;
>>>       unsigned int idx = state->hdm_decoder_idx;
>>>
>>>       if (ct3d->hostvmem) {
>>>
>>> --
>>> 2.55.0
>>
>>

Re: [PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Akihiko Odaki]

On 2026/07/27 6:07, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 27, 2026 at 12:53 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote:
>>> When compiled with -Og, gcc produces many false-positives
>>> gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).
>>
>> it hurts if you do it? so don't do it then?
> 
> We are not far from getting it working, we can accommodate a bit of
> code while making it a bit clearer for the reader too.
> 
>>
>>> We already use auto-var-init=zero, but better be explicit.
>>
>> explicit about false positives?
> 
> Explicit initialization

These are the same concerns I raised in my previous review:

https://lore.kernel.org/qemu-devel/f919e684-93ed-4eca-8ddc-785e69dd8add@rsg.ci.i.u-tokyo.ac.jp/

I do not think this patch make the code clearer. They add values that 
are never consumed, which obscures rather than clarifies the data flow. 
Leaving a variable uninitialized until its actual value is assigned 
makes that flow more explicit.

Still, I think this change makes sense. Failure with -Og shows that
whether GCC emits the warning depends on subtle optimization details.
Avoiding that dependency is worthwhile.

> 
>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>   target/i386/cpu.c             | 3 ++-
>>>   target/i386/emulate/x86_mmu.c | 4 ++--
>>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index 5805d33ab92d..28e4435df23f 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object *obj, Visitor *v,
>>>   static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
>>>                                            Error **errp)
>>>   {
>>> -    const AVX10VersionDefinition *def;
>>> +    const AVX10VersionDefinition *def = NULL;
>>>       CPUX86State *env = &cpu->env;
>>>
>>>       if (!version) {
>>> @@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
>>>               break;
>>>           }
>>>       }
>>> +    assert(def != NULL);
>>>
>>>       if (def->version < version) {
>>>           error_setg(errp, "avx10-version can be at most %d", def->version);
>>
>>
>> what does assert have to do with gcc warnings?
> 
> That's what is expected at this point, unfortunately the compiler
> doesn't care we help it here. I can drop it

I would keep it. It reduces the downside of introducing an otherwise 
unused initial value by documenting the invariant.

> 
>>> diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c
>>> index 8d4371467fd7..65bcccd4751e 100644
>>> --- a/target/i386/emulate/x86_mmu.c
>>> +++ b/target/i386/emulate/x86_mmu.c
>>> @@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, target_ulong addr, MMUTranslat
>>>       int largeness = 0;
>>>       target_ulong cr3 = x86_read_cr(cpu, 3);
>>>       uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
>>> -    MMUTranslateResult res;
>>> -
>>> +    MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED;
>>> +
>>
>>
>> This is not 0 as the commit log implies.
> 
> Initializing with 0 implies defaulting to MMU_TRANSLATE_SUCCESS - not
> the best choice here.

Since the value is ultimately overwritten anyway, the specific 
placeholder chosen doesn't strictly matter for execution, but 
MMU_TRANSLATE_SUCCESS feels like a more natural default behavior.

walk_gpt() checks each page-table level until one rejects the mapping or
all levels have been traversed. If top_level == 0 (i.e., there is zero 
levels to traverse), nothing would have rejected the mapping, so success 
is more natural than MMU_TRANSLATE_PAGE_NOT_MAPPED.

Regards,
Akihiko Odaki

Re: [PATCH v2 08/12] hw/hexagon: fix machine->fdt leak in qom-test

[Philippe Mathieu-Daudé]

On 26/7/26 22:44, Marc-André Lureau wrote:
> virt_instance_init() built the FDT unconditionally at QOM
> instance-init time, so simply instantiating the object (e.g. via
> qom-test's introspection, without ever realizing the machine) leaked
> the 1MB FDT blob: machine_finalize() does not free machine->fdt.
> 
> Other boards (arm/virt, riscv/virt, ...) build the FDT lazily from
> their MachineClass::init callback, which only runs when the machine
> is actually selected to boot. Do the same here by moving create_fdt()
> into virt_init().
> 
> Fixes: 88a8bc7f43ff ("hw/hexagon: Define hexagon "virt" machine")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/hexagon/virt.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v2 03/12] block/blkio: fix compiler false-positive warning

[Philippe Mathieu-Daudé]

On 26/7/26 22:44, Marc-André Lureau wrote:
> Under some optimization, gcc produces a false-positive:
>      ../block/blkio.c: In function ‘blkio_co_getlength’:
>      ../block/blkio.c:943:8: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>        943 |     if (ret < 0) {
>            |        ^
> 
> Replace WITH_QEMU_LOCK_GUARD with the simpler QEMU_LOCK_GUARD.
> 
> Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   block/blkio.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Philippe Mathieu-Daudé]

On 27/7/26 06:40, Akihiko Odaki wrote:
> On 2026/07/27 6:09, Marc-André Lureau wrote:
>> On Mon, Jul 27, 2026 at 12:49 AM Michael S. Tsirkin <mst@redhat.com> 
>> wrote:
>>>
>>> On Mon, Jul 27, 2026 at 12:44:04AM +0400, Marc-André Lureau wrote:
>>>> Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path 
>>>> for the non interleaved cases")
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> Not commit log really?
>>
>> SSIA, but I can make it longer if it helps.
> 
> I agree the subject is sufficient so:
> 
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 
> That said, we can quote docs/devel/style.rst if desired:
> 
>  > * Variables declared with g_auto* MUST always be initialized,
>  >   otherwise the cleanup function will use uninitialized stack memory

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v2 06/12] hw/i3c: fix default return value of d2_i3c_send()

[Philippe Mathieu-Daudé]

On 26/7/26 22:44, Marc-André Lureau wrote:
> Fixes: 208772a189f9 ("hw/i3c/dw-i3c: Add data TX and RX")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/i3c/dw-i3c.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
> index 6386499dd9fc..7b887fbd60cb 100644
> --- a/hw/i3c/dw-i3c.c
> +++ b/hw/i3c/dw-i3c.c
> @@ -430,7 +430,7 @@ static int dw_i3c_send_start(DWI3C *s, uint8_t addr, bool is_recv, bool is_i2c)
>   static int dw_i3c_send(DWI3C *s, const uint8_t *data, uint32_t num_to_send,
>                          uint32_t *num_sent, bool is_i2c)
>   {
> -    int ret;
> +    int ret = 0;

So IIUC the issue is being called with num_to_send=0.
Can that happen? Apparently no. Could we assert it is
non-zero?

Re: [PATCH v2 05/12] hw/cxl: fix invalid free on early return

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 01:40:48PM +0900, Akihiko Odaki wrote:
> On 2026/07/27 6:09, Marc-André Lureau wrote:
> > On Mon, Jul 27, 2026 at 12:49 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > 
> > > On Mon, Jul 27, 2026 at 12:44:04AM +0400, Marc-André Lureau wrote:
> > > > Fixes: 680935c9a6ff ("hw/cxl: Add a performant (and correct) path for the non interleaved cases")
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > 
> > > Not commit log really?
> > 
> > SSIA, but I can make it longer if it helps.
> 
> I agree the subject is sufficient so:
> 
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 
> That said, we can quote docs/devel/style.rst if desired:
> 
> > * Variables declared with g_auto* MUST always be initialized,
> >   otherwise the cleanup function will use uninitialized stack memory

I like that. But the question is not just whether it's required by
the coding style. The question is if there's a bug and how is it
reacheable, which can guide things like backporting decisions.
Thanks!

> Regards,
> Akihiko Odaki
> 
> > 
> > > 
> > > > ---
> > > >   hw/cxl/cxl-host.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> > > > index 7e744312f1d8..eba13c9e7cba 100644
> > > > --- a/hw/cxl/cxl-host.c
> > > > +++ b/hw/cxl/cxl-host.c
> > > > @@ -279,7 +279,7 @@ static void cxl_fmws_direct_passthrough_setup(CXLDirectPTState *state,
> > > >       MemoryRegion *mr = NULL;
> > > >       uint64_t vmr_size = 0, pmr_size = 0, offset = 0;
> > > >       MemoryRegion *direct_mr;
> > > > -    g_autofree char *direct_mr_name;
> > > > +    g_autofree char *direct_mr_name = NULL;
> > > >       unsigned int idx = state->hdm_decoder_idx;
> > > > 
> > > >       if (ct3d->hostvmem) {
> > > > 
> > > > --
> > > > 2.55.0
> > > 
> > > 

Re: [PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 01:07:32AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 27, 2026 at 12:53 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote:
> > > When compiled with -Og, gcc produces many false-positives
> > > gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).
> >
> > it hurts if you do it? so don't do it then?
> 
> We are not far from getting it working, we can accommodate a bit of
> code while making it a bit clearer for the reader too.

I don't know, dead code does not make things clear to me.
And it burns tokens for LLMs)


I simply do not at all cherish to have yet another weird config
that will trip up CI and then I am wasting time either
"fixing" it myself or wasting even more time communicating with
contributors.



Used uninitialized warnings is already not great with -O2.
How about simply disabling them with -Og?
Is -Wno-error=maybe-uninitialized enough?
Default build will catch enough errors and that seems enough for me.

> >
> > > We already use auto-var-init=zero, but better be explicit.
> >
> > explicit about false positives?
> 
> Explicit initialization

I don't really think it's better. Maybe it's required to make the
tool work, but it would be even better to avoid the noise.


If I see A = 123; I expect it was worth my time to read it.

> >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  target/i386/cpu.c             | 3 ++-
> > >  target/i386/emulate/x86_mmu.c | 4 ++--
> > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index 5805d33ab92d..28e4435df23f 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object *obj, Visitor *v,
> > >  static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
> > >                                           Error **errp)
> > >  {
> > > -    const AVX10VersionDefinition *def;
> > > +    const AVX10VersionDefinition *def = NULL;
> > >      CPUX86State *env = &cpu->env;
> > >
> > >      if (!version) {
> > > @@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version,
> > >              break;
> > >          }
> > >      }
> > > +    assert(def != NULL);
> > >
> > >      if (def->version < version) {
> > >          error_setg(errp, "avx10-version can be at most %d", def->version);
> >
> >
> > what does assert have to do with gcc warnings?
> 
> That's what is expected at this point,

of course - it is dereferenced 1 line below.
what I worry about is people seeing this and blindly adding
these asserts all over.

> unfortunately the compiler
> doesn't care we help it here. I can drop it
> 
> > > diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c
> > > index 8d4371467fd7..65bcccd4751e 100644
> > > --- a/target/i386/emulate/x86_mmu.c
> > > +++ b/target/i386/emulate/x86_mmu.c
> > > @@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, target_ulong addr, MMUTranslat
> > >      int largeness = 0;
> > >      target_ulong cr3 = x86_read_cr(cpu, 3);
> > >      uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
> > > -    MMUTranslateResult res;
> > > -
> > > +    MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED;
> > > +
> >
> >
> > This is not 0 as the commit log implies.
> 
> Initializing with 0 implies defaulting to MMU_TRANSLATE_SUCCESS - not
> the best choice here.

when I see this I go "ok there is probably an early exit I better
remember it" - and then it turns out there isn't and I am left confused.

-- 
MST

Re: [PATCH v2 00/12] Fixes for 11.1

[Michael S. Tsirkin]

On Mon, Jul 27, 2026 at 01:08:28AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 27, 2026 at 12:50 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Jul 27, 2026 at 12:43:59AM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > Here are some assorted patches worth considering for the 11.1 freeze
> > > bug-fix period.
> >
> > why are these in a single patchset? who is expected to merge it?
> 
> Given that they are kind of general coding fixes, I think I can send a
> PR once the patch are reviewed.

I have no problem with that just pls mention this down the road
when it's ready.

> >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > Changes in v2, after Akihiko comments:
> > > - split the "Fix many -Werror=maybe-uninitialized" patch
> > > - qdev-monitor: add drain_rcu parameter to avoid deadlock from virtio-net
> > > - rewrite WITH_QEMU_LOCK_GUARD/WITH_RCU_READ_LOCK_GUARD macros to suppress
> > >   GCC false-positive warnings (new patch)
> > > - block/blkio: use QEMU_LOCK_GUARD instead of WITH_QEMU_LOCK_GUARD in
> > >   getlength() to avoid false-positive (new patch)
> > > - hw/cxl: split invalid-free fix into its own patch
> > > - hw/i3c: fix default return value of d2_i3c_send() (new patch)
> > > - hw/ppc: fix dangling machine->fdt reference after spapr fdt updates (new patch)
> > > - Link to v1: https://lore.kernel.org/qemu-devel/20260725-fix2-v1-0-5c9a6ce7b96a@redhat.com
> > >
> > > ---
> > > Marc-André Lureau (12):
> > >       hw/display/virtio-gpu-rutabaga: zero-init capset info response
> > >       block/blkio: fix error return value on getlength()
> > >       block/blkio: fix compiler false-positive warning
> > >       include/qemu: adjust LOCK_GUARD macros to avoid potential warning
> > >       hw/cxl: fix invalid free on early return
> > >       hw/i3c: fix default return value of d2_i3c_send()
> > >       Fix some -Werror=maybe-uninitialized
> > >       hw/hexagon: fix machine->fdt leak in qom-test
> > >       hw/ppc: fix dangling fdt reference
> > >       hw/core/machine: free machine->fdt in machine_finalize()
> > >       migration/multifd: fix Error leak in multifd_recv_terminate_threads()
> > >       qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict
> > >
> > >  include/monitor/qdev.h           |  3 ++-
> > >  include/qemu/lockable.h          | 19 +++++++++++-------
> > >  include/qemu/rcu.h               | 17 ++++++++++------
> > >  block/blkio.c                    |  7 +++----
> > >  hw/audio/intel-hda.c             |  2 +-
> > >  hw/core/machine.c                |  1 +
> > >  hw/cxl/cxl-host.c                |  2 +-
> > >  hw/display/virtio-gpu-rutabaga.c |  2 ++
> > >  hw/hexagon/virt.c                |  9 +--------
> > >  hw/i3c/dw-i3c.c                  |  2 +-
> > >  hw/net/virtio-net.c              |  2 +-
> > >  hw/ppc/spapr.c                   |  1 +
> > >  hw/ppc/spapr_hcall.c             |  1 +
> > >  migration/multifd.c              |  1 +
> > >  system/qdev-monitor.c            | 42 +++++++++++++++-------------------------
> > >  target/i386/cpu.c                |  3 ++-
> > >  target/i386/emulate/x86_mmu.c    |  4 ++--
> > >  17 files changed, 59 insertions(+), 59 deletions(-)
> > > ---
> > > base-commit: 300438ffbb8d9430cac2fcc15cba6f482b2c0587
> > > change-id: 20260725-fix2-6e51d231794b
> > >
> > > Best regards,
> > > --
> > > Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> >

Re: [PATCH v2 07/12] Fix some -Werror=maybe-uninitialized

[Daniel P. Berrangé]

On Mon, Jul 27, 2026 at 02:25:39PM +0900, Akihiko Odaki wrote:
> On 2026/07/27 6:07, Marc-André Lureau wrote:
> > Hi
> > 
> > On Mon, Jul 27, 2026 at 12:53 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > 
> > > On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote:
> > > > When compiled with -Og, gcc produces many false-positives
> > > > gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2).
> > > 
> > > it hurts if you do it? so don't do it then?
> > 
> > We are not far from getting it working, we can accommodate a bit of
> > code while making it a bit clearer for the reader too.
> > 
> > > 
> > > > We already use auto-var-init=zero, but better be explicit.
> > > 
> > > explicit about false positives?
> > 
> > Explicit initialization
> 
> These are the same concerns I raised in my previous review:
> 
> https://lore.kernel.org/qemu-devel/f919e684-93ed-4eca-8ddc-785e69dd8add@rsg.ci.i.u-tokyo.ac.jp/
> 
> I do not think this patch make the code clearer. They add values that are
> never consumed, which obscures rather than clarifies the data flow. Leaving
> a variable uninitialized until its actual value is assigned makes that flow
> more explicit.

We've seen time & again that human reviewers fail to reliably identify
when variables are initialized vs uninitialized, leading to countless
CVEs over the years.

We want everything initialized to reduce our security risk profile.

We added auto-var-init=zero as a backstop to have implicit initialization
on the basis that zero-init is the right thing to do 95% of the time.

None the less, we should be adding explicit initialization to everywhere
so that we're clear about when we want zero vs non-zero initialization,
plus a few places we explicitly mark & disable initialization for
performance reasons.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH v2 11/12] migration/multifd: fix Error leak in multifd_recv_terminate_threads()

[Fabiano Rosas]

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

> If err != NULL, free it.
>
> Fixes: 11dd7be57524 ("migration/multifd: Remove p->quit from recv side")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  migration/multifd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index dbad525d2a28..503014f76ba5 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -1056,6 +1056,7 @@ static void multifd_recv_terminate_threads(Error *err)
>      trace_multifd_recv_terminate_threads(err != NULL);
>  
>      if (qatomic_xchg(&multifd_recv_state->exiting, 1)) {
> +        error_free(err);
>          return;
>      }

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH v2 11/12] migration/multifd: fix Error leak in multifd_recv_terminate_threads()

[Peter Xu]

On Mon, Jul 27, 2026 at 12:44:10AM +0400, Marc-André Lureau wrote:
> If err != NULL, free it.
> 
> Fixes: 11dd7be57524 ("migration/multifd: Remove p->quit from recv side")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu