From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPZzd-0006me-IE for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:43:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPZzb-0004Ug-AR for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:43:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPZzb-0004UY-2W for qemu-devel@nongnu.org; Tue, 19 Jul 2016 14:43:43 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7B34185547 for ; Tue, 19 Jul 2016 18:43:42 +0000 (UTC) References: <1468946744-28293-1-git-send-email-marcel@redhat.com> <20160719204506-mutt-send-email-mst@redhat.com> From: Marcel Apfelbaum Message-ID: <578E74DB.8070602@redhat.com> Date: Tue, 19 Jul 2016 21:43:39 +0300 MIME-Version: 1.0 In-Reply-To: <20160719204506-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org On 07/19/2016 08:47 PM, Michael S. Tsirkin wrote: > On Tue, Jul 19, 2016 at 07:45:44PM +0300, Marcel Apfelbaum wrote: >> Modern machines are expected to be used by newer setups with >> modern guests aiming the use of the latest features. >> >> Enable modern and disable legacy for virtio devices >> plugged into PCIe ports (Root ports or Downstream ports). >> Using the Virtio 1 mode will remove the limitation >> of the number of devices that can be attached to a machine >> by removing the need for the IO BAR. >> >> Convert 'disable-modern' and 'disable-legacy' properties to OnOffAuto >> with default Auto. >> >> Signed-off-by: Marcel Apfelbaum > > > Sounds good, but I think we need to stick to existing > defaults for old machine types. > Pls add compat entries accordingly. > Added and posted v2. Thanks for the quick review! Marcel >> --- >> >> Hi, >> >> If everyone agrees, I am thinking about getting it into 2.7 >> to avoid the ~15 virtio devices limitation per machine. >> >> Notes: >> - The non PCIe machines behaviour should remain the same. >> - I hope is OK to make the disable-* properties OnOffAuto. Previous setups >> using them can be affected, but libvirt is not using them yet (as far as I know) >> - My tests were limited to checking all possible disable-* configurations (and make check for all archs) >> >> Thanks, >> Marcel >> >> hw/virtio/virtio-pci.c | 31 ++++++++++++++++++++++++------- >> hw/virtio/virtio-pci.h | 2 ++ >> 2 files changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 2b34b43..ec9e84f 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1716,6 +1716,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) >> { >> VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); >> VirtioPCIClass *k = VIRTIO_PCI_GET_CLASS(pci_dev); >> + bool pcie_port = (pci_bus_is_express(pci_dev->bus) && >> + !pci_bus_is_root(pci_dev->bus)); >> >> /* >> * virtio pci bar layout used by default. >> @@ -1766,8 +1768,23 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) >> >> address_space_init(&proxy->modern_as, &proxy->modern_cfg, "virtio-pci-cfg-as"); >> >> - if (pci_is_express(pci_dev) && pci_bus_is_express(pci_dev->bus) && >> - !pci_bus_is_root(pci_dev->bus)) { >> + if ((pcie_port && (proxy->disable_modern == ON_OFF_AUTO_AUTO)) >> + || (proxy->disable_modern == ON_OFF_AUTO_OFF)) { >> + proxy->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; >> + pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; >> + } else { >> + proxy->flags |= VIRTIO_PCI_FLAG_DISABLE_MODERN; >> + pci_dev->cap_present &= ~QEMU_PCI_CAP_EXPRESS; >> + } >> + >> + if ((pcie_port && (proxy->disable_legacy == ON_OFF_AUTO_AUTO)) >> + || (proxy->disable_legacy == ON_OFF_AUTO_ON)) { >> + proxy->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; >> + } else { >> + proxy->flags &= ~VIRTIO_PCI_FLAG_DISABLE_LEGACY; >> + } >> + >> + if (pcie_port && pci_is_express(pci_dev)) { >> int pos; >> >> pos = pcie_endpoint_cap_init(pci_dev, 0); > > down the road, I really think we need a new type of > property that does not need this manual code. > E.g. specify a location to store the bit. > > This can wait though. > > >> @@ -1821,10 +1838,10 @@ static void virtio_pci_reset(DeviceState *qdev) >> static Property virtio_pci_properties[] = { >> DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false), >> - DEFINE_PROP_BIT("disable-legacy", VirtIOPCIProxy, flags, >> - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, false), >> - DEFINE_PROP_BIT("disable-modern", VirtIOPCIProxy, flags, >> - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true), >> + DEFINE_PROP_ON_OFF_AUTO("disable-legacy", VirtIOPCIProxy, disable_legacy, >> + ON_OFF_AUTO_AUTO), >> + DEFINE_PROP_ON_OFF_AUTO("disable-modern", VirtIOPCIProxy, disable_modern, >> + ON_OFF_AUTO_AUTO), >> DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), >> DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, >> @@ -1841,7 +1858,7 @@ static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp) >> PCIDevice *pci_dev = &proxy->pci_dev; >> >> if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) && >> - !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN)) { >> + !(proxy->disable_modern == ON_OFF_AUTO_ON)) { >> pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; >> } >> >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index e4548c2..4f219d4 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -144,6 +144,8 @@ struct VirtIOPCIProxy { >> uint32_t modern_mem_bar; >> int config_cap; >> uint32_t flags; >> + OnOffAuto disable_modern; >> + OnOffAuto disable_legacy; >> uint32_t class_code; >> uint32_t nvectors; >> uint32_t dfselect; >> -- >> 2.4.3