From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEsTR-0000kO-6M for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:14:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bEsTN-0002zl-Qg for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:14:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:39306) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEsTN-0002zY-CA for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:14:13 -0400 References: <1466403224-16604-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1466403224-16604-9-git-send-email-caoj.fnst@cn.fujitsu.com> From: Hannes Reinecke Message-ID: <576789B4.7010906@suse.de> Date: Mon, 20 Jun 2016 08:14:12 +0200 MIME-Version: 1.0 In-Reply-To: <1466403224-16604-9-git-send-email-caoj.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v9 08/13] pci: Convert msi_init() to Error and fix callers to check it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin , qemu-devel@nongnu.org Cc: Gerd Hoffmann , John Snow , Dmitry Fleytman , Jason Wang , "Michael S. Tsirkin" , Paolo Bonzini , Alex Williamson , Markus Armbruster , Marcel Apfelbaum On 06/20/2016 08:13 AM, Cao jin wrote: > msi_init() reports errors with error_report(), which is wrong > when it's used in realize(). >=20 > Fix by converting it to Error. >=20 > Fix its callers to handle failure instead of ignoring it. >=20 > For those callers who don't handle the failure, it might happen: > when user want msi on, but he doesn't get what he want because of > msi_init fails silently. >=20 > cc: Gerd Hoffmann > cc: John Snow > cc: Dmitry Fleytman > cc: Jason Wang > cc: Michael S. Tsirkin > cc: Hannes Reinecke > cc: Paolo Bonzini > cc: Alex Williamson > cc: Markus Armbruster > cc: Marcel Apfelbaum >=20 > Reviewed-by: Markus Armbruster > Signed-off-by: Cao jin > --- > hw/audio/intel-hda.c | 24 ++++++++++++++++++++---- > hw/ide/ich.c | 7 +++++-- > hw/net/e1000e.c | 8 ++------ > hw/net/vmxnet3.c | 37 ++++++++++++------------------= ------- > hw/pci-bridge/ioh3420.c | 6 +++++- > hw/pci-bridge/pci_bridge_dev.c | 20 ++++++++++++++++---- > hw/pci-bridge/xio3130_downstream.c | 6 +++++- > hw/pci-bridge/xio3130_upstream.c | 6 +++++- > hw/pci/msi.c | 11 ++++++++--- > hw/scsi/megasas.c | 26 +++++++++++++++++++++----- > hw/scsi/mptsas.c | 31 ++++++++++++++++++++++++------= - > hw/scsi/vmw_pvscsi.c | 2 +- > hw/usb/hcd-xhci.c | 23 +++++++++++++++++++---- > hw/vfio/pci.c | 7 +++++-- > include/hw/pci/msi.h | 3 ++- > 15 files changed, 150 insertions(+), 67 deletions(-) >=20 > diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c > index 6b4dda0..82101f8 100644 > --- a/hw/audio/intel-hda.c > +++ b/hw/audio/intel-hda.c > @@ -1135,6 +1135,8 @@ static void intel_hda_realize(PCIDevice *pci, Err= or **errp) > { > IntelHDAState *d =3D INTEL_HDA(pci); > uint8_t *conf =3D d->pci.config; > + Error *err =3D NULL; > + int ret; > =20 > d->name =3D object_get_typename(OBJECT(d)); > =20 > @@ -1143,13 +1145,27 @@ static void intel_hda_realize(PCIDevice *pci, E= rror **errp) > /* HDCTL off 0x40 bit 0 selects signaling mode (1-HDA, 0 - Ac97) 1= 8.1.19 */ > conf[0x40] =3D 0x01; > =20 > + if (d->msi !=3D ON_OFF_AUTO_OFF) { > + ret =3D msi_init(&d->pci, d->old_msi_addr ? 0x50 : 0x60, > + 1, true, false, &err); > + /* Any error other than -ENOTSUP(board's MSI support is broken= ) > + * is a programming error */ > + assert(!ret || ret =3D=3D -ENOTSUP); > + if (ret && d->msi =3D=3D ON_OFF_AUTO_ON) { > + /* Can't satisfy user's explicit msi=3Don request, fail */ > + error_append_hint(&err, "You have to use msi=3Dauto (defau= lt) or " > + "msi=3Doff with this machine type.\n"); > + error_propagate(errp, err); > + return; > + } > + assert(!err || d->msi =3D=3D ON_OFF_AUTO_AUTO); > + /* With msi=3Dauto, we fall back to MSI off silently */ > + error_free(err); > + } > + > memory_region_init_io(&d->mmio, OBJECT(d), &intel_hda_mmio_ops, d, > "intel-hda", 0x4000); > pci_register_bar(&d->pci, 0, 0, &d->mmio); > - if (d->msi !=3D ON_OFF_AUTO_OFF) { > - /* TODO check for errors */ > - msi_init(&d->pci, d->old_msi_addr ? 0x50 : 0x60, 1, true, fals= e); > - } > =20 > hda_codec_bus_init(DEVICE(pci), &d->codecs, sizeof(d->codecs), > intel_hda_response, intel_hda_xfer); > diff --git a/hw/ide/ich.c b/hw/ide/ich.c > index 0a13334..920ec27 100644 > --- a/hw/ide/ich.c > +++ b/hw/ide/ich.c > @@ -68,7 +68,6 @@ > #include > #include "sysemu/block-backend.h" > #include "sysemu/dma.h" > - > #include > #include > =20 > @@ -111,6 +110,7 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, E= rror **errp) > int sata_cap_offset; > uint8_t *sata_cap; > d =3D ICH_AHCI(dev); > + int ret; > =20 > ahci_realize(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6)= ; > =20 > @@ -146,7 +146,10 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, = Error **errp) > /* Although the AHCI 1.3 specification states that the first capab= ility > * should be PMCAP, the Intel ICH9 data sheet specifies that the I= CH9 > * AHCI device puts the MSI capability first, pointing to 0x80. */ > - msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); > + ret =3D msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false, 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 =3D=3D -ENOTSUP); > } > =20 > static void pci_ich9_uninit(PCIDevice *dev) > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c > index 692283f..a06d184 100644 > --- a/hw/net/e1000e.c > +++ b/hw/net/e1000e.c > @@ -268,13 +268,9 @@ e1000e_init_msi(E1000EState *s) > { > int res; > =20 > - res =3D msi_init(PCI_DEVICE(s), > - 0xD0, /* MSI capability offset */ > - 1, /* MAC MSI interrupts */ > - true, /* 64-bit message addresses supported */ > - false); /* Per vector mask supported */ > + res =3D msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL); > =20 > - if (res > 0) { > + if (!res) { > s->intr_state |=3D E1000E_USE_MSI; > } else { > trace_e1000e_msi_init_fail(res); > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index d978976..63f8904 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -2197,27 +2197,6 @@ vmxnet3_cleanup_msix(VMXNET3State *s) > } > } > =20 > -#define VMXNET3_USE_64BIT (true) > -#define VMXNET3_PER_VECTOR_MASK (false) > - > -static bool > -vmxnet3_init_msi(VMXNET3State *s) > -{ > - PCIDevice *d =3D PCI_DEVICE(s); > - int res; > - > - res =3D msi_init(d, VMXNET3_MSI_OFFSET(s), VMXNET3_MAX_NMSIX_INTRS= , > - VMXNET3_USE_64BIT, VMXNET3_PER_VECTOR_MASK); > - if (0 > res) { > - VMW_WRPRN("Failed to initialize MSI, error %d", res); > - s->msi_used =3D false; > - } else { > - s->msi_used =3D true; > - } > - > - return s->msi_used; > -} > - > static void > vmxnet3_cleanup_msi(VMXNET3State *s) > { > @@ -2279,10 +2258,15 @@ static uint64_t vmxnet3_device_serial_num(VMXNE= T3State *s) > return dsn_payload; > } > =20 > + > +#define VMXNET3_USE_64BIT (true) > +#define VMXNET3_PER_VECTOR_MASK (false) > + > static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) > { > DeviceState *dev =3D DEVICE(pci_dev); > VMXNET3State *s =3D VMXNET3(pci_dev); > + int ret; > =20 > VMW_CBPRN("Starting init..."); > =20 > @@ -2306,14 +2290,17 @@ static void vmxnet3_pci_realize(PCIDevice *pci_= dev, Error **errp) > /* Interrupt pin A */ > pci_dev->config[PCI_INTERRUPT_PIN] =3D 0x01; > =20 > + ret =3D msi_init(pci_dev, VMXNET3_MSI_OFFSET(s), VMXNET3_MAX_NMSIX= _INTRS, > + VMXNET3_USE_64BIT, VMXNET3_PER_VECTOR_MASK, 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 =3D=3D -ENOTSUP); > + s->msi_used =3D !ret; > + > if (!vmxnet3_init_msix(s)) { > VMW_WRPRN("Failed to initialize MSI-X, configuration is incons= istent."); > } > =20 > - if (!vmxnet3_init_msi(s)) { > - VMW_WRPRN("Failed to initialize MSI, configuration is inconsis= tent."); > - } > - > vmxnet3_net_init(s); > =20 > if (pci_is_express(pci_dev)) { > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c > index b4a7806..93c6f0b 100644 > --- a/hw/pci-bridge/ioh3420.c > +++ b/hw/pci-bridge/ioh3420.c > @@ -25,6 +25,7 @@ > #include "hw/pci/msi.h" > #include "hw/pci/pcie.h" > #include "ioh3420.h" > +#include "qapi/error.h" > =20 > #define PCI_DEVICE_ID_IOH_EPORT 0x3420 /* D0:F0 express mode = */ > #define PCI_DEVICE_ID_IOH_REV 0x2 > @@ -97,6 +98,7 @@ static int ioh3420_initfn(PCIDevice *d) > PCIEPort *p =3D PCIE_PORT(d); > PCIESlot *s =3D PCIE_SLOT(d); > int rc; > + Error *err =3D NULL; > =20 > pci_bridge_initfn(d, TYPE_PCIE_BUS); > pcie_port_init_reg(d); > @@ -109,8 +111,10 @@ static int ioh3420_initfn(PCIDevice *d) > =20 > rc =3D msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR, > IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT, > - IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT); > + IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT, = &err); > if (rc < 0) { > + assert(rc =3D=3D -ENOTSUP); > + error_report_err(err); > goto err_bridge; > } > =20 > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_= dev.c > index 0fbecc4..5dbd933 100644 > --- a/hw/pci-bridge/pci_bridge_dev.c > +++ b/hw/pci-bridge/pci_bridge_dev.c > @@ -54,6 +54,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev) > PCIBridge *br =3D PCI_BRIDGE(dev); > PCIBridgeDev *bridge_dev =3D PCI_BRIDGE_DEV(dev); > int err; > + Error *local_err =3D NULL; > =20 > pci_bridge_initfn(dev, TYPE_PCI_BUS); > =20 > @@ -75,12 +76,23 @@ static int pci_bridge_dev_initfn(PCIDevice *dev) > goto slotid_error; > } > =20 > - if (bridge_dev->msi !=3D ON_OFF_AUTO_OFF && > - msi_nonbroken) { > - err =3D msi_init(dev, 0, 1, true, true); > - if (err < 0) { > + if (bridge_dev->msi !=3D ON_OFF_AUTO_OFF) { > + /* it means SHPC exists, because MSI is needed by SHPC */ > + > + err =3D msi_init(dev, 0, 1, true, true, &local_err); > + /* Any error other than -ENOTSUP(board's MSI support is broken= ) > + * is a programming error */ > + assert(!err || err =3D=3D -ENOTSUP); > + if (err && bridge_dev->msi =3D=3D ON_OFF_AUTO_ON) { > + /* Can't satisfy user's explicit msi=3Don request, fail */ > + error_append_hint(&local_err, "You have to use msi=3Dauto = (default) " > + "or msi=3Doff with this machine type.\n"); > + error_report_err(local_err); > goto msi_error; > } > + assert(!local_err || bridge_dev->msi =3D=3D ON_OFF_AUTO_AUTO); > + /* With msi=3Dauto, we fall back to MSI off silently */ > + error_free(local_err); > } > =20 > if (shpc_present(dev)) { > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130= _downstream.c > index e6d653d..f6149a3 100644 > --- a/hw/pci-bridge/xio3130_downstream.c > +++ b/hw/pci-bridge/xio3130_downstream.c > @@ -24,6 +24,7 @@ > #include "hw/pci/msi.h" > #include "hw/pci/pcie.h" > #include "xio3130_downstream.h" > +#include "qapi/error.h" > =20 > #define PCI_DEVICE_ID_TI_XIO3130D 0x8233 /* downstream port */ > #define XIO3130_REVISION 0x1 > @@ -60,14 +61,17 @@ static int xio3130_downstream_initfn(PCIDevice *d) > PCIEPort *p =3D PCIE_PORT(d); > PCIESlot *s =3D PCIE_SLOT(d); > int rc; > + Error *err =3D NULL; > =20 > pci_bridge_initfn(d, TYPE_PCIE_BUS); > pcie_port_init_reg(d); > =20 > rc =3D msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR, > XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT, > - XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT)= ; > + XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,= &err); > if (rc < 0) { > + assert(rc =3D=3D -ENOTSUP); > + error_report_err(err); > goto err_bridge; > } > =20 > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_u= pstream.c > index d976844..487edac 100644 > --- a/hw/pci-bridge/xio3130_upstream.c > +++ b/hw/pci-bridge/xio3130_upstream.c > @@ -24,6 +24,7 @@ > #include "hw/pci/msi.h" > #include "hw/pci/pcie.h" > #include "xio3130_upstream.h" > +#include "qapi/error.h" > =20 > #define PCI_DEVICE_ID_TI_XIO3130U 0x8232 /* upstream port */ > #define XIO3130_REVISION 0x2 > @@ -56,14 +57,17 @@ static int xio3130_upstream_initfn(PCIDevice *d) > { > PCIEPort *p =3D PCIE_PORT(d); > int rc; > + Error *err =3D NULL; > =20 > pci_bridge_initfn(d, TYPE_PCIE_BUS); > pcie_port_init_reg(d); > =20 > rc =3D msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR, > XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT, > - XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT)= ; > + XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,= &err); > if (rc < 0) { > + assert(rc =3D=3D -ENOTSUP); > + error_report_err(err); > goto err_bridge; > } > =20 > diff --git a/hw/pci/msi.c b/hw/pci/msi.c > index ed79225..a87b227 100644 > --- a/hw/pci/msi.c > +++ b/hw/pci/msi.c > @@ -22,6 +22,7 @@ > #include "hw/pci/msi.h" > #include "hw/xen/xen.h" > #include "qemu/range.h" > +#include "qapi/error.h" > =20 > /* PCI_MSI_ADDRESS_LO */ > #define PCI_MSI_ADDRESS_LO_MASK (~0x3) > @@ -173,7 +174,8 @@ bool msi_enabled(const PCIDevice *dev) > * If @msi64bit, make the device capable of sending a 64-bit message > * address. > * If @msi_per_vector_mask, make the device support per-vector masking= . > - * Return 0 on success, return -errno on error. > + * @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 @offset is non-zero, > @@ -181,7 +183,8 @@ bool msi_enabled(const PCIDevice *dev) > * if a real HW is broken. > */ > int msi_init(struct PCIDevice *dev, uint8_t offset, > - unsigned int nr_vectors, bool msi64bit, bool msi_per_vect= or_mask) > + unsigned int nr_vectors, bool msi64bit, > + bool msi_per_vector_mask, Error **errp) > { > unsigned int vectors_order; > uint16_t flags; > @@ -189,6 +192,7 @@ int msi_init(struct PCIDevice *dev, uint8_t offset, > int config_offset; > =20 > if (!msi_nonbroken) { > + error_setg(errp, "MSI is not supported by interrupt controller= "); > return -ENOTSUP; > } > =20 > @@ -212,7 +216,8 @@ int msi_init(struct PCIDevice *dev, uint8_t offset, > } > =20 > cap_size =3D msi_cap_sizeof(flags); > - config_offset =3D pci_add_capability(dev, PCI_CAP_ID_MSI, offset, = cap_size); > + config_offset =3D pci_add_capability2(dev, PCI_CAP_ID_MSI, offset, > + cap_size, errp); > if (config_offset < 0) { > return config_offset; > } > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c > index 7a4301b..345783d 100644 > --- a/hw/scsi/megasas.c > +++ b/hw/scsi/megasas.c > @@ -29,7 +29,7 @@ > #include "hw/scsi/scsi.h" > #include "block/scsi.h" > #include "trace.h" > - > +#include "qapi/error.h" > #include "mfi.h" > =20 > #define MEGASAS_VERSION_GEN1 "1.70" > @@ -2333,6 +2333,8 @@ static void megasas_scsi_realize(PCIDevice *dev, = Error **errp) > MegasasBaseClass *b =3D MEGASAS_DEVICE_GET_CLASS(s); > uint8_t *pci_conf; > int i, bar_type; > + Error *err =3D NULL; > + int ret; > =20 > pci_conf =3D dev->config; > =20 > @@ -2341,6 +2343,24 @@ static void megasas_scsi_realize(PCIDevice *dev,= Error **errp) > /* Interrupt pin 1 */ > pci_conf[PCI_INTERRUPT_PIN] =3D 0x01; > =20 > + if (megasas_use_msi(s)) { > + ret =3D msi_init(dev, 0x50, 1, true, false, &err); > + /* Any error other than -ENOTSUP(board's MSI support is broken= ) > + * is a programming error */ > + assert(!ret || ret =3D=3D -ENOTSUP); > + if (ret && s->msi =3D=3D ON_OFF_AUTO_ON) { > + /* Can't satisfy user's explicit msi=3Don request, fail */ > + error_append_hint(&err, "You have to use msi=3Dauto (defau= lt) or " > + "msi=3Doff with this machine type.\n"); > + error_propagate(errp, err); > + return; > + } else if (ret) { > + /* With msi=3Dauto, we fall back to MSI off silently */ > + s->msi =3D ON_OFF_AUTO_OFF; > + error_free(err); > + } > + } > + > memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s= , > "megasas-mmio", 0x4000); > memory_region_init_io(&s->port_io, OBJECT(s), &megasas_port_ops, s= , > @@ -2348,10 +2368,6 @@ static void megasas_scsi_realize(PCIDevice *dev,= Error **errp) > memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops,= s, > "megasas-queue", 0x40000); > =20 > - if (megasas_use_msi(s) && > - msi_init(dev, 0x50, 1, true, false)) { > - s->msi =3D ON_OFF_AUTO_OFF; > - } > if (megasas_use_msix(s) && > msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000, > &s->mmio_io, b->mmio_bar, 0x3800, 0x68)) { > diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c > index dfbc0c4..698be42 100644 > --- a/hw/scsi/mptsas.c > +++ b/hw/scsi/mptsas.c > @@ -32,7 +32,7 @@ > #include "hw/scsi/scsi.h" > #include "block/scsi.h" > #include "trace.h" > - > +#include "qapi/error.h" > #include "mptsas.h" > #include "mpi.h" > =20 > @@ -1273,10 +1273,33 @@ static void mptsas_scsi_realize(PCIDevice *dev,= Error **errp) > { > DeviceState *d =3D DEVICE(dev); > MPTSASState *s =3D MPT_SAS(dev); > + Error *err =3D NULL; > + int ret; > =20 > dev->config[PCI_LATENCY_TIMER] =3D 0; > dev->config[PCI_INTERRUPT_PIN] =3D 0x01; > =20 > + if (s->msi !=3D ON_OFF_AUTO_OFF) { > + ret =3D msi_init(dev, 0, 1, true, false, &err); > + /* Any error other than -ENOTSUP(board's MSI support is broken= ) > + * is a programming error */ > + assert(!ret || ret =3D=3D -ENOTSUP); > + if (ret && s->msi =3D=3D ON_OFF_AUTO_ON) { > + /* Can't satisfy user's explicit msi=3Don request, fail */ > + error_append_hint(&err, "You have to use msi=3Dauto (defau= lt) or " > + "msi=3Doff with this machine type.\n"); > + error_propagate(errp, err); > + s->msi_in_use =3D false; > + return; > + } else if (ret) { > + /* With msi=3Dauto, we fall back to MSI off silently */ > + error_free(err); > + s->msi_in_use =3D false; > + } else { > + s->msi_in_use =3D true; > + } > + } > + > memory_region_init_io(&s->mmio_io, OBJECT(s), &mptsas_mmio_ops, s, > "mptsas-mmio", 0x4000); > memory_region_init_io(&s->port_io, OBJECT(s), &mptsas_port_ops, s, > @@ -1284,12 +1307,6 @@ static void mptsas_scsi_realize(PCIDevice *dev, = Error **errp) > memory_region_init_io(&s->diag_io, OBJECT(s), &mptsas_diag_ops, s, > "mptsas-diag", 0x10000); > =20 > - if (s->msi !=3D ON_OFF_AUTO_OFF && > - msi_init(dev, 0, 1, true, false) >=3D 0) { > - /* TODO check for errors */ > - s->msi_in_use =3D true; > - } > - > pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->port_io); > pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY | > PCI_BASE_ADDRESS_MEM_TYPE_32, &s->mmi= o_io); > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c > index e035fce..ecd6077 100644 > --- a/hw/scsi/vmw_pvscsi.c > +++ b/hw/scsi/vmw_pvscsi.c > @@ -1063,7 +1063,7 @@ pvscsi_init_msi(PVSCSIState *s) > PCIDevice *d =3D PCI_DEVICE(s); > =20 > res =3D msi_init(d, PVSCSI_MSI_OFFSET(s), PVSCSI_MSIX_NUM_VECTORS, > - PVSCSI_USE_64BIT, PVSCSI_PER_VECTOR_MASK); > + PVSCSI_USE_64BIT, PVSCSI_PER_VECTOR_MASK, NULL); > if (res < 0) { > trace_pvscsi_init_msi_fail(res); > s->msi_used =3D false; > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index 0a5510f..1a3377f 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -26,6 +26,7 @@ > #include "hw/pci/msi.h" > #include "hw/pci/msix.h" > #include "trace.h" > +#include "qapi/error.h" > =20 > //#define DEBUG_XHCI > //#define DEBUG_DATA > @@ -3581,6 +3582,7 @@ static void usb_xhci_init(XHCIState *xhci) > static void usb_xhci_realize(struct PCIDevice *dev, Error **errp) > { > int i, ret; > + Error *err =3D NULL; > =20 > XHCIState *xhci =3D XHCI(dev); > =20 > @@ -3591,6 +3593,23 @@ static void usb_xhci_realize(struct PCIDevice *d= ev, Error **errp) > =20 > usb_xhci_init(xhci); > =20 > + if (xhci->msi !=3D ON_OFF_AUTO_OFF) { > + ret =3D 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 =3D=3D -ENOTSUP); > + if (ret && xhci->msi =3D=3D ON_OFF_AUTO_ON) { > + /* Can't satisfy user's explicit msi=3Don request, fail */ > + error_append_hint(&err, "You have to use msi=3Dauto (defau= lt) or " > + "msi=3Doff with this machine type.\n"); > + error_propagate(errp, err); > + return; > + } > + assert(!err || xhci->msi =3D=3D ON_OFF_AUTO_AUTO); > + /* With msi=3Dauto, we fall back to MSI off silently */ > + error_free(err); > + } > + > if (xhci->numintrs > MAXINTRS) { > xhci->numintrs =3D MAXINTRS; > } > @@ -3648,10 +3667,6 @@ static void usb_xhci_realize(struct PCIDevice *d= ev, Error **errp) > assert(ret >=3D 0); > } > =20 > - if (xhci->msi !=3D ON_OFF_AUTO_OFF) { > - /* TODO check for errors */ > - msi_init(dev, 0x70, xhci->numintrs, true, false); > - } > if (xhci->msix !=3D ON_OFF_AUTO_OFF) { > /* TODO check for errors */ > msix_init(dev, xhci->numintrs, > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 53b87b7..9469ac2 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -31,6 +31,7 @@ > #include "sysemu/sysemu.h" > #include "pci.h" > #include "trace.h" > +#include "qapi/error.h" > =20 > #define MSIX_CAP_LENGTH 12 > =20 > @@ -1170,6 +1171,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, in= t pos) > uint16_t ctrl; > bool msi_64bit, msi_maskbit; > int ret, entries; > + Error *err =3D NULL; > =20 > if (pread(vdev->vbasedev.fd, &ctrl, sizeof(ctrl), > vdev->config_offset + pos + PCI_CAP_FLAGS) !=3D sizeof(c= trl)) { > @@ -1183,12 +1185,13 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, = int pos) > =20 > trace_vfio_msi_setup(vdev->vbasedev.name, pos); > =20 > - ret =3D msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit= ); > + ret =3D msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit= , &err); > if (ret < 0) { > if (ret =3D=3D -ENOTSUP) { > return 0; > } > - error_report("vfio: msi_init failed"); > + error_prepend(&err, "vfio: msi_init failed: "); > + error_report_err(err); > return ret; > } > vdev->msi_cap_size =3D 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit = ? 0x4 : 0); > diff --git a/include/hw/pci/msi.h b/include/hw/pci/msi.h > index 8124908..4837bcf 100644 > --- a/include/hw/pci/msi.h > +++ b/include/hw/pci/msi.h > @@ -35,7 +35,8 @@ void msi_set_message(PCIDevice *dev, MSIMessage msg); > MSIMessage msi_get_message(PCIDevice *dev, unsigned int vector); > bool msi_enabled(const PCIDevice *dev); > int msi_init(struct PCIDevice *dev, uint8_t offset, > - unsigned int nr_vectors, bool msi64bit, bool msi_per_vect= or_mask); > + unsigned int nr_vectors, bool msi64bit, > + bool msi_per_vector_mask, Error **errp); > void msi_uninit(struct PCIDevice *dev); > void msi_reset(PCIDevice *dev); > void msi_notify(PCIDevice *dev, unsigned int vector); >=20 For the megasas bits: Reviewed-by: Hannes Reinecke Cheers, Hannes --=20 Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: F. Imend=F6rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N=FCrnberg)