Skip to content

Vnc264#5

Closed
berrange wants to merge 3072 commits intomasterfrom
vnc264
Closed

Vnc264#5
berrange wants to merge 3072 commits intomasterfrom
vnc264

Conversation

@berrange
Copy link
Copy Markdown
Owner

@berrange berrange commented Apr 24, 2025

As defined by:

https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#open-h-264-encoding

The noVNC HTML application recently added support for this encoding. There is
also an open pull request to add audio support to noVNC:

novnc/noVNC#1952

With that in place, the web based VNC console is good enough to display
a VM showing a video with reasonable bandwidth.

Possible improvements:

  • Dynamic switching to/from H264 mode at high change rates
  • do not compute all rects in vnc_update_client to reduce CPU load

We may also extend the RFB Audio protocol with "opus" encoding, because uncompressed
audio need too much bandwidth.

Summary by Sourcery

Add H.264 video encoding support for VNC using GStreamer, enabling more efficient video streaming over VNC connections

New Features:

  • Implement H.264 video encoding for VNC using multiple GStreamer encoders
  • Add dynamic encoder selection with support for nvh264enc, vaapih264enc, x264enc, and openh264enc
  • Introduce configuration option to enable/disable H.264 encoding

Enhancements:

  • Optimize VNC refresh mechanism to support H.264 encoding
  • Add support for keeping dirty frames for smoother video transmission
  • Implement flexible encoder initialization and context management

Build:

  • Add GStreamer as an optional build dependency
  • Update meson build configuration to support H.264 encoding

CI:

  • Add GStreamer support to build options and configuration scripts

Summary by CodeRabbit

  • New Features

    • Added optional H.264 video encoding support for VNC using GStreamer, improving video streaming performance and efficiency.
    • Introduced a new build option to enable or disable GStreamer-based H.264 encoding for VNC.
    • Added a VNC server option to configure H.264 encoder selection or disable H.264 support.
    • Enhanced VNC client negotiation and update logic to support H.264 encoding, including optimized update scheduling.
  • Chores

    • Updated build scripts and help documentation to reflect new GStreamer and H.264 VNC options.

stefanhaRH and others added 30 commits March 13, 2025 17:57
Allow virtio-scsi virtqueues to be assigned to different IOThreads. This
makes it possible to take advantage of host multi-queue block layer
scalability by assigning virtqueues that have affinity with vCPUs to
different IOThreads that have affinity with host CPUs. The same feature
was introduced for virtio-blk in the past:
https://developers.redhat.com/articles/2024/09/05/scaling-virtio-blk-disk-io-iothread-virtqueue-mapping

Here are fio randread 4k iodepth=64 results from a 4 vCPU guest with an
Intel P4800X SSD:
iothreads IOPS
------------------------------
1         189576
2         312698
4         346744

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-ID: <[email protected]>
Tested-by: Peter Krempa <[email protected]>
[kwolf: Updated 051 output, virtio-scsi can now use any iothread]
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
Previously the ctrl virtqueue was handled in the AioContext where SCSI
requests are processed. When IOThread Virtqueue Mapping was added things
become more complicated because SCSI requests could run in other
AioContexts.

Simplify by handling the ctrl virtqueue in the main loop where reset
operations can be performed. Note that BHs are still used canceling SCSI
requests in their AioContexts but at least the mean loop activity
doesn't need BHs anymore.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-ID: <[email protected]>
Tested-by: Peter Krempa <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
Peter Krempa and Kevin Wolf observed that iothread-vq-mapping is
confusing to use because the control and event virtqueues have a fixed
location before the command virtqueues but need to be treated
differently.

Only expose the command virtqueues via iothread-vq-mapping so that the
command-line parameter is intuitive: it controls where SCSI requests are
processed.

The control virtqueue needs to be hardcoded to the main loop thread for
technical reasons anyway. Kevin also pointed out that it's better to
place the event virtqueue in the main loop thread since its no poll
behavior would prevent polling if assigned to an IOThread.

This change is its own commit to avoid squashing the previous commit.

Suggested-by: Kevin Wolf <[email protected]>
Suggested-by: Peter Krempa <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-ID: <[email protected]>
Tested-by: Peter Krempa <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
This tool converts a disk image to qcow2, writing the result directly
to stdout. This can be used for example to send the generated file
over the network.

This is equivalent to using qemu-img to convert a file to qcow2 and
then writing the result to stdout, with the difference that this tool
does not need to create this temporary qcow2 file and therefore does
not need any additional disk space.

Implementing this directly in qemu-img is not really an option because
it expects the output file to be seekable and it is also meant to be a
generic tool that supports all combinations of file formats and image
options. Instead, this tool can only produce qcow2 files with the
basic options, without compression, encryption or other features.

The input file is read twice. The first pass is used to determine
which clusters contain non-zero data and that information is used to
create the qcow2 header, refcount table and blocks, and L1 and L2
tables. After all that metadata is created then the second pass is
used to write the guest data.

By default qcow2-to-stdout.py expects the input to be a raw file, but
if qemu-storage-daemon is available then it can also be used to read
images in other formats. Alternatively the user can also run qemu-nbd
or qemu-storage-daemon manually instead.

Signed-off-by: Alberto Garcia <[email protected]>
Signed-off-by: Madeeha Javed <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
…into staging

* Various fixes for functional tests
* Fix the name of the "configs" directory in the documentation

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEEJ7iIR+7gJQEY8+q5LtnXdP5wLbUFAmfSjagRHHRodXRoQHJl
# ZGhhdC5jb20ACgkQLtnXdP5wLbWBmA//RhAHuF/fTmQagBsZPETXjU1g8ifw9aqm
# WPZcQEXyQFlqYYQZmtV7dk3aTGEw4kBDmm+SKTSQz1yUcBGptMl8xuWaxgdpcOw0
# Bqt+lYNgwGL9/OocCdNolU3+aVbETljr5l+rzbnwsTVIqGk63Qhmtwdupb8h1nfY
# 4vCXU+sY3BkvBF8HbV6Wb1aPtqC+iH/Ln8+yoKkC8UePD623dK58SsOVrhUQDfFr
# U/HUy4BZlHFCfGGmDVGBjHdEbOzQkLQ9N3ilsNSWcF87RPkWPft+qLs4RjDFW+oT
# oksXEFHcr8XQO03fwHBNTyv+NUfnrvDY8V+gl6C9ItQr58SZzse57caZKWrYppZ3
# l5iHoaLMV3juZFDNXNHkWHuveXi05+0V0UbZihzBeC4+zjNRyh3e1GuDoh5VoG8o
# XIb55RxU8eBG2/ulHZ71eAYrGpxO+tDdsdnak1coPFsU8HrC9QzRfywiAZe1Wwmx
# 5t5AHbZ7RdnxgStU1lWTUT2IDVSini4DKevt/FzhKkv1aD8NbhI/ooGDC0zbS6SU
# XK6PP2G5a5OnjQ904oRCQbnhrxFa5qNfryylvvreT2bVgX0BiE4pJ9JXdgQOMYlP
# kZERZZQcv3y6VVavAT67yeNKQpyb4HSHdTDQ2irgXP1UwHRpwLpKdqB1UhzNJ8m8
# k0faA8RXir4=
# =VtGZ
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 13 Mar 2025 15:47:52 HKT
# gpg:                using RSA key 27B88847EEE0250118F3EAB92ED9D774FE702DB5
# gpg:                issuer "[email protected]"
# gpg: Good signature from "Thomas Huth <[email protected]>" [full]
# gpg:                 aka "Thomas Huth <[email protected]>" [full]
# gpg:                 aka "Thomas Huth <[email protected]>" [full]
# gpg:                 aka "Thomas Huth <[email protected]>" [unknown]
# Primary key fingerprint: 27B8 8847 EEE0 2501 18F3  EAB9 2ED9 D774 FE70 2DB5

* tag 'pull-request-2025-03-13' of https://gitlab.com/thuth/qemu:
  tests/functional: skip vulkan test if missing vulkaninfo
  tests/functional/asset: Add AssetError exception class
  tests/functional/asset: Verify downloaded size
  tests/functional/asset: Fail assert fetch when retries are exceeded
  docs/system: Fix the information on how to run certain functional tests
  tests/functional: Bump up arm_replay timeout
  tests/functional: Require 'user' netdev for ppc64 e500 test
  docs: Rename default-configs to configs

Signed-off-by: Stefan Hajnoczi <[email protected]>
Block layer patches

- virtio-scsi: add iothread-vq-mapping parameter
- Improve writethrough performance
- Fix missing zero init in bdrv_snapshot_goto()
- Added scripts/qcow2-to-stdout.py
- Code cleanup and iotests fixes

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmfTDysRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9Yz6A//asOl37zjbtf9pYjY/gliH859TQOppPGD
# LB9IIr+nTDME0wfUkCOlag+CeEYZwkeo2PF+XeopsyzlJeBOk4tL7AkY57XYe3lZ
# M5hlnNrn6l3gb6iioMg60pEKSMrpKprB16vT3nAtyN6aEXsm9TvtPkWPFTCFGVeK
# W74VCr7wuXbfdEJcOGd8WhB9ZHIgwoWYnoL41tvCoefW2yNaMA6X0TLn98toXzOi
# il50ZnnchTQngns5R+n+1R1Ma995t393D+CArQcYVRzxKGOs5p0y4otz4gCkMhdp
# GVL09R7Ge4TteSJ2myxlN/EjYOxmdoMrVDajr4xPdHBw12MKzgk8i82h4/Es/Q5o
# 3Npgx74+jDyqlICb/czTVM5KJINpyO80vO3N3WpYUOQGyTCcYgv7pIpy8pB2o6Te
# RPlv0W9bHVSSgThFFLQ0Ud8WRGJe1K/ar8bdmiWN08Wez1avENWaYmsv5zGnFL24
# vD6cNXMR4mF7mzyeWda/5hGKv75djVgX+ZfzvWNT3qgizD56JBOA3RdCRwBZJOJb
# TvJkfi5RGyaji9BfKVCYBL3/iDELJEVDW8jxvIIUrS0aPcTHpAQ5gTO7VAokreqZ
# 5Smll11eeoEgPPvNLw8ikmOGTWOMkJGrmExP2K1ApANq3kSbBSU4jroEr0BG9PZT
# 6Y0hUdtFSdU=
# =w2Ri
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 14 Mar 2025 01:00:27 HKT
# gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg:                issuer "[email protected]"
# gpg: Good signature from "Kevin Wolf <[email protected]>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (23 commits)
  scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout
  virtio-scsi: only expose cmd vqs via iothread-vq-mapping
  virtio-scsi: handle ctrl virtqueue in main loop
  virtio-scsi: add iothread-vq-mapping parameter
  virtio: extract iothread-vq-mapping.h API
  virtio-blk: tidy up iothread_vq_mapping functions
  virtio-blk: extract cleanup_iothread_vq_mapping() function
  virtio-scsi: perform TMFs in appropriate AioContexts
  virtio-scsi: protect events_dropped field
  virtio-scsi: introduce event and ctrl virtqueue locks
  scsi: introduce requests_lock
  scsi: track per-SCSIRequest AioContext
  dma: use current AioContext for dma_blk_io()
  scsi-disk: drop unused SCSIDiskState->bh field
  iotests: Limit qsd-migrate to working formats
  aio-posix: Adjust polling time also for new handlers
  aio-posix: Separate AioPolledEvent per AioHandler
  aio-posix: Factor out adjust_polling_time()
  aio: Create AioPolledEvent
  block/io: Ignore FUA with cache.no-flush=on
  ...

Signed-off-by: Stefan Hajnoczi <[email protected]>
The description of feature @unstable is three paragraphs.  The second
and third became part of the description by accident in commit
9fb49da (qapi: Mark unstable QMP parts with feature 'unstable').

The second paragraph describes a defect in terms of the
implementation.  Fine, but doesn't belong into user-facing
documentation.  Turn it into a TODO section.

Rewrite everything else for clarity and completeness.

Signed-off-by: Markus Armbruster <[email protected]>
Message-ID: <[email protected]>
Acked-by: Alberto Garcia <[email protected]>
When using the annotations feature, type hints do not need to be
imported at runtime, only at type check time. Move type-check-only
imports into a conditional to reduce the number of imports needed at
runtime.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Currently, only the definition name is stored in the tree metadata; but
the node property is confusingly called "fullname". Rectify this by
always storing the FQN in the tree metadata.

... While we're here, re-organize the code in preparation for namespace
support to make it a bit easier to add additional components of the
FQN. With this change, there is now extremely little code left that's
taken directly from the Python domain :)

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
This patch adds a namespace component to the "Fully Qualified Name", in
the form of "domain:module.name". As there are no namespace directives
or options yet, this component will simply be empty as of this patch.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Akin to the :module: override option, the :namespace: options allows you
to forcibly override the contextual namespace associatied with a
definition.

We don't necessarily actually need this, but I felt compelled to stick
close to how the Python domain works that offers context overrides.

As of this commit, it is possible to add e.g. ":namespace: QMP" to any
QAPI directive to forcibly associate that definition with a given
namespace.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Add a new directive that marks the beginning of a QAPI "namespace", for
example; "QMP", "QGA" or "QSD". This directive will associate all
subsequent QAPI directives in a document with the specified
namespace. This does not change the visual display of any of the
definitions or index entries, but does change the "Fully Qualified Name"
inside the QAPI domain's object table. This allows for two different
"namespaces" to define entities with otherwise identical names -- which
will come in handy for documenting both QEMU QMP and the QEMU Storage
Daemon.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Add a :namespace: option to the qapi-doc directive, which inserts a
qapi:namespace directive into the start of the generated document. This,
in turn, associates all auto-generated definitions by this directive
with the specified namespace.

The source info for these generated lines are credited to the start of
the qapi-doc directive, which isn't precisely correct, but I wasn't sure
how to get it more accurate without some re-parsing shenanigans.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
This patch does three things:

1. Record the current namespace context in pending_xrefs so it can be
   used for link resolution later,
2. Pass that recorded namespace context to find_obj() when resolving a
   reference, and
3. Wildly and completely rewrite find_obj().

cross-reference support is expanded to tolerate the presence or absence
of either namespace or module, and to cope with the presence or absence
of contextual information for either.

References now work like this:

1. If the explicit reference target is recorded in the domain's object
   registry, we link to that target and stop looking. We do this lookup
   regardless of how fully qualified the target is, which allows direct
   references to modules (which don't have a module component to their
   names) or direct references to definitions that may or may not belong
   to a namespace or module.

2. If contextual information is available from qapi:namespace or
   qapi:module directives, try using those components to find a direct
   match to the implied target name.

3. If both prior lookups fail, generate a series of regular expressions
   looking for wildcard matches in order from most to least
   specific. Any explicitly provided components (namespace, module)
   *must* match exactly, but both contextual and entirely omitted
   components are allowed to differ from the search result. Note that if
   more than one result is found, Sphinx will emit a warning (a build
   error for QEMU) and list all of the candidate references.

The practical upshot is that in the large majority of cases, namespace
and module information is not required when creating simple `references`
to definitions from within the same context -- even when identical
definitions exist in other contexts.

Even when using simple `references` from elsewhere in the QEMU
documentation manual, explicit namespace info is not required if there
is only one definition by that name.

Disambiguation *will* be required from outside of the QAPI documentation
when referencing e.g. block-core definitions, which are shared between
QEMU QMP and the QEMU Storage Daemon. In that case, there are two
options:

A: References can be made partially or fully explicit,
   e.g. `QMP:block-dirty-bitmap-add` will link to the QEMU version of
   the definition, while `QSD:block-dirty-bitmap-add` would link to the
   QSD version.

B: If all of the references in a document are intended to go to the same
   place, you can insert a "qapi:namespace:: QMP" directive to influence
   the fuzzy-searching for later references.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
[Commit message typo fixed]
Signed-off-by: Markus Armbruster <[email protected]>
Generate an index-per-namespace for the QAPI domain. Due to a limitation
with Sphinx's architecture, these indices must be defined during setup
time and cannot be dynamically created on-demand when a namespace
directive is encountered.

Owing to that limitation, add a configuration value to conf.py that
specifies which QAPI namespaces we'll generate indices for.

Indices will be named after their namespace, e.g. the "QMP" namespace
will generate to "qapi-qmp-index.html" and can be referenced using
`qapi-qmp-index`.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
This also creates the qapi-qmp-index.html index and cross-reference
target.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Before we enable the QGA and QSD namespaces, we need to disambiguate
some of the references that would become ambiguous as a result!

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
This also creates the `qapi-qsd-index` and `qapi-qga-index` QMP indices.

Signed-off-by: John Snow <[email protected]>
Message-ID: <[email protected]>
Acked-by: Markus Armbruster <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
The A32_BANKED_REG_{GET,SET} macros are only used inside target/arm;
move their definitions to cpregs.h. There's no need to have them
defined in all the code that includes cpu.h.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
We would like to move arm_el_is_aa64() to internals.h; however, it is
used by access_secure_reg().  Make that function not be inline, so
that it can stay in cpu.h.

access_secure_reg() is used only in two places:
 * in hflags.c
 * in the user-mode arm emulators, to decide whether to store
   the TLS value in the secure or non-secure banked field

The second of these is not on a super-hot path that would care about
the inlining (and incidentally will always use the NS banked field
because our user-mode CPUs never set ARM_FEATURE_EL3); put the
definition of access_secure_reg() in hflags.c, near its only use
inside target/arm.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
At the top of linux-user/aarch64/cpu_loop.c we define a set of
macros for reading and writing data and code words, but we never
use these macros. Delete them.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
In linux-user/arm/cpu_loop.c we define a full set of get/put
macros for both code and data (since the endianness handling
is different between the two). However the only one we actually
use is get_user_code_u32(). Remove the rest.

We leave a comment noting how data-side accesses should be handled
for big-endian, because that's a subtle point and we just removed the
macros that were effectively documenting it.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
The arm_cpu_data_is_big_endian() and related functions are now used
only in target/arm; they can be moved to internals.h.

The motivation here is that we would like to move arm_current_el()
to internals.h.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
The functions arm_current_el() and arm_el_is_aa64() are used only in
target/arm and in hw/intc/arm_gicv3_cpuif.c.  They're functions that
query internal state of the CPU.  Move them out of cpu.h and into
internals.h.

This means we need to include internals.h in arm_gicv3_cpuif.c, but
this is justifiable because that file is implementing the GICv3 CPU
interface, which really is part of the CPU proper; we just ended up
implementing it in code in hw/intc/ for historical reasons.

The motivation for this move is that we'd like to change
arm_el_is_aa64() to add a condition that uses cpu_isar_feature();
but we don't want to include cpu-features.h in cpu.h.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
…AArch32

The definition of SCR_EL3.RW says that its effective value is 1 if:
 - EL2 is implemented and does not support AArch32, and SCR_EL3.NS is 1
 - the effective value of SCR_EL3.{EEL2,NS} is {1,0} (i.e. we are
   Secure and Secure EL2 is disabled)

We implement the second of these in arm_el_is_aa64(), but forgot the
first.

Provide a new function arm_scr_rw_eff() to return the effective
value of SCR_EL3.RW, and use it in arm_el_is_aa64() and the other
places that currently look directly at the bit value.

(scr_write() enforces that the RW bit is RAO/WI if neither EL1 nor
EL2 have AArch32 support, but if EL1 does but EL2 does not then the
bit must still be writeable.)

This will mean that if code at EL3 attempts to perform an exception
return to AArch32 EL2 when EL2 is AArch64-only we will correctly
handle this as an illegal exception return: it will be caught by the
"return to an EL which is configured for a different register width"
check in HELPER(exception_return).

We do already have some CPU types which don't implement AArch32
above EL0, so this is technically a bug; it doesn't seem worth
backporting to stable because no sensible guest code will be
deliberately attempting to set the RW bit to a value corresponding
to an unimplemented execution state and then checking that we
did the right thing.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Define the cpr_is_incoming helper, to be used in several cpr fix patches.

Signed-off-by: Steve Sistare <[email protected]>
Reviewed-by: Peter Xu <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
During normal migration, new QEMU creates and initializes memory regions,
then loads the preserved contents of the region from vmstate.

During CPR, memory regions are preserved in place, then the realize
method initializes the regions contents, losing the old contents.  To
fix, skip the re-init during CPR.

Signed-off-by: Steve Sistare <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
During normal migration, new QEMU creates and initializes memory regions,
then loads the preserved contents of the region from vmstate.

During CPR, memory regions are preserved in place, then the realize
method initializes the regions contents, losing the old contents.  To
fix, skip the re-init during CPR.

Signed-off-by: Steve Sistare <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
During normal migration, new QEMU creates and initializes memory regions,
then loads the preserved contents of the region from vmstate.

During CPR, memory regions are preserved in place, then the realize
method initializes the regions contents, losing the old contents.  To
fix, skip writes to the qxl memory regions during CPR load.

Reported-by: [email protected]
Tested-by: [email protected]
Signed-off-by: Steve Sistare <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
When EL1 doesn't support AArch32, the HCR_EL2.RW bit is supposed to
be RAO/WI. Enforce the RAO/WI behaviour.

Note that we handle "reset value should honour RES1 bits" in the same
way that SCR_EL3 does, via a reset function.

We do already have some CPU types which don't implement AArch32
above EL0, so this is technically a bug; it doesn't seem worth
backporting to stable because no sensible guest code will be
deliberately attempting to set the RW bit to a value corresponding
to an unimplemented execution state and then checking that we
did the right thing.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
berrange pushed a commit that referenced this pull request Sep 8, 2025
The ICH9 PCI device uses qemu_init_irq() in its instance_init method,
but fails to clean it up in its uninit. This results in a leak,
detected by ASAN when running the device-introspect-test:

Direct leak of 96 byte(s) in 1 object(s) allocated from:
    #0 0x58f3b53ecde3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
u-system-arm+0x21f1de3) (BuildId: 8dcd38b1d76bd7bd44f905c38200f4cceafd7ca4)
    #1 0x72e446dd5b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x72e446db745a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #3 0x58f3b7c6fc67 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
u/build/arm-asan/../../qom/object.c:568:23
    #4 0x58f3b7c6f670 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
m-asan/../../qom/object.c:578:5
    #5 0x58f3b7c6611b in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/irq.c:48:5
    #6 0x58f3b5c6e931 in pci_ich9_ahci_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/ide/ich.c:117:5

We could call qemu_free_irq() in pci_ich9_uninit(), but
since we have a method of initializing the IRQ that doesn't
need manual freeing, use that instead: qemu_init_irq_child().

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Sep 8, 2025
In pca9554_set_pin() we have a string property which we parse in
order to set some non-string fields in the device state.  So we call
visit_type_str(), passing it the address of the local variable
state_str.

visit_type_str() will allocate a new copy of the string; we
never free this string, so the result is a memory leak, detected
by ASAN during a "make check" run:

Direct leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x5d605212ede3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-arm+0x21f1de3) (
BuildId: 3d5373c89317f58bfcd191a33988c7347714be14)
    #1 0x7f7edea57b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b68282
9a6913cf682d75)
    #2 0x7f7edea6d4d8 in g_strdup (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x784d8) (BuildId: 1eb6131419edb83b2178b68282
9a6913cf682d75)
    #3 0x5d6055289a91 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321:10
    #4 0x5d6055289a91 in qobject_input_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qo
bject-input-visitor.c:542:12
    #5 0x5d605528479c in visit_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qapi-visit
-core.c:349:10
    #6 0x5d60528bdd87 in pca9554_set_pin /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/gpio/pca9554.c:179:10
    #7 0x5d60549bcbbb in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1450:5
    #8 0x5d60549d2055 in object_property_set_qobject /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/qom-qobject.c:28:10
    #9 0x5d60549bcdf1 in object_property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1458:15
    #10 0x5d605439d077 in gb200nvl_bmc_i2c_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:1267:5
    #11 0x5d60543a3bbc in aspeed_machine_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:493:9

Make the state_str g_autofree, so that we will always free
it, on both error-exit and success codepaths.

Cc: [email protected]
Fixes: de0c7d5 ("misc: Add a pca9554 GPIO device model")
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Glenn Miles <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Sep 8, 2025
In the xnlx_dp_init() function we create the s->dpcd and
s->edid objects with qdev_new(); then in xlnx_dp_realize()
we realize the dpcd with qdev_realize() and the edid with
qdev_realize_and_unref().

This is inconsistent, and both ways result in a memory
leak for the instance_init -> deinit lifecycle tested
by device-introspect-test:

Indirect leak of 1968 byte(s) in 1 object(s) allocated from:
    #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c5
3fecd904ba5fc1f521d7da080a0e4103b)
    #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5aded7b9211c in object_new_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:767:15
    #3 0x5aded7b92240 in object_new /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:789:12
    #4 0x5aded7b773e4 in qdev_new /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/qdev.c:149:19
    #5 0x5aded54458be in xlnx_dp_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/display/xlnx_dp.c:1272:20

Direct leak of 344 byte(s) in 1 object(s) allocated from:
    #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
    #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5aded7b9211c in object_new_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:767:15
    #3 0x5aded7b92240 in object_new /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:789:12
    #4 0x5aded7b773e4 in qdev_new /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/qdev.c:149:19
    #5 0x5aded5445a56 in xlnx_dp_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/display/xlnx_dp.c:1275:22

Instead, explicitly object_unref() after we have added the objects as
child properties of the device.  This means they will automatically
be freed when this device is deinited.  When we do this,
qdev_realize() is the correct way to realize them in
xlnx_dp_realize().

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Manos Pitsidianakis <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Sep 8, 2025
When running the bios-tables-test under ASAN we see leaks like this:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x5bc58579b00d in calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x250400d) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
    #1 0x7b4ad90337b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5bc5861826db in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:307:34
    #3 0x5bc587a9edb6 in arm_load_dtb /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/boot.c:656:15

Indirect leak of 28 byte(s) in 2 object(s) allocated from:
    #0 0x5bc58579ae23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x2503e23) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
    #1 0x7b4ad6c8f947 in __vasprintf_internal libio/vasprintf.c:116:16
    #2 0x7b4ad9080a52 in g_vasprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xb0a52) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #3 0x7b4ad90515e4 in g_strdup_vprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x815e4) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #4 0x7b4ad9051940 in g_strdup_printf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x81940) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #5 0x5bc5885eb739 in object_get_canonical_path /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:2123:19
    #6 0x5bc58618dca8 in pc_dimm_md_fill_device_info /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/pc-dimm.c:268:18
    #7 0x5bc586182792 in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:310:9

This happens because we declared the MemoryDeviceInfoList *md_list
with g_autofree, which will free the direct memory with g_free() but
doesn't free all the other data structures referenced by it.  Instead
what we want is to declare the pointer with g_autoptr(), which will
automatically call the qapi_free_MemoryDeviceInfoList() cleanup
function when the variable goes out of scope.

Fixes: 36bc78a ("hw/arm: add static NVDIMMs in device tree")
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Manos Pitsidianakis <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Oct 7, 2025
When we unrealize a CPU object (which happens on vCPU hot-unplug), we
should destroy all the AddressSpace objects we created via calls to
cpu_address_space_init() when the CPU was realized.

Commit 24bec42 added a function to do this for a specific
AddressSpace, but did not add any places where the function was
called.

Since we always want to destroy all the AddressSpaces on unrealize,
regardless of the target architecture, we don't need to try to keep
track of how many are still undestroyed, or make the target
architecture code manually call a destroy function for each AS it
created.  Instead we can adjust the function to always completely
destroy the whole cpu->ases array, and arrange for it to be called
during CPU unrealize as part of the common code.

Without this fix, AddressSanitizer will report a leak like this
from a run where we hot-plugged and then hot-unplugged an x86 KVM
vCPU:

Direct leak of 416 byte(s) in 1 object(s) allocated from:
    #0 0x5b638565053d in calloc (/data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/qemu-system-x86_64+0x1ee153d) (BuildId: c1cd6022b195142106e1bffeca23498c2b752bca)
    #1 0x7c28083f77b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5b6386999c7c in cpu_address_space_init /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../system/physmem.c:797:25
    #3 0x5b638727f049 in kvm_cpu_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../target/i386/kvm/kvm-cpu.c:102:5
    #4 0x5b6385745f40 in accel_cpu_common_realize /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../accel/accel-common.c:101:13
    #5 0x5b638568fe3c in cpu_exec_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/cpu-common.c:232:10
    #6 0x5b63874a2cd5 in x86_cpu_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../target/i386/cpu.c:9321:5
    #7 0x5b6387a0469a in device_set_realized /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/qdev.c:494:13
    #8 0x5b6387a27d9e in property_set_bool /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:2375:5
    #9 0x5b6387a2090b in object_property_set /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:1450:5
    #10 0x5b6387a35b05 in object_property_set_qobject /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/qom-qobject.c:28:10
    #11 0x5b6387a21739 in object_property_set_bool /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:1520:15
    #12 0x5b63879fe510 in qdev_realize /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/qdev.c:276:12

Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2517
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Xu <[email protected]>
berrange added a commit that referenced this pull request Oct 7, 2025
Launch QEMU with

  $ qemu-img create \
      --object secret,id=sec0,data=123456 \
      -f luks -o key-secret=sec0 demo.luks 1g

  $ qemu-system-x86_64 \
      --object secret,id=sec0,data=123456 \
      -blockdev  driver=luks,key-secret=sec0,file.filename=demo.luks,file.driver=file,node-name=luks

Then in QMP shell attempt

  x-blockdev-amend job-id=fish node-name=luks options={'state':'active','new-secret':'sec0','driver':'luks'}

It will result in an assertion

  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
  #1  0x00007fad18b73f63 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:89
  #2  0x00007fad18b19f3e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007fad18b016d0 in __GI_abort () at abort.c:77
  #4  0x00007fad18b01639 in __assert_fail_base
      (fmt=<optimized out>, assertion=<optimized out>, file=<optimized out>, line=<optimized out>, function=<optimized out>) at assert.c:118
  #5  0x00007fad18b120af in __assert_fail (assertion=<optimized out>, file=<optimized out>, line=<optimized out>, function=<optimized out>)
      at assert.c:127
  #6  0x000055ff74fdbd46 in bdrv_graph_rdlock_main_loop () at ../block/graph-lock.c:260
  #7  0x000055ff7548521b in graph_lockable_auto_lock_mainloop (x=<optimized out>)
      at /usr/src/debug/qemu-9.2.4-1.fc42.x86_64/include/block/graph-lock.h:266
  #8  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x55ffb6d66ef0 "", buflen=256000, opaque=0x55ffb5edcc30, errp=0x55ffb6f00700)
      at ../block/crypto.c:71
  #9  0x000055ff75439f8b in qcrypto_block_luks_load_key
      (block=block@entry=0x55ffb5edbe90, slot_idx=slot_idx@entry=0, password=password@entry=0x55ffb67dc260 "123456", masterkey=masterkey@entry=0x55ffb5fb0c40 "", readfunc=readfunc@entry=0x55ff754851e0 <block_crypto_read_func>, opaque=opaque@entry=0x55ffb5edcc30, errp=0x55ffb6f00700)
      at ../crypto/block-luks.c:927
  #10 0x000055ff7543b90f in qcrypto_block_luks_find_key
      (block=<optimized out>, password=<optimized out>, masterkey=<optimized out>, readfunc=<optimized out>, opaque=<optimized out>, errp=<optimized out>) at ../crypto/block-luks.c:1045
  #11 qcrypto_block_luks_amend_add_keyslot
      (block=0x55ffb5edbe90, readfunc=0x55ff754851e0 <block_crypto_read_func>, writefunc=0x55ff75485100 <block_crypto_write_func>, opaque=0x55ffb5edcc3, opts_luks=0x7fad1715aef8, force=<optimized out>, errp=0x55ffb6f00700) at ../crypto/block-luks.c:1673
  #12 qcrypto_block_luks_amend_options
      (block=0x55ffb5edbe90, readfunc=0x55ff754851e0 <block_crypto_read_func>, writefunc=0x55ff75485100 <block_crypto_write_func>, opaque=0x55ffb5edcc30, options=0x7fad1715aef0, force=<optimized out>, errp=0x55ffb6f00700) at ../crypto/block-luks.c:1865
  #13 0x000055ff75485b95 in block_crypto_amend_options_generic_luks
      (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>, errp=<optimized out>) at ../block/crypto.c:949
  #14 0x000055ff75485c28 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
      at ../block/crypto.c:1008
  #15 0x000055ff754778e5 in blockdev_amend_run (job=0x55ffb6f00640, errp=0x55ffb6f00700) at ../block/amend.c:52
  #16 0x000055ff75468b90 in job_co_entry (opaque=0x55ffb6f00640) at ../job.c:1106
  #17 0x000055ff755a0fc2 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../util/coroutine-ucontext.c:175

This changes the read/write callbacks to not assert that they
are run in mainloop context if already in a coroutine.

Fixes: 1f051dc
Signed-off-by: Daniel P. Berrangé <[email protected]>
berrange added a commit that referenced this pull request Oct 31, 2025
Launch QEMU with

  $ qemu-img create \
      --object secret,id=sec0,data=123456 \
      -f luks -o key-secret=sec0 demo.luks 1g

  $ qemu-system-x86_64 \
      --object secret,id=sec0,data=123456 \
      -blockdev  driver=luks,key-secret=sec0,file.filename=demo.luks,file.driver=file,node-name=luks

Then in QMP shell attempt

  x-blockdev-amend job-id=fish node-name=luks options={'state':'active','new-secret':'sec0','driver':'luks'}

It will result in an assertion

  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
  #1  0x00007fad18b73f63 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:89
  #2  0x00007fad18b19f3e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007fad18b016d0 in __GI_abort () at abort.c:77
  #4  0x00007fad18b01639 in __assert_fail_base
      (fmt=<optimized out>, assertion=<optimized out>, file=<optimized out>, line=<optimized out>, function=<optimized out>) at assert.c:118
  #5  0x00007fad18b120af in __assert_fail (assertion=<optimized out>, file=<optimized out>, line=<optimized out>, function=<optimized out>)
      at assert.c:127
  #6  0x000055ff74fdbd46 in bdrv_graph_rdlock_main_loop () at ../block/graph-lock.c:260
  #7  0x000055ff7548521b in graph_lockable_auto_lock_mainloop (x=<optimized out>)
      at /usr/src/debug/qemu-9.2.4-1.fc42.x86_64/include/block/graph-lock.h:266
  #8  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x55ffb6d66ef0 "", buflen=256000, opaque=0x55ffb5edcc30, errp=0x55ffb6f00700)
      at ../block/crypto.c:71
  #9  0x000055ff75439f8b in qcrypto_block_luks_load_key
      (block=block@entry=0x55ffb5edbe90, slot_idx=slot_idx@entry=0, password=password@entry=0x55ffb67dc260 "123456", masterkey=masterkey@entry=0x55ffb5fb0c40 "", readfunc=readfunc@entry=0x55ff754851e0 <block_crypto_read_func>, opaque=opaque@entry=0x55ffb5edcc30, errp=0x55ffb6f00700)
      at ../crypto/block-luks.c:927
  #10 0x000055ff7543b90f in qcrypto_block_luks_find_key
      (block=<optimized out>, password=<optimized out>, masterkey=<optimized out>, readfunc=<optimized out>, opaque=<optimized out>, errp=<optimized out>) at ../crypto/block-luks.c:1045
  #11 qcrypto_block_luks_amend_add_keyslot
      (block=0x55ffb5edbe90, readfunc=0x55ff754851e0 <block_crypto_read_func>, writefunc=0x55ff75485100 <block_crypto_write_func>, opaque=0x55ffb5edcc3, opts_luks=0x7fad1715aef8, force=<optimized out>, errp=0x55ffb6f00700) at ../crypto/block-luks.c:1673
  #12 qcrypto_block_luks_amend_options
      (block=0x55ffb5edbe90, readfunc=0x55ff754851e0 <block_crypto_read_func>, writefunc=0x55ff75485100 <block_crypto_write_func>, opaque=0x55ffb5edcc30, options=0x7fad1715aef0, force=<optimized out>, errp=0x55ffb6f00700) at ../crypto/block-luks.c:1865
  #13 0x000055ff75485b95 in block_crypto_amend_options_generic_luks
      (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>, errp=<optimized out>) at ../block/crypto.c:949
  #14 0x000055ff75485c28 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
      at ../block/crypto.c:1008
  #15 0x000055ff754778e5 in blockdev_amend_run (job=0x55ffb6f00640, errp=0x55ffb6f00700) at ../block/amend.c:52
  #16 0x000055ff75468b90 in job_co_entry (opaque=0x55ffb6f00640) at ../job.c:1106
  #17 0x000055ff755a0fc2 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../util/coroutine-ucontext.c:175

This changes the read/write callbacks to not assert that they
are run in mainloop context if already in a coroutine.

This is also reproduced by qemu-iotests cases 295 and 296.

Fixes: 1f051dc
Signed-off-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
berrange pushed a commit that referenced this pull request Jan 7, 2026
When the Bus Master bit is disabled in a PCI device's Command Register,
the device's DMA address space becomes unassigned memory (i.e. the
io_mem_unassigned MemoryRegion).

This can lead to deadlocks with IOThreads since io_mem_unassigned
accesses attempt to acquire the Big QEMU Lock (BQL). For example,
virtio-pci devices deadlock in virtio_write_config() ->
virtio_pci_stop_ioeventfd() when waiting for the IOThread while holding
the BQL. The IOThread is unable to acquire the BQL but the vcpu thread
won't release the BQL while waiting for the IOThread.

io_mem_unassigned is trivially thread-safe since it has no state, it
simply rejects all load/store accesses. Therefore it is safe to enable
lockless I/O on io_mem_unassigned to eliminate this deadlock.

Here is the backtrace described above:

  Thread 9 (Thread 0x7fccfcdff6c0 (LWP 247832) "CPU 4/KVM"):
  #0  0x00007fcd11529d46 in ppoll () from target:/lib64/libc.so.6
  #1  0x000056468a1a9bad in ppoll (__fds=<optimized out>, __nfds=<optimized out>, __timeout=0x0, __ss=0x0) at /usr/include/bits/poll2.h:88
  #2  0x000056468a18f9d9 in fdmon_poll_wait (ctx=0x5646c6a1dc30, ready_list=0x7fccfcdfb310, timeout=-1) at ../util/fdmon-poll.c:79
  #3  0x000056468a18f14f in aio_poll (ctx=<optimized out>, blocking=blocking@entry=true) at ../util/aio-posix.c:730
  #4  0x000056468a1ad842 in aio_wait_bh_oneshot (ctx=<optimized out>, cb=cb@entry=0x564689faa420 <virtio_blk_ioeventfd_stop_vq_bh>, opaque=<optimized out>) at ../util/aio-wait.c:85
  #5  0x0000564689faaa89 in virtio_blk_stop_ioeventfd (vdev=0x5646c8fd7e90) at ../hw/block/virtio-blk.c:1644
  #6  0x0000564689d77880 in virtio_bus_stop_ioeventfd (bus=bus@entry=0x5646c8fd7e08) at ../hw/virtio/virtio-bus.c:264
  #7  0x0000564689d780db in virtio_bus_stop_ioeventfd (bus=bus@entry=0x5646c8fd7e08) at ../hw/virtio/virtio-bus.c:256
  #8  0x0000564689d7d98a in virtio_pci_stop_ioeventfd (proxy=0x5646c8fcf8e0) at ../hw/virtio/virtio-pci.c:413
  #9  virtio_write_config (pci_dev=0x5646c8fcf8e0, address=4, val=<optimized out>, len=<optimized out>) at ../hw/virtio/virtio-pci.c:803
  #10 0x0000564689dcb45a in memory_region_write_accessor (mr=mr@entry=0x5646c6dc2d30, addr=3145732, value=value@entry=0x7fccfcdfb528, size=size@entry=2, shift=<optimized out>, mask=mask@entry=65535, attrs=...) at ../system/memory.c:491
  #11 0x0000564689dcaeb0 in access_with_adjusted_size (addr=addr@entry=3145732, value=value@entry=0x7fccfcdfb528, size=size@entry=2, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x564689dcb3f0 <memory_region_write_accessor>, mr=0x5646c6dc2d30, attrs=...) at ../system/memory.c:567
  #12 0x0000564689dcb156 in memory_region_dispatch_write (mr=mr@entry=0x5646c6dc2d30, addr=addr@entry=3145732, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) at ../system/memory.c:1554
  #13 0x0000564689dd389a in flatview_write_continue_step (attrs=..., attrs@entry=..., buf=buf@entry=0x7fcd05b87028 "", mr_addr=3145732, l=l@entry=0x7fccfcdfb5f0, mr=0x5646c6dc2d30, len=2) at ../system/physmem.c:3266
  #14 0x0000564689dd3adb in flatview_write_continue (fv=0x7fcadc0d8930, addr=3761242116, attrs=..., ptr=0xe0300004, len=2, mr_addr=<optimized out>, l=<optimized out>, mr=<optimized out>) at ../system/physmem.c:3296
  #15 flatview_write (fv=0x7fcadc0d8930, addr=addr@entry=3761242116, attrs=attrs@entry=..., buf=buf@entry=0x7fcd05b87028, len=len@entry=2) at ../system/physmem.c:3327
  #16 0x0000564689dd7191 in address_space_write (as=0x56468b433600 <address_space_memory>, addr=3761242116, attrs=..., buf=0x7fcd05b87028, len=2) at ../system/physmem.c:3447
  #17 address_space_rw (as=0x56468b433600 <address_space_memory>, addr=3761242116, attrs=attrs@entry=..., buf=buf@entry=0x7fcd05b87028, len=2, is_write=<optimized out>) at ../system/physmem.c:3457
  #18 0x0000564689ff1ef6 in kvm_cpu_exec (cpu=cpu@entry=0x5646c6dab810) at ../accel/kvm/kvm-all.c:3248
  #19 0x0000564689ff32f5 in kvm_vcpu_thread_fn (arg=arg@entry=0x5646c6dab810) at ../accel/kvm/kvm-accel-ops.c:53
  #20 0x000056468a19225c in qemu_thread_start (args=0x5646c6db6190) at ../util/qemu-thread-posix.c:393
  #21 0x00007fcd114c5b68 in start_thread () from target:/lib64/libc.so.6
  #22 0x00007fcd115364e4 in clone () from target:/lib64/libc.so.6

  Thread 3 (Thread 0x7fcd0503a6c0 (LWP 247825) "IO iothread1"):
  #0  0x00007fcd114c2d30 in __lll_lock_wait () from target:/lib64/libc.so.6
  #1  0x00007fcd114c8fe2 in pthread_mutex_lock@@GLIBC_2.2.5 () from target:/lib64/libc.so.6
  #2  0x000056468a192538 in qemu_mutex_lock_impl (mutex=0x56468b432e60 <bql>, file=0x56468a1e26a5 "../system/physmem.c", line=3198) at ../util/qemu-thread-posix.c:94
  #3  0x0000564689dc12e2 in bql_lock_impl (file=file@entry=0x56468a1e26a5 "../system/physmem.c", line=line@entry=3198) at ../system/cpus.c:566
  #4  0x0000564689ddc151 in prepare_mmio_access (mr=0x56468b433800 <io_mem_unassigned>) at ../system/physmem.c:3198
  #5  address_space_lduw_internal_cached_slow (cache=<optimized out>, addr=2, attrs=..., result=0x0, endian=DEVICE_LITTLE_ENDIAN) at ../system/memory_ldst.c.inc:211
  #6  address_space_lduw_le_cached_slow (cache=<optimized out>, addr=addr@entry=2, attrs=attrs@entry=..., result=result@entry=0x0) at ../system/memory_ldst.c.inc:253
  #7  0x0000564689fd692c in address_space_lduw_le_cached (result=0x0, cache=<optimized out>, addr=2, attrs=...) at /var/tmp/qemu/include/exec/memory_ldst_cached.h.inc:35
  #8  lduw_le_phys_cached (cache=<optimized out>, addr=2) at /var/tmp/qemu/include/exec/memory_ldst_phys.h.inc:66
  #9  virtio_lduw_phys_cached (vdev=<optimized out>, cache=<optimized out>, pa=2) at /var/tmp/qemu/include/hw/virtio/virtio-access.h:166
  #10 vring_avail_idx (vq=0x5646c8fe2470) at ../hw/virtio/virtio.c:396
  #11 virtio_queue_split_set_notification (vq=0x5646c8fe2470, enable=0) at ../hw/virtio/virtio.c:534
  #12 virtio_queue_set_notification (vq=0x5646c8fe2470, enable=0) at ../hw/virtio/virtio.c:595
  #13 0x000056468a18e7a8 in poll_set_started (ctx=ctx@entry=0x5646c6c74e30, ready_list=ready_list@entry=0x7fcd050366a0, started=started@entry=true) at ../util/aio-posix.c:247
  #14 0x000056468a18f2bb in poll_set_started (ctx=0x5646c6c74e30, ready_list=0x7fcd050366a0, started=true) at ../util/aio-posix.c:226
  #15 try_poll_mode (ctx=0x5646c6c74e30, ready_list=0x7fcd050366a0, timeout=<synthetic pointer>) at ../util/aio-posix.c:612
  #16 aio_poll (ctx=0x5646c6c74e30, blocking=blocking@entry=true) at ../util/aio-posix.c:689
  #17 0x000056468a032c26 in iothread_run (opaque=opaque@entry=0x5646c69f3380) at ../iothread.c:63
  #18 0x000056468a19225c in qemu_thread_start (args=0x5646c6c75410) at ../util/qemu-thread-posix.c:393
  #19 0x00007fcd114c5b68 in start_thread () from target:/lib64/libc.so.6
  #20 0x00007fcd115364e4 in clone () from target:/lib64/libc.so.6

Buglink: https://issues.redhat.com/browse/RHEL-71933
Reported-by: Peixiu Hou <[email protected]>
Cc: Kevin Wolf <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Xu <[email protected]>
berrange pushed a commit that referenced this pull request Jan 7, 2026
Caught by inspection, but ASAN also reports:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
 #0 in malloc
 #1 in g_malloc
 #2 in g_memdup
 #3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
 #4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
 #5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
 #6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
 #7 in migrate_params_apply ../migration/options.c:1407:13
 #8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
 #9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
 #10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5

Note that this is entirely harmless because the migration object which
contains the MigrationParameters structure is kept until the QEMU
process exits.

Reviewed-by: Markus Armbruster <[email protected]>
Reviewed-by: Peter Xu <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Xu <[email protected]>
berrange pushed a commit that referenced this pull request Feb 9, 2026
Add a 'preserve_config' field in struct GPEXConfig and, if set, generate
the _DSM function #5 for preserving PCI boot configurations.

This will be used for SMMUv3 accel=on support in subsequent patch. When
SMMUv3 acceleration (accel=on) is enabled, QEMU exposes IORT Reserved
Memory Region (RMR) nodes to support MSI doorbell translations. As per
the Arm IORT specification, using IORT RMRs mandates the presence of
_DSM function #5 so that the OS retains the firmware-assigned PCI
configuration. Hence, this patch adds conditional support for generating
_DSM #5.

According to the ACPI Specification, Revision 6.6, Section 9.1.1 -
“_DSM (Device Specific Method)”,

"
If Function Index is zero, the return is a buffer containing one bit for
each function index, starting with zero. Bit 0 indicates whether there
is support for any functions other than function 0 for the specified
UUID and Revision ID. If set to zero, no functions are supported (other
than function zero) for the specified UUID and Revision ID. If set to
one, at least one additional function is supported. For all other bits
in the buffer, a bit is set to zero to indicate if that function index
is not supported for the specific UUID and Revision ID. (For example,
bit 1 set to 0 indicates that function index 1 is not supported for the
specific UUID and Revision ID.)
"

Please refer PCI Firmware Specification, Revision 3.3, Section 4.6.5 —
"_DSM for Preserving PCI Boot Configurations" for Function 5 of _DSM
method.

Also, while at it, move the byte_list declaration to the top of the
function for clarity.

At the moment, DSM generation is not yet enabled.

The resulting AML when preserve_config=true is:

    Method (_DSM, 4, NotSerialized)
        {
            If ((Arg0 == ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d")))
                {
                    If ((Arg2 == Zero))
                    {
                        Return (Buffer (One)
                        {
                             0x21
                        })
                    }

                    If ((Arg2 == 0x05))
                    {
                        Return (Zero)
                    }
                }
         ...
      }

Cc: Michael S. Tsirkin <[email protected]>
Signed-off-by: Eric Auger <[email protected]>
Signed-off-by: Shameer Kolothum <[email protected]>
Signed-off-by: Shameer Kolothum <[email protected]>
Tested-by: Zhangfei Gao <[email protected]>
Tested-by: Eric Auger <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Michael S. Tsirkin <[email protected]>
Message-id: [email protected]
[Shameer: Removed possible duplicate _DSM creations]
Signed-off-by: Shameer Kolothum <[email protected]>
Tested-by: Zhangfei Gao <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Michael S. Tsirkin <[email protected]>
Tested-by: Eric Auger <[email protected]>
Signed-off-by: Shameer Kolothum <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
berrange pushed a commit that referenced this pull request Feb 9, 2026
Introduce a new pci_preserve_config field in virt machine state which
allows the generation of DSM #5. This field is only set if accel SMMU
is instantiated.

In a subsequent patch, SMMUv3 accel mode will make use of IORT RMR nodes
to enable nested translation of MSI doorbell addresses. IORT RMR requires
_DSM #5 to be set for the PCI host bridge so that the Guest kernel
preserves the PCI boot configuration.

Reviewed-by: Jonathan Cameron <[email protected]>
Tested-by: Zhangfei Gao <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
Tested-by: Eric Auger <[email protected]>
Signed-off-by: Shameer Kolothum <[email protected]>
Message-id: [email protected]
Signed-off-by: Peter Maydell <[email protected]>
berrange pushed a commit that referenced this pull request Mar 3, 2026
…n coredump

Commit 772f868 ("scripts/qemu-gdb: Support coroutine dumps in
coredumps") introduced coroutine traces in coredumps using raw stack
unwinding.  While this works, this approach does not allow to view the
function arguments in the corresponding stack frames.

As an alternative, we can obtain saved registers from the coroutine's
jmpbuf, patch them into the coredump's struct elf_prstatus in place, and
execute another gdb subprocess to get backtrace from the patched temporary
coredump.

While providing more detailed info, this alternative approach, however, is
more invasive as it might potentially corrupt the coredump file. We do take
precautions by saving the original registers values into a separate binary
blob /path/to/coredump.ptregs, so that it can be restores in the next
GDB session.  Still, instead of making it a new deault, let's keep raw unwind
the default behaviour, but add the '--detailed' option for 'qemu bt' and
'qemu coroutine' command which would enforce the new behaviour.

That's how this looks:

  (gdb) qemu coroutine 0x7fda9335a508
  #0  0x5602bdb41c26 in qemu_coroutine_switch<+214> () at ../util/coroutine-ucontext.c:321
  #1  0x5602bdb3e8fe in qemu_aio_coroutine_enter<+493> () at ../util/qemu-coroutine.c:293
  #2  0x5602bdb3c4eb in co_schedule_bh_cb<+538> () at ../util/async.c:547
  #3  0x5602bdb3b518 in aio_bh_call<+119> () at ../util/async.c:172
  #4  0x5602bdb3b79a in aio_bh_poll<+457> () at ../util/async.c:219
  #5  0x5602bdb10f22 in aio_poll<+1201> () at ../util/aio-posix.c:719
  #6  0x5602bd8fb1ac in iothread_run<+123> () at ../iothread.c:63
  #7  0x5602bdb18a24 in qemu_thread_start<+355> () at ../util/qemu-thread-posix.c:393

  (gdb) qemu coroutine 0x7fda9335a508 --detailed
  patching core file /tmp/tmpq4hmk2qc
  found "CORE" at 0x10c48
  assume pt_regs at 0x10cbc
  write r15 at 0x10cbc
  write r14 at 0x10cc4
  write r13 at 0x10ccc
  write r12 at 0x10cd4
  write rbp at 0x10cdc
  write rbx at 0x10ce4
  write rip at 0x10d3c
  write rsp at 0x10d54

  #0  0x00005602bdb41c26 in qemu_coroutine_switch (from_=0x7fda9335a508, to_=0x7fda8400c280, action=COROUTINE_ENTER) at ../util/coroutine-ucontext.c:321
  #1  0x00005602bdb3e8fe in qemu_aio_coroutine_enter (ctx=0x5602bf7147c0, co=0x7fda8400c280) at ../util/qemu-coroutine.c:293
  #2  0x00005602bdb3c4eb in co_schedule_bh_cb (opaque=0x5602bf7147c0) at ../util/async.c:547
  #3  0x00005602bdb3b518 in aio_bh_call (bh=0x5602bf714a40) at ../util/async.c:172
  #4  0x00005602bdb3b79a in aio_bh_poll (ctx=0x5602bf7147c0) at ../util/async.c:219
  #5  0x00005602bdb10f22 in aio_poll (ctx=0x5602bf7147c0, blocking=true) at ../util/aio-posix.c:719
  #6  0x00005602bd8fb1ac in iothread_run (opaque=0x5602bf42b100) at ../iothread.c:63
  #7  0x00005602bdb18a24 in qemu_thread_start (args=0x5602bf7164a0) at ../util/qemu-thread-posix.c:393
  #8  0x00007fda9e89f7f2 in start_thread (arg=<optimized out>) at pthread_create.c:443
  #9  0x00007fda9e83f450 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

CC: Vladimir Sementsov-Ogievskiy <[email protected]>
CC: Peter Xu <[email protected]>
Originally-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Andrey Drobyshev <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Message-id: [email protected]
Signed-off-by: Stefan Hajnoczi <[email protected]>
berrange pushed a commit that referenced this pull request Mar 3, 2026
Some Proxmox users reported an occasional assertion failure [0][1] in
busy VMs when using drive mirror with active mode. In particular, the
failure may occur for zero writes shorter than the job granularity:

> #0  0x00007b421154b507 in abort ()
> #1  0x00007b421154b420 in ?? ()
> #2  0x0000641c582e061f in bitmap_set (map=0x7b4204014e00, start=14, nr=-1)
> #3  0x0000641c58062824 in do_sync_target_write (job=0x641c7e73d1e0,
>       method=MIRROR_METHOD_ZERO, offset=852480, bytes=4096, qiov=0x0, flags=0)
> #4  0x0000641c58062250 in bdrv_mirror_top_do_write (bs=0x641c7e62e1f0,
        method=MIRROR_METHOD_ZERO, copy_to_target=true, offset=852480,
        bytes=4096, qiov=0x0, flags=0)
> #5  0x0000641c58061f31 in bdrv_mirror_top_pwrite_zeroes (bs=0x641c7e62e1f0,
        offset=852480, bytes=4096, flags=0)

The range for the dirty bitmap described by dirty_bitmap_offset and
dirty_bitmap_end is narrower than the original range and in fact,
dirty_bitmap_end might be smaller than dirty_bitmap_offset. There
already is a check for 'dirty_bitmap_offset < dirty_bitmap_end' before
resetting the dirty bitmap. Add such a check for setting the zero
bitmap too, which uses the same narrower range.

[0]: https://forum.proxmox.com/threads/177981/
[1]: https://bugzilla.proxmox.com/show_bug.cgi?id=7222

Cc: [email protected]
Fixes: 7e27754 ("mirror: Skip writing zeroes when target is already zero")
Signed-off-by: Fiona Ebner <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
berrange pushed a commit that referenced this pull request Mar 3, 2026
Commit e27194e ("virtio-gpu-virgl: correct parent for blob memory
region") made the name member of MemoryRegion unset, causing a NULL
pointer dereference[1]:
> Thread 2 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> (gdb) bt
> #0  0x00007ffff56565e2 in __strcmp_evex () at /lib64/libc.so.6
> #1  0x0000555555841bdb in find_fd (head=0x5555572337d0 <cpr_state>,
> name=0x0, id=0) at ../migration/cpr.c:68
> #2  cpr_delete_fd (name=name@entry=0x0, id=id@entry=0) at
> ../migration/cpr.c:77
> #3  0x000055555582290a in qemu_ram_free (block=0x7ff7e93aa7f0) at
> ../system/physmem.c:2615
> #4  0x000055555581ae02 in memory_region_finalize (obj=<optimized out>)
> at ../system/memory.c:1816
> #5  0x0000555555a70ab9 in object_deinit (obj=<optimized out>,
> type=<optimized out>) at ../qom/object.c:715
> #6  object_finalize (data=0x7ff7e936eff0) at ../qom/object.c:729
> #7  object_unref (objptr=0x7ff7e936eff0) at ../qom/object.c:1232
> #8  0x0000555555814fae in memory_region_unref (mr=<optimized out>) at
> ../system/memory.c:1848
> #9  flatview_destroy (view=0x555559ed6c40) at ../system/memory.c:301
> #10 0x0000555555bfc122 in call_rcu_thread (opaque=<optimized out>) at
> ../util/rcu.c:324
> #11 0x0000555555bf17a7 in qemu_thread_start (args=0x555557b99520) at
> ../util/qemu-thread-posix.c:393
> #12 0x00007ffff556f464 in start_thread () at /lib64/libc.so.6
> #13 0x00007ffff55f25ac in __clone3 () at /lib64/libc.so.6

The intention of the aforementioned commit is to prevent a MemoryRegion
from parenting itself while its references is counted indendependently
of the device. To achieve the same goal, add a type of QOM objects that
count references and parent MemoryRegions.

[1] https://lore.kernel.org/qemu-devel/[email protected]/

Cc: [email protected]
Fixes: e27194e ("virtio-gpu-virgl: correct parent for blob memory region")
Signed-off-by: Akihiko Odaki <[email protected]>
Tested-by: Dmitry Osipenko <[email protected]>
Tested-by: Joelle van Dyne <[email protected]>
Reviewed-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Message-Id: <[email protected]>
berrange pushed a commit that referenced this pull request Mar 3, 2026
When removing the 'emergency-write' property in commit d0660e5
we neglected to remove the code reducing the virtio_console_config
structure size, allowing to access up to the unallocated 'emerg_wr'
field.

Can be reproduced running:

  $ cat << EOF | qemu-system-i386 -nodefaults \
                     -machine q35 -m 512M \
                     -device virtio-serial \
                     -display none \
                     -machine accel=qtest -qtest stdio
  outl 0xcf8 0x80000810
  outl 0xcfc 0xc000
  outl 0xcf8 0x80000804
  outw 0xcfc 0x01
  outl 0xc014 0x00
  EOF
  ==3210206==ERROR: AddressSanitizer: heap-buffer-overflow
      on address 0x502000090858 at pc 0x5638f1300a9b bp 0x7fff6b525b80 sp 0x7fff6b525b70
  READ of size 4 at 0x502000090858 thread T0
      #0 0x5638f1300a9a in set_config hw/char/virtio-serial-bus.c:590
      #1 0x5638f0bccdcf in virtio_config_writel hw/virtio/virtio-config-io.c:104
      #2 0x5638f0bd0c89 in virtio_pci_config_write hw/virtio/virtio-pci.c:637
      #3 0x5638f0cf90cf in memory_region_write_accessor system/memory.c:491
      #4 0x5638f0cf975b in access_with_adjusted_size system/memory.c:567
      #5 0x5638f0d01d3f in memory_region_dispatch_write system/memory.c:1547
      #6 0x5638f0d2fa1e in address_space_stm_internal system/memory_ldst.c.inc:85
      #7 0x5638f0d30013 in address_space_stl_le system/memory_ldst_endian.c.inc:53
      #8 0x5638f0ceb568 in cpu_outl system/ioport.c:79
      #9 0x5638f0d3c0f9 in qtest_process_command system/qtest.c:483

  0x502000090858 is located 0 bytes to the right of 8-byte region [0x502000090850,0x502000090858)
  allocated by thread T0 here:
      #0 0x7f0dc32cba57 in __interceptor_calloc src/libsanitizer/asan/asan_malloc_linux.cpp:154
      #1 0x7f0dc2382c50 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5ec50)
      #2 0x5638f1303c27 in virtio_serial_device_realize hw/char/virtio-serial-bus.c:1046
      #3 0x5638f1396a9c in virtio_device_realize hw/virtio/virtio.c:4053
      #4 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523
      #5 0x5638f13fdaf6 in property_set_bool qom/object.c:2376
      #6 0x5638f13f9098 in object_property_set qom/object.c:1450
      #7 0x5638f140283c in object_property_set_qobject qom/qom-qobject.c:28
      #8 0x5638f13f9616 in object_property_set_bool qom/object.c:1520
      #9 0x5638f13e91cc in qdev_realize hw/core/qdev.c:276
      #10 0x5638f0c3d94b in virtio_serial_pci_realize hw/virtio/virtio-serial-pci.c:69
      #11 0x5638f0bda886 in virtio_pci_realize hw/virtio/virtio-pci.c:2351
      #12 0x5638f09bc2ae in pci_qdev_realize hw/pci/pci.c:2310
      #13 0x5638f0bdb2f2 in virtio_pci_dc_realize hw/virtio/virtio-pci.c:2473
      #14 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523

    SUMMARY: AddressSanitizer: heap-buffer-overflow hw/char/virtio-serial-bus.c:590 in set_config

Fixes: d0660e5 ("hw/char/virtio-serial: Do not expose the 'emergency-write' property")
Reported-by: Alexander Bulekov <[email protected]>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3303
Buglink: https://issues.oss-fuzz.com/issues/484647006
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Alexander Bulekov <[email protected]>
Message-Id: <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
In patch_hwaddr() we allocate a GByteArray for the data we read back
from the guest; however we forget to free it, and the leak sanitizer
complains:

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x56c00ad48293 in malloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-x86_64+0x1a9f293) (BuildId: 62e2a7dbe5ff146b2fa14d26e24e443f1967edd9)
    #1 0x7b3e4cc91ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x7b3e4cc54c12 in g_array_sized_new (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x25c12) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #3 0x7b3e44b06b49 in patch_hwaddr /home/pm215/qemu/build/x86-tgt-san/../../tests/tcg/plugins/patch.c:68:29

Indirect leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x56c00ad486b0 in realloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-x86_64+0x1a9f6b0) (BuildId: 62e2a7dbe5ff146b2fa14d26e24e443f1967edd9)
    #1 0x7b3e4cc92819 in g_realloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63819) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x7b3e4cc54b36  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x25b36) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #3 0x7b3e4cc55276 in g_array_set_size (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x26276) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #4 0x7b3e4cc55574 in g_byte_array_set_size (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x26574) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #5 0x56c00be2ccc1 in qemu_plugin_read_memory_hwaddr /home/pm215/qemu/build/x86-tgt-san/../../plugins/api.c:524:5

Mark the variable as g_autoptr(), as we already do in the equivalent
code in patch_vaddr().

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Pierrick Bouvier <[email protected]>
Link: https://lore.kernel.org/qemu-devel/[email protected]
Signed-off-by: Pierrick Bouvier <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The QPCIDevice we get via qpci_device_foreach() is allocated
memory, and we need to g_free() it after use.

This fixes asan leaks like this:

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x622a5f16913d in calloc (/home/pm215/qemu/build/arm-clang/tests/qtest/iommu-smmuv3-test+0x1d413d) (BuildId: bc598be1f4ad6d1a9a600c55aeef36108bdb6a04)
    #1 0x73ee41c0f771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x622a5f1d4cec in qpci_device_find /home/pm215/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:82:11
    #3 0x622a5f1d4cec in qpci_device_foreach /home/pm215/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:34:19
    #4 0x622a5f23cc73 in setup_qtest_pci_device /home/pm215/qemu/build/arm-clang/../../tests/qtest/iommu-smmuv3-test.c:45:5
    #5 0x622a5f23cc73 in run_smmuv3_translation /home/pm215/qemu/build/arm-clang/../../tests/qtest/iommu-smmuv3-test.c:74:11

Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Message-id: [email protected]
berrange pushed a commit that referenced this pull request Apr 8, 2026
The IOWatchPoll holds a reference to the iochannel while the "child"
source (iwp->src) is removed from the context and freed. Freeing the
source leads to the iochannel being also freed at
qio_channel_fd_source_finalize().

Later, io_watch_poll_prepare() tries to create another source with the
same iochannel and hits an use after free:

==8241==ERROR: AddressSanitizer: heap-use-after-free on address 0x514000000040
READ of size 8 at 0x514000000040 thread T2
    #0 0x561c2d272fcd in object_get_class  ../qom/object.c:1043:17
    #1 0x561c2d338f84 in QIO_CHANNEL_GET_CLASS  include/io/channel.h:29:1
    #2 0x561c2d33b26f in qio_channel_create_watch  ../io/channel.c:388:30
    #3 0x561c2d2f0993 in io_watch_poll_prepare  ../chardev/char-io.c:65:20
    ...

0x514000000040 is located 0 bytes inside of 392-byte region [0x514000000040,0x5140000001c8)
freed by thread T2 here:
    #0 0x561c2d2319a5 in free
    #1 0x7fb2c0926638 in g_free
    #2 0x561c2d276507 in object_finalize  ../qom/object.c:734:9
    #3 0x561c2d271d0d in object_unref  ../qom/object.c:1231:9
    #4 0x561c2d32ef1d in qio_channel_fd_source_finalize  ../io/channel-watch.c:95:5
    #5 0x7fb2c091d124 in g_source_unref_internal ../glib/gmain.c:2298
    #6 0x561c2d2f0b6c in io_watch_poll_prepare  ../chardev/char-io.c:71:9
    ...

previously allocated by thread T3 (connect) here:
    #0 0x561c2d231c69 in malloc
    #1 0x7fb2c0926518 in g_malloc
    #2 0x561c2d27246e in object_new_with_type  ../qom/object.c:767:15
    #3 0x561c2d272530 in object_new  ../qom/object.c:789:12
    #4 0x561c2d320193 in qio_channel_socket_new  ../io/channel-socket.c:64:31
    #5 0x561c2d308013 in tcp_chr_connect_client_async  ../chardev/char-socket.c:1181:12
    #6 0x561c2d3002e7 in qmp_chardev_open_socket_client  ../chardev/char-socket.c:1281:9
    ...

Fix the issue by incrementing the iochannel reference count when the
IOWatchPoll takes a reference and decrementing when it is finalized.

Signed-off-by: Fabiano Rosas <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Message-id: [email protected]
[PMM: rebased]
Signed-off-by: Peter Maydell <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
tcp_chr_free_connection() can be called multiple times in succession,
in which case the yank function will get as argument a NULL s->sioc
that has been cleared by the previous tcp_chr_free_connection() call.

This leads to an abort() at yank_unregister_function().

 #0  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51
 #1  __GI_abort () at abort.c:79
 #2  qtest_check_status (s=0x513000005600) at ../tests/qtest/libqtest.c:209
 #3  qtest_wait_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:273
 #4  qtest_kill_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:285
 #5  kill_qemu_hook_func (s=0x513000005600) at ../tests/qtest/libqtest.c:294
 #6  g_hook_list_invoke (hook_list=0x55ea9cc750c0 <abrt_hooks>, may_recurse=0) at ../glib/ghook.c:534
 #7  sigabrt_handler (signo=6) at ../tests/qtest/libqtest.c:299
 #8  <signal handler called>
 #9  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51
 #10 __GI_abort () at abort.c:79
 #11 yank_unregister_function (instance=0x7fb26f2ea9a0,
     func=0x55ea9bcc0a10 <char_socket_yank_iochannel>, opaque=0x0) at
     ../util/yank.c:151
 #12 tcp_chr_free_connection (chr=0x51300000ffc0) at ../chardev/char-socket.c:385
 #13 tcp_chr_disconnect_locked (chr=0x51300000ffc0) at ../chardev/char-socket.c:477
 #14 tcp_chr_disconnect (chr=0x51300000ffc0) at ../chardev/char-socket.c:495
 #15 tcp_chr_hup (channel=0x514000000040, cond=G_IO_HUP, opaque=0x51300000ffc0) at ../chardev/char-socket.c:536
 #16 qio_channel_fd_source_dispatch (source=0x50c0000b5fc0, callback=0x55ea9bcd6770 <tcp_chr_hup>,
     user_data=0x51300000ffc0) at ../io/channel-watch.c:84
 #17 g_main_dispatch (context=0x50f000000040) at ../glib/gmain.c:3381
 #18 g_main_context_dispatch (context=context@entry=0x50f000000040) at ../glib/gmain.c:4099
 #19 g_main_context_iterate (context=0x50f000000040, block=block@entry=1, dispatch=dispatch@entry=1,
     self=<optimized out>) at ../glib/gmain.c:4175
 #20 g_main_loop_run (loop=0x502000055690) at ../glib/gmain.c:4373

Commit ebae647 ("chardev: check if the chardev is registered for
yanking") seems to have encountered a similar issue, but checking
s->registered_yank is not a complete solution because that flag
pertains to the yank instance, not to each individual function.

Skip the yank_unregister_function() in case s->sioc is already NULL,
which indicates the last yank function was already removed.

Signed-off-by: Fabiano Rosas <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Message-id: [email protected]
[PMM: rebased]
Signed-off-by: Peter Maydell <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
Details: https://gitlab.com/qemu-project/qemu/-/issues/3144

The function schedule_next_request is called with tg->lock held and
it may call throttle_group_co_restart_queue, which takes
tgm->throttled_reqs_lock, qemu_co_mutex_lock may leave current
coroutine if other iothread has taken the lock. If the next
coroutine will call throttle_group_co_io_limits_intercept - it
will try to take the mutex tg->lock which will never be released.

Here is the backtrace of the iothread:
Thread 30 (Thread 0x7f8aad1fd6c0 (LWP 24240) "IO iothread2"):
 #0  futex_wait (futex_word=0x5611adb7d828, expected=2, private=0) at ../sysdeps/nptl/futex-internal.h:146
 #1  __GI___lll_lock_wait (futex=futex@entry=0x5611adb7d828, private=0) at lowlevellock.c:49
 #2  0x00007f8ab5a97501 in lll_mutex_lock_optimized (mutex=0x5611adb7d828) at pthread_mutex_lock.c:48
 #3  ___pthread_mutex_lock (mutex=0x5611adb7d828) at pthread_mutex_lock.c:93
 #4  0x00005611823f5482 in qemu_mutex_lock_impl (mutex=0x5611adb7d828, file=0x56118289daca "../block/throttle-groups.c", line=372) at ../util/qemu-thread-posix.c:94
 #5  0x00005611822b0b39 in throttle_group_co_io_limits_intercept (tgm=0x5611af1bb4d8, bytes=4096, direction=THROTTLE_READ) at ../block/throttle-groups.c:372
 #6  0x00005611822473b1 in blk_co_do_preadv_part (blk=0x5611af1bb490, offset=15972311040, bytes=4096, qiov=0x7f8aa4000f98, qiov_offset=0, flags=BDRV_REQ_REGISTERED_BUF) at ../block/block-backend.c:1354
 #7  0x0000561182247fa0 in blk_aio_read_entry (opaque=0x7f8aa4005910) at ../block/block-backend.c:1619
 #8  0x000056118241952e in coroutine_trampoline (i0=-1543497424, i1=32650) at ../util/coroutine-ucontext.c:175
 #9  0x00007f8ab5a56f70 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:66 from target:/lib64/libc.so.6
 #10 0x00007f8aad1ef190 in ?? ()
 #11 0x0000000000000000 in ?? ()

The lock is taken in line 386:
(gdb) p tg.lock
$1 = {lock = {__data = {__lock = 2, __count = 0, __owner = 24240, __nusers = 1, __kind = 0, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}},
    __size = "\002\000\000\000\000\000\000\000\260^\000\000\001", '\000' <repeats 26 times>, __align = 2}, file = 0x56118289daca "../block/throttle-groups.c",
  line = 386, initialized = true}

The solution is to use tg->lock to protect both ThreadGroup fields and
ThrottleGroupMember.throttled_reqs. It doesn't seem to be possible
to use separate locks because we need to first manipulate ThrottleGroup
fields, then schedule next coroutine using throttled_reqs and after than
update token field from ThrottleGroup depending on the throttled_reqs
state.

Signed-off-by: Dmitry Guryanov <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Hanna Czenczek <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The m68k mcf_intc interrupt controller currently implements its
inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
This results in leaks like this:

Direct leak of 2944 byte(s) in 46 object(s) allocated from:
    #0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
    #1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
    #3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
    #4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
    #5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11

This isn't an important leak, as it is memory we allocate once at
QEMU startup and that has to stay live for the lifetime of the
system.  However it does point at a code improvement.

Modernise this to have the device itself create inbound GPIOs with
qdev_init_gpio_in() that the board can then refer to and wire up
individually.

As the device is used in only a single board, we can update device
and board in a single patch rather than having to try to figure out
some way to change the API more piecemeal.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Thomas Huth <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
In the sun4m machine init, we set up the cpu_irqs[] array
with the real inbound IRQs for each CPU, followed by some
dummy IRQs for the remaining slots from smp_cpus up to
MAX_CPUS. These dummy IRQs do nothing when set/cleared
because the dummy_cpu_set_irq() function does nothing.

Instead of creating these "do nothing" qemu_irqs, instead
pass the number of CPUs to slavio_intctl_init() so that it
can only wire up the interrupt controller's interrupts
for the CPUs that actually exist. Calling qemu_set_irq()
on an irq that isn't connected does nothing, so this is
a simpler way to achieve the same result.

This cleanup fixes an unimportant memory leak reported by
the address sanitizer that happens because we allocate these
dummy IRQs with qemu_allocate_irqs():

Direct leak of 1920 byte(s) in 15 object(s) allocated from:
    #0 0x5cb7b120cf63 in malloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0bf63) (BuildId: d27f9230a7cc82ebfaf0cf9e439dc215ddd7ac68)
    #1 0x743cd6dc5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5cb7b1a42fb4 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
    #3 0x5cb7b19e7e72 in sun4m_hw_init /home/pm215/qemu/build/san/../../hw/sparc/sun4m.c:845:23
    #4 0x5cb7b141d3dd in machine_run_board_init /home/pm215/qemu/build/san/../../hw/core/machine.c:1709:5
    #5 0x5cb7b1542895 in qemu_init_board /home/pm215/qemu/build/san/../../system/vl.c:2717:5
    #6 0x5cb7b1542895 in qmp_x_exit_preconfig /home/pm215/qemu/build/san/../../system/vl.c:2811:5
    #7 0x5cb7b15493ac in qemu_init /home/pm215/qemu/build/san/../../system/vl.c:3849:9
    #8 0x5cb7b1f3f201 in main /home/pm215/qemu/build/san/../../system/main.c:71:5
    #9 0x743cd4a2a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #10 0x743cd4a2a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #11 0x5cb7b1172114 in _start (/home/pm215/qemu/build/san/qemu-system-sparc+0xd71114) (BuildId: d27f9230a7cc82ebfaf0cf9e439dc215ddd7ac68)

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Mark Cave-Ayland <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The slavio_timer device's instance_init function allocates memory for
TimerContext structs and a ptimer, but it never frees this memory, so
we will leak it if the QMP interface does introspection of this
device type, as reported by the clang address sanitizer:

Indirect leak of 4896 byte(s) in 17 object(s) allocated from:
    #0 0x5f2948d9b14d in calloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0c14d) (BuildId: 7210711bdf6f7fbd0b863bd2dfcc7c42c7175db1)
    #1 0x758584b11771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5f2949097b8a in slavio_timer_init /home/pm215/qemu/build/san/../../hw/timer/slavio_timer.c:403:14
    #3 0x5f29495d790f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #4 0x5f29495d96ef in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
    #5 0x5f2949a30a26 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11

Indirect leak of 1632 byte(s) in 17 object(s) allocated from:
    #0 0x5f2948d9b14d in calloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0c14d) (BuildId: 7210711bdf6f7fbd0b863bd2dfcc7c42c7175db1)
    #1 0x758584b11771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5f2948f7c65a in ptimer_init /home/pm215/qemu/build/san/../../hw/core/ptimer.c:464:9
    #3 0x5f2949097c1f in slavio_timer_init /home/pm215/qemu/build/san/../../hw/timer/slavio_timer.c:407:32
    #4 0x5f29495d790f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #5 0x5f29495d96ef in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
    #6 0x5f2949a30a26 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11

Avoid the TimerContext leaks by making them an array inside the
SLAVIO_TimerState struct instead of allocating a compile-time-fixed
number of them each individually with g_new0() and then throwing away
the pointer.

Avoid the ptimer() leak by calling ptimer_free in
instance_finalize().

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Mark Cave-Ayland <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The current implementation of qdev_get_printable_name() sometimes
returns a string that must not be freed (vdev->id or the fixed
fallback string "<unknown device>" and sometimes returns a string
that must be freed (the return value of qdev_get_dev_path()). This
forces callers to leak the string in the "must be freed" case.

Make the function consistent that it always returns a string that
the caller must free, and make the three callsites free it.

This fixes leaks like this that show up when running "make check"
with the address sanitizer enabled:

Direct leak of 13 byte(s) in 1 object(s) allocated from:
    #0 0x5561de21f293 in malloc (/home/pm215/qemu/build/san/qemu-system-i386+0x1a2d293) (BuildId: 6d6fad7130fd5c8dbbc03401df554f68b8034936)
    #1 0x767ad7a82ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5561deaf34f2 in pcibus_get_dev_path /home/pm215/qemu/build/san/../../hw/pci/pci.c:2792:12
    #3 0x5561df9d8830 in qdev_get_printable_name /home/pm215/qemu/build/san/../../hw/core/qdev.c:431:24
    #4 0x5561deebdca2 in virtio_init_region_cache /home/pm215/qemu/build/san/../../hw/virtio/virtio.c:298:17
    #5 0x5561df05f842 in memory_region_write_accessor /home/pm215/qemu/build/san/../../system/memory.c:491:5
    #6 0x5561df05ed1b in access_with_adjusted_size /home/pm215/qemu/build/san/../../system/memory.c:567:18
    #7 0x5561df05e3fa in memory_region_dispatch_write /home/pm215/qemu/build/san/../../system/memory.c
    #8 0x5561df0aa805 in address_space_stm_internal /home/pm215/qemu/build/san/../../system/memory_ldst.c.inc:85:13
    #9 0x5561df0bcad3 in qtest_process_command /home/pm215/qemu/build/san/../../system/qtest.c:480:13

Cc: [email protected]
Fixes: e209d4d ("virtio: improve virtqueue mapping error messages")
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The sifive_e_aon watchdog creates a timer with timer_new_ns() in its
instance_init method, but does not free it in instance_finalize.
This means that QMP introspection of the device leaks it:

Direct leak of 48 byte in 1 object allocated from:
    #0  in calloc
    #1  in g_malloc0
    #2  in timer_new_full /home/pm215/qemu/include/qemu/timer.h:520:21
    #3  in timer_new /home/pm215/qemu/include/qemu/timer.h:543:12
    #4  in timer_new_ns /home/pm215/qemu/include/qemu/timer.h:563:12
    #5  in sifive_e_aon_init /home/pm215/qemu/build/san/../../hw/misc/sifive_e_aon.c:286:21
    #6  in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #7  in object_initialize /home/pm215/qemu/build/san/../../qom/object.c:578:5
    #8  in object_initialize_child_with_propsv /home/pm215/qemu/build/san/../../qom/object.c:608:5
    #9  in object_initialize_child_with_props /home/pm215/qemu/build/san/../../qom/object.c:591:10
    #10  in object_initialize_child_internal /home/pm215/qemu/build/san/../../qom/object.c:645:5
    #11  in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #12  in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
    #13  in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11

Allocating a separate QEMUTimer with timer_new() is not the preferred
interface (per the comments in include/qemu/timer.h); switch to an
inline struct initialized with timer_init(), which we can clean up
with timer_del() in finalize.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The TYPE_RISCV_CPC device allocates an array in its instance_init,
but does not free this, leading to leaks like this from QOM/QMP
introspection:

Direct leak of 512 byte in 1 object allocated from:
    #0  in calloc
    #1  in g_malloc0
    #2  in riscv_cpc_init /home/pm215/qemu/build/san/../../hw/misc/riscv_cpc.c:175:15
    #3  in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #4  in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
    #5  in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
    #6  in qdev_device_help /home/pm215/qemu/build/san/../../system/qdev-monitor.c:313:17
    #7  in hmp_device_add /home/pm215/qemu/build/san/../../system/qdev-monitor.c:1005:9

Free the array in instance_finalize.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
Recent fixes to TLS tasks memory handling have left the TLS bye task
uncovered. Fix by freeing the task in the same way the handshake task
is freed.

Direct leak of 704 byte(s) in 4 object(s) allocated from:
    #1 0x7f5909b1d6a0 in g_malloc0 ../glib/gmem.c:163
    #2 0x557650496d61 in qio_task_new ../io/task.c:58:12
    #3 0x557650475d7f in qio_channel_tls_bye ../io/channel-tls.c:352:12
    #4 0x55764f7a1bb4 in migration_tls_channel_end ../migration/tls.c:159:5
    #5 0x55764f709750 in migration_ioc_shutdown_gracefully ../migration/multifd.c:462:9
    #6 0x55764f6fcf53 in multifd_send_terminate_threads ../migration/multifd.c:493:13
    #7 0x55764f6fcafb in multifd_send_shutdown ../migration/multifd.c:580:5
    #8 0x55764f6e1b14 in migration_cleanup ../migration/migration.c:1323:9
    #9 0x55764f6f5bac in migration_cleanup_bh ../migration/migration.c:1350:5

Fixes: d39d0f3 ("io: fix cleanup for TLS I/O source data on cancellation")
Reviewed-by: Daniel P. Berrangé <[email protected]>
Acked-by: Daniel P. Berrangé <[email protected]>
Link: https://lore.kernel.org/qemu-devel/[email protected]
Signed-off-by: Fabiano Rosas <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
Some tests can cause QEMU to exit(1) too early while the incoming
coroutine has not yielded for a first time yet. This trips ASAN
because resources related to dispatching the incoming process will
still be allocated in the io/channel.c layer without a
straight-forward way for the migration code to clean them up.

As an example of one such issue, the UUID validation happens early
enough that the temporary socket from qio_net_listener_channel_func()
still has an elevated refcount. If it fails, the listener dispatch
code never gets to free the resource:

Direct leak of 400 byte(s) in 1 object(s) allocated from:
    #0 0x55e668890a07 in malloc asan_malloc_linux.cpp:68:3
    #1 0x7f3c7e2b6648 in g_malloc ../glib/gmem.c:130
    #2 0x55e66a8ef05f in object_new_with_type ../qom/object.c:767:15
    #3 0x55e66a8ef178 in object_new ../qom/object.c:789:12
    #4 0x55e66a93bcc6 in qio_channel_socket_new ../io/channel-socket.c:70:31
    #5 0x55e66a93f34f in qio_channel_socket_accept ../io/channel-socket.c:401:12
    #6 0x55e66a96752a in qio_net_listener_channel_func ../io/net-listener.c:64:12
    #7 0x55e66a94bdac in qio_channel_fd_source_dispatch ../io/channel-watch.c:84:12
    #8 0x7f3c7e2adf4b in g_main_dispatch ../glib/gmain.c:3476
    #9 0x7f3c7e2adf4b in g_main_context_dispatch_unlocked ../glib/gmain.c:4284
    #10 0x7f3c7e2b00c8 in g_main_context_dispatch ../glib/gmain.c:4272

The exit(1) also requires some tests to setup qtest to expect a return
code of 1 from the QEMU process. Although we can check migration
status changes to be fairly certain where the failure happened, there
is always the possibility of QEMU exiting for another reason and the
test passing. This happens frequently with sanitizers enabled, but
also risks masking issues in the regular build.

Stop allowing the incoming migration to exit and instead require the
tests to wait for the FAILED state and end QEMU gracefully with
qtest_quit.

In practice this means setting exit-on-error=false for every incoming
migration, changing MIG_TEST_FAIL_DEST_QUIT_ERR to MIG_TEST_FAIL and
waiting for a change of state where necessary.

With this, the MIG_TEST_FAIL_DEST_QUIT_ERR error result is now unused,
remove it.

The affected tests are:
validate_uuid_error
multifd_tcp_cancel
dirty_limit
precopy_unix_tls_x509_default_host
precopy_tcp_tls_no_hostname
tcp_tls_x509_mismatch_host
dbus_vmstate_missing_src
dbus_vmstate_missing_dst

Also add a comment to QEMU source explaining that the incoming
coroutine might block for a while until it yields as this is the
actual root cause of the issue.

Reviewed-by: Peter Xu <[email protected]>
Reviewed-by: Prasad Pandit <[email protected]>
Link: https://lore.kernel.org/qemu-devel/[email protected]
[assert that key doesn't already exists]
Signed-off-by: Fabiano Rosas <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
…lize

The riscv-iommu device makes various allocations in its
instance_init method. These will leak when QMP inits an
object of this type to introspect it, as can be seen if you
run 'make check' with the address sanitizer enabled:

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x5d8415b6ed9d in calloc (/home/pm215/qemu/build/san/qemu-system-riscv32+0x1832d9d) (BuildId: fedcc313e48ba803d63837329c37fd609dd50849)
    #1 0x75c0502f1771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x5d8416d09391 in riscv_iommu_instance_init /home/pm215/qemu/build/san/../../hw/riscv/riscv-iommu.c:2463:18
    #3 0x5d841710483f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #4 0x5d8417104ee9 in object_initialize /home/pm215/qemu/build/san/../../qom/object.c:578:5
    #5 0x5d8417104ee9 in object_initialize_child_with_propsv /home/pm215/qemu/build/san/../../qom/object.c:608:5
    #6 0x5d8417104db1 in object_initialize_child_with_props /home/pm215/qemu/build/san/../../qom/object.c:591:10
    #7 0x5d8417106506 in object_initialize_child_internal /home/pm215/qemu/build/san/../../qom/object.c:645:5
    #8 0x5d8416d16a12 in riscv_iommu_sys_init /home/pm215/qemu/build/san/../../hw/riscv/riscv-iommu-sys.c:199:5
    #9 0x5d841710483f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
    #10 0x5d841710661f in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
    #11 0x5d841755d956 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11

(and other similar backtraces).

Fix these by freeing the resources we allocate in instance_init in
instance_finalize.  In some cases we were freeing these in unrealize,
and in some cases not at all.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Reviewed-by: Chao Liu <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Alistair Francis <[email protected]>
berrange pushed a commit that referenced this pull request Apr 8, 2026
The pci_piix_realize() function's use of qemu_allocate_irqs()
results in a memory leak:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
    #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
    #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
    #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
    #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
    #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
    #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
    #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
    #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
    #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
    #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5

(The i386 PC sets the has-pic property to 'false', so this only
affects the MIPS Malta board.)

Fix this by embedding the i8259 irq in the device state instead of
allocating it.  This is a similar fix to the one we used for vt82c686
in commit 2225dc5, except that we use qemu_init_irq_child()
instead of qemu_init_irq().  The behaviour is identical except that
the _child() version avoids what would be a leak if we ever
unrealized the device.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: BALATON Zoltan <[email protected]>
Reviewed-by: Bernhard Beschow <[email protected]>
Message-id: [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.