qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] virtio-mmio: format transport base address in B


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] virtio-mmio: format transport base address in BusClass.get_dev_path
Date: Mon, 11 Jul 2016 18:40:14 +0100

On 7 July 2016 at 17:15, Andrew Jones <address@hidden> wrote:
> On Tue, Jul 05, 2016 at 07:23:14PM +0200, Laszlo Ersek wrote:
>> At the moment the following QEMU command line triggers an assertion
>> failure (minimal reproducer by Cole):
>>
>>   qemu-system-aarch64 \
>>     -machine virt-2.6,accel=tcg \
>>     -nodefaults \
>>     -no-user-config \
>>     -nographic -monitor stdio \
>>     -device virtio-scsi-device,id=scsi0 \
>>     -device virtio-scsi-device,id=scsi1 \
>>     -drive file=foo.img,format=raw,if=none,id=d0 \
>>     -device scsi-hd,bus=scsi0.0,drive=d0 \
>>     -drive file=foo.img,format=raw,if=none,id=d1 \
>>     -device scsi-hd,bus=scsi1.0,drive=d1
>>
>>   qemu-system-aarch64: migration/savevm.c:615:
>>   vmstate_register_with_alias_id:
>>   Assertion `!se->compat || se->instance_id == 0' failed.
>>
>> The reason is that the vmstate sections for the two scsi-hd devices are
>> not uniquely identifiable by name.
>>
>> The direct parent buses of the scsi-hd devices -- scsi0.0 and scsi1.0 --
>> support the BusClass.get_dev_path member function. scsibus_get_dev_path()
>> formats a device path prefix with the help of its topologically parent
>> bus, and then appends the chan:id:lun triplet to it. For both scsi-hd
>> devices, this triplet is 0:0:0.
>>
>> (Here we use "device path" in the QEMU migration sense, for vmstate
>> section identification, not in the OFW or UEFI device path senses.)
>>
>> The virtio-scsi HBA is plugged into the virtio-mmio bus (implemented by
>> the internal VirtIOMMIOProxy device). This bus class
>> (TYPE_VIRTIO_MMIO_BUS) inherits, as its get_dev_path() member function,
>> the virtio_bus_get_dev_path() method from its parent class
>> (TYPE_VIRTIO_BUS).
>>
>> virtio_bus_get_dev_path() does not format any kind of device address on
>> its own; "virtio addresses" are transport-specific. Therefore
>> virtio_bus_get_dev_path() asks the topologically parent bus of the proxy
>> object (implementing the specific virtio transport) to format the address
>> of the proxy object.
>>
>> (For virtio-pci devices (where the proxy is an instance of VirtIOPCIProxy,
>> plugged into a PCI bus), this ends up in pcibus_get_dev_path().)
>>
>> However, VirtIOMMIOProxy is usually (in practice: always) plugged into
>> "main-system-bus", the singleton TYPE_SYSTEM_BUS object. This BusClass
>> does not support formatting QEMU vmstate device paths at all (as
>> SysBusDevice objects can have zero or more IO ports and zero or more MMIO
>> regions). Hence the formatting request delegated from
>> virtio_bus_get_dev_path() gets answered with NULL.
>>
>> The end result is that the two scsi-hd devices end up with the same device
>> path "0:0:0", which triggers the assert.
>>
>> We can solve this by recognizing that virtio-mmio transports are
>> distinguished from each other by their base addresses in MMIO address
>> space. Implement virtio_mmio_bus_get_dev_path() as follows:
>>
>> (1) The virtio device whose devpath is to be formatted resides on a
>>     virtio-mmio bus that is implemented by a VirtIOMMIOProxy object. Ask
>>     the parent bus of VirtIOMMIOProxy to format the device path of
>>     VirtIOMMIOProxy, as a path prefix. (This is identical to what
>>     virtio_bus_get_dev_path() does.)
>>
>> (2) Append the base address of VirtIOMMIOProxy to the device path, such
>>     as:
>>     - address@hidden,
>>     - address@hidden
>>
>> Given that these device paths are placed in the migration stream, step (2)
>> above, if done unconditionally, would break migration. So make that step
>> conditional on a new VirtIOMMIOProxy property, which is enabled for 2.7
>> machine types and later.
>>
>> Cc: "Michael S. Tsirkin" <address@hidden>
>> Cc: Cole Robinson <address@hidden>
>> Cc: Dr. David Alan Gilbert <address@hidden>
>> Cc: Kevin Zhao <address@hidden>
>> Cc: Peter Maydell <address@hidden>
>> Cc: Tom Hanson <address@hidden>
>> Reported-by: Kevin Zhao <address@hidden>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1594239
>> Signed-off-by: Laszlo Ersek <address@hidden>

> FWIW, this patch looks good to me.
>
> Reviewed-by: Andrew Jones <address@hidden>

Thanks; applied to target-arm.next.

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]