diff mbox

[V7,15/16] virtio-pci: increase the maximum number of virtqueues to 513

Message ID 1429770109-23873-16-git-send-email-jasowang@redhat.com
State New
Headers show

Commit Message

Jason Wang April 23, 2015, 6:21 a.m. UTC
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 on recent Linux host (which supports up to 256 tuntap 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 | 6 +++++-
 4 files changed, 20 insertions(+), 1 deletion(-)

Comments

Cornelia Huck April 23, 2015, 11:24 a.m. UTC | #1
On Thu, 23 Apr 2015 14:21:48 +0800
Jason Wang <jasowang@redhat.com> 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 on recent Linux host (which supports up to 256 tuntap 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 | 6 +++++-
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
(...)
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 7d01500..c510cb7 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -42,7 +42,11 @@
>   * configuration space */
>  #define VIRTIO_PCI_CONFIG_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
> 
> -#define VIRTIO_PCI_QUEUE_MAX 64
> +/* The number was chose to be greater than both the the number of max
> + * vcpus supported by host and the number of max tuntap queues support
> + * by host and also leave some spaces for future.
> + */
> +#define VIRTIO_PCI_QUEUE_MAX 1024
                                ^^^^

I think you need to fixup subject and patch description :)

> 
>  static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
>                                 VirtIOPCIProxy *dev);
Michael S. Tsirkin April 27, 2015, 11:02 a.m. UTC | #2
On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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.

This isn't a documented interface, and no guest that I know of does
this.  Accordingly, I think we should drop everything except the
hw/virtio/virtio-pci.c change.

> 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 | 6 +++++-
>  4 files changed, 20 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 7d01500..c510cb7 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -42,7 +42,11 @@
>   * configuration space */
>  #define VIRTIO_PCI_CONFIG_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
>  
> -#define VIRTIO_PCI_QUEUE_MAX 64
> +/* The number was chose to be greater than both the the number of max
> + * vcpus supported by host and the number of max tuntap queues support
> + * by host and also leave some spaces for future.
> + */
> +#define VIRTIO_PCI_QUEUE_MAX 1024
>  
>  static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
>                                 VirtIOPCIProxy *dev);
> -- 
> 2.1.0
Jason Wang April 28, 2015, 3:05 a.m. UTC | #3
On Thu, Apr 23, 2015 at 7:24 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Thu, 23 Apr 2015 14:21:48 +0800
> Jason Wang <jasowang@redhat.com> 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 on recent Linux host (which supports up to 256 tuntap 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 | 6 +++++-
>>   4 files changed, 20 insertions(+), 1 deletion(-)
>>  
> (...)
>>  diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>  index 7d01500..c510cb7 100644
>>  --- a/hw/virtio/virtio-pci.c
>>  +++ b/hw/virtio/virtio-pci.c
>>  @@ -42,7 +42,11 @@
>>    * configuration space */
>>   #define VIRTIO_PCI_CONFIG_SIZE(dev)     
>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
>>  
>>  -#define VIRTIO_PCI_QUEUE_MAX 64
>>  +/* The number was chose to be greater than both the the number of 
>> max
>>  + * vcpus supported by host and the number of max tuntap queues 
>> support
>>  + * by host and also leave some spaces for future.
>>  + */
>>  +#define VIRTIO_PCI_QUEUE_MAX 1024
>                                 ^^^^
> 
> I think you need to fixup subject and patch description :)

Yes, will fix this.

Thanks
> 
>>  
>>   static void virtio_pci_bus_new(VirtioBusState *bus, size_t 
>> bus_size,
>>                                  VirtIOPCIProxy *dev);
>
Jason Wang April 28, 2015, 3:12 a.m. UTC | #4
On Mon, Apr 27, 2015 at 7:02 PM, Michael S. Tsirkin <mst@redhat.com> 
wrote:
> On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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.
> 
> This isn't a documented interface, and no guest that I know of does
> this.  Accordingly, I think we should drop everything except the
> hw/virtio/virtio-pci.c change.

We leave a chance for guest to use such undocumented behavior, so 
technically we'd better keep it and it maybe too late for us to fix if 
we find such a guest in the future. And consider keeping this 
compatibility was really not hard, so I suggest to include this.
  
> 
> 
>>  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 | 6 +++++-
>>   4 files changed, 20 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 7d01500..c510cb7 100644
>>  --- a/hw/virtio/virtio-pci.c
>>  +++ b/hw/virtio/virtio-pci.c
>>  @@ -42,7 +42,11 @@
>>    * configuration space */
>>   #define VIRTIO_PCI_CONFIG_SIZE(dev)     
>> VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
>>   
>>  -#define VIRTIO_PCI_QUEUE_MAX 64
>>  +/* The number was chose to be greater than both the the number of 
>> max
>>  + * vcpus supported by host and the number of max tuntap queues 
>> support
>>  + * by host and also leave some spaces for future.
>>  + */
>>  +#define VIRTIO_PCI_QUEUE_MAX 1024
>>   
>>   static void virtio_pci_bus_new(VirtioBusState *bus, size_t 
>> bus_size,
>>                                  VirtIOPCIProxy *dev);
>>  -- 
>>  2.1.0
Michael S. Tsirkin April 28, 2015, 7:17 a.m. UTC | #5
On Tue, Apr 28, 2015 at 11:12:10AM +0800, Jason Wang wrote:
> 
> 
> On Mon, Apr 27, 2015 at 7:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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.
> >
> >This isn't a documented interface, and no guest that I know of does
> >this.  Accordingly, I think we should drop everything except the
> >hw/virtio/virtio-pci.c change.
> 
> We leave a chance for guest to use such undocumented behavior, so
> technically we'd better keep it and it maybe too late for us to fix if we
> find such a guest in the future. And consider keeping this compatibility was
> really not hard, so I suggest to include this.

Reminds me of  https://xkcd.com/1172/
We don't do this kind of thing.

> >
> >
> >> 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 | 6 +++++-
> >>  4 files changed, 20 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 7d01500..c510cb7 100644
> >> --- a/hw/virtio/virtio-pci.c
> >> +++ b/hw/virtio/virtio-pci.c
> >> @@ -42,7 +42,11 @@
> >>   * configuration space */
> >>  #define VIRTIO_PCI_CONFIG_SIZE(dev)
> >>VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
> >> -#define VIRTIO_PCI_QUEUE_MAX 64
> >> +/* The number was chose to be greater than both the the number of max
> >> + * vcpus supported by host and the number of max tuntap queues support
> >> + * by host and also leave some spaces for future.
> >> + */
> >> +#define VIRTIO_PCI_QUEUE_MAX 1024
> >>     static void virtio_pci_bus_new(VirtioBusState *bus, size_t
> >>bus_size,
> >>                                 VirtIOPCIProxy *dev);
> >> --  2.1.0
Jason Wang May 13, 2015, 7:47 a.m. UTC | #6
On 04/28/2015 03:17 PM, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2015 at 11:12:10AM +0800, Jason Wang wrote:
>> > 
>> > 
>> > On Mon, Apr 27, 2015 at 7:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> > >On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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.
>>> > >
>>> > >This isn't a documented interface, and no guest that I know of does
>>> > >this.  Accordingly, I think we should drop everything except the
>>> > >hw/virtio/virtio-pci.c change.
>> > 
>> > We leave a chance for guest to use such undocumented behavior, so
>> > technically we'd better keep it and it maybe too late for us to fix if we
>> > find such a guest in the future. And consider keeping this compatibility was
>> > really not hard, so I suggest to include this.
> Reminds me of  https://xkcd.com/1172/
> We don't do this kind of thing.

Ok, but let's consider for management:

If we don't do this, consider src has qemu 2.4 and dst has qemu 2.3.
Then libvirt can create 2.3 machine on src with more than 64 queues.
What happens if it want to migrate to dst? I believe we don't want to
teach libvirt about the queue limit for each machine type?
Michael S. Tsirkin May 13, 2015, 8:16 a.m. UTC | #7
On Wed, May 13, 2015 at 03:47:51PM +0800, Jason Wang wrote:
> 
> 
> On 04/28/2015 03:17 PM, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2015 at 11:12:10AM +0800, Jason Wang wrote:
> >> > 
> >> > 
> >> > On Mon, Apr 27, 2015 at 7:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>> > >On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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.
> >>> > >
> >>> > >This isn't a documented interface, and no guest that I know of does
> >>> > >this.  Accordingly, I think we should drop everything except the
> >>> > >hw/virtio/virtio-pci.c change.
> >> > 
> >> > We leave a chance for guest to use such undocumented behavior, so
> >> > technically we'd better keep it and it maybe too late for us to fix if we
> >> > find such a guest in the future. And consider keeping this compatibility was
> >> > really not hard, so I suggest to include this.
> > Reminds me of  https://xkcd.com/1172/
> > We don't do this kind of thing.
> 
> Ok, but let's consider for management:
> 
> If we don't do this, consider src has qemu 2.4 and dst has qemu 2.3.
> Then libvirt can create 2.3 machine on src with more than 64 queues.
> What happens if it want to migrate to dst?
> I believe we don't want to
> teach libvirt about the queue limit for each machine type?

The basic requirement for migration is to supply identical
configuration at both sides. If you don't, migration won't
work, and that's expected.
Eduardo Habkost May 14, 2015, 6:54 p.m. UTC | #8
On Thu, Apr 23, 2015 at 02:21:48PM +0800, 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 on recent Linux host (which supports up to 256 tuntap 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 | 6 +++++-
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
[...]
>  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_3(MachineState *machine)
>  {
> +    ObjectClass *klass = object_class_by_name("virtio-pci-bus");
> +    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
> +
> +    k->queue_max = 64;
>  }
[...]
>  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;
>  }

If you use compat_props for this, you will just need to add it to
HW_COMPAT_2_3 without duplicating the same compat code on all machines.
diff mbox

Patch

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 7d01500..c510cb7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -42,7 +42,11 @@ 
  * configuration space */
 #define VIRTIO_PCI_CONFIG_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_enabled(dev))
 
-#define VIRTIO_PCI_QUEUE_MAX 64
+/* The number was chose to be greater than both the the number of max
+ * vcpus supported by host and the number of max tuntap queues support
+ * by host and also leave some spaces for future.
+ */
+#define VIRTIO_PCI_QUEUE_MAX 1024
 
 static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtIOPCIProxy *dev);