[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 2/6] pci: Convert msix_init() to Error and fix calle
|
From: |
Cao jin |
|
Subject: |
[Qemu-devel] [PATCH 2/6] pci: Convert msix_init() to Error and fix callers to check it |
|
Date: |
Wed, 17 Aug 2016 22:39:03 +0800 |
msix_init() has the same issue with msi_init(), which reports errors
via error_report(), that is not suitable when it's used in realize().
Fix it by converting it to Error, also fix its callers to
handle failure instead of ignoring it.
Cc: Jiri Pirko <address@hidden>
CC: Gerd Hoffmann <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>
Signed-off-by: Cao jin <address@hidden>
---
hw/misc/ivshmem.c | 1 +
hw/net/e1000e.c | 2 +-
hw/net/rocker/rocker.c | 2 +-
hw/net/vmxnet3.c | 42 +++++++++--------------------
hw/pci/msix.c | 18 +++++++++----
hw/scsi/megasas.c | 25 ++++++++++++++----
hw/usb/hcd-xhci.c | 71 ++++++++++++++++++++++++++++++--------------------
hw/vfio/pci.c | 7 +++--
hw/virtio/virtio-pci.c | 7 ++---
include/hw/pci/msix.h | 3 ++-
10 files changed, 101 insertions(+), 77 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 40a2ebc..b8511ee 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -899,6 +899,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error
**errp)
if (ivshmem_setup_interrupts(s) < 0) {
error_setg(errp, "failed to initialize interrupts");
+ error_append_hint(errp, "MSI-X is not supported by interrupt
controller");
return;
}
}
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index d001c96..82a7be1 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -295,7 +295,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..769b1fd 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1262,7 +1262,7 @@ static int rocker_msix_init(Rocker *r)
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, NULL);
if (err) {
return err;
}
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index bbf44ad..0ee80b7 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2177,32 +2177,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)
{
@@ -2311,9 +2285,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);
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 384a29d..27fee0a 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
@@ -242,7 +243,8 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned
nentries)
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 +252,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 +270,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;
}
@@ -336,7 +340,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, NULL);
if (ret) {
return ret;
}
@@ -445,8 +449,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
{
MSIMessage msg;
- if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
+ if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) {
return;
+ }
+
if (msix_is_masked(dev, vector)) {
msix_set_pending(dev, vector);
return;
@@ -481,8 +487,10 @@ void msix_reset(PCIDevice *dev)
/* Mark vector as used. */
int msix_vector_use(PCIDevice *dev, unsigned vector)
{
- if (vector >= dev->msix_entries_nr)
+ if (vector >= dev->msix_entries_nr) {
return -EINVAL;
+ }
+
dev->msix_entry_used[vector]++;
return 0;
}
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e968302..61efb0f 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2349,16 +2349,31 @@ 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");
+ 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;
- }
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);
+ }
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);
- }
}
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..fae0bf1 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1659,11 +1659,8 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
int err = msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors,
proxy->msix_bar);
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("unable to init msix: "
+ "MSI-X is not supported by interrupt controller");
proxy->nvectors = 0;
}
}
diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
index 048a29d..0b0e31b 100644
--- a/include/hw/pci/msix.h
+++ b/include/hw/pci/msix.h
@@ -9,7 +9,8 @@ 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);
--
2.1.0
- Re: [Qemu-devel] [PATCH 3/6] e1000e: fix for migration compatibility, (continued)
[Qemu-devel] [PATCH 5/6] megasas: remove unnecessary megasas_use_msix(), Cao jin, 2016/08/17
[Qemu-devel] [PATCH 6/6] megasas: undo the overwrites of user configuration, Cao jin, 2016/08/17
[Qemu-devel] [PATCH 1/6] msix_init: assert programming error, Cao jin, 2016/08/17
[Qemu-devel] [PATCH 2/6] pci: Convert msix_init() to Error and fix callers to check it,
Cao jin <=