From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55067) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3iBJ-00019x-Tp for qemu-devel@nongnu.org; Tue, 01 Dec 2015 05:29:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3iBE-0005Q7-Tr for qemu-devel@nongnu.org; Tue, 01 Dec 2015 05:29:09 -0500 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]:35813) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3iBE-0005Ph-Kq for qemu-devel@nongnu.org; Tue, 01 Dec 2015 05:29:04 -0500 Received: by wmuu63 with SMTP id u63so166439637wmu.0 for ; Tue, 01 Dec 2015 02:29:03 -0800 (PST) Date: Tue, 1 Dec 2015 12:29:00 +0200 From: Shmulik Ladkani Message-ID: <20151201122900.530ef562@pixies> In-Reply-To: <1447155689-26230-1-git-send-email-marcel@redhat.com> References: <1447155689-26230-1-git-send-email-marcel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V5] hw/virtio: Add PCIe capability to virtio devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , mst@redhat.com Cc: qemu-devel@nongnu.org, ehabkost@redhat.com, kraxel@redhat.com Hi, On Tue, 10 Nov 2015 13:41:29 +0200, marcel@redhat.com wrote: > The virtio devices are converted to PCI-Express > if they are plugged into a PCI-Express bus and > the 'modern' protocol is enabled. > > @@ -1592,6 +1592,26 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) > > + if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) > + && !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN) > + && pci_bus_is_express(pci_dev->bus) > + && !pci_bus_is_root(pci_dev->bus)) { > + int pos; > + > + pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; Setting QEMU_PCI_CAP_EXPRESS here in 'virtio_pci_realize' is too late. This is since 'pci_qdev_realize' (DeviceClass.realize of TYPE_PCI_DEVICE) is invoked prior the PCIDeviceClass's specific 'realize' method (virtio_pci_realize in this case). During 'pci_qdev_realize' (specifically, within do_pci_register_device), the QEMU_PCI_CAP_EXPRESS gets tested indirectly, when pci_is_express and pci_config_size helpers are called. For example: 'pci_config_alloc' uses 'pci_config_size' which relies on QEMU_PCI_CAP_EXPRESS property. Since virtio_pci sets QEMU_PCI_CAP_EXPRESS *after* pci_qdev_realize has finished, we end up having an insufficient pci config space allocated for the virtio "pcie" device. May I suggest the following: - Expose 'pci_qdev_realize' - Have 'virtio_pci_class_init' arm it's own dc->realize, which will first set 'QEMU_PCI_CAP_EXPRESS' flag as needed, and then call 'pci_qdev_realize' - Now, in 'virtio_pci_realize' we may use 'pci_is_express' instead of directly checking the proxy->flags If this sounds ok, I'll submit a fix. Regards, Shmulik