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

[David Hildenbrand <david@redhat.com>]

On 24.08.21 00:05, Michael S. Tsirkin wrote:
> On Mon, Aug 23, 2021 at 03:18:51PM -0400, Peter Xu wrote:
>> On Mon, Aug 23, 2021 at 02:49:12PM -0400, Eduardo Habkost wrote:
>>> On Wed, Aug 18, 2021 at 03:43:18PM -0400, Peter Xu wrote:
>>>> QEMU creates -device objects in order as specified by the user's cmdline.
>>>> However that ordering may not be the ideal order.  For example, some platform
>>>> devices (vIOMMUs) may want to be created earlier than most of the rest
>>>> devices (e.g., vfio-pci, virtio).
>>>>
>>>> This patch orders the QemuOptsList of '-device's so they'll be sorted first
>>>> before kicking off the device realizations.  This will allow the device
>>>> realization code to be able to use APIs like pci_device_iommu_address_space()
>>>> correctly, because those functions rely on the platfrom devices being realized.
>>>>
>>>> Now we rely on vmsd->priority which is defined as MigrationPriority to provide
>>>> the ordering, as either VM init and migration completes will need such an
>>>> ordering.  In the future we can move that priority information out of vmsd.
>>>>
>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>
>>> Can we be 100% sure that changing the ordering of every single
>>> device being created won't affect guest ABI?  (I don't think we can)
>>
>> That's a good question, however I doubt whether there's any real-world guest
>> ABI for that.  As a developer, I normally specify cmdline parameter in an adhoc
>> way, so that I assume most parameters are not sensitive to ordering and I can
>> tune the ordering as wish.  I'm not sure whether that's common for qemu users,
>> I would expect so, but I may have missed something that I'm not aware of.
>>
>> Per my knowledge the only "guest ABI" change is e.g. when we specify "vfio-pci"
>> to be before "intel-iommu": it'll be constantly broken before this patchset,
>> while after this series it'll be working.  It's just that I don't think those
>> "guest ABI" is necessary to be kept, and that's exactly what I want to fix with
>> the patchset..
>>
>>>
>>> How many device types in QEMU have non-default vmsd priority?
>>
>> 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.

I do wonder if migration priorities are really what we want to reuse 
here. I guess it works right, but just by pure luck (because we ignore 
the implicit dependency regarding priorities)

-- 
Thanks,

David / dhildenb