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 <pbonz...@redhat.com> > Cc: Richard Henderson <r...@twiddle.net> > Cc: Michael S. Tsirkin <m...@redhat.com> > Cc: Alexander Graf <ag...@suse.de> > Cc: qemu-...@nongnu.org > Signed-off-by: Jason Wang <jasow...@redhat.com>
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. > @@ -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