Re: [PATCH 2/2] vfio/migration: Parallelize device state transitions

[Peter Xu <peterx@redhat.com>]

On Tue, Jul 21, 2026 at 05:07:53PM +0200, Maciej S. Szmigiero wrote:
> 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?

Yes.  The rename change I mentioned above should have done so.

> 
> > 
> >    - 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(),

I thought this is required with whatever priority impl we will have, even
if I don't think we need to fix it right now unless you wanted to..

> 
> * 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,

Is anyone using VFIO migration on PPC that you're aware of?  If not, maybe
we don't need to worry too much for now.  Meanwhile, this looks like an
existing issue as well.

For the fix whenever wanted, IIUC the intr controller should register both
HIGH and NORMAL priority notifiers, then when VM stop, disable the
controller only in NORMAL notifier, and when VM starts, enable the
controller only in HIGH notifier (we could think about better names for the
three steps, maybe it shouldn't be called as "priority" as well even if the
executions will be strictly ordered).

> 
> * 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?

It's only stopping the tracking, results of the tracking shouldn't matter
anymore on whether some DMA was trapped.  It looks safe no matter where we
put it.

> 
> * 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.

Doesn't look relevant.  IIUC, only anything that is DMA capable can be
relevant.

> 
> 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.

IIUC we don't need a 4th step here, correct me otherwise, while the 3rd
SYNC was only needed for VFIO's threaded approach.

> 
> 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.

Yes. VFIO device only need its SYNC / MEDIUM priority notifier be ordered
with NORMAL notifiers, to make sure all VFIO devices switched to P2P state
before the NORMAL notifiers.

> 
> 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.

We shouldn't justify a solution by "how many subsystems it touches".

I was suggesting to solve one problem at a time, but then prepare_cb()
already does it.  You pointed out there might be real regression on VM
start path, I agreed.

I thought you wanted to fix all things, which I'm ok with and agree it is
better, then it's unavoidable to touch all modules.  But yes, we need to
reproduce the problems and verify the fix.

The middle ground is we define the notifiers to suite for the solution we
think will fix everything, but you only need to fix VFIO and leave the rest
for later. We can discuss the problems in the cover letter or commit logs
for future works on top.  After all, in this case of VFIO to do the right
things it does need three steps / notifiers.

Thanks,

-- 
Peter Xu