From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRhy-0007ar-7s for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:09:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTRhu-0005Hg-Sv for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:09:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38827) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRhu-0005HW-LO for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:09:10 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 6716614CAB5 for ; Wed, 10 Feb 2016 10:09:10 +0000 (UTC) References: <56BA240A.6060409@redhat.com> <56BB0864.5080701@redhat.com> From: Laurent Vivier Message-ID: <56BB0C43.2070800@redhat.com> Date: Wed, 10 Feb 2016 11:09:07 +0100 MIME-Version: 1.0 In-Reply-To: <56BB0864.5080701@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , qemu-devel qemu-devel Cc: Jason Wang , Eduardo Habkost , "Michael S. Tsirkin" On 10/02/2016 10:52, Marcel Apfelbaum wrote: > On 02/09/2016 07:38 PM, Laurent Vivier wrote: >> Hi, >> >> I'm playing with a qemu-2.5.0 and pc-i440fx-2.4 machine type, and >> perhaps I don't understand correctly the compat_props machinery but >> there is something strange for me: >> >> in qemu-2.5.0, hw/virtio/virtio-pci.c: >> >> 1880 DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags, >> 1881 VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false), >> >> So I guess a new default machine (pc-i440fx-2.5) will be defined by >> default with "x-disable-pcie" set to "false". > > Correct, but we are talking about both pc and q35. > By default, virtio devices *can be* PCIe for all new machines. > >> >> and in qemu-2.5, include/hw/compat.h: >> >> 3 >> 4 #define HW_COMPAT_2_4 \ >> 5 {\ >> .. >> 14 .driver = "virtio-pci",\ >> 15 .property = "x-disable-pcie",\ >> 16 .value = "on",\ >> 17 },{\ >> >> So I guess a new machine pc-i440fx-2.4 created with qemu-2.5.0 will have >> "x-disable-pcie" set to true (as with qemu-2.4.0 this feature is not >> available). > > Exactly, machines before 2.5 will not have virtio devices PCIe > Old q35 machines are also included. > >> >> But in this case "x-disable-pcie" is always set to "false". > > For older machines yes, for newer no. > >> >> So the questions are: >> - do I understand correctly? > > Does the above respond to your question? > >> - I check "x-disable-pcie" for "virtio-*-pci" devices (virtio-net-pci, >> virtio-balloon-pci, ..."), should they inherit the behavior from their >> parent "virtio-pci"? (they should, as virtio-pci is an abstract device >> type) > > Yes, they do inherit this property. > >> >> My test: >> >> - take qemu-2.5.0 (a8c40fa Update version for v2.5.0 release) >> - start it with "-M pc-i440fx-2.5 -device virtio-balloon-pci -S", >> "info qtree": >> >> bus: pci.0 >> type PCI >> dev: virtio-balloon-pci, id "" >> ... >> x-disable-pcie = false > > This is correct again. > >> >> - start it with "-M pc-i440fx-2.4 -device virtio-balloon-pci -S", >> "info qtree": >> >> bus: pci.0 >> type PCI >> dev: virtio-balloon-pci, id "" >> ... >> x-disable-pcie = false > > This is a bug! Thanks for finding it. > It took a little but I found the root cause. > > We have 2 virtio features mapped into the same bit: > - #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4 > - #define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 4 > > Since both properties appear in HW_COMPAT_2_4, but > migrate-extra is the last one used, it wins the race. > > > To the obvious question of "how did that happen?" I can say we had an > unlucky break. > Both Jason and me worked on a new different virtio feature in the same > time, > and they were both merged in the same pull request. > We both saw BIT 3 as the last used :) > > https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03041.html > > I'll send a patch to map virtio pcie feature to a different bit. > Thanks, Thank you Marcel! You have answered to all my questions and find a fix. This is perfect :) Laurent