| Message ID | 1427876112-12615-17-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/01/2015 10:15 AM, Jason Wang wrote: > This patch increases the maximum number of virtqueues for pci from 64 > to 513. This will allow booting a virtio-net-pci device with 256 queue > pairs. > > To keep migration compatibility, 64 was kept for legacy machine > types. This is because qemu in fact allows guest to probe the limit of > virtqueues through trying to set queue_sel. So for legacy machine > type, we should make sure setting queue_sel to more than 63 won't > make effect. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: qemu-ppc@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/i386/pc_piix.c | 5 +++++ > hw/i386/pc_q35.c | 5 +++++ > hw/ppc/spapr.c | 5 +++++ > hw/virtio/virtio-pci.c | 2 +- > 4 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 212e263..6e098ce 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -49,6 +49,7 @@ > #include "hw/acpi/acpi.h" > #include "cpu.h" > #include "qemu/error-report.h" > +#include "hw/virtio/virtio-bus.h" > #ifdef CONFIG_XEN > # include <xen/hvm/hvm_info_table.h> > #endif > @@ -312,6 +313,10 @@ static void pc_init_pci(MachineState *machine) > > static void pc_compat_2_3(MachineState *machine) > { > + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); > + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); > + > + k->queue_max = 64; > } > > static void pc_compat_2_2(MachineState *machine) > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index e67f2de..ff7c414 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -45,6 +45,7 @@ > #include "hw/usb.h" > #include "hw/cpu/icc_bus.h" > #include "qemu/error-report.h" > +#include "include/hw/virtio/virtio-bus.h" > > /* ICH9 AHCI has 6 ports */ > #define MAX_SATA_PORTS 6 > @@ -291,6 +292,10 @@ static void pc_q35_init(MachineState *machine) > > static void pc_compat_2_3(MachineState *machine) > { > + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); > + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); > + > + k->queue_max = 64; > } > > static void pc_compat_2_2(MachineState *machine) > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 8e43aa2..ee8f6a3 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -50,6 +50,7 @@ > #include "hw/pci/pci.h" > #include "hw/scsi/scsi.h" > #include "hw/virtio/virtio-scsi.h" > +#include "hw/virtio/virtio-bus.h" > > #include "exec/address-spaces.h" > #include "hw/usb.h" > @@ -1827,6 +1828,10 @@ static const TypeInfo spapr_machine_info = { > > static void spapr_compat_2_3(Object *obj) > { > + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); > + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); > + > + k->queue_max = 64; > } > > static void spapr_compat_2_2(Object *obj) > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 9a5242a..02e3ce8 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -42,7 +42,7 @@ > * configuration space */ > #define VIRTIO_PCI_CONFIG_SIZE(dev) VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) > > -#define VIRTIO_PCI_QUEUE_MAX 64 > +#define VIRTIO_PCI_QUEUE_MAX 513 513 is an interesting number. Any particular reason for it? Maybe this was mentioned before and I just missed it ;) Alex
On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> wrote: > On 04/01/2015 10:15 AM, Jason Wang wrote: > >> This patch increases the maximum number of virtqueues for pci from 64 >> to 513. This will allow booting a virtio-net-pci device with 256 queue >> pairs. >> ... >> >> * configuration space */ >> #define VIRTIO_PCI_CONFIG_SIZE(dev) VIRTIO_PCI_CONFIG_OFF(msix_ >> enabled(dev)) >> -#define VIRTIO_PCI_QUEUE_MAX 64 >> +#define VIRTIO_PCI_QUEUE_MAX 513 >> > > 513 is an interesting number. Any particular reason for it? Maybe this was > mentioned before and I just missed it ;) > > quite large, too. I thought multiple queue pairs were useful to split the load for multicore machines, but targeting VMs with up to 256 cores (and presumably an equal number in the host) seems really forward looking. On the other hand, the message is dated april first... cheers luigi
On 04/07/2015 08:06 PM, Luigi Rizzo wrote: > > > On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de > <mailto:agraf@suse.de>> wrote: > > On 04/01/2015 10:15 AM, Jason Wang wrote: > > This patch increases the maximum number of virtqueues for pci > from 64 > to 513. This will allow booting a virtio-net-pci device with > 256 queue > pairs. > ... > > * configuration space */ > #define VIRTIO_PCI_CONFIG_SIZE(dev) > VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) > -#define VIRTIO_PCI_QUEUE_MAX 64 > +#define VIRTIO_PCI_QUEUE_MAX 513 > > > 513 is an interesting number. Any particular reason for it? Maybe > this was mentioned before and I just missed it ;) > > > quite large, too. I thought multiple queue pairs were useful > to split the load for multicore machines, but targeting VMs with > up to 256 cores (and presumably an equal number in the host) > seems really forward looking. They can also be useful in case your host tap queue is full, so going higher than the host core count may make sense for throughput. However, I am in doubt that there is a one-size-fits-all answer to this. Could we maybe make the queue size configurable via a qdev property? Alex
On 04/08/2015 12:54 AM, Alexander Graf wrote: > On 04/01/2015 10:15 AM, Jason Wang wrote: >> This patch increases the maximum number of virtqueues for pci from 64 >> to 513. This will allow booting a virtio-net-pci device with 256 queue >> pairs. >> >> To keep migration compatibility, 64 was kept for legacy machine >> types. This is because qemu in fact allows guest to probe the limit of >> virtqueues through trying to set queue_sel. So for legacy machine >> type, we should make sure setting queue_sel to more than 63 won't >> make effect. >> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Richard Henderson <rth@twiddle.net> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: qemu-ppc@nongnu.org >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/i386/pc_piix.c | 5 +++++ >> hw/i386/pc_q35.c | 5 +++++ >> hw/ppc/spapr.c | 5 +++++ >> hw/virtio/virtio-pci.c | 2 +- >> 4 files changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 212e263..6e098ce 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -49,6 +49,7 @@ >> #include "hw/acpi/acpi.h" >> #include "cpu.h" >> #include "qemu/error-report.h" >> +#include "hw/virtio/virtio-bus.h" >> #ifdef CONFIG_XEN >> # include <xen/hvm/hvm_info_table.h> >> #endif >> @@ -312,6 +313,10 @@ static void pc_init_pci(MachineState *machine) >> static void pc_compat_2_3(MachineState *machine) >> { >> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >> + >> + k->queue_max = 64; >> } >> static void pc_compat_2_2(MachineState *machine) >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index e67f2de..ff7c414 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -45,6 +45,7 @@ >> #include "hw/usb.h" >> #include "hw/cpu/icc_bus.h" >> #include "qemu/error-report.h" >> +#include "include/hw/virtio/virtio-bus.h" >> /* ICH9 AHCI has 6 ports */ >> #define MAX_SATA_PORTS 6 >> @@ -291,6 +292,10 @@ static void pc_q35_init(MachineState *machine) >> static void pc_compat_2_3(MachineState *machine) >> { >> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >> + >> + k->queue_max = 64; >> } >> static void pc_compat_2_2(MachineState *machine) >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 8e43aa2..ee8f6a3 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -50,6 +50,7 @@ >> #include "hw/pci/pci.h" >> #include "hw/scsi/scsi.h" >> #include "hw/virtio/virtio-scsi.h" >> +#include "hw/virtio/virtio-bus.h" >> #include "exec/address-spaces.h" >> #include "hw/usb.h" >> @@ -1827,6 +1828,10 @@ static const TypeInfo spapr_machine_info = { >> static void spapr_compat_2_3(Object *obj) >> { >> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >> + >> + k->queue_max = 64; >> } >> static void spapr_compat_2_2(Object *obj) >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 9a5242a..02e3ce8 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -42,7 +42,7 @@ >> * configuration space */ >> #define VIRTIO_PCI_CONFIG_SIZE(dev) >> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >> -#define VIRTIO_PCI_QUEUE_MAX 64 >> +#define VIRTIO_PCI_QUEUE_MAX 513 > > 513 is an interesting number. Any particular reason for it? Maybe this > was mentioned before and I just missed it ;) > > > Alex > > The number were chose to match kernel limit for tuntap queues. Recent kernel allow up to 256 queues for tuntap, so 513 is in fact 256 * 2 plus 1 control vq.
> Am 08.04.2015 um 10:10 schrieb Jason Wang <jasowang@redhat.com>: > > > >> On 04/08/2015 12:54 AM, Alexander Graf wrote: >>> On 04/01/2015 10:15 AM, Jason Wang wrote: >>> This patch increases the maximum number of virtqueues for pci from 64 >>> to 513. This will allow booting a virtio-net-pci device with 256 queue >>> pairs. >>> >>> To keep migration compatibility, 64 was kept for legacy machine >>> types. This is because qemu in fact allows guest to probe the limit of >>> virtqueues through trying to set queue_sel. So for legacy machine >>> type, we should make sure setting queue_sel to more than 63 won't >>> make effect. >>> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Richard Henderson <rth@twiddle.net> >>> Cc: Michael S. Tsirkin <mst@redhat.com> >>> Cc: Alexander Graf <agraf@suse.de> >>> Cc: qemu-ppc@nongnu.org >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> --- >>> hw/i386/pc_piix.c | 5 +++++ >>> hw/i386/pc_q35.c | 5 +++++ >>> hw/ppc/spapr.c | 5 +++++ >>> hw/virtio/virtio-pci.c | 2 +- >>> 4 files changed, 16 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >>> index 212e263..6e098ce 100644 >>> --- a/hw/i386/pc_piix.c >>> +++ b/hw/i386/pc_piix.c >>> @@ -49,6 +49,7 @@ >>> #include "hw/acpi/acpi.h" >>> #include "cpu.h" >>> #include "qemu/error-report.h" >>> +#include "hw/virtio/virtio-bus.h" >>> #ifdef CONFIG_XEN >>> # include <xen/hvm/hvm_info_table.h> >>> #endif >>> @@ -312,6 +313,10 @@ static void pc_init_pci(MachineState *machine) >>> static void pc_compat_2_3(MachineState *machine) >>> { >>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>> + >>> + k->queue_max = 64; >>> } >>> static void pc_compat_2_2(MachineState *machine) >>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >>> index e67f2de..ff7c414 100644 >>> --- a/hw/i386/pc_q35.c >>> +++ b/hw/i386/pc_q35.c >>> @@ -45,6 +45,7 @@ >>> #include "hw/usb.h" >>> #include "hw/cpu/icc_bus.h" >>> #include "qemu/error-report.h" >>> +#include "include/hw/virtio/virtio-bus.h" >>> /* ICH9 AHCI has 6 ports */ >>> #define MAX_SATA_PORTS 6 >>> @@ -291,6 +292,10 @@ static void pc_q35_init(MachineState *machine) >>> static void pc_compat_2_3(MachineState *machine) >>> { >>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>> + >>> + k->queue_max = 64; >>> } >>> static void pc_compat_2_2(MachineState *machine) >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 8e43aa2..ee8f6a3 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -50,6 +50,7 @@ >>> #include "hw/pci/pci.h" >>> #include "hw/scsi/scsi.h" >>> #include "hw/virtio/virtio-scsi.h" >>> +#include "hw/virtio/virtio-bus.h" >>> #include "exec/address-spaces.h" >>> #include "hw/usb.h" >>> @@ -1827,6 +1828,10 @@ static const TypeInfo spapr_machine_info = { >>> static void spapr_compat_2_3(Object *obj) >>> { >>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>> + >>> + k->queue_max = 64; >>> } >>> static void spapr_compat_2_2(Object *obj) >>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >>> index 9a5242a..02e3ce8 100644 >>> --- a/hw/virtio/virtio-pci.c >>> +++ b/hw/virtio/virtio-pci.c >>> @@ -42,7 +42,7 @@ >>> * configuration space */ >>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>> +#define VIRTIO_PCI_QUEUE_MAX 513 >> >> 513 is an interesting number. Any particular reason for it? Maybe this >> was mentioned before and I just missed it ;) >> >> >> Alex > > The number were chose to match kernel limit for tuntap queues. Recent > kernel allow up to 256 queues for tuntap, so 513 is in fact 256 * 2 plus > 1 control vq. I see. I'm not fully convinced that it's a good idea to bake that limit into qemu, but we have to have a limit somewhere. However, could you please make it more obvious in the code where thid number stems from? Either by using defines or with a comment. Alex
On Wed, Apr 8, 2015 at 2:06 AM, Luigi Rizzo <rizzo@iet.unipi.it> wrote: > > > On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> wrote: >> On 04/01/2015 10:15 AM, Jason Wang wrote: >>> This patch increases the maximum number of virtqueues for pci from >>> 64 >>> to 513. This will allow booting a virtio-net-pci device with 256 >>> queue >>> pairs. >>> ... >>> * configuration space */ >>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>> +#define VIRTIO_PCI_QUEUE_MAX 513 >> >> 513 is an interesting number. Any particular reason for it? Maybe >> this was mentioned before and I just missed it ;) >> > > quite large, too. I thought multiple queue pairs were useful > to split the load for multicore machines, but targeting VMs with > up to 256 cores (and presumably an equal number in the host) > seems really forward looking. Probably not, since KVM can support up to 255 vcpus now. > > On the other hand, the message is dated april first... > cheers > luigi
On Wed, Apr 8, 2015 at 2:33 AM, Alexander Graf <agraf@suse.de> wrote: > On 04/07/2015 08:06 PM, Luigi Rizzo wrote: >> >> >> On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> wrote: >>> On 04/01/2015 10:15 AM, Jason Wang wrote: >>>> This patch increases the maximum number of virtqueues for pci from >>>> 64 >>>> to 513. This will allow booting a virtio-net-pci device with 256 >>>> queue >>>> pairs. >>>> ... >>>> * configuration space */ >>>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>>> +#define VIRTIO_PCI_QUEUE_MAX 513 >>> >>> 513 is an interesting number. Any particular reason for it? Maybe >>> this was mentioned before and I just missed it ;) >>> >> >> quite large, too. I thought multiple queue pairs were useful >> to split the load for multicore machines, but targeting VMs with >> up to 256 cores (and presumably an equal number in the host) >> seems really forward looking. > > They can also be useful in case your host tap queue is full, so going > higher than the host core count may make sense for throughput. > > However, I am in doubt that there is a one-size-fits-all answer to > this. Could we maybe make the queue size configurable via a qdev > property? We can do this on top but I'm not sure I understand the question. Do you mean a per-device limitation? > > > Alex >
On Wed, Apr 8, 2015 at 4:13 PM, Alexander Graf <agraf@suse.de> wrote: > > > >> Am 08.04.2015 um 10:10 schrieb Jason Wang <jasowang@redhat.com>: >> >> >> >>> On 04/08/2015 12:54 AM, Alexander Graf wrote: >>>> On 04/01/2015 10:15 AM, Jason Wang wrote: >>>> This patch increases the maximum number of virtqueues for pci >>>> from 64 >>>> to 513. This will allow booting a virtio-net-pci device with 256 >>>> queue >>>> pairs. >>>> >>>> To keep migration compatibility, 64 was kept for legacy machine >>>> types. This is because qemu in fact allows guest to probe the >>>> limit of >>>> virtqueues through trying to set queue_sel. So for legacy machine >>>> type, we should make sure setting queue_sel to more than 63 won't >>>> make effect. >>>> >>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>> Cc: Richard Henderson <rth@twiddle.net> >>>> Cc: Michael S. Tsirkin <mst@redhat.com> >>>> Cc: Alexander Graf <agraf@suse.de> >>>> Cc: qemu-ppc@nongnu.org >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>> --- >>>> hw/i386/pc_piix.c | 5 +++++ >>>> hw/i386/pc_q35.c | 5 +++++ >>>> hw/ppc/spapr.c | 5 +++++ >>>> hw/virtio/virtio-pci.c | 2 +- >>>> 4 files changed, 16 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >>>> index 212e263..6e098ce 100644 >>>> --- a/hw/i386/pc_piix.c >>>> +++ b/hw/i386/pc_piix.c >>>> @@ -49,6 +49,7 @@ >>>> #include "hw/acpi/acpi.h" >>>> #include "cpu.h" >>>> #include "qemu/error-report.h" >>>> +#include "hw/virtio/virtio-bus.h" >>>> #ifdef CONFIG_XEN >>>> # include <xen/hvm/hvm_info_table.h> >>>> #endif >>>> @@ -312,6 +313,10 @@ static void pc_init_pci(MachineState >>>> *machine) >>>> static void pc_compat_2_3(MachineState *machine) >>>> { >>>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>>> + >>>> + k->queue_max = 64; >>>> } >>>> static void pc_compat_2_2(MachineState *machine) >>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >>>> index e67f2de..ff7c414 100644 >>>> --- a/hw/i386/pc_q35.c >>>> +++ b/hw/i386/pc_q35.c >>>> @@ -45,6 +45,7 @@ >>>> #include "hw/usb.h" >>>> #include "hw/cpu/icc_bus.h" >>>> #include "qemu/error-report.h" >>>> +#include "include/hw/virtio/virtio-bus.h" >>>> /* ICH9 AHCI has 6 ports */ >>>> #define MAX_SATA_PORTS 6 >>>> @@ -291,6 +292,10 @@ static void pc_q35_init(MachineState >>>> *machine) >>>> static void pc_compat_2_3(MachineState *machine) >>>> { >>>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>>> + >>>> + k->queue_max = 64; >>>> } >>>> static void pc_compat_2_2(MachineState *machine) >>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>> index 8e43aa2..ee8f6a3 100644 >>>> --- a/hw/ppc/spapr.c >>>> +++ b/hw/ppc/spapr.c >>>> @@ -50,6 +50,7 @@ >>>> #include "hw/pci/pci.h" >>>> #include "hw/scsi/scsi.h" >>>> #include "hw/virtio/virtio-scsi.h" >>>> +#include "hw/virtio/virtio-bus.h" >>>> #include "exec/address-spaces.h" >>>> #include "hw/usb.h" >>>> @@ -1827,6 +1828,10 @@ static const TypeInfo spapr_machine_info = >>>> { >>>> static void spapr_compat_2_3(Object *obj) >>>> { >>>> + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); >>>> + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); >>>> + >>>> + k->queue_max = 64; >>>> } >>>> static void spapr_compat_2_2(Object *obj) >>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >>>> index 9a5242a..02e3ce8 100644 >>>> --- a/hw/virtio/virtio-pci.c >>>> +++ b/hw/virtio/virtio-pci.c >>>> @@ -42,7 +42,7 @@ >>>> * configuration space */ >>>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>>> +#define VIRTIO_PCI_QUEUE_MAX 513 >>> >>> 513 is an interesting number. Any particular reason for it? Maybe >>> this >>> was mentioned before and I just missed it ;) >>> >>> >>> Alex >> >> The number were chose to match kernel limit for tuntap queues. >> Recent >> kernel allow up to 256 queues for tuntap, so 513 is in fact 256 * 2 >> plus >> 1 control vq. > > I see. I'm not fully convinced that it's a good idea to bake that > limit into qemu, but we have to have a limit somewhere. > > However, could you please make it more obvious in the code where thid > number stems from? Either by using defines or with a comment. > > > Alex Will add a comment for this. Thanks.
On 08.04.15 10:29, Jason Wang wrote: > > > On Wed, Apr 8, 2015 at 2:33 AM, Alexander Graf <agraf@suse.de> wrote: >> On 04/07/2015 08:06 PM, Luigi Rizzo wrote: >>> >>> >>> On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> wrote: >>>> On 04/01/2015 10:15 AM, Jason Wang wrote: >>>>> This patch increases the maximum number of virtqueues for pci from 64 >>>>> to 513. This will allow booting a virtio-net-pci device with 256 queue >>>>> pairs. >>>>> ... * configuration space */ >>>>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>>>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>>>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>>>> +#define VIRTIO_PCI_QUEUE_MAX 513 >>>> >>>> 513 is an interesting number. Any particular reason for it? Maybe >>>> this was mentioned before and I just missed it ;) >>>> >>> >>> quite large, too. I thought multiple queue pairs were useful >>> to split the load for multicore machines, but targeting VMs with >>> up to 256 cores (and presumably an equal number in the host) >>> seems really forward looking. >> >> They can also be useful in case your host tap queue is full, so going >> higher than the host core count may make sense for throughput. >> >> However, I am in doubt that there is a one-size-fits-all answer to >> this. Could we maybe make the queue size configurable via a qdev >> property? > > We can do this on top but I'm not sure I understand the question. Do you > mean a per-device limitation? Ok, let me rephrase the question. Why isn't the limit 64k? Would we hit resource constraints? Imagine that in Linux 5.0 the kernel tap interface extends to way more queues, we'd be stuck in the same situation as today, no? Alex
On Wed, Apr 8, 2015 at 4:41 PM, Alexander Graf <agraf@suse.de> wrote: > > > On 08.04.15 10:29, Jason Wang wrote: >> >> >> On Wed, Apr 8, 2015 at 2:33 AM, Alexander Graf <agraf@suse.de> >> wrote: >>> On 04/07/2015 08:06 PM, Luigi Rizzo wrote: >>>> >>>> >>>> On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> >>>> wrote: >>>>> On 04/01/2015 10:15 AM, Jason Wang wrote: >>>>>> This patch increases the maximum number of virtqueues for pci >>>>>> from 64 >>>>>> to 513. This will allow booting a virtio-net-pci device with >>>>>> 256 queue >>>>>> pairs. >>>>>> ... * configuration space */ >>>>>> #define VIRTIO_PCI_CONFIG_SIZE(dev) >>>>>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) >>>>>> -#define VIRTIO_PCI_QUEUE_MAX 64 >>>>>> +#define VIRTIO_PCI_QUEUE_MAX 513 >>>>> >>>>> 513 is an interesting number. Any particular reason for it? Maybe >>>>> this was mentioned before and I just missed it ;) >>>>> >>>> >>>> quite large, too. I thought multiple queue pairs were useful >>>> to split the load for multicore machines, but targeting VMs with >>>> up to 256 cores (and presumably an equal number in the host) >>>> seems really forward looking. >>> >>> They can also be useful in case your host tap queue is full, so >>> going >>> higher than the host core count may make sense for throughput. >>> >>> However, I am in doubt that there is a one-size-fits-all answer to >>> this. Could we maybe make the queue size configurable via a qdev >>> property? >> >> We can do this on top but I'm not sure I understand the question. >> Do you >> mean a per-device limitation? > > Ok, let me rephrase the question. Why isn't the limit 64k? Because there's no real use case for 64k but I agree to make more space for the future extension. > Would we hit > resource constraints? Probably not since VirtQueue is rather small. 64K will consume about 4 or 5 MB, not sure it was too big but looks ok. > Imagine that in Linux 5.0 the kernel tap interface > extends to way more queues, we'd be stuck in the same situation as > today, no? Yes, but any limit will be exceeded in the future. > > > Alex
On Tue, Apr 07, 2015 at 08:06:14PM +0200, Luigi Rizzo wrote: > > > On Tue, Apr 7, 2015 at 6:54 PM, Alexander Graf <agraf@suse.de> wrote: > > On 04/01/2015 10:15 AM, Jason Wang wrote: > > This patch increases the maximum number of virtqueues for pci from 64 > to 513. This will allow booting a virtio-net-pci device with 256 queue > pairs. > ... > > * configuration space */ > #define VIRTIO_PCI_CONFIG_SIZE(dev) VIRTIO_PCI_CONFIG_OFF > (msix_enabled(dev)) > -#define VIRTIO_PCI_QUEUE_MAX 64 > +#define VIRTIO_PCI_QUEUE_MAX 513 > > > 513 is an interesting number. Any particular reason for it? Maybe this was > mentioned before and I just missed it ;) > > > > quite large, too. I thought multiple queue pairs were useful > to split the load for multicore machines, but targeting VMs with > up to 256 cores (and presumably an equal number in the host) > seems really forward looking. Well, that's the # of VCPUs QEMU supports ATM, no? Seems silly to have limit on vqs block us from creating such VMs. Maybe just define it as max cpus * 2 + 1. > On the other hand, the message is dated april first... > cheers > luigi
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 212e263..6e098ce 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -49,6 +49,7 @@ #include "hw/acpi/acpi.h" #include "cpu.h" #include "qemu/error-report.h" +#include "hw/virtio/virtio-bus.h" #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif @@ -312,6 +313,10 @@ static void pc_init_pci(MachineState *machine) static void pc_compat_2_3(MachineState *machine) { + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); + + k->queue_max = 64; } static void pc_compat_2_2(MachineState *machine) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e67f2de..ff7c414 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -45,6 +45,7 @@ #include "hw/usb.h" #include "hw/cpu/icc_bus.h" #include "qemu/error-report.h" +#include "include/hw/virtio/virtio-bus.h" /* ICH9 AHCI has 6 ports */ #define MAX_SATA_PORTS 6 @@ -291,6 +292,10 @@ static void pc_q35_init(MachineState *machine) static void pc_compat_2_3(MachineState *machine) { + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); + + k->queue_max = 64; } static void pc_compat_2_2(MachineState *machine) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 8e43aa2..ee8f6a3 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -50,6 +50,7 @@ #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" #include "hw/virtio/virtio-scsi.h" +#include "hw/virtio/virtio-bus.h" #include "exec/address-spaces.h" #include "hw/usb.h" @@ -1827,6 +1828,10 @@ static const TypeInfo spapr_machine_info = { static void spapr_compat_2_3(Object *obj) { + ObjectClass *klass = object_class_by_name("virtio-pci-bus"); + VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); + + k->queue_max = 64; } static void spapr_compat_2_2(Object *obj) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 9a5242a..02e3ce8 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -42,7 +42,7 @@ * configuration space */ #define VIRTIO_PCI_CONFIG_SIZE(dev) VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev)) -#define VIRTIO_PCI_QUEUE_MAX 64 +#define VIRTIO_PCI_QUEUE_MAX 513 static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size, VirtIOPCIProxy *dev);
This patch increases the maximum number of virtqueues for pci from 64 to 513. This will allow booting a virtio-net-pci device with 256 queue pairs. To keep migration compatibility, 64 was kept for legacy machine types. This is because qemu in fact allows guest to probe the limit of virtqueues through trying to set queue_sel. So for legacy machine type, we should make sure setting queue_sel to more than 63 won't make effect. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Alexander Graf <agraf@suse.de> Cc: qemu-ppc@nongnu.org Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/i386/pc_piix.c | 5 +++++ hw/i386/pc_q35.c | 5 +++++ hw/ppc/spapr.c | 5 +++++ hw/virtio/virtio-pci.c | 2 +- 4 files changed, 16 insertions(+), 1 deletion(-)