| Message ID | 1468946744-28293-1-git-send-email-marcel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <marcel@redhat.com> Sounds good, but I think we need to stick to existing defaults for old machine types. Pls add compat entries accordingly. > --- > > 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
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 <marcel@redhat.com> > > > 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
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); @@ -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;
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 <marcel@redhat.com> --- 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(-)