| Message ID | 1469028501-23780-1-git-send-email-marcel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/20/2016 06:28 PM, Marcel Apfelbaum wrote: > Enable transitional virtio devices by default. > Enable virtio-1.0 for 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. > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > --- > > Hi, > > v2 -> v3: > - Various code tweaks to simplify if statements (Michael) > - Enable virtio modern by default (Gerd and Cornelia) > - Replace virtio flags with actual fields (Gerd) > - Wrappers for more readable code Hi, I forgot to mention I also verified that the code in virtio_pci_realize runs before any other code that updates the "disable*" properties. (Michael) Thanks, Marcel > > v1 -> v2: > - Stick to existing defaults for old machine types (Michael S. Tsirkin) > > If everyone agrees, I am thinking about getting it into 2.7 > to avoid the ~15 virtio devices limitation per machine. > > My tests were limited to checking all possible disable-* configurations (and make check for all archs) > > Thanks, > Marcel > > hw/display/virtio-gpu-pci.c | 4 +--- > hw/display/virtio-vga.c | 4 +--- > hw/virtio/virtio-pci.c | 34 ++++++++++++++++++---------------- > hw/virtio/virtio-pci.h | 21 +++++++++++++++++---- > include/hw/compat.h | 8 ++++++++ > 5 files changed, 45 insertions(+), 26 deletions(-) > > diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c > index a71b230..34a724c 100644 > --- a/hw/display/virtio-gpu-pci.c > +++ b/hw/display/virtio-gpu-pci.c > @@ -30,9 +30,7 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > int i; > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > > for (i = 0; i < g->conf.max_outputs; i++) { > diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c > index 315b7fc..5b510a1 100644 > --- a/hw/display/virtio-vga.c > +++ b/hw/display/virtio-vga.c > @@ -134,9 +134,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > > /* init virtio bits */ > qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(g), true, "realized", &err); > if (err) { > error_propagate(errp, err); > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 2b34b43..11cd634 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -161,7 +161,7 @@ static bool virtio_pci_modern_state_needed(void *opaque) > { > VirtIOPCIProxy *proxy = opaque; > > - return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + return virtio_pci_modern(proxy); > } > > static const VMStateDescription vmstate_virtio_pci_modern_state = { > @@ -300,8 +300,8 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier, > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > VirtQueue *vq = virtio_get_queue(vdev, n); > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool fast_mmio = kvm_ioeventfd_any_length_enabled(); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > MemoryRegion *modern_mr = &proxy->notify.mr; > @@ -1576,8 +1576,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > VirtioBusState *bus = &proxy->bus; > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > uint8_t *config; > uint32_t size; > @@ -1696,7 +1696,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > static void virtio_pci_device_unplugged(DeviceState *d) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > > virtio_pci_stop_ioeventfd(proxy); > @@ -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,11 @@ 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 (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > + proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > + } > + > + if (pcie_port && pci_is_express(pci_dev)) { > int pos; > > pos = pcie_endpoint_cap_init(pci_dev, 0); > @@ -1821,10 +1826,9 @@ 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_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), > DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), > DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, > @@ -1841,7 +1845,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)) { > + virtio_pci_modern(proxy)) { > pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; > } > > @@ -2303,9 +2307,7 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > DeviceState *vdev = DEVICE(&vinput->vdev); > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > } > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index e4548c2..25fbf8a 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass; > enum { > VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, > - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, > - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, > VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, > VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, > @@ -77,8 +75,6 @@ enum { > #define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT) > > /* virtio version flags */ > -#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) > > /* migrate extra state */ > @@ -144,6 +140,8 @@ struct VirtIOPCIProxy { > uint32_t modern_mem_bar; > int config_cap; > uint32_t flags; > + bool disable_modern; > + OnOffAuto disable_legacy; > uint32_t class_code; > uint32_t nvectors; > uint32_t dfselect; > @@ -158,6 +156,21 @@ struct VirtIOPCIProxy { > VirtioBusState bus; > }; > > +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > +{ > + return !proxy->disable_modern; > +} > + > +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) > +{ > + return proxy->disable_legacy == ON_OFF_AUTO_OFF; > +} > + > +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) > +{ > + proxy->disable_modern = false; > + proxy->disable_legacy = ON_OFF_AUTO_ON; > +} > > /* > * virtio-scsi-pci: This extends VirtioPCIProxy. > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 9914e7a..1531399 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -6,6 +6,14 @@ > .driver = "virtio-mmio",\ > .property = "format_transport_address",\ > .value = "off",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-modern",\ > + .value = "on",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-legacy",\ > + .value = "off",\ > }, > > #define HW_COMPAT_2_5 \ >
On Mi, 2016-07-20 at 18:28 +0300, Marcel Apfelbaum wrote: > Enable transitional virtio devices by default. > Enable virtio-1.0 for 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. Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
On Wed, 20 Jul 2016 18:28:21 +0300 Marcel Apfelbaum <marcel@redhat.com> wrote: > Enable transitional virtio devices by default. > Enable virtio-1.0 for devices plugged into > PCIe ports (Root ports or Downstream ports). Add "by default", as this can still be overridden? > > Using the virtio-1 mode will remove the limitation s/Using the virtio-1 mode/Disabling the legacy mode/ ? > of the number of devices that can be attached to a machine > by removing the need for the IO BAR. > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> (...) > +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > +{ > + return !proxy->disable_modern; > +} > + > +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) > +{ > + return proxy->disable_legacy == ON_OFF_AUTO_OFF; > +} > + > +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) One thing I still find a bit confusing is that you refer to 'modern' above, but force to 'virtio_1' here... but that's a minor thing. > +{ > + proxy->disable_modern = false; > + proxy->disable_legacy = ON_OFF_AUTO_ON; > +} > > /* > * virtio-scsi-pci: This extends VirtioPCIProxy. > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 9914e7a..1531399 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -6,6 +6,14 @@ > .driver = "virtio-mmio",\ > .property = "format_transport_address",\ > .value = "off",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-modern",\ > + .value = "on",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-legacy",\ > + .value = "off",\ After looking at the code, I think this will work - did you test this with a compat machine, though? > }, > > #define HW_COMPAT_2_5 \ But generally, looks good and I think this is also an improvement in readability.
On 07/21/2016 11:54 AM, Cornelia Huck wrote: > On Wed, 20 Jul 2016 18:28:21 +0300 > Marcel Apfelbaum <marcel@redhat.com> wrote: > >> Enable transitional virtio devices by default. >> Enable virtio-1.0 for devices plugged into >> PCIe ports (Root ports or Downstream ports). > Hi Cornelia, Thank you for the review. > Add "by default", as this can still be overridden? > Yes, using -device virtio*,disable-modern=x,disable-legacy=y are respected as before. >> >> Using the virtio-1 mode will remove the limitation > > s/Using the virtio-1 mode/Disabling the legacy mode/ > > ? > Well, the way I see it virtio-1 'pure' is not using the IO BAR. This is why virtio-1 == disable-modern=off && disable-legacy=on IMHO. If you or Michael see this differently I have nothing against re-wording it. >> of the number of devices that can be attached to a machine >> by removing the need for the IO BAR. >> >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > > (...) > >> +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) >> +{ >> + return !proxy->disable_modern; >> +} >> + >> +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) >> +{ >> + return proxy->disable_legacy == ON_OFF_AUTO_OFF; >> +} >> + >> +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) > > One thing I still find a bit confusing is that you refer to 'modern' > above, but force to 'virtio_1' here... but that's a minor thing. > I went for 'virtio-1' because of the existing comments (force virtio-1) and also because 'modern' does not imply "no legacy" - those are independent flags. BTW, instead of the 'disable*' properties (which I find hard to follow) I would go for one property : "mode" with "legacy"/"transitional"/"virtio-1"/"auto" values. But is too late for that (at least for 2.7). >> +{ >> + proxy->disable_modern = false; >> + proxy->disable_legacy = ON_OFF_AUTO_ON; >> +} >> >> /* >> * virtio-scsi-pci: This extends VirtioPCIProxy. >> diff --git a/include/hw/compat.h b/include/hw/compat.h >> index 9914e7a..1531399 100644 >> --- a/include/hw/compat.h >> +++ b/include/hw/compat.h >> @@ -6,6 +6,14 @@ >> .driver = "virtio-mmio",\ >> .property = "format_transport_address",\ >> .value = "off",\ >> + },{\ >> + .driver = "virtio-pci",\ >> + .property = "disable-modern",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-pci",\ >> + .property = "disable-legacy",\ >> + .value = "off",\ > > After looking at the code, I think this will work - did you test this > with a compat machine, though? > Yes, I tested it with pc/q35 2.5 and 2.6 machines. The previous behavior remains the same. >> }, >> >> #define HW_COMPAT_2_5 \ > > But generally, looks good and I think this is also an improvement in > readability. > Thanks! Marcel
On Thu, 21 Jul 2016 12:26:03 +0300 Marcel Apfelbaum <marcel@redhat.com> wrote: > On 07/21/2016 11:54 AM, Cornelia Huck wrote: > > On Wed, 20 Jul 2016 18:28:21 +0300 > > Marcel Apfelbaum <marcel@redhat.com> wrote: > > > >> Enable transitional virtio devices by default. > >> Enable virtio-1.0 for devices plugged into > >> PCIe ports (Root ports or Downstream ports). > > > > Hi Cornelia, > Thank you for the review. > > > Add "by default", as this can still be overridden? > > > > Yes, using -device virtio*,disable-modern=x,disable-legacy=y > are respected as before. > > > >> > >> Using the virtio-1 mode will remove the limitation > > > > s/Using the virtio-1 mode/Disabling the legacy mode/ > > > > ? > > > > Well, the way I see it virtio-1 'pure' is not using the IO BAR. > This is why virtio-1 == disable-modern=off && disable-legacy=on IMHO. IMV: transitional = legacy + modern and therefore modern = transitional - legacy (with modern and virtio-1 the same for pci). I think taking legacy away from transitional is clearer. > > If you or Michael see this differently I have nothing against re-wording it. Let's see what Michael prefers. > > >> of the number of devices that can be attached to a machine > >> by removing the need for the IO BAR. > >> > >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > > > > (...) > > > >> +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > >> +{ > >> + return !proxy->disable_modern; > >> +} > >> + > >> +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) > >> +{ > >> + return proxy->disable_legacy == ON_OFF_AUTO_OFF; > >> +} > >> + > >> +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) > > > > One thing I still find a bit confusing is that you refer to 'modern' > > above, but force to 'virtio_1' here... but that's a minor thing. > > > > I went for 'virtio-1' because of the existing comments (force virtio-1) > and also because 'modern' does not imply "no legacy" - those are independent flags. Hm, for my view see above. > > BTW, instead of the 'disable*' properties (which I find hard to follow) I would go for one > property : "mode" with "legacy"/"transitional"/"virtio-1"/"auto" values. > But is too late for that (at least for 2.7). It would need some compat handling as well. If you really want to go with such a 'mode' property, I'd prefer 'modern' over 'virtio-1': 'modern' is a virtio-pci concept, while 'virtio-1' is transport-agnostic. But it's all just words in the end :) Regardless what you end up with, feel free to add my Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
On Wed, Jul 20, 2016 at 06:28:21PM +0300, Marcel Apfelbaum wrote: > Enable transitional virtio devices by default. > Enable virtio-1.0 for devices plugged into disable legacy is better, I agree. > 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. > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> I think you also want to add some comment with a description explaining *why* you are disabling legacy for these specific devices. > --- > > Hi, > > v2 -> v3: > - Various code tweaks to simplify if statements (Michael) > - Enable virtio modern by default (Gerd and Cornelia) > - Replace virtio flags with actual fields (Gerd) > - Wrappers for more readable code > > v1 -> v2: > - Stick to existing defaults for old machine types (Michael S. Tsirkin) > > If everyone agrees, I am thinking about getting it into 2.7 > to avoid the ~15 virtio devices limitation per machine. > > My tests were limited to checking all possible disable-* configurations (and make check for all archs) > > Thanks, > Marcel > > hw/display/virtio-gpu-pci.c | 4 +--- > hw/display/virtio-vga.c | 4 +--- > hw/virtio/virtio-pci.c | 34 ++++++++++++++++++---------------- > hw/virtio/virtio-pci.h | 21 +++++++++++++++++---- > include/hw/compat.h | 8 ++++++++ > 5 files changed, 45 insertions(+), 26 deletions(-) > > diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c > index a71b230..34a724c 100644 > --- a/hw/display/virtio-gpu-pci.c > +++ b/hw/display/virtio-gpu-pci.c > @@ -30,9 +30,7 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > int i; > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > > for (i = 0; i < g->conf.max_outputs; i++) { > diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c > index 315b7fc..5b510a1 100644 > --- a/hw/display/virtio-vga.c > +++ b/hw/display/virtio-vga.c > @@ -134,9 +134,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > > /* init virtio bits */ > qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(g), true, "realized", &err); > if (err) { > error_propagate(errp, err); > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 2b34b43..11cd634 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -161,7 +161,7 @@ static bool virtio_pci_modern_state_needed(void *opaque) > { > VirtIOPCIProxy *proxy = opaque; > > - return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + return virtio_pci_modern(proxy); > } > > static const VMStateDescription vmstate_virtio_pci_modern_state = { > @@ -300,8 +300,8 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier, > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > VirtQueue *vq = virtio_get_queue(vdev, n); > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool fast_mmio = kvm_ioeventfd_any_length_enabled(); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > MemoryRegion *modern_mr = &proxy->notify.mr; > @@ -1576,8 +1576,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > VirtioBusState *bus = &proxy->bus; > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > uint8_t *config; > uint32_t size; > @@ -1696,7 +1696,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > static void virtio_pci_device_unplugged(DeviceState *d) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > > virtio_pci_stop_ioeventfd(proxy); > @@ -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,11 @@ 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 (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > + proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > + } > + > + if (pcie_port && pci_is_express(pci_dev)) { > int pos; > > pos = pcie_endpoint_cap_init(pci_dev, 0); > @@ -1821,10 +1826,9 @@ 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_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), > DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), > DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, > @@ -1841,7 +1845,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)) { > + virtio_pci_modern(proxy)) { > pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; > } > > @@ -2303,9 +2307,7 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > DeviceState *vdev = DEVICE(&vinput->vdev); > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > } > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index e4548c2..25fbf8a 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass; > enum { > VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, > - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, > - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, > VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, > VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, > @@ -77,8 +75,6 @@ enum { > #define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT) > > /* virtio version flags */ > -#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) > > /* migrate extra state */ > @@ -144,6 +140,8 @@ struct VirtIOPCIProxy { > uint32_t modern_mem_bar; > int config_cap; > uint32_t flags; > + bool disable_modern; > + OnOffAuto disable_legacy; > uint32_t class_code; > uint32_t nvectors; > uint32_t dfselect; > @@ -158,6 +156,21 @@ struct VirtIOPCIProxy { > VirtioBusState bus; > }; > > +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > +{ > + return !proxy->disable_modern; > +} > + > +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) > +{ > + return proxy->disable_legacy == ON_OFF_AUTO_OFF; > +} > + > +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) > +{ > + proxy->disable_modern = false; > + proxy->disable_legacy = ON_OFF_AUTO_ON; > +} > > /* > * virtio-scsi-pci: This extends VirtioPCIProxy. > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 9914e7a..1531399 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -6,6 +6,14 @@ > .driver = "virtio-mmio",\ > .property = "format_transport_address",\ > .value = "off",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-modern",\ > + .value = "on",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-legacy",\ > + .value = "off",\ > }, > > #define HW_COMPAT_2_5 \ > -- > 2.4.3
On Wed, Jul 20, 2016 at 06:28:21PM +0300, Marcel Apfelbaum wrote: > Enable transitional virtio devices by default. > Enable virtio-1.0 for 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. > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > --- > > Hi, > > v2 -> v3: > - Various code tweaks to simplify if statements (Michael) > - Enable virtio modern by default (Gerd and Cornelia) > - Replace virtio flags with actual fields (Gerd) > - Wrappers for more readable code > > v1 -> v2: > - Stick to existing defaults for old machine types (Michael S. Tsirkin) Actually this can still break existing scripts: stick a device on express bus but add disable-modern=on Gave you a legacy device previously but it no longer does. > If everyone agrees, I am thinking about getting it into 2.7 > to avoid the ~15 virtio devices limitation per machine. > > My tests were limited to checking all possible disable-* configurations (and make check for all archs) > > Thanks, > Marcel > > hw/display/virtio-gpu-pci.c | 4 +--- > hw/display/virtio-vga.c | 4 +--- > hw/virtio/virtio-pci.c | 34 ++++++++++++++++++---------------- > hw/virtio/virtio-pci.h | 21 +++++++++++++++++---- > include/hw/compat.h | 8 ++++++++ > 5 files changed, 45 insertions(+), 26 deletions(-) > > diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c > index a71b230..34a724c 100644 > --- a/hw/display/virtio-gpu-pci.c > +++ b/hw/display/virtio-gpu-pci.c > @@ -30,9 +30,7 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > int i; > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > > for (i = 0; i < g->conf.max_outputs; i++) { > diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c > index 315b7fc..5b510a1 100644 > --- a/hw/display/virtio-vga.c > +++ b/hw/display/virtio-vga.c > @@ -134,9 +134,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > > /* init virtio bits */ > qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(g), true, "realized", &err); > if (err) { > error_propagate(errp, err); > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 2b34b43..11cd634 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -161,7 +161,7 @@ static bool virtio_pci_modern_state_needed(void *opaque) > { > VirtIOPCIProxy *proxy = opaque; > > - return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + return virtio_pci_modern(proxy); > } > > static const VMStateDescription vmstate_virtio_pci_modern_state = { > @@ -300,8 +300,8 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier, > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > VirtQueue *vq = virtio_get_queue(vdev, n); > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool fast_mmio = kvm_ioeventfd_any_length_enabled(); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > MemoryRegion *modern_mr = &proxy->notify.mr; > @@ -1576,8 +1576,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > VirtioBusState *bus = &proxy->bus; > - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool legacy = virtio_pci_legacy(proxy); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > uint8_t *config; > uint32_t size; > @@ -1696,7 +1696,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > static void virtio_pci_device_unplugged(DeviceState *d) > { > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > + bool modern = virtio_pci_modern(proxy); > bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; > > virtio_pci_stop_ioeventfd(proxy); > @@ -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,11 @@ 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 (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > + proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > + } > + > + if (pcie_port && pci_is_express(pci_dev)) { > int pos; > > pos = pcie_endpoint_cap_init(pci_dev, 0); > @@ -1821,10 +1826,9 @@ 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_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), > DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), > DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, > @@ -1841,7 +1845,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)) { > + virtio_pci_modern(proxy)) { > pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; > } > > @@ -2303,9 +2307,7 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > DeviceState *vdev = DEVICE(&vinput->vdev); > > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > - /* force virtio-1.0 */ > - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; > - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; > + virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", errp); > } > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index e4548c2..25fbf8a 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass; > enum { > VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, > - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, > - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, > VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, > VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, > @@ -77,8 +75,6 @@ enum { > #define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT) > > /* virtio version flags */ > -#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) > > /* migrate extra state */ > @@ -144,6 +140,8 @@ struct VirtIOPCIProxy { > uint32_t modern_mem_bar; > int config_cap; > uint32_t flags; > + bool disable_modern; > + OnOffAuto disable_legacy; > uint32_t class_code; > uint32_t nvectors; > uint32_t dfselect; > @@ -158,6 +156,21 @@ struct VirtIOPCIProxy { > VirtioBusState bus; > }; > > +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > +{ > + return !proxy->disable_modern; > +} > + > +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) > +{ > + return proxy->disable_legacy == ON_OFF_AUTO_OFF; > +} > + > +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) > +{ > + proxy->disable_modern = false; > + proxy->disable_legacy = ON_OFF_AUTO_ON; > +} > > /* > * virtio-scsi-pci: This extends VirtioPCIProxy. > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 9914e7a..1531399 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -6,6 +6,14 @@ > .driver = "virtio-mmio",\ > .property = "format_transport_address",\ > .value = "off",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-modern",\ > + .value = "on",\ > + },{\ > + .driver = "virtio-pci",\ > + .property = "disable-legacy",\ > + .value = "off",\ > }, > > #define HW_COMPAT_2_5 \ > -- > 2.4.3
Hi, > Actually this can still break existing scripts: > stick a device on express bus but add disable-modern=on > Gave you a legacy device previously but it no longer does. Unlikely to happen in practice because there is little reason to use disable-modern=on in 2.6 & older because that is the default ... Still we can default to legacy=yes in case disable-modern=on + disable-legacy=auto. And throw and error in case both modern and legacy are explicitly disabled (as already suggested elsewhere in this thread). cheers, Gerd
On Thu, Jul 21, 2016 at 11:58:52PM +0200, Gerd Hoffmann wrote: > Hi, > > > Actually this can still break existing scripts: > > stick a device on express bus but add disable-modern=on > > Gave you a legacy device previously but it no longer does. > > Unlikely to happen in practice because there is little reason to use > disable-modern=on in 2.6 & older because that is the default ... Good point, I forgot. > Still we can default to legacy=yes in case disable-modern=on + > disable-legacy=auto. Given the above I'm not sure it's worth it. I'll leave it to Marcel to decide. > And throw and error in case both modern and legacy > are explicitly disabled (as already suggested elsewhere in this thread). > > cheers, > Gerd
On 07/21/2016 11:18 PM, Michael S. Tsirkin wrote: > On Wed, Jul 20, 2016 at 06:28:21PM +0300, Marcel Apfelbaum wrote: >> Enable transitional virtio devices by default. >> Enable virtio-1.0 for devices plugged into > > disable legacy is better, I agree. > >> 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. >> >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > > I think you also want to add some comment with a description explaining > *why* you are disabling legacy for these specific devices. Hi Michael, I thought the above paragraph in the commit message explains it: " 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." What do you think I should add? Thanks, Marcel > > >> --- >> >> Hi, >> >> v2 -> v3: >> - Various code tweaks to simplify if statements (Michael) >> - Enable virtio modern by default (Gerd and Cornelia) >> - Replace virtio flags with actual fields (Gerd) >> - Wrappers for more readable code >> >> v1 -> v2: >> - Stick to existing defaults for old machine types (Michael S. Tsirkin) >> >> If everyone agrees, I am thinking about getting it into 2.7 >> to avoid the ~15 virtio devices limitation per machine. >> >> My tests were limited to checking all possible disable-* configurations (and make check for all archs) >> >> Thanks, >> Marcel >> >> hw/display/virtio-gpu-pci.c | 4 +--- >> hw/display/virtio-vga.c | 4 +--- >> hw/virtio/virtio-pci.c | 34 ++++++++++++++++++---------------- >> hw/virtio/virtio-pci.h | 21 +++++++++++++++++---- >> include/hw/compat.h | 8 ++++++++ >> 5 files changed, 45 insertions(+), 26 deletions(-) >> >> diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c >> index a71b230..34a724c 100644 >> --- a/hw/display/virtio-gpu-pci.c >> +++ b/hw/display/virtio-gpu-pci.c >> @@ -30,9 +30,7 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> int i; >> >> qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); >> - /* force virtio-1.0 */ >> - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; >> - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; >> + virtio_pci_force_virtio_1(vpci_dev); >> object_property_set_bool(OBJECT(vdev), true, "realized", errp); >> >> for (i = 0; i < g->conf.max_outputs; i++) { >> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c >> index 315b7fc..5b510a1 100644 >> --- a/hw/display/virtio-vga.c >> +++ b/hw/display/virtio-vga.c >> @@ -134,9 +134,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> >> /* init virtio bits */ >> qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); >> - /* force virtio-1.0 */ >> - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; >> - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; >> + virtio_pci_force_virtio_1(vpci_dev); >> object_property_set_bool(OBJECT(g), true, "realized", &err); >> if (err) { >> error_propagate(errp, err); >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 2b34b43..11cd634 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -161,7 +161,7 @@ static bool virtio_pci_modern_state_needed(void *opaque) >> { >> VirtIOPCIProxy *proxy = opaque; >> >> - return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); >> + return virtio_pci_modern(proxy); >> } >> >> static const VMStateDescription vmstate_virtio_pci_modern_state = { >> @@ -300,8 +300,8 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier, >> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); >> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); >> VirtQueue *vq = virtio_get_queue(vdev, n); >> - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); >> - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); >> + bool legacy = virtio_pci_legacy(proxy); >> + bool modern = virtio_pci_modern(proxy); >> bool fast_mmio = kvm_ioeventfd_any_length_enabled(); >> bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; >> MemoryRegion *modern_mr = &proxy->notify.mr; >> @@ -1576,8 +1576,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> { >> VirtIOPCIProxy *proxy = VIRTIO_PCI(d); >> VirtioBusState *bus = &proxy->bus; >> - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); >> - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); >> + bool legacy = virtio_pci_legacy(proxy); >> + bool modern = virtio_pci_modern(proxy); >> bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; >> uint8_t *config; >> uint32_t size; >> @@ -1696,7 +1696,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> static void virtio_pci_device_unplugged(DeviceState *d) >> { >> VirtIOPCIProxy *proxy = VIRTIO_PCI(d); >> - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); >> + bool modern = virtio_pci_modern(proxy); >> bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; >> >> virtio_pci_stop_ioeventfd(proxy); >> @@ -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,11 @@ 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 (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { >> + proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; >> + } >> + >> + if (pcie_port && pci_is_express(pci_dev)) { >> int pos; >> >> pos = pcie_endpoint_cap_init(pci_dev, 0); >> @@ -1821,10 +1826,9 @@ 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_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), >> DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), >> DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, >> @@ -1841,7 +1845,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)) { >> + virtio_pci_modern(proxy)) { >> pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; >> } >> >> @@ -2303,9 +2307,7 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> DeviceState *vdev = DEVICE(&vinput->vdev); >> >> qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); >> - /* force virtio-1.0 */ >> - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; >> - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; >> + virtio_pci_force_virtio_1(vpci_dev); >> object_property_set_bool(OBJECT(vdev), true, "realized", errp); >> } >> >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index e4548c2..25fbf8a 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass; >> enum { >> VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, >> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, >> - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, >> - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, >> VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, >> VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, >> VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, >> @@ -77,8 +75,6 @@ enum { >> #define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT) >> >> /* virtio version flags */ >> -#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) >> >> /* migrate extra state */ >> @@ -144,6 +140,8 @@ struct VirtIOPCIProxy { >> uint32_t modern_mem_bar; >> int config_cap; >> uint32_t flags; >> + bool disable_modern; >> + OnOffAuto disable_legacy; >> uint32_t class_code; >> uint32_t nvectors; >> uint32_t dfselect; >> @@ -158,6 +156,21 @@ struct VirtIOPCIProxy { >> VirtioBusState bus; >> }; >> >> +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) >> +{ >> + return !proxy->disable_modern; >> +} >> + >> +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) >> +{ >> + return proxy->disable_legacy == ON_OFF_AUTO_OFF; >> +} >> + >> +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) >> +{ >> + proxy->disable_modern = false; >> + proxy->disable_legacy = ON_OFF_AUTO_ON; >> +} >> >> /* >> * virtio-scsi-pci: This extends VirtioPCIProxy. >> diff --git a/include/hw/compat.h b/include/hw/compat.h >> index 9914e7a..1531399 100644 >> --- a/include/hw/compat.h >> +++ b/include/hw/compat.h >> @@ -6,6 +6,14 @@ >> .driver = "virtio-mmio",\ >> .property = "format_transport_address",\ >> .value = "off",\ >> + },{\ >> + .driver = "virtio-pci",\ >> + .property = "disable-modern",\ >> + .value = "on",\ >> + },{\ >> + .driver = "virtio-pci",\ >> + .property = "disable-legacy",\ >> + .value = "off",\ >> }, >> >> #define HW_COMPAT_2_5 \ >> -- >> 2.4.3
On 07/22/2016 01:21 AM, Michael S. Tsirkin wrote: > On Thu, Jul 21, 2016 at 11:58:52PM +0200, Gerd Hoffmann wrote: >> Hi, >> >>> Actually this can still break existing scripts: >>> stick a device on express bus but add disable-modern=on >>> Gave you a legacy device previously but it no longer does. >> >> Unlikely to happen in practice because there is little reason to use >> disable-modern=on in 2.6 & older because that is the default ... > > Good point, I forgot. > >> Still we can default to legacy=yes in case disable-modern=on + >> disable-legacy=auto. > > Given the above I'm not sure it's worth it. I'll leave it to Marcel > to decide. Hi Michael,Gerd, I think it doesn't worth to make the code more complicated for an uninteresting scenario. > >> And throw and error in case both modern and legacy >> are explicitly disabled (as already suggested elsewhere in this thread). >> There is already a patch for this upstream: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg05263.html Michael, since the rc1 is approaching fast, can you please advice on what changes to make to the commit message and if you prefer me to add the Greg patch to the series and re-send, or you can take it separately? Thanks, Marcel >> cheers, >> Gerd
diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c index a71b230..34a724c 100644 --- a/hw/display/virtio-gpu-pci.c +++ b/hw/display/virtio-gpu-pci.c @@ -30,9 +30,7 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) int i; qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - /* force virtio-1.0 */ - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; + virtio_pci_force_virtio_1(vpci_dev); object_property_set_bool(OBJECT(vdev), true, "realized", errp); for (i = 0; i < g->conf.max_outputs; i++) { diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index 315b7fc..5b510a1 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -134,9 +134,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) /* init virtio bits */ qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); - /* force virtio-1.0 */ - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; + virtio_pci_force_virtio_1(vpci_dev); object_property_set_bool(OBJECT(g), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 2b34b43..11cd634 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -161,7 +161,7 @@ static bool virtio_pci_modern_state_needed(void *opaque) { VirtIOPCIProxy *proxy = opaque; - return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); + return virtio_pci_modern(proxy); } static const VMStateDescription vmstate_virtio_pci_modern_state = { @@ -300,8 +300,8 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier *notifier, VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); VirtQueue *vq = virtio_get_queue(vdev, n); - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); + bool legacy = virtio_pci_legacy(proxy); + bool modern = virtio_pci_modern(proxy); bool fast_mmio = kvm_ioeventfd_any_length_enabled(); bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; MemoryRegion *modern_mr = &proxy->notify.mr; @@ -1576,8 +1576,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) { VirtIOPCIProxy *proxy = VIRTIO_PCI(d); VirtioBusState *bus = &proxy->bus; - bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY); - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); + bool legacy = virtio_pci_legacy(proxy); + bool modern = virtio_pci_modern(proxy); bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; uint8_t *config; uint32_t size; @@ -1696,7 +1696,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) static void virtio_pci_device_unplugged(DeviceState *d) { VirtIOPCIProxy *proxy = VIRTIO_PCI(d); - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); + bool modern = virtio_pci_modern(proxy); bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; virtio_pci_stop_ioeventfd(proxy); @@ -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,11 @@ 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 (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { + proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; + } + + if (pcie_port && pci_is_express(pci_dev)) { int pos; pos = pcie_endpoint_cap_init(pci_dev, 0); @@ -1821,10 +1826,9 @@ 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_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, @@ -1841,7 +1845,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)) { + virtio_pci_modern(proxy)) { pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; } @@ -2303,9 +2307,7 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) DeviceState *vdev = DEVICE(&vinput->vdev); qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - /* force virtio-1.0 */ - vpci_dev->flags &= ~VIRTIO_PCI_FLAG_DISABLE_MODERN; - vpci_dev->flags |= VIRTIO_PCI_FLAG_DISABLE_LEGACY; + virtio_pci_force_virtio_1(vpci_dev); object_property_set_bool(OBJECT(vdev), true, "realized", errp); } diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index e4548c2..25fbf8a 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -61,8 +61,6 @@ typedef struct VirtioBusClass VirtioPCIBusClass; enum { VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, - VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, - VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, @@ -77,8 +75,6 @@ enum { #define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT) /* virtio version flags */ -#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) /* migrate extra state */ @@ -144,6 +140,8 @@ struct VirtIOPCIProxy { uint32_t modern_mem_bar; int config_cap; uint32_t flags; + bool disable_modern; + OnOffAuto disable_legacy; uint32_t class_code; uint32_t nvectors; uint32_t dfselect; @@ -158,6 +156,21 @@ struct VirtIOPCIProxy { VirtioBusState bus; }; +static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) +{ + return !proxy->disable_modern; +} + +static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) +{ + return proxy->disable_legacy == ON_OFF_AUTO_OFF; +} + +static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) +{ + proxy->disable_modern = false; + proxy->disable_legacy = ON_OFF_AUTO_ON; +} /* * virtio-scsi-pci: This extends VirtioPCIProxy. diff --git a/include/hw/compat.h b/include/hw/compat.h index 9914e7a..1531399 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -6,6 +6,14 @@ .driver = "virtio-mmio",\ .property = "format_transport_address",\ .value = "off",\ + },{\ + .driver = "virtio-pci",\ + .property = "disable-modern",\ + .value = "on",\ + },{\ + .driver = "virtio-pci",\ + .property = "disable-legacy",\ + .value = "off",\ }, #define HW_COMPAT_2_5 \
Enable transitional virtio devices by default. Enable virtio-1.0 for 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. Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> --- Hi, v2 -> v3: - Various code tweaks to simplify if statements (Michael) - Enable virtio modern by default (Gerd and Cornelia) - Replace virtio flags with actual fields (Gerd) - Wrappers for more readable code v1 -> v2: - Stick to existing defaults for old machine types (Michael S. Tsirkin) If everyone agrees, I am thinking about getting it into 2.7 to avoid the ~15 virtio devices limitation per machine. My tests were limited to checking all possible disable-* configurations (and make check for all archs) Thanks, Marcel hw/display/virtio-gpu-pci.c | 4 +--- hw/display/virtio-vga.c | 4 +--- hw/virtio/virtio-pci.c | 34 ++++++++++++++++++---------------- hw/virtio/virtio-pci.h | 21 +++++++++++++++++---- include/hw/compat.h | 8 ++++++++ 5 files changed, 45 insertions(+), 26 deletions(-)