From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53743) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGv3a-0006Nc-7s for qemu-devel@nongnu.org; Thu, 29 Jan 2015 14:47:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGv3V-0003OR-9A for qemu-devel@nongnu.org; Thu, 29 Jan 2015 14:47:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41143) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGv3V-0003OI-1y for qemu-devel@nongnu.org; Thu, 29 Jan 2015 14:47:09 -0500 Message-ID: <54CA8E37.8070009@redhat.com> Date: Thu, 29 Jan 2015 20:47:03 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <54CA6CF6.7090308@redhat.com> <54CA7BF5.8020800@redhat.com> <54CA8637.2080306@redhat.com> In-Reply-To: <54CA8637.2080306@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] address order of virtio-mmio devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu devel list 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 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