Re: [Qemu-devel] address order of virtio-mmio devices

[Laszlo Ersek]

On 01/29/15 20:12, Laszlo Ersek wrote:
> On 01/29/15 20:01, Peter Maydell wrote:
>> On 29 January 2015 at 18:29, Laszlo Ersek <address@hidden> wrote:
>>> Yes. The DTB is 100% fine. However, we're talking two independent
>>> mappings here. The lower level mapping is the DTB, and that's completely
>>> fine. It's handled by the second loop in create_virtio_devices(). But,
>>> that mapping in the DTB doesn't know about actual virtio devices at all,
>>> it only describes virtio-mmio transports.
>>>
>>> The bug is in the other mapping, ie. how devices specified with the
>>> -device options are mapped onto the (then already existing) virtio-mmio
>>> transports. The processing of -device is fine, it goes forward. The
>>> issue is that qbus_find_recursive(), in response to each -device option,
>>> finds the next free transport in decreasing address order, *because*
>>> qbus_realize() *prepended* each transport, not appended it, when
>>> create_virtio_devices() called sysbus_create_simple() in increasing
>>> address order at startup.
>>>
>>>> Device tree nodes appear in the tree lowest address
>>>> first, which is exactly what the code above is
>>>> claiming to do.
>>>
>>> The comment makes two statements, and two loops follow it.
>>
>> I see what you mean now.
>>
>> Does the ordering we have result in the guest kernel (usually)
>> allocating vda to the first command line device, vdb to the
>> second, and so on? I'm pretty sure I recall testing at the
>> time and that that's why we have the funny ordering.
> 
> That's exactly what we're seeing. The first virtio-blk-device on the
> qemu command line ends up getting the highest /dev/vdX in the guest, and
> the last one on the qemu cmdline becomes /dev/vda in the guest.
> Libguestfs expects the last cmdline option to become the highest /dev/vdX.
> 
>> (Looking back through the archives, when we last looked at
>> this I think it turned out that the kernel probes the mmio
>> devices in one order and then assigns them vda/vdb/etc
>> in the opposite order.)
> 
> Ugh! That would be the perfect explanation for the current qemu code.
> 
>>
>>> The first statement in the comment is wrong:
>>>
>>>      /* Note that we have to create the transports in forwards order
>>>       * so that command line devices are inserted lowest address first,
>>>
>>> The first loop matches the first statement in the comment, hence the
>>> first loop is also wrong.
>>
>> Regardless, we can't change the ordering now without breaking
>> existing command lines (I'm sure there are people out there
>> who aren't using UUIDs).
>>
>> We could fix the comment, though.
> 
> If the guest kernel changed its "assignment strategy" at some point, but
> earlier it used to match the comment (and the code), then whichever way
> we shape the comment will be wrong for the other kernel strategy. :) So,
> in that case this code is probably best left undisturbed.
> 
> I'll try to dig out some kernel commit for more evidence.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=70161ff3

Laszlo