On 20.07.2026 16:55, Peter Xu wrote:
On Fri, Jul 17, 2026 at 04:31:32PM +0200, Maciej S. Szmigiero wrote:
Inside the VFIO code such sync point for VFIO devices could/should indeed
be created, but then it's not available for these other (non-VFIO) devices
for the purpose of avoiding the regression from the previous paragraph.
However, replacing the order priority/adjustment mechanism from patch 1
with the "pre" and "post" handlers I described above would avoid having
that regression (and help fix the vhost-net case too).
I'm not sure I fully get the pre/post handlers idea, I think it sounds
working, but in all cases I want to decouple it with patch 1: I don't think
it requires patch 1, am I right?
It wouldn't require the *current* patch 1 but it would need to be implemented
in some preparatory patch so it would effectively be a new patch 1 (or a set
of patches replacing patch 1).
Assuming that we would go this route instead of the one you described below.
Now if we want to avoid this "theoretical regression" and solve both things
together.. I think we may need to refactor the notifiers mechanism.
Firstly, I hope we're on the same page that essentially prepare_cb() is the
priority mechanism here, we only have HIGH and NORMAL priority, where cb()
is the NORMAL priority.
Yeah, I think the current situation could be described this way.
We also need to persist depth concept per-notifier, I think we should start
by renaming VMChangeStateEntry.priority to depth, add a comment explaining
it (on different order of invokations on VM start/shutdown).
But then, I don't think we need anything as complex as pre/post hooks with
hashes. I think you're right then we need SYNC point which can be
essentially a priority notifier that is in the middle of HIGH and NORMAL.
Hence, I want to see if below should be the easiest:
- Rename VMChangeStateEntry.priority to depth
- Normalize prepare_cb() into VM_CHANGE_NOTIFY_PRI_HIGH, making cb() to
be NORMAL, OTOH. With this, prepare_cb() needs to be registered
separately with qemu_add_vm_change_state_handler_prio_full(). The
function now should drop prepare_cb() but instead take a real
"priority" value of VM_CHANGE_NOTIFY_PRI_*.
And where the qdev tree depth will go in a call to
qemu_add_vm_change_state_handler_prio_full() - as an additional "depth"
parameter?
- Introduce VM_CHANGE_NOTIFY_PRI_SYNC, in the middle of PREPARE / NORMAL.
- VFIO can now register its 3rd notifier against SYNC.
All rest devices will need to shift part of its logic into PRI_HIGH to
either quiesce DMA or enable the backend to accept DMA (when on dest QEMU).
None of them will need SYNC only if they'll also switch to an async thread
model.
In principle, this *should* work but with some caveats:
* vhost-net would need the DMA-stopping part of its cb() handler separated
and moved into prepare_cb(),
* There's some restore code in "PowerPC sPAPR XIVE interrupt controller"
running from cb() at prio 0, so originally always completing before VFIO device
was switched into RUNNING state.
I'm not 100% sure if it's okay to make VFIO device reach this state before
that interrupt controller restore code finishes,
* The postponed dirty memory logging stop handler also runs from cb() at
prio 0, so currently always finishes before VFIO device was switched into
RUNNING state.
This handler also stops DMA logging for VFIO device or its container,
but here I presume it should be safe to do so while the VFIO device has already
started dirtying memory even though this couldn't happen in the existing code?
* There's some sync/setup of x86 vAPIC state (for 32-bit guests?) also running
from cb() at prio 0.
Not sure if this needs to finish before VFIO device gets switched into
RUNNING state.
These 3 possible issues above (besides vhost-net move/refactor) could be
theoretically worked around by moving these callbacks from cb() to a new
priority *before* VM_CHANGE_NOTIFY_PRI_SYNC (maybe called
VM_CHANGE_NOTIFY_PRI_SYNC_BEFORE or similar) as I presume they won't need the
VFIO device to reach DMA-accepting state but at the same time they shouldn't
delay the launch of VFIO device state changing threads from
VM_CHANGE_NOTIFY_PRI_HIGH.
Also, the final VFIO thread joining/collection does *not* need to be ordered
with exiting cb() handlers, so it should really run at VM_CHANGE_NOTIFY_PRI_LOW
(below PRI_NORMAL) for best parallelism.
The disadvantages of this overall design are that it would need coordination
between different sub-maintainers, increase the "blast radius" of the patch
set to different parts of QEMU code, and so obviously would be more
regression-risky than the aforementioned "pre" and "post" handlers design.
Especially that probably few people would be able to test, for example,
the PPC XIVE case with VFIO.
Thanks,
Thanks,
Maciej
