Re: [PATCH 4/4] vl: Prioritize realizations of devices

[David Hildenbrand]

On 24.08.21 21:52, Peter Xu wrote:
> On Tue, Aug 24, 2021 at 06:24:27PM +0200, David Hildenbrand wrote:
> > > > Not so much; here's the list of priorities and the devices using it:
> > > >
> > > >          |--------------------+---------|
> > > >          | priority           | devices |
> > > >          |--------------------+---------|
> > > >          | MIG_PRI_IOMMU      |       3 |
> > > >          | MIG_PRI_PCI_BUS    |       7 |
> > > >          | MIG_PRI_VIRTIO_MEM |       1 |
> > > >          | MIG_PRI_GICV3_ITS  |       1 |
> > > >          | MIG_PRI_GICV3      |       1 |
> > > >          |--------------------+---------|
> > >
> > > iommu is probably ok. I think virtio mem is ok too,
> > > in that it is normally created by virtio-mem-pci ...
> >
> > IIRC:
> >
> > intel-iommu has to be created on the QEMU cmdline before creating
> > virtio-mem-pci.
> >
> > -device intel-iommu,caching-mode=on,intremap=on,device-iotlb=on \
> > ...
> > -device 
> > virtio-mem-pci,disable-legacy=on,disable-modern=off,iommu_platform=on,ats=on,id=vm0,...
> >
> > Creating virtio-mem-pci will implicitly create virtio-mem. virtio-mem device
> > state has to be migrated before migrating intel-iommu state.
>
> Since we're at it.. frankly I didn't fully digest why virtio-mem needs to be
> migrated before when reading commit 0fd7616e0f1171b.  It gives me the feeling
> more like that virtio-mem has a ordering dependency with vfio-pci not
> virtio-mem, but I could be wrong.

"We have to take care of incoming migration: at the point the
 IOMMUs get restored and start creating mappings in vfio,
 RamDiscardManager implementations might not be back up and running yet:
 let's add runstate priorities to enforce the order when restoring.

s/runstate/vmstate/ :|

I recall that we can see IOMMU_NOTIFIER_MAP events when restoring an 
IOMMU device. vfio_get_xlat_addr() would be called and could reject 
mappings targeting virtio-mem memory in case the virtio-mem device has 
not restored its bitmap from the migration stream such that 
ram_discard_manager_is_populated() would be reliable. Consequently, we 
have to restore the virtio-mem device state (not the virtio-mem-pci 
device state!) before restoring an IOMMU.



> E.g., the IOMMU unit shouldn't be walking page table if without a device using
> it, then it won't fail until the device walks it in region_add() hooks when
> memory replay happens.

I recall it happened when switching to the iommu region e.g., in 
vtd_post_load()->vtd_switch_address_space(). But I forgot the exact call 
path.

--
Thanks,

David / dhildenb