[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 08/13] pci: Convert msi_init() to Error and f
|
From: |
Hannes Reinecke |
|
Subject: |
Re: [Qemu-devel] [PATCH v9 08/13] pci: Convert msi_init() to Error and fix callers to check it |
|
Date: |
Mon, 20 Jun 2016 08:14:12 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 |
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().
>
> Fix by converting it to Error.
>
> Fix its callers to handle failure instead of ignoring it.
>
> 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.
>
> cc: Gerd Hoffmann <address@hidden>
> cc: John Snow <address@hidden>
> cc: Dmitry Fleytman <address@hidden>
> cc: Jason Wang <address@hidden>
> cc: Michael S. Tsirkin <address@hidden>
> cc: Hannes Reinecke <address@hidden>
> cc: Paolo Bonzini <address@hidden>
> cc: Alex Williamson <address@hidden>
> cc: Markus Armbruster <address@hidden>
> cc: Marcel Apfelbaum <address@hidden>
>
> Reviewed-by: Markus Armbruster <address@hidden>
> Signed-off-by: Cao jin <address@hidden>
> ---
> 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(-)
>
> 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, Error
> **errp)
> {
> IntelHDAState *d = INTEL_HDA(pci);
> uint8_t *conf = d->pci.config;
> + Error *err = NULL;
> + int ret;
>
> d->name = object_get_typename(OBJECT(d));
>
> @@ -1143,13 +1145,27 @@ static void intel_hda_realize(PCIDevice *pci, Error
> **errp)
> /* HDCTL off 0x40 bit 0 selects signaling mode (1-HDA, 0 - Ac97) 18.1.19
> */
> conf[0x40] = 0x01;
>
> + if (d->msi != ON_OFF_AUTO_OFF) {
> + ret = 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 == -ENOTSUP);
> + if (ret && d->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 || d->msi == ON_OFF_AUTO_AUTO);
> + /* With msi=auto, 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 != ON_OFF_AUTO_OFF) {
> - /* TODO check for errors */
> - msi_init(&d->pci, d->old_msi_addr ? 0x50 : 0x60, 1, true, false);
> - }
>
> 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 <hw/isa/isa.h>
> #include "sysemu/block-backend.h"
> #include "sysemu/dma.h"
> -
> #include <hw/ide/pci.h>
> #include <hw/ide/ahci.h>
>
> @@ -111,6 +110,7 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error
> **errp)
> int sata_cap_offset;
> uint8_t *sata_cap;
> d = ICH_AHCI(dev);
> + int ret;
>
> ahci_realize(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6);
>
> @@ -146,7 +146,10 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error
> **errp)
> /* Although the AHCI 1.3 specification states that the first capability
> * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9
> * AHCI device puts the MSI capability first, pointing to 0x80. */
> - msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
> + ret = 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 == -ENOTSUP);
> }
>
> 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;
>
> - res = 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 = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL);
>
> - if (res > 0) {
> + if (!res) {
> s->intr_state |= 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)
> }
> }
>
> -#define VMXNET3_USE_64BIT (true)
> -#define VMXNET3_PER_VECTOR_MASK (false)
> -
> -static bool
> -vmxnet3_init_msi(VMXNET3State *s)
> -{
> - PCIDevice *d = PCI_DEVICE(s);
> - int res;
> -
> - res = 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 = false;
> - } else {
> - s->msi_used = true;
> - }
> -
> - return s->msi_used;
> -}
> -
> static void
> vmxnet3_cleanup_msi(VMXNET3State *s)
> {
> @@ -2279,10 +2258,15 @@ static uint64_t
> vmxnet3_device_serial_num(VMXNET3State *s)
> return dsn_payload;
> }
>
> +
> +#define VMXNET3_USE_64BIT (true)
> +#define VMXNET3_PER_VECTOR_MASK (false)
> +
> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> {
> DeviceState *dev = DEVICE(pci_dev);
> VMXNET3State *s = VMXNET3(pci_dev);
> + int ret;
>
> VMW_CBPRN("Starting init...");
>
> @@ -2306,14 +2290,17 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev,
> Error **errp)
> /* Interrupt pin A */
> pci_dev->config[PCI_INTERRUPT_PIN] = 0x01;
>
> + ret = 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 == -ENOTSUP);
> + s->msi_used = !ret;
> +
> if (!vmxnet3_init_msix(s)) {
> VMW_WRPRN("Failed to initialize MSI-X, configuration is
> inconsistent.");
> }
>
> - if (!vmxnet3_init_msi(s)) {
> - VMW_WRPRN("Failed to initialize MSI, configuration is
> inconsistent.");
> - }
> -
> vmxnet3_net_init(s);
>
> 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"
>
> #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 = PCIE_PORT(d);
> PCIESlot *s = PCIE_SLOT(d);
> int rc;
> + Error *err = NULL;
>
> pci_bridge_initfn(d, TYPE_PCIE_BUS);
> pcie_port_init_reg(d);
> @@ -109,8 +111,10 @@ static int ioh3420_initfn(PCIDevice *d)
>
> rc = 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 == -ENOTSUP);
> + error_report_err(err);
> goto err_bridge;
> }
>
> 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 = PCI_BRIDGE(dev);
> PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
> int err;
> + Error *local_err = NULL;
>
> pci_bridge_initfn(dev, TYPE_PCI_BUS);
>
> @@ -75,12 +76,23 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> goto slotid_error;
> }
>
> - if (bridge_dev->msi != ON_OFF_AUTO_OFF &&
> - msi_nonbroken) {
> - err = msi_init(dev, 0, 1, true, true);
> - if (err < 0) {
> + if (bridge_dev->msi != ON_OFF_AUTO_OFF) {
> + /* it means SHPC exists, because MSI is needed by SHPC */
> +
> + err = 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 == -ENOTSUP);
> + if (err && bridge_dev->msi == ON_OFF_AUTO_ON) {
> + /* Can't satisfy user's explicit msi=on request, fail */
> + error_append_hint(&local_err, "You have to use msi=auto
> (default) "
> + "or msi=off with this machine type.\n");
> + error_report_err(local_err);
> goto msi_error;
> }
> + assert(!local_err || bridge_dev->msi == ON_OFF_AUTO_AUTO);
> + /* With msi=auto, we fall back to MSI off silently */
> + error_free(local_err);
> }
>
> 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"
>
> #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 = PCIE_PORT(d);
> PCIESlot *s = PCIE_SLOT(d);
> int rc;
> + Error *err = NULL;
>
> pci_bridge_initfn(d, TYPE_PCIE_BUS);
> pcie_port_init_reg(d);
>
> rc = 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 == -ENOTSUP);
> + error_report_err(err);
> goto err_bridge;
> }
>
> diff --git a/hw/pci-bridge/xio3130_upstream.c
> b/hw/pci-bridge/xio3130_upstream.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"
>
> #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 = PCIE_PORT(d);
> int rc;
> + Error *err = NULL;
>
> pci_bridge_initfn(d, TYPE_PCIE_BUS);
> pcie_port_init_reg(d);
>
> rc = 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 == -ENOTSUP);
> + error_report_err(err);
> goto err_bridge;
> }
>
> 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"
>
> /* 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_vector_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;
>
> if (!msi_nonbroken) {
> + error_setg(errp, "MSI is not supported by interrupt controller");
> return -ENOTSUP;
> }
>
> @@ -212,7 +216,8 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
> }
>
> cap_size = msi_cap_sizeof(flags);
> - config_offset = pci_add_capability(dev, PCI_CAP_ID_MSI, offset,
> cap_size);
> + config_offset = 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"
>
> #define MEGASAS_VERSION_GEN1 "1.70"
> @@ -2333,6 +2333,8 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
> **errp)
> MegasasBaseClass *b = MEGASAS_DEVICE_GET_CLASS(s);
> uint8_t *pci_conf;
> int i, bar_type;
> + Error *err = NULL;
> + int ret;
>
> pci_conf = dev->config;
>
> @@ -2341,6 +2343,24 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
> **errp)
> /* Interrupt pin 1 */
> pci_conf[PCI_INTERRUPT_PIN] = 0x01;
>
> + if (megasas_use_msi(s)) {
> + ret = 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 == -ENOTSUP);
> + if (ret && s->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;
> + } else if (ret) {
> + /* With msi=auto, we fall back to MSI off silently */
> + s->msi = 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);
>
> - if (megasas_use_msi(s) &&
> - msi_init(dev, 0x50, 1, true, false)) {
> - s->msi = 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"
>
> @@ -1273,10 +1273,33 @@ static void mptsas_scsi_realize(PCIDevice *dev, Error
> **errp)
> {
> DeviceState *d = DEVICE(dev);
> MPTSASState *s = MPT_SAS(dev);
> + Error *err = NULL;
> + int ret;
>
> dev->config[PCI_LATENCY_TIMER] = 0;
> dev->config[PCI_INTERRUPT_PIN] = 0x01;
>
> + if (s->msi != ON_OFF_AUTO_OFF) {
> + ret = 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 == -ENOTSUP);
> + if (ret && s->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);
> + s->msi_in_use = false;
> + return;
> + } else if (ret) {
> + /* With msi=auto, we fall back to MSI off silently */
> + error_free(err);
> + s->msi_in_use = false;
> + } else {
> + s->msi_in_use = 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);
>
> - if (s->msi != ON_OFF_AUTO_OFF &&
> - msi_init(dev, 0, 1, true, false) >= 0) {
> - /* TODO check for errors */
> - s->msi_in_use = 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->mmio_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 = PCI_DEVICE(s);
>
> res = 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 = 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"
>
> //#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 = NULL;
>
> XHCIState *xhci = XHCI(dev);
>
> @@ -3591,6 +3593,23 @@ static void usb_xhci_realize(struct PCIDevice *dev,
> Error **errp)
>
> 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;
> }
> @@ -3648,10 +3667,6 @@ static void usb_xhci_realize(struct PCIDevice *dev,
> Error **errp)
> assert(ret >= 0);
> }
>
> - if (xhci->msi != ON_OFF_AUTO_OFF) {
> - /* TODO check for errors */
> - msi_init(dev, 0x70, xhci->numintrs, true, false);
> - }
> if (xhci->msix != 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"
>
> #define MSIX_CAP_LENGTH 12
>
> @@ -1170,6 +1171,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
> uint16_t ctrl;
> bool msi_64bit, msi_maskbit;
> int ret, entries;
> + Error *err = NULL;
>
> if (pread(vdev->vbasedev.fd, &ctrl, sizeof(ctrl),
> vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
> @@ -1183,12 +1185,13 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int
> pos)
>
> trace_vfio_msi_setup(vdev->vbasedev.name, pos);
>
> - ret = msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit);
> + ret = msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit, &err);
> if (ret < 0) {
> if (ret == -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 = 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_vector_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);
>
For the megasas bits:
Reviewed-by: Hannes Reinecke <address@hidden>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
address@hidden +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
- Re: [Qemu-devel] [PATCH v9 06/13] megasas: change msi/msix property type, (continued)
- [Qemu-devel] [PATCH v9 10/13] mptsas: remove unnecessary internal msi state flag, Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 03/13] usb xhci: change msi/msix property type, Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 13/13] vmw_pvscsi: remove unnecessary internal msi state flag, Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 12/13] e1000e: remove unnecessary internal msi state flag, Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 11/13] vmxnet3: remove unnecessary internal msi state flag, Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 09/13] megasas: remove unnecessary megasas_use_msi(), Cao jin, 2016/06/20
- [Qemu-devel] [PATCH v9 08/13] pci: Convert msi_init() to Error and fix callers to check it, Cao jin, 2016/06/20
- Re: [Qemu-devel] [PATCH v9 08/13] pci: Convert msi_init() to Error and fix callers to check it,
Hannes Reinecke <=
- Re: [Qemu-devel] [PATCH v9 00/13] Add param Error ** for msi_init()--part2, Cao jin, 2016/06/28