| Message ID | 1426671309-13645-19-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 18, 2015 at 05:35:08PM +0800, Jason Wang wrote: > This patch let msix_init_exclusive_bar() can accept bar_size parameter > other than a hard-coded limit 4096. Then caller can specify a bar_size > depends on msix entries and can use up to 2048 msix entries as PCI > spec allows. To keep migration compatibility, 4096 is used for all > callers and pba were start from half of bar size. > > Cc: Keith Busch <keith.busch@intel.com> > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> Hmm this API looks strange. Caller will then have to have msi specific knowledge. Can't we keep the size iternal to msix.c? > --- > hw/block/nvme.c | 2 +- > hw/misc/ivshmem.c | 2 +- > hw/pci/msix.c | 18 +++++++----------- > hw/virtio/virtio-pci.c | 2 +- > include/hw/pci/msix.h | 2 +- > 5 files changed, 11 insertions(+), 15 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 0f3dfb9..09d7884 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -787,7 +787,7 @@ static int nvme_init(PCIDevice *pci_dev) > pci_register_bar(&n->parent_obj, 0, > PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, > &n->iomem); > - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4); > + msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, 4096); > > id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); > id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index 5d272c8..3e2a2d4 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -631,7 +631,7 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > > static void ivshmem_setup_msi(IVShmemState * s) > { > - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) { > + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, 4096)) { > IVSHMEM_DPRINTF("msix initialization failed\n"); > exit(1); > } > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > index 24de260..9a1894f 100644 > --- a/hw/pci/msix.c > +++ b/hw/pci/msix.c > @@ -291,33 +291,29 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, > } > > int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, > - uint8_t bar_nr) > + uint8_t bar_nr, uint32_t bar_size) > { > int ret; > char *name; > + uint32_t bar_pba_offset = bar_size / 2; Spec says we must give BIR 4k of it's own, but for larger BARs, using up half the BAR just for BIR seems wasteful. > > /* > * Migration compatibility dictates that this remains a 4k > * BAR with the vector table in the lower half and PBA in > * the upper half. Do not use these elsewhere! > */ You've left comment here where it no longer makes any sense. > -#define MSIX_EXCLUSIVE_BAR_SIZE 4096 > -#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0 > -#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2) > -#define MSIX_EXCLUSIVE_CAP_OFFSET 0 > - > - if (nentries * PCI_MSIX_ENTRY_SIZE > MSIX_EXCLUSIVE_BAR_PBA_OFFSET) { > + if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) { > return -EINVAL; > } > > name = g_strdup_printf("%s-msix", dev->name); > - memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, MSIX_EXCLUSIVE_BAR_SIZE); > + memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, bar_size); > g_free(name); > > ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr, > - MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, &dev->msix_exclusive_bar, > - bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET, > - MSIX_EXCLUSIVE_CAP_OFFSET); > + 0, &dev->msix_exclusive_bar, > + bar_nr, bar_pba_offset, > + 0); > if (ret) { > return ret; > } > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 02e3ce8..4a5febb 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -937,7 +937,7 @@ static void virtio_pci_device_plugged(DeviceState *d) > config[PCI_INTERRUPT_PIN] = 1; > > if (proxy->nvectors && > - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { > + msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, 4096)) { > error_report("unable to init msix vectors to %" PRIu32, > proxy->nvectors); > proxy->nvectors = 0; > diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h > index 954d82b..43edebc 100644 > --- a/include/hw/pci/msix.h > +++ b/include/hw/pci/msix.h > @@ -11,7 +11,7 @@ int msix_init(PCIDevice *dev, unsigned short nentries, > unsigned table_offset, MemoryRegion *pba_bar, > uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos); > int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, > - uint8_t bar_nr); > + uint8_t bar_nr, uint32_t bar_size); > > void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len); > > -- > 2.1.0
On Wed, Mar 18, 2015 at 8:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Mar 18, 2015 at 05:35:08PM +0800, Jason Wang wrote: >> This patch let msix_init_exclusive_bar() can accept bar_size >> parameter >> other than a hard-coded limit 4096. Then caller can specify a >> bar_size >> depends on msix entries and can use up to 2048 msix entries as PCI >> spec allows. To keep migration compatibility, 4096 is used for all >> callers and pba were start from half of bar size. >> >> Cc: Keith Busch <keith.busch@intel.com> >> Cc: Kevin Wolf <kwolf@redhat.com> >> Cc: Stefan Hajnoczi <stefanha@redhat.com> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > > Hmm this API looks strange. Caller will then have to have > msi specific knowledge. > Can't we keep the size iternal to msix.c? Looks like we can, but still need an extra parameter e.g a boolean compat_size to let msix_init_exclusive_bar() to use 4096 as bar size. > > >> --- >> hw/block/nvme.c | 2 +- >> hw/misc/ivshmem.c | 2 +- >> hw/pci/msix.c | 18 +++++++----------- >> hw/virtio/virtio-pci.c | 2 +- >> include/hw/pci/msix.h | 2 +- >> 5 files changed, 11 insertions(+), 15 deletions(-) >> >> diff --git a/hw/block/nvme.c b/hw/block/nvme.c >> index 0f3dfb9..09d7884 100644 >> --- a/hw/block/nvme.c >> +++ b/hw/block/nvme.c >> @@ -787,7 +787,7 @@ static int nvme_init(PCIDevice *pci_dev) >> pci_register_bar(&n->parent_obj, 0, >> PCI_BASE_ADDRESS_SPACE_MEMORY | >> PCI_BASE_ADDRESS_MEM_TYPE_64, >> &n->iomem); >> - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4); >> + msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, >> 4096); >> >> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); >> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + >> PCI_SUBSYSTEM_VENDOR_ID)); >> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c >> index 5d272c8..3e2a2d4 100644 >> --- a/hw/misc/ivshmem.c >> +++ b/hw/misc/ivshmem.c >> @@ -631,7 +631,7 @@ static uint64_t ivshmem_get_size(IVShmemState * >> s) { >> >> static void ivshmem_setup_msi(IVShmemState * s) >> { >> - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) { >> + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, >> 4096)) { >> IVSHMEM_DPRINTF("msix initialization failed\n"); >> exit(1); >> } >> diff --git a/hw/pci/msix.c b/hw/pci/msix.c >> index 24de260..9a1894f 100644 >> --- a/hw/pci/msix.c >> +++ b/hw/pci/msix.c >> @@ -291,33 +291,29 @@ int msix_init(struct PCIDevice *dev, unsigned >> short nentries, >> } >> >> int msix_init_exclusive_bar(PCIDevice *dev, unsigned short >> nentries, >> - uint8_t bar_nr) >> + uint8_t bar_nr, uint32_t bar_size) >> { >> int ret; >> char *name; >> + uint32_t bar_pba_offset = bar_size / 2; > > Spec says we must give BIR 4k of it's own, but for > larger BARs, using up half the BAR just for BIR seems > wasteful. I see, I will make PBA size more accurate. > > >> >> /* >> * Migration compatibility dictates that this remains a 4k >> * BAR with the vector table in the lower half and PBA in >> * the upper half. Do not use these elsewhere! >> */ > > You've left comment here where it no longer makes > any sense. Will remove this. > > >> -#define MSIX_EXCLUSIVE_BAR_SIZE 4096 >> -#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0 >> -#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2) >> -#define MSIX_EXCLUSIVE_CAP_OFFSET 0 >> - >> - if (nentries * PCI_MSIX_ENTRY_SIZE > >> MSIX_EXCLUSIVE_BAR_PBA_OFFSET) { >> + if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) { >> return -EINVAL; >> } >> >> name = g_strdup_printf("%s-msix", dev->name); >> - memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), >> name, MSIX_EXCLUSIVE_BAR_SIZE); >> + memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), >> name, bar_size); >> g_free(name); >> >> ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, >> bar_nr, >> - MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, >> &dev->msix_exclusive_bar, >> - bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET, >> - MSIX_EXCLUSIVE_CAP_OFFSET); >> + 0, &dev->msix_exclusive_bar, >> + bar_nr, bar_pba_offset, >> + 0); >> if (ret) { >> return ret; >> } >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 02e3ce8..4a5febb 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -937,7 +937,7 @@ static void >> virtio_pci_device_plugged(DeviceState *d) >> config[PCI_INTERRUPT_PIN] = 1; >> >> if (proxy->nvectors && >> - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, >> 1)) { >> + msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, >> 1, 4096)) { >> error_report("unable to init msix vectors to %" PRIu32, >> proxy->nvectors); >> proxy->nvectors = 0; >> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h >> index 954d82b..43edebc 100644 >> --- a/include/hw/pci/msix.h >> +++ b/include/hw/pci/msix.h >> @@ -11,7 +11,7 @@ int msix_init(PCIDevice *dev, unsigned short >> nentries, >> unsigned table_offset, MemoryRegion *pba_bar, >> uint8_t pba_bar_nr, unsigned pba_offset, uint8_t >> cap_pos); >> int msix_init_exclusive_bar(PCIDevice *dev, unsigned short >> nentries, >> - uint8_t bar_nr); >> + uint8_t bar_nr, uint32_t bar_size); >> >> void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t >> val, int len); >> >> -- >> 2.1.0
On Thu, Mar 19, 2015 at 01:19:23PM +0800, Jason Wang wrote: > > > On Wed, Mar 18, 2015 at 8:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > >On Wed, Mar 18, 2015 at 05:35:08PM +0800, Jason Wang wrote: > >> This patch let msix_init_exclusive_bar() can accept bar_size parameter > >> other than a hard-coded limit 4096. Then caller can specify a bar_size > >> depends on msix entries and can use up to 2048 msix entries as PCI > >> spec allows. To keep migration compatibility, 4096 is used for all > >> callers and pba were start from half of bar size. > >> Cc: Keith Busch <keith.busch@intel.com> > >> Cc: Kevin Wolf <kwolf@redhat.com> > >> Cc: Stefan Hajnoczi <stefanha@redhat.com> > >> Cc: Michael S. Tsirkin <mst@redhat.com> > >> Signed-off-by: Jason Wang <jasowang@redhat.com> > > > >Hmm this API looks strange. Caller will then have to have > >msi specific knowledge. > >Can't we keep the size iternal to msix.c? > > Looks like we can, but still need an extra parameter e.g a boolean > compat_size to let msix_init_exclusive_bar() to use 4096 as bar size. Why not just figure the size out from # of vectors? > > > > > >> --- > >> hw/block/nvme.c | 2 +- > >> hw/misc/ivshmem.c | 2 +- > >> hw/pci/msix.c | 18 +++++++----------- > >> hw/virtio/virtio-pci.c | 2 +- > >> include/hw/pci/msix.h | 2 +- > >> 5 files changed, 11 insertions(+), 15 deletions(-) > >> diff --git a/hw/block/nvme.c b/hw/block/nvme.c > >> index 0f3dfb9..09d7884 100644 > >> --- a/hw/block/nvme.c > >> +++ b/hw/block/nvme.c > >> @@ -787,7 +787,7 @@ static int nvme_init(PCIDevice *pci_dev) > >> pci_register_bar(&n->parent_obj, 0, > >> PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, > >> &n->iomem); > >> - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4); > >> + msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, 4096); > >> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); > >> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + > >>PCI_SUBSYSTEM_VENDOR_ID)); > >> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > >> index 5d272c8..3e2a2d4 100644 > >> --- a/hw/misc/ivshmem.c > >> +++ b/hw/misc/ivshmem.c > >> @@ -631,7 +631,7 @@ static uint64_t ivshmem_get_size(IVShmemState * s) > >>{ > >> static void ivshmem_setup_msi(IVShmemState * s) > >> { > >> - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) { > >> + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, 4096)) { > >> IVSHMEM_DPRINTF("msix initialization failed\n"); > >> exit(1); > >> } > >> diff --git a/hw/pci/msix.c b/hw/pci/msix.c > >> index 24de260..9a1894f 100644 > >> --- a/hw/pci/msix.c > >> +++ b/hw/pci/msix.c > >> @@ -291,33 +291,29 @@ int msix_init(struct PCIDevice *dev, unsigned > >>short nentries, > >> } > >> int msix_init_exclusive_bar(PCIDevice *dev, unsigned short > >>nentries, > >> - uint8_t bar_nr) > >> + uint8_t bar_nr, uint32_t bar_size) > >> { > >> int ret; > >> char *name; > >> + uint32_t bar_pba_offset = bar_size / 2; > > > >Spec says we must give BIR 4k of it's own, but for > >larger BARs, using up half the BAR just for BIR seems > >wasteful. > > I see, I will make PBA size more accurate. > > > > > > >> /* > >> * Migration compatibility dictates that this remains a 4k > >> * BAR with the vector table in the lower half and PBA in > >> * the upper half. Do not use these elsewhere! > >> */ > > > >You've left comment here where it no longer makes > >any sense. > > Will remove this. > > > > > > >> -#define MSIX_EXCLUSIVE_BAR_SIZE 4096 > >> -#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0 > >> -#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2) > >> -#define MSIX_EXCLUSIVE_CAP_OFFSET 0 > >> - > >> - if (nentries * PCI_MSIX_ENTRY_SIZE > > >>MSIX_EXCLUSIVE_BAR_PBA_OFFSET) { > >> + if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) { > >> return -EINVAL; > >> } > >> name = g_strdup_printf("%s-msix", dev->name); > >> - memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, > >>MSIX_EXCLUSIVE_BAR_SIZE); > >> + memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, > >>bar_size); > >> g_free(name); > >> ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, > >>bar_nr, > >> - MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, > >>&dev->msix_exclusive_bar, > >> - bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET, > >> - MSIX_EXCLUSIVE_CAP_OFFSET); > >> + 0, &dev->msix_exclusive_bar, > >> + bar_nr, bar_pba_offset, > >> + 0); > >> if (ret) { > >> return ret; > >> } > >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > >> index 02e3ce8..4a5febb 100644 > >> --- a/hw/virtio/virtio-pci.c > >> +++ b/hw/virtio/virtio-pci.c > >> @@ -937,7 +937,7 @@ static void virtio_pci_device_plugged(DeviceState > >>*d) > >> config[PCI_INTERRUPT_PIN] = 1; > >> if (proxy->nvectors && > >> - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) > >>{ > >> + msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, > >>4096)) { > >> error_report("unable to init msix vectors to %" PRIu32, > >> proxy->nvectors); > >> proxy->nvectors = 0; > >> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h > >> index 954d82b..43edebc 100644 > >> --- a/include/hw/pci/msix.h > >> +++ b/include/hw/pci/msix.h > >> @@ -11,7 +11,7 @@ int msix_init(PCIDevice *dev, unsigned short > >>nentries, > >> unsigned table_offset, MemoryRegion *pba_bar, > >> uint8_t pba_bar_nr, unsigned pba_offset, uint8_t > >>cap_pos); > >> int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, > >> - uint8_t bar_nr); > >> + uint8_t bar_nr, uint32_t bar_size); > >> void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t > >>val, int len); > >> -- 2.1.0
On Thu, Mar 19, 2015 at 6:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Thu, Mar 19, 2015 at 01:19:23PM +0800, Jason Wang wrote: >> >> >> On Wed, Mar 18, 2015 at 8:52 PM, Michael S. Tsirkin >> <mst@redhat.com> wrote: >> >On Wed, Mar 18, 2015 at 05:35:08PM +0800, Jason Wang wrote: >> >> This patch let msix_init_exclusive_bar() can accept bar_size >> parameter >> >> other than a hard-coded limit 4096. Then caller can specify a >> bar_size >> >> depends on msix entries and can use up to 2048 msix entries as >> PCI >> >> spec allows. To keep migration compatibility, 4096 is used for >> all >> >> callers and pba were start from half of bar size. >> >> Cc: Keith Busch <keith.busch@intel.com> >> >> Cc: Kevin Wolf <kwolf@redhat.com> >> >> Cc: Stefan Hajnoczi <stefanha@redhat.com> >> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> > >> >Hmm this API looks strange. Caller will then have to have >> >msi specific knowledge. >> >Can't we keep the size iternal to msix.c? >> >> Looks like we can, but still need an extra parameter e.g a boolean >> compat_size to let msix_init_exclusive_bar() to use 4096 as bar >> size. > > Why not just figure the size out from # of vectors? Yes, for 2.4 we can figure the size out from the number of vectors.
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 0f3dfb9..09d7884 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -787,7 +787,7 @@ static int nvme_init(PCIDevice *pci_dev) pci_register_bar(&n->parent_obj, 0, PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem); - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4); + msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, 4096); id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index 5d272c8..3e2a2d4 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -631,7 +631,7 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { static void ivshmem_setup_msi(IVShmemState * s) { - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) { + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, 4096)) { IVSHMEM_DPRINTF("msix initialization failed\n"); exit(1); } diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 24de260..9a1894f 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -291,33 +291,29 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, } int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, - uint8_t bar_nr) + uint8_t bar_nr, uint32_t bar_size) { int ret; char *name; + uint32_t bar_pba_offset = bar_size / 2; /* * Migration compatibility dictates that this remains a 4k * BAR with the vector table in the lower half and PBA in * the upper half. Do not use these elsewhere! */ -#define MSIX_EXCLUSIVE_BAR_SIZE 4096 -#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0 -#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2) -#define MSIX_EXCLUSIVE_CAP_OFFSET 0 - - if (nentries * PCI_MSIX_ENTRY_SIZE > MSIX_EXCLUSIVE_BAR_PBA_OFFSET) { + if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) { return -EINVAL; } name = g_strdup_printf("%s-msix", dev->name); - memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, MSIX_EXCLUSIVE_BAR_SIZE); + memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, bar_size); g_free(name); ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr, - MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, &dev->msix_exclusive_bar, - bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET, - MSIX_EXCLUSIVE_CAP_OFFSET); + 0, &dev->msix_exclusive_bar, + bar_nr, bar_pba_offset, + 0); if (ret) { return ret; } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 02e3ce8..4a5febb 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -937,7 +937,7 @@ static void virtio_pci_device_plugged(DeviceState *d) config[PCI_INTERRUPT_PIN] = 1; if (proxy->nvectors && - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { + msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, 4096)) { error_report("unable to init msix vectors to %" PRIu32, proxy->nvectors); proxy->nvectors = 0; diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h index 954d82b..43edebc 100644 --- a/include/hw/pci/msix.h +++ b/include/hw/pci/msix.h @@ -11,7 +11,7 @@ int msix_init(PCIDevice *dev, unsigned short nentries, unsigned table_offset, MemoryRegion *pba_bar, uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos); int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, - uint8_t bar_nr); + uint8_t bar_nr, uint32_t bar_size); void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);
This patch let msix_init_exclusive_bar() can accept bar_size parameter other than a hard-coded limit 4096. Then caller can specify a bar_size depends on msix entries and can use up to 2048 msix entries as PCI spec allows. To keep migration compatibility, 4096 is used for all callers and pba were start from half of bar size. Cc: Keith Busch <keith.busch@intel.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/block/nvme.c | 2 +- hw/misc/ivshmem.c | 2 +- hw/pci/msix.c | 18 +++++++----------- hw/virtio/virtio-pci.c | 2 +- include/hw/pci/msix.h | 2 +- 5 files changed, 11 insertions(+), 15 deletions(-)