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

[Peter Xu <peterx@redhat.com>]

On Mon, Jul 13, 2026 at 10:23:07PM +0200, Maciej S. Szmigiero wrote:
> On 6.07.2026 18:23, Peter Xu wrote:
> > On Thu, Jul 02, 2026 at 08:36:32PM +0200, Maciej S. Szmigiero wrote:
> > > I looked more deeply into this and even prototyped a similar new design.
> > 
> > Sorry for a late response; off work most of last week.
> 
> Also sorry for my late response - was extremely busy the entire previous week.
> 
> > > 
> > > However, in the end I think that running VFIO state transitions in
> > > parallel with other non-VFIO cb() handlers just isn't safe.
> > > 
> > > For example, on the migration source, the second transition is
> > > PRE_COPY_P2P -> STOP_COPY.
> > > PRE_COPY_P2P is the last state during migration that still allows the
> > > device to accept DMA and similar interactions, while STOP_COPY is
> > > already the "quiescent" state.
> > > 
> > > So if a VFIO has already reached STOP_COPY but a non-VFIO device
> > > callback still hasn't finished, then any DMA/MMIO/P2P transaction from
> > > that "unfinished" device to the VFIO device would now hit a VFIO device
> > > that no longer can accept such transactions.
> > > This may result in hard-to-debug memory corruption during live migration.
> > 
> > Do you have any solid example of this problem?
> > 
> > AFAIU, P2P in VFIO state's term only means P2P DMAs _between_ host VFIO
> > devices, nothing to do with emulated.
> 
> I presume that quiescent VFIO devices can't accept *any* DMA, the spec (vfio.h)
> even says that:
> > The user should take steps to restrict access to vfio device regions while
> > the device is in STOP_COPY or risk corruption of the device migration data
> > stream.
> 
> Correspondingly, I think the same goes for RESUMING state on the migration
> target since it is described there as:
> > The device is *stopped* and is loading a new internal state
> 
> While you are right that the spec only talks about P2P DMA I don't see how
> host-initiated DMA would be different here - after all, it's about changing
> internal device state in a state where the device can't accept that anymore/yet.
> 
> > I am not aware of any P2P that can be initiated from emulated devices, even
> > if it exists, it is only P2P from guest perspective not host: it will
> > become host operations finally from processor side.  I really don't think
> > it's a concern.
> 
> Yeah, it's normal DMA from the VFIO device perspective.
>  > My limited understanding of VFIO's P2P state is: when switching to P2P
> > state, the VFIO device can still accept P2P DMA from other devices, but
> > never initiating P2P DMAs, and the latter sentence is critical, it means
> > the real quiecent state of the whole system happens after the last VFIO
> > device switching to P2P state: then it means none of the VFIO devices can
> > initiate P2P DMA anymore, whole system is quiesced.
> 
> Just having the VFIO devices in the system prevented from being able to
> initiate DMA is not enough for them to reach "quiecent" or fully stable
> state as long as there are other possible sources of DMA transactions
> targeting them in the system.
> 
> Glancing at the existing vm_state_change handlers there are some
> concerning ones there:
> * vhost-net (called via virtio_vmstate_change() in base virtio-net),
> 
> In the vhost-net case the vm_state_change handler for this device
> ultimately reaches vhost_net_stop() and do_vhost_dev_stop() and these
> seem to stop this network device rings at this point.
> 
> Now, having vhost-net packet buffers in a VFIO device MMIO BAR would
> be rather atypical configuration, but nevertheless letting VFIO
> device possibly reach "quiecent" state before this handler finishes
> would theoretically be a regression.

If this is an issue, it was an issue for master branch too, right?  Because
the master branch (with prepare_cb()s) already switch to P2P_DMA state for
all VFIO devices before invoking any cb().

It is also an issue then even before P2P_DMA state introduced, because
before that all cb()s can be run at random orders.

What you said might be an issue indeed in that extremely corner case, but I
think it still needs some justification, and it's IMHO orthogonal to what
we're talking about.  If it's an issue, maybe we want to fix it first
before attempting to move further to any concurrent approaches, but is it
really an issue is to be better justified.

Applies to all below.

> 
> * virtio-blk virtio_blk_dma_restart_cb() vm_state_change handler
> submits some block requests to its (possibly dedicated) IOThread.
> 
> Again, having block device request buffers in a VFIO device MMIO BAR
> would not be a typical configuration, however letting this handler
> run before making sure that VFIO devices can accept DMA would
> theoretically make a regression too.
> 
> * same for scsi-bus scsi_dma_restart_cb() that does
> scsi_dma_restart_req() for each pending request.
> 
> 
> The above is not an exhaustive list, these are only the cases
> I was able to identify relatively easily by going through the
> code.
> And I think that with possible memory corruption issues its better
> to be safe than sorry, as they are pain to debug.
> 
> The only real non-VFIO cost of being safe about all of this is
> ~15 code lines addition to vm-change-state-handler.c, all the
> remaining complexity is already on the VFIO side of things.
> > That happens earlier
> > than reaching any of the cb()s here, because VFIO's prepare_cb() does that
> > P2P state switch.
> 
> I thought your proposed design was to launch (per-VFIO device)
> device state changing thread in prepare_cb(), make it do *both*
> VFIO device state transitions and collect it in cb()?

The idea was only about trying to leverage prepare_cb() to achieve real
"prioritized callbacks" for a device, based on patch 1 being problematic.

What prepare_cb() does can be anything.  Certainly, we can make it only do
step 1 then in cb() kick all threads to move on to step 2, and sync again.

> 
> In this case the transitions are *NOT* ordered with respect to
> other cb() handlers (besides them being finished by the time
> the VFIO device qdev tree depth cb() handlers are reached by
> virtue of the threads being collected at this point - this is
> actually necessary in order to preserve proper ordering with
> respect to some types of interrupt controllers on the source).
> 
> So on the migration source the thread could already put device
> into "quiecent" state by the time other cb() handlers could run
> and on the migration target the device could still be
> in an "unprepared" state by the time other cb() handlers are called.

Thanks,

-- 
Peter Xu