* [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines
@ 2016-07-19 16:45 Marcel Apfelbaum
2016-07-19 17:47 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Marcel Apfelbaum @ 2016-07-19 16:45 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst
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(-)
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;
--
2.4.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines
2016-07-19 16:45 [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines Marcel Apfelbaum
@ 2016-07-19 17:47 ` Michael S. Tsirkin
2016-07-19 18:43 ` Marcel Apfelbaum
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2016-07-19 17:47 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines
2016-07-19 17:47 ` Michael S. Tsirkin
@ 2016-07-19 18:43 ` Marcel Apfelbaum
0 siblings, 0 replies; 3+ messages in thread
From: Marcel Apfelbaum @ 2016-07-19 18:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-19 18:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-19 16:45 [Qemu-devel] [PATCH] hw/virtio-pci: fix virtio behaviour on modern (PCIe) machines Marcel Apfelbaum
2016-07-19 17:47 ` Michael S. Tsirkin
2016-07-19 18:43 ` Marcel Apfelbaum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).