From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38430) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtBoE-00035E-At for qemu-devel@nongnu.org; Mon, 02 Nov 2015 04:53:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtBoB-0003vy-27 for qemu-devel@nongnu.org; Mon, 02 Nov 2015 04:53:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56782) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtBoA-0003vc-Pz for qemu-devel@nongnu.org; Mon, 02 Nov 2015 04:53:46 -0500 References: <1446119788-19722-1-git-send-email-marcel@redhat.com> <20151102104253.50e5dbe6@bahia.local> From: Marcel Apfelbaum Message-ID: <563732A6.8050704@redhat.com> Date: Mon, 2 Nov 2015 11:53:42 +0200 MIME-Version: 1.0 In-Reply-To: <20151102104253.50e5dbe6@bahia.local> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V3] hw/virtio: Add PCIe capability to virtio devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: mst@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com, kraxel@redhat.com On 11/02/2015 11:42 AM, Greg Kurz wrote: > On Thu, 29 Oct 2015 13:56:28 +0200 > Marcel Apfelbaum wrote: > >> The virtio devices are converted to PCI-Express >> if they are plugged into a PCI-Express bus and >> the 'modern' protocol is enabled. >> >> Devices plugged directly into the Root Complex as >> Integrated Endpoints remain PCI. >> >> Signed-off-by: Marcel Apfelbaum >> --- >> v2 -> v3: >> - Addressed Michael S. Tsirkin's comments: >> - enable pcie only for 2.5+ machines. >> >> v1 -> v2: >> - Addressed Michael S. Tsirkin's comments: >> - Added the minimum required capabilities for PCIe devices >> - Integrated Endpoints remain PCI >> >> - Use pcie_endpoint_cap_init instead of manually creating the pcie capability. >> >> - Regarding Gerd Hoffman's comments: >> - Creating virtio-pcie devices: >> For the moment I prefer to not duplicate the virtio definitions, >> at least until we don't have a consensus (Personally I don't like it) >> - Removing the IO bar: >> This would be my next patch on the "virtio to express" series, I plan >> to remove it only for "modern" devices. >> >> Thanks, >> Marcel >> >> hw/virtio/virtio-pci.c | 22 ++++++++++++++++++++++ >> hw/virtio/virtio-pci.h | 2 ++ >> include/hw/compat.h | 46 +++++++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 69 insertions(+), 1 deletion(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index f55dd2b..a288d8b 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1592,6 +1592,26 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) >> >> address_space_init(&proxy->modern_as, &proxy->modern_cfg, "virtio-pci-cfg-as"); >> >> + 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; >> + pos = pcie_endpoint_cap_init(pci_dev, 0); >> + assert(pos > 0); >> + >> + pos = pci_add_capability(pci_dev, PCI_CAP_ID_PM, 0, PCI_PM_SIZEOF); >> + assert(pos > 0); >> + >> + /* >> + * Indicates that this function complies with revision 1.2 of the >> + * PCI Power Management Interface Specification. >> + */ >> + pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); >> + } >> + >> virtio_pci_bus_new(&proxy->bus, sizeof(proxy->bus), proxy); >> if (k->realize) { >> k->realize(proxy, errp); >> @@ -1622,6 +1642,8 @@ static Property virtio_pci_properties[] = { >> VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, false), >> DEFINE_PROP_BIT("disable-modern", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true), >> + DEFINE_PROP_BIT("disable-pcie", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index 801c23a..1a487fc 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -72,8 +72,10 @@ typedef struct VirtioBusClass VirtioPCIBusClass; >> /* virtio version flags */ >> #define VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT 2 >> #define VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT 3 >> +#define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 4 >> #define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT) >> #define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT) >> +#define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT) >> >> typedef struct { >> MSIMessage msg; >> diff --git a/include/hw/compat.h b/include/hw/compat.h >> index 095de5d..0a08531 100644 >> --- a/include/hw/compat.h >> +++ b/include/hw/compat.h >> @@ -2,7 +2,51 @@ >> #define HW_COMPAT_H >> >> #define HW_COMPAT_2_4 \ >> - /* empty */ >> + {\ >> + .driver = "virtio-blk-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-scsi-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-net-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-input-host-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-keyboard-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-mouse-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-serial-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-tablet-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-gpu-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-balloon-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-rng-pci",\ >> + .property = "disable-pcie",\ >> + .value = "on",\ >> + }, >> > > What about "virtio-9p-pci" ? Thanks for pointing it out: 1. It was missed because: -device help 2>&1 | grep virtio | grep pci didn't return it. This is because my config didn't include VIRTFS. Next time I'll grep the code. 2. I sent V4 of this patch that applies the property to the "virtio-pci" base class rather than to individual devices. Thanks, Marcel > >> #define HW_COMPAT_2_3 \ >> {\ >