Commit 1108b2f8 authored by Cao jin's avatar Cao jin Committed by Michael S. Tsirkin
Browse files

pci: Convert msi_init() to Error and fix callers to check it 



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 <kraxel@redhat.com>
cc: John Snow <jsnow@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>

Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarCao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
parent 69b205bb

hw/audio/intel-hda.c

+20 −4
Original line number Diff line number Diff line
@@ -1132,6 +1132,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));
@@ -1140,13 +1142,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);

hw/ide/ich.c

+5 −2
Original line number Diff line number Diff line
@@ -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)

hw/net/e1000e.c

+2 −6
Original line number Diff line number Diff line
@@ -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);

hw/net/vmxnet3.c

+12 −25
Original line number Diff line number Diff line
@@ -2216,27 +2216,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)

{
@@ -2298,10 +2277,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...");
@@ -2325,14 +2309,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)) {

hw/pci-bridge/ioh3420.c

+5 −1
Original line number Diff line number Diff line
@@ -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;

    }