qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V4 19/19] virtio-pci: introduce auto_msix_bar_si


From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH V4 19/19] virtio-pci: introduce auto_msix_bar_size property
Date: Thu, 19 Mar 2015 13:23:56 +0800



On Wed, Mar 18, 2015 at 8:57 PM, Michael S. Tsirkin <address@hidden> wrote:
On Wed, Mar 18, 2015 at 05:35:09PM +0800, Jason Wang wrote:
 Currently we don't support more than 128 MSI-X vectors for a pci
devices, trying to use vector=129 for a virtio-net-pci device may get: qemu-system-x86_64: -device virtio-net-pci,netdev=hn0,vectors=129:
 unable to init msix vectors to 129
This this because the MSI-X bar size were hard-coded as 4096. So this
 patch introduces boolean auto_msix_bar_size property for
virito-pci devices. Enable this will let the device calculate the msix bar size based on the number of MSI-X entries instead of previous 4096
 hard-coded limit.
This is a must to let virtio-net can up to 256 queues and each queue
 were associated with a specific MSI-X entry.
Cc: Paolo Bonzini <address@hidden>
 Cc: Richard Henderson <address@hidden>
 Cc: Michael S. Tsirkin <address@hidden>
 Cc: Alexander Graf <address@hidden>
 Cc: address@hidden
 Signed-off-by: Jason Wang <address@hidden>

I don't understand what this property does.
What if I *don't* set auto_msix_bar_size?
vectors<=128 works exactly like it dif previously, vectors=129 fails?
Does not seem like useful behaviour, to me.


 ---
  hw/i386/pc_piix.c      |  8 ++++++++
  hw/i386/pc_q35.c       |  8 ++++++++
  hw/ppc/spapr.c         | 11 ++++++++++-
  hw/virtio/virtio-pci.c | 17 +++++++++++++++--
  hw/virtio/virtio-pci.h |  3 +++
  include/hw/compat.h    | 11 +++++++++++
  6 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
 index 0796719..8808500 100644
 --- a/hw/i386/pc_piix.c
 +++ b/hw/i386/pc_piix.c
 @@ -552,6 +552,10 @@ static QEMUMachine pc_i440fx_machine_v2_3 = {
      PC_I440FX_2_3_MACHINE_OPTIONS,
      .name = "pc-i440fx-2.3",
      .init = pc_init_pci_2_3,
 +    .compat_props = (GlobalProperty[]) {
 +        HW_COMPAT_2_3,
 +        { /* end of list */ }
 +    },
  };
#define PC_I440FX_2_2_MACHINE_OPTIONS PC_I440FX_2_3_MACHINE_OPTIONS
 @@ -560,6 +564,10 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
      PC_I440FX_2_2_MACHINE_OPTIONS,
      .name = "pc-i440fx-2.2",
      .init = pc_init_pci_2_2,
 +    .compat_props = (GlobalProperty[]) {
 +        HW_COMPAT_2_2,
 +        { /* end of list */ }
 +    },
  };
#define PC_I440FX_2_1_MACHINE_OPTIONS \
 diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
 index a8a34a4..4a34349 100644
 --- a/hw/i386/pc_q35.c
 +++ b/hw/i386/pc_q35.c
 @@ -448,6 +448,10 @@ static QEMUMachine pc_q35_machine_v2_3 = {
      PC_Q35_2_3_MACHINE_OPTIONS,
      .name = "pc-q35-2.3",
      .init = pc_q35_init_2_3,
 +    .compat_props = (GlobalProperty[]) {
 +        HW_COMPAT_2_3,
 +        { /* end of list */ }
 +    },
  };
#define PC_Q35_2_2_MACHINE_OPTIONS PC_Q35_2_3_MACHINE_OPTIONS
 @@ -456,6 +460,10 @@ static QEMUMachine pc_q35_machine_v2_2 = {
      PC_Q35_2_2_MACHINE_OPTIONS,
      .name = "pc-q35-2.2",
      .init = pc_q35_init_2_2,
 +    .compat_props = (GlobalProperty[]) {
 +        HW_COMPAT_2_2,
 +        { /* end of list */ }
 +    },
  };
#define PC_Q35_2_1_MACHINE_OPTIONS \
 diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
 index 5f25dd3..853a5cc 100644
 --- a/hw/ppc/spapr.c
 +++ b/hw/ppc/spapr.c
 @@ -1794,12 +1794,16 @@ static const TypeInfo spapr_machine_info = {
      },
  };
+#define SPAPR_COMPAT_2_3 \
 +        HW_COMPAT_2_3
 +
  #define SPAPR_COMPAT_2_2 \
 +        SPAPR_COMPAT_2_3, \
          {\
              .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
              .property = "mem_win_size",\
              .value    = "0x20000000",\
 -        }
 +        } \
#define SPAPR_COMPAT_2_1 \
          SPAPR_COMPAT_2_2
@@ -1883,10 +1887,15 @@ static const TypeInfo spapr_machine_2_2_info = { static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
  {
 +    static GlobalProperty compat_props[] = {
 +        SPAPR_COMPAT_2_3,
 +        { /* end of list */ }
 +    };
      MachineClass *mc = MACHINE_CLASS(oc);
mc->name = "pseries-2.3";
      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
 +    mc->compat_props = compat_props;
  }
static const TypeInfo spapr_machine_2_3_info = {
 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index 4a5febb..f4cd405 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
@@ -925,7 +925,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
      VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
      VirtioBusState *bus = &proxy->bus;
      uint8_t *config;
 -    uint32_t size;
 +    uint32_t size, bar_size;
config = proxy->pci_dev.config;
      if (proxy->class_code) {
@@ -936,8 +936,19 @@ static void virtio_pci_device_plugged(DeviceState *d) pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
      config[PCI_INTERRUPT_PIN] = 1;
+ if (proxy->flags & VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE) {
 +        bar_size = proxy->nvectors * PCI_MSIX_ENTRY_SIZE * 2;
 +        if (bar_size & (bar_size - 1)) {
 +            bar_size = 1 << qemu_fls(bar_size);
 +        }
 +    } else {
 +        /* For migration compatibility */
 +        bar_size = 4096;
 +    }
 +
      if (proxy->nvectors &&
- msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, 4096)) { + msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1,
 +                                bar_size)) {
          error_report("unable to init msix vectors to %" PRIu32,
                       proxy->nvectors);
          proxy->nvectors = 0;


As I expected, msix format stuff spreads out to virtio.
Consider "vectors * PCI_MSIX_ENTRY_SIZE * 2"
That's because you use half the BAR for BIR in msix.c
So any change will have to be done in two places,
that's bad.

Yes, will move to msix.c by passing the compat flags to msix_init_exclusive_bar().



@@ -1370,6 +1381,8 @@ static const TypeInfo virtio_serial_pci_info = {
  static Property virtio_net_properties[] = {
      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                      VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
 +    DEFINE_PROP_BIT("auto_msix_bar_size", VirtIOPCIProxy, flags,
 +                    VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT, true),
      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
      DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
      DEFINE_PROP_END_OF_LIST(),
 diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
 index 3bac016..82a6782 100644
 --- a/hw/virtio/virtio-pci.h
 +++ b/hw/virtio/virtio-pci.h
 @@ -62,6 +62,9 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
   * vcpu thread using ioeventfd for some devices. */
  #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
#define VIRTIO_PCI_FLAG_USE_IOEVENTFD (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
 +#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT 2
 +#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE \
 +    (1 << VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT)
typedef struct {
      MSIMessage msg;
 diff --git a/include/hw/compat.h b/include/hw/compat.h
 index 313682a..3186275 100644
 --- a/include/hw/compat.h
 +++ b/include/hw/compat.h
 @@ -1,7 +1,18 @@
  #ifndef HW_COMPAT_H
  #define HW_COMPAT_H
+#define HW_COMPAT_2_3 \
 +        {\
 +            .driver   = "virtio-net-pci",\
 +            .property = "auto_msix_bar_size",\
 +            .value    = "off",\
 +        }
 +
 +#define HW_COMPAT_2_2 \
 +        HW_COMPAT_2_3
 +
  #define HW_COMPAT_2_1 \
 +        HW_COMPAT_2_2, \
          {\
              .driver   = "intel-hda",\
              .property = "old_msi_addr",\
-- 2.1.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]