pci: Convert msix_init() to Error and fix callers

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. In order to make the API change as small as possible,
leave the return value check to later patch.

For some devices(like e1000e, vmxnet3, nvme) who won't fail because of
msix_init's failure, suppress the error report by passing NULL error
object.

Bonus: add comment for msix_init.

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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
Cao jin 2017-01-17 14:18:48 +08:00 committed by Michael S. Tsirkin
parent 20729dbd01
commit ee640c625e
12 changed files with 58 additions and 24 deletions

View File

@ -872,7 +872,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, NULL);
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));

View File

@ -749,13 +749,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;
}
@ -898,8 +898,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
qemu_chr_fe_set_handlers(&s->server_chr, ivshmem_can_receive,
ivshmem_read, NULL, s, NULL, true);
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;
}
}

View File

@ -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);

View File

@ -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;
}

View File

@ -2191,7 +2191,7 @@ vmxnet3_init_msix(VMXNET3State *s)
VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
&s->msix_bar,
VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s),
VMXNET3_MSIX_OFFSET(s));
VMXNET3_MSIX_OFFSET(s), NULL);
if (0 > res) {
VMW_WRPRN("Failed to initialize MSI-X, error %d", res);

View File

@ -29,11 +29,10 @@ static int gen_rp_interrupts_init(PCIDevice *d, Error **errp)
{
int rc;
rc = msix_init_exclusive_bar(d, GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR, 0);
rc = msix_init_exclusive_bar(d, GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR, 0, errp);
if (rc < 0) {
assert(rc == -ENOTSUP);
error_setg(errp, "Unable to init msix vectors");
} else {
msix_vector_use(d, 0);
}

View File

@ -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,31 @@ 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,10 +271,12 @@ 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;
}
if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) {
error_setg(errp, "The number of MSI-X vectors is invalid");
return -EINVAL;
}
@ -266,10 +289,13 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
table_offset + table_size > memory_region_size(table_bar) ||
pba_offset + pba_size > memory_region_size(pba_bar) ||
(table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) {
error_setg(errp, "table & pba overlap, or they don't fit in BARs,"
" or don't align");
return -EINVAL;
}
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;
}
@ -306,7 +332,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;
@ -338,7 +364,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;
}

View File

@ -2367,9 +2367,11 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
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->mmio_io, b->mmio_bar, 0x3800, 0x68, NULL)) {
/* TODO: check msix_init's error, and should fail on msix=on */
s->msix = ON_OFF_AUTO_OFF;
}
if (pci_is_express(dev)) {
pcie_endpoint_cap_init(dev, 0xa0);
}

View File

@ -3703,11 +3703,11 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
}
if (xhci->msix != ON_OFF_AUTO_OFF) {
/* TODO check for errors */
/* TODO check for errors, and should fail when msix=on */
msix_init(dev, xhci->numintrs,
&xhci->mem, 0, OFF_MSIX_TABLE,
&xhci->mem, 0, OFF_MSIX_PBA,
0x90);
0x90, NULL);
}
}

View File

@ -1432,6 +1432,7 @@ static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
{
int ret;
Error *err = NULL;
vdev->msix->pending = g_malloc0(BITS_TO_LONGS(vdev->msix->entries) *
sizeof(unsigned long));
@ -1439,12 +1440,15 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
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) {
error_report_err(err);
return 0;
}
error_setg(errp, "msix_init failed");
error_propagate(errp, err);
return ret;
}

View File

@ -1686,9 +1686,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_idx);
proxy->msix_bar_idx, NULL);
if (err) {
/* Notice when a system that supports MSIx can't initialize it. */
/* 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);

View File

@ -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);