Re: [Qemu-devel] [PATCH v2 3/5] pci: Convert msix_init() to Error and fix callers to check it

[Markus Armbruster <armbru@redhat.com>]

Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> msix_init() reports errors with error_report(), which is wrong when
> it's used in realize().  The same issue was fixed for msi_init() in
> commit 1108b2f.
>
> For some devices like e1000e & vmxnet3 who won't fail because of
> msi_init's failure, suppress the error report by passing NULL error object.
>
> CC: Jiri Pirko <jiri@resnulli.us>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Dmitry Fleytman <dmitry@daynix.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Hannes Reinecke <hare@suse.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Alex Williamson <alex.williamson@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
>  hw/block/nvme.c        |  5 +++-
>  hw/misc/ivshmem.c      |  8 +++---
>  hw/net/e1000e.c        |  2 +-
>  hw/net/rocker/rocker.c |  4 ++-
>  hw/net/vmxnet3.c       | 42 +++++++++--------------------
>  hw/pci/msix.c          | 31 ++++++++++++++++++----
>  hw/scsi/megasas.c      | 26 ++++++++++++++----
>  hw/usb/hcd-xhci.c      | 71 ++++++++++++++++++++++++++++++--------------------
>  hw/vfio/pci.c          |  7 +++--
>  hw/virtio/virtio-pci.c |  8 ++----
>  include/hw/pci/msix.h  |  5 ++--
>  11 files changed, 125 insertions(+), 84 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index cef3bb4..ae84dc7 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -829,6 +829,7 @@ static int nvme_init(PCIDevice *pci_dev)
>  {
>      NvmeCtrl *n = NVME(pci_dev);
>      NvmeIdCtrl *id = &n->id_ctrl;
> +    Error *err = NULL;
>  
>      int i;
>      int64_t bs_size;
> @@ -870,7 +871,9 @@ 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);
> +    if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, &err)) {
> +        error_report_err(err);
> +    }
>  
>      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 40a2ebc..a1060ec 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -750,13 +750,13 @@ static void ivshmem_reset(DeviceState *d)
>      }
>  }
>  
> -static int ivshmem_setup_interrupts(IVShmemState *s)
> +static int ivshmem_setup_interrupts(IVShmemState *s, Error **errp)
>  {
>      /* allocate QEMU callback data for receiving interrupts */
>      s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
>  
>      if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
> -        if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
> +        if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, errp)) {
>              return -1;
>          }
>  
> @@ -897,8 +897,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>          qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive,
>                                ivshmem_read, NULL, s);
>  
> -        if (ivshmem_setup_interrupts(s) < 0) {
> -            error_setg(errp, "failed to initialize interrupts");
> +        if (ivshmem_setup_interrupts(s, errp) < 0) {
> +            error_prepend(errp, "Failed to initialize interrupts: ");
>              return;
>          }
>      }
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index bad43f4..72aad21 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -292,7 +292,7 @@ e1000e_init_msix(E1000EState *s)
>                          E1000E_MSIX_IDX, E1000E_MSIX_TABLE,
>                          &s->msix,
>                          E1000E_MSIX_IDX, E1000E_MSIX_PBA,
> -                        0xA0);
> +                        0xA0, NULL);
>  
>      if (res < 0) {
>          trace_e1000e_msix_init_fail(res);
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 30f2ce4..e421ebb 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1256,14 +1256,16 @@ static int rocker_msix_init(Rocker *r)
>  {
>      PCIDevice *dev = PCI_DEVICE(r);
>      int err;
> +    Error *local_err = NULL;
>  
>      err = msix_init(dev, ROCKER_MSIX_VEC_COUNT(r->fp_ports),
>                      &r->msix_bar,
>                      ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET,
>                      &r->msix_bar,
>                      ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
> -                    0);
> +                    0, &local_err);
>      if (err) {
> +        error_report_err(local_err);
>          return err;
>      }
>  
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 90f6943..4824f8d 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2181,32 +2181,6 @@ vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors)
>      return true;
>  }
>  
> -static bool
> -vmxnet3_init_msix(VMXNET3State *s)
> -{
> -    PCIDevice *d = PCI_DEVICE(s);
> -    int res = msix_init(d, VMXNET3_MAX_INTRS,
> -                        &s->msix_bar,
> -                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
> -                        &s->msix_bar,
> -                        VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
> -                        VMXNET3_MSIX_OFFSET(s));
> -
> -    if (0 > res) {
> -        VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
> -        s->msix_used = false;
> -    } else {
> -        if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
> -            VMW_WRPRN("Failed to use MSI-X vectors, error %d", res);
> -            msix_uninit(d, &s->msix_bar, &s->msix_bar);
> -            s->msix_used = false;
> -        } else {
> -            s->msix_used = true;
> -        }
> -    }
> -    return s->msix_used;
> -}
> -
>  static void
>  vmxnet3_cleanup_msix(VMXNET3State *s)
>  {
> @@ -2315,9 +2289,19 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>       * is a programming error. Fall back to INTx silently on -ENOTSUP */
>      assert(!ret || ret == -ENOTSUP);
>  
> -    if (!vmxnet3_init_msix(s)) {
> -        VMW_WRPRN("Failed to initialize MSI-X, configuration is inconsistent.");
> -    }
> +    ret = msix_init(pci_dev, VMXNET3_MAX_INTRS,
> +                    &s->msix_bar,
> +                    VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
> +                    &s->msix_bar,
> +                    VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
> +                    VMXNET3_MSIX_OFFSET(s), NULL);
> +    /* Any error other than -ENOTSUP(board's MSI support is broken)
> +     * is a programming error. Fall back to INTx silently on -ENOTSUP */
> +    assert(!ret || ret == -ENOTSUP);
> +    s->msix_used = !ret;
> +    /* VMXNET3_MAX_INTRS is passed, so it will never fail when mark vector.
> +     * For simplicity, no need to check return value. */
> +    vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS);
>  
>      vmxnet3_net_init(s);

Uh, this is more than just a conversion to Error.  Before, the code
falls back to not using MSI-X on any error, with a warning.  After, it
falls back on ENOTSUP only, silently, and crashes on any other error.
Such a change needs to be documented in the commit message, or be in a
separate patch.  I prefer separate patch.

>  
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index 0aadc0c..568c051 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -21,6 +21,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/xen/xen.h"
>  #include "qemu/range.h"
> +#include "qapi/error.h"
>  
>  #define MSIX_CAP_LENGTH 12
>  
> @@ -238,11 +239,29 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
>      }
>  }
>  
> -/* Initialize the MSI-X structures */
> +/* Make PCI device @dev MSI-X capable
> + * @nentries is the max number of MSI-X vectors that the device support.
> + * @table_bar is the MemoryRegion that MSI-X table structure resides.
> + * @table_bar_nr is number of base address register corresponding to @table_bar.
> + * @table_offset indicates the offset that the MSI-X table structure starts with
> + * in @table_bar.
> + * @pba_bar is the MemoryRegion that the Pending Bit Array structure resides.
> + * @pba_bar_nr is number of base address register corresponding to @pba_bar.
> + * @pba_offset indicates the offset that the Pending Bit Array structure
> + * starts with in @pba_bar.
> + * Non-zero @cap_pos puts capability MSI-X at that offset in PCI config space.
> + * @errp is for returning errors.
> + *
> + * Return 0 on success; set @errp and return -errno on error.
> + * -ENOTSUP means lacking msi support for a msi-capable platform.
> + * -EINVAL means capability overlap, happens when @cap_pos is non-zero,
> + * also means a programming error, except device assignment, which can check
> + * if a real HW is broken.*/
>  int msix_init(struct PCIDevice *dev, unsigned short nentries,
>                MemoryRegion *table_bar, uint8_t table_bar_nr,
>                unsigned table_offset, MemoryRegion *pba_bar,
> -              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos)
> +              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos,
> +              Error **errp)
>  {
>      int cap;
>      unsigned table_size, pba_size;
> @@ -250,6 +269,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
>  
>      /* Nothing to do if MSI is not supported by interrupt controller */
>      if (!msi_nonbroken) {
> +        error_setg(errp, "MSI-X is not supported by interrupt controller");
>          return -ENOTSUP;
>      }
>  
> @@ -267,7 +287,8 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
>          assert(0);
>      }
>  
> -    cap = pci_add_capability(dev, PCI_CAP_ID_MSIX, cap_pos, MSIX_CAP_LENGTH);
> +    cap = pci_add_capability2(dev, PCI_CAP_ID_MSIX,
> +                              cap_pos, MSIX_CAP_LENGTH, errp);
>      if (cap < 0) {
>          return cap;
>      }
> @@ -304,7 +325,7 @@ 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, Error **errp)
>  {
>      int ret;
>      char *name;
> @@ -336,7 +357,7 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
>      ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr,
>                      0, &dev->msix_exclusive_bar,
>                      bar_nr, bar_pba_offset,
> -                    0);
> +                    0, errp);
>      if (ret) {
>          return ret;
>      }
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index e968302..6d45025 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2349,16 +2349,32 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
>  
>      memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
>                            "megasas-mmio", 0x4000);
> +    if (megasas_use_msix(s)) {
> +        ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
> +                        &s->mmio_io, b->mmio_bar, 0x3800, 0x68, &err);
> +        /* Any error other than -ENOTSUP(board's MSI support is broken)
> +         * is a programming error */
> +        assert(!ret || ret == -ENOTSUP);
> +        if (ret && s->msix == ON_OFF_AUTO_ON) {
> +            /* Can't satisfy user's explicit msix=on request, fail */
> +            error_append_hint(&err, "You have to use msix=auto (default) or "
> +                    "msix=off with this machine type.\n");
> +            /* No instance_finalize method, need to free the resource here */
> +            object_unref(OBJECT(&s->mmio_io));
> +            error_propagate(errp, err);
> +            return;
> +        } else if (ret) {
> +            /* With msix=auto, we fall back to MSI off silently */
> +            s->msix = ON_OFF_AUTO_OFF;
> +            error_free(err);
> +        }
> +    }
> +
>      memory_region_init_io(&s->port_io, OBJECT(s), &megasas_port_ops, s,
>                            "megasas-io", 256);
>      memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
>                            "megasas-queue", 0x40000);
>  
> -    if (megasas_use_msix(s) &&
> -        msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
> -                  &s->mmio_io, b->mmio_bar, 0x3800, 0x68)) {
> -        s->msix = ON_OFF_AUTO_OFF;
> -    }

Before your patch, msix=on behaves just like msix=auto.

Afterwards, msix=on fails when MSI-X can't be enabled.

That's a good change, but it needs to be documented in the commit
message, or be in a separate patch.  I prefer separate patch.

>      if (pci_is_express(dev)) {
>          pcie_endpoint_cap_init(dev, 0xa0);
>      }
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 188f954..4280c5d 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3594,25 +3594,6 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>      dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
>      dev->config[0x60] = 0x30; /* release number */
>  
> -    usb_xhci_init(xhci);
> -
> -    if (xhci->msi != ON_OFF_AUTO_OFF) {
> -        ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
> -        /* Any error other than -ENOTSUP(board's MSI support is broken)
> -         * is a programming error */
> -        assert(!ret || ret == -ENOTSUP);
> -        if (ret && xhci->msi == ON_OFF_AUTO_ON) {
> -            /* Can't satisfy user's explicit msi=on request, fail */
> -            error_append_hint(&err, "You have to use msi=auto (default) or "
> -                    "msi=off with this machine type.\n");
> -            error_propagate(errp, err);
> -            return;
> -        }
> -        assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
> -        /* With msi=auto, we fall back to MSI off silently */
> -        error_free(err);
> -    }
> -
>      if (xhci->numintrs > MAXINTRS) {
>          xhci->numintrs = MAXINTRS;
>      }
> @@ -3622,21 +3603,60 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>      if (xhci->numintrs < 1) {
>          xhci->numintrs = 1;
>      }
> +
>      if (xhci->numslots > MAXSLOTS) {
>          xhci->numslots = MAXSLOTS;
>      }
>      if (xhci->numslots < 1) {
>          xhci->numslots = 1;
>      }
> +
>      if (xhci_get_flag(xhci, XHCI_FLAG_ENABLE_STREAMS)) {
>          xhci->max_pstreams_mask = 7; /* == 256 primary streams */
>      } else {
>          xhci->max_pstreams_mask = 0;
>      }
>  
> -    xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
> +    if (xhci->msi != ON_OFF_AUTO_OFF) {
> +        ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
> +        /* Any error other than -ENOTSUP(board's MSI support is broken)
> +         * is a programming error */
> +        assert(!ret || ret == -ENOTSUP);
> +        if (ret && xhci->msi == ON_OFF_AUTO_ON) {
> +            /* Can't satisfy user's explicit msi=on request, fail */
> +            error_append_hint(&err, "You have to use msi=auto (default) or "
> +                    "msi=off with this machine type.\n");
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
> +        /* With msi=auto, we fall back to MSI off silently */
> +        error_free(err);
> +    }

Can you explain why you're moving this code?

>  
>      memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
> +    if (xhci->msix != ON_OFF_AUTO_OFF) {
> +        ret = msix_init(dev, xhci->numintrs,
> +                        &xhci->mem, 0, OFF_MSIX_TABLE,
> +                        &xhci->mem, 0, OFF_MSIX_PBA,
> +                        0x90, &err);
> +        /* Any error other than -ENOTSUP(board's MSI support is broken)
> +         * is a programming error */
> +        assert(!ret || ret == -ENOTSUP);
> +        if (ret && xhci->msix == ON_OFF_AUTO_ON) {
> +            /* Can't satisfy user's explicit msix=on request, fail */
> +            error_append_hint(&err, "You have to use msix=auto (default) or "
> +                    "msic=off with this machine type.\n");
> +            /* No instance_finalize method, need to free the resource here */
> +            object_unref(OBJECT(&xhci->mem));
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        assert(!err || xhci->msix == ON_OFF_AUTO_AUTO);
> +        /* With msix=auto, we fall back to MSI off silently */
> +        error_free(err);
> +    }
> +
>      memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
>                            "capabilities", LEN_CAP);
>      memory_region_init_io(&xhci->mem_oper, OBJECT(xhci), &xhci_oper_ops, xhci,
> @@ -3664,19 +3684,14 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>                       PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64,
>                       &xhci->mem);
>  
> +    usb_xhci_init(xhci);
> +    xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
> +
>      if (pci_bus_is_express(dev->bus) ||
>          xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
>          ret = pcie_endpoint_cap_init(dev, 0xa0);
>          assert(ret >= 0);
>      }
> -
> -    if (xhci->msix != ON_OFF_AUTO_OFF) {
> -        /* TODO check for errors */
> -        msix_init(dev, xhci->numintrs,
> -                  &xhci->mem, 0, OFF_MSIX_TABLE,
> -                  &xhci->mem, 0, OFF_MSIX_PBA,
> -                  0x90);
> -    }

You're resolving the TODO.  Good, but it needs to be documented in the
commit message, or be in a separate patch.

>  }
>  
>  static void usb_xhci_exit(PCIDevice *dev)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 7bfa17c..87f4e11 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1349,6 +1349,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev)
>  static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
>  {
>      int ret;
> +    Error *err = NULL;
>  
>      vdev->msix->pending = g_malloc0(BITS_TO_LONGS(vdev->msix->entries) *
>                                      sizeof(unsigned long));
> @@ -1356,12 +1357,14 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
>                      vdev->bars[vdev->msix->table_bar].region.mem,
>                      vdev->msix->table_bar, vdev->msix->table_offset,
>                      vdev->bars[vdev->msix->pba_bar].region.mem,
> -                    vdev->msix->pba_bar, vdev->msix->pba_offset, pos);
> +                    vdev->msix->pba_bar, vdev->msix->pba_offset, pos,
> +                    &err);
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
>              return 0;
>          }
> -        error_report("vfio: msix_init failed");
> +        error_prepend(&err, "vfio: msix_init failed: ");
> +        error_report_err(err);
>          return ret;
>      }
>  
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 755f921..2e6b9bc 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1657,13 +1657,9 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
>  
>      if (proxy->nvectors) {
>          int err = msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors,
> -                                          proxy->msix_bar);
> +                                          proxy->msix_bar, errp);
>          if (err) {
> -            /* Notice when a system that supports MSIx can't initialize it.  */
> -            if (err != -ENOTSUP) {
> -                error_report("unable to init msix vectors to %" PRIu32,
> -                             proxy->nvectors);
> -            }
> +            error_report_err(*errp);
>              proxy->nvectors = 0;
>          }
>      }
> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
> index 048a29d..1f27658 100644
> --- a/include/hw/pci/msix.h
> +++ b/include/hw/pci/msix.h
> @@ -9,9 +9,10 @@ MSIMessage msix_get_message(PCIDevice *dev, unsigned int vector);
>  int msix_init(PCIDevice *dev, unsigned short nentries,
>                MemoryRegion *table_bar, uint8_t table_bar_nr,
>                unsigned table_offset, MemoryRegion *pba_bar,
> -              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos);
> +              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos,
> +              Error **errp);
>  int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
> -                            uint8_t bar_nr);
> +                            uint8_t bar_nr, Error **errp);
>  
>  void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);