From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRS0-000380-FC for qemu-devel@nongnu.org; Wed, 10 Feb 2016 04:52:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTRRw-0001HS-Ck for qemu-devel@nongnu.org; Wed, 10 Feb 2016 04:52:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47451) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRRw-0001HJ-54 for qemu-devel@nongnu.org; Wed, 10 Feb 2016 04:52:40 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 1E275A789 for ; Wed, 10 Feb 2016 09:52:39 +0000 (UTC) References: <56BA240A.6060409@redhat.com> From: Marcel Apfelbaum Message-ID: <56BB0864.5080701@redhat.com> Date: Wed, 10 Feb 2016 11:52:36 +0200 MIME-Version: 1.0 In-Reply-To: <56BA240A.6060409@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed 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: Laurent Vivier , qemu-devel qemu-devel Cc: Jason Wang , Eduardo Habkost , "Michael S. Tsirkin" 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, Marcel > > Any explanation is very welcome. > > Thanks, > Laurent >