* [Qemu-devel] [PATCH v8 01/10] msix: Follow CODING_STYLE
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
@ 2017-01-13 7:11 ` Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 02/10] hcd-xhci: check & correct param before using it Cao jin
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Marcel Apfelbaum, Michael S. Tsirkin
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/pci/msix.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb14fc60..0cee631ecc55 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -447,8 +447,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;
@@ -483,8 +485,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;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 02/10] hcd-xhci: check & correct param before using it
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 01/10] msix: Follow CODING_STYLE Cao jin
@ 2017-01-13 7:11 ` Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 03/10] pci: Convert msix_init() to Error and fix callers to check it Cao jin
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:11 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Markus Armbruster, Marcel Apfelbaum,
Michael S. Tsirkin
usb_xhci_realize() corrects invalid values of property "intrs"
automatically, but the uncorrected value is passed to msi_init(),
which chokes on invalid values. Delay that until after the
correction.
Resources allocated by usb_xhci_init() are leaked when msi_init()
fails. Fix by calling it after msi_init().
CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/usb/hcd-xhci.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6dd8c0..0ace273da472 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3627,25 +3627,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;
}
@@ -3667,6 +3648,24 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
xhci->max_pstreams_mask = 0;
}
+ 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);
+ }
+
+ usb_xhci_init(xhci);
xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 03/10] pci: Convert msix_init() to Error and fix callers to check it
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 01/10] msix: Follow CODING_STYLE Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 02/10] hcd-xhci: check & correct param before using it Cao jin
@ 2017-01-13 7:11 ` Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 04/10] megasas: change behaviour of msix switch Cao jin
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:11 UTC (permalink / raw)
To: qemu-devel
Cc: Jiri Pirko, Gerd Hoffmann, Dmitry Fleytman, Jason Wang,
Michael S. Tsirkin, Hannes Reinecke, Paolo Bonzini,
Alex Williamson, Markus Armbruster, Marcel Apfelbaum
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.
For some devices(like e1000e, vmxnet3) 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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/block/nvme.c | 5 ++++-
hw/misc/ivshmem.c | 8 ++++----
hw/net/e1000e.c | 6 +++++-
hw/net/rocker/rocker.c | 7 ++++++-
hw/net/vmxnet3.c | 8 ++++++--
hw/pci/msix.c | 34 +++++++++++++++++++++++++++++-----
hw/scsi/megasas.c | 5 ++++-
hw/usb/hcd-xhci.c | 13 ++++++++-----
hw/vfio/pci.c | 8 ++++++--
hw/virtio/virtio-pci.c | 11 +++++------
include/hw/pci/msix.h | 5 +++--
11 files changed, 80 insertions(+), 30 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d479fd22f573..2d703c8a712a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -831,6 +831,7 @@ static int nvme_init(PCIDevice *pci_dev)
{
NvmeCtrl *n = NVME(pci_dev);
NvmeIdCtrl *id = &n->id_ctrl;
+ Error *err = NULL;
int i;
int64_t bs_size;
@@ -872,7 +873,9 @@ 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);
+ if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, &err)) {
+ error_report_err(err);
+ }
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));
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index abeaf3da0800..70e71a597b9c 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -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;
}
@@ -897,8 +897,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;
}
}
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1ca0062..74cbbef30366 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -292,7 +292,11 @@ e1000e_init_msix(E1000EState *s)
E1000E_MSIX_IDX, E1000E_MSIX_TABLE,
&s->msix,
E1000E_MSIX_IDX, E1000E_MSIX_PBA,
- 0xA0);
+ 0xA0, NULL);
+
+ /* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. Fall back to INTx silently on -ENOTSUP */
+ assert(!res || res == -ENOTSUP);
if (res < 0) {
trace_e1000e_msix_init_fail(res);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index e9d215aa4df1..8f829f2946d8 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1256,14 +1256,19 @@ 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);
+ /* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. */
+ assert(!err || err == -ENOTSUP);
if (err) {
+ error_report_err(local_err);
return err;
}
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 92f6af9620f1..a433cc017cb1 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2191,10 +2191,14 @@ 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);
+
+ /* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. Fall back to INTx on -ENOTSUP */
+ assert(!res || res == -ENOTSUP);
if (0 > res) {
- VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
+ VMW_WRPRN("Failed to initialize MSI-X, board's MSI support is broken");
s->msix_used = false;
} else {
if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0cee631ecc55..d03016f124a9 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
@@ -238,11 +239,29 @@ 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 +269,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 +287,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 +330,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 +362,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;
}
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 67fc1e7893f8..a946424ab234 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2366,9 +2366,12 @@ 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, &err)) {
+ /*TODO: check msix_init's error, and should fail on msix=on */
+ error_report_err(err);
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 0ace273da472..153b006ca01b 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3703,11 +3703,14 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
}
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);
+ /* TODO check for errors, and should fail when msix=on */
+ ret = msix_init(dev, xhci->numintrs,
+ &xhci->mem, 0, OFF_MSIX_TABLE,
+ &xhci->mem, 0, OFF_MSIX_PBA,
+ 0x90, &err);
+ if (ret) {
+ error_report_err(err);
+ }
}
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d7dbe0e3e04e..0bcfaad0a095 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -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 *local_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,
+ &local_err);
if (ret < 0) {
if (ret == -ENOTSUP) {
+ error_report_err(local_err);
return 0;
}
- error_setg(errp, "msix_init failed");
+
+ error_propagate(errp, local_err);
return ret;
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 21c2b9dbfc97..9562c265aa26 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1670,13 +1670,12 @@ 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, errp);
+ /* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. */
+ assert(!err || err == -ENOTSUP);
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_err(*errp);
proxy->nvectors = 0;
}
}
diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
index 048a29dd2f65..1f27658d352f 100644
--- a/include/hw/pci/msix.h
+++ b/include/hw/pci/msix.h
@@ -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);
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 04/10] megasas: change behaviour of msix switch
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (2 preceding siblings ...)
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 03/10] pci: Convert msix_init() to Error and fix callers to check it Cao jin
@ 2017-01-13 7:11 ` Cao jin
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 05/10] hcd-xhci: " Cao jin
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:11 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Hannes Reinecke, Paolo Bonzini,
Markus Armbruster, Marcel Apfelbaum
Resolve the TODO, msix=auto means msix on; if user specify msix=on,
then device creation fail on msix_init failure.
Also undo the overwrites of user configuration of msix.
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Hannes Reinecke <hare@suse.de>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/scsi/megasas.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index a946424ab234..14d6e0c6d565 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2359,19 +2359,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");
+ /* No instance_finalize method, need to free the resource here */
+ object_unref(OBJECT(&s->mmio_io));
+ error_propagate(errp, err);
+ return;
+ }
+ assert(!err || s->msix == ON_OFF_AUTO_AUTO);
+ /* With msix=auto, we fall back to MSI off silently */
+ 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, &err)) {
- /*TODO: check msix_init's error, and should fail on msix=on */
- error_report_err(err);
- s->msix = ON_OFF_AUTO_OFF;
- }
-
if (pci_is_express(dev)) {
pcie_endpoint_cap_init(dev, 0xa0);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 05/10] hcd-xhci: change behaviour of msix switch
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (3 preceding siblings ...)
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 04/10] megasas: change behaviour of msix switch Cao jin
@ 2017-01-13 7:11 ` Cao jin
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 06/10] megasas: undo the overwrites of msi user configuration Cao jin
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:11 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Michael S. Tsirkin, Markus Armbruster,
Marcel Apfelbaum
Resolve the TODO, msix=auto means msix on; if user specify msix=on,
then device creation fail on msix_init failure.
CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/usb/hcd-xhci.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 153b006ca01b..aaca57cb5f1f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3636,12 +3636,14 @@ 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 {
@@ -3669,6 +3671,28 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
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 "
+ "msix=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,
@@ -3701,17 +3725,6 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
ret = pcie_endpoint_cap_init(dev, 0xa0);
assert(ret >= 0);
}
-
- if (xhci->msix != ON_OFF_AUTO_OFF) {
- /* TODO check for errors, and should fail when msix=on */
- ret = msix_init(dev, xhci->numintrs,
- &xhci->mem, 0, OFF_MSIX_TABLE,
- &xhci->mem, 0, OFF_MSIX_PBA,
- 0x90, &err);
- if (ret) {
- error_report_err(err);
- }
- }
}
static void usb_xhci_exit(PCIDevice *dev)
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 06/10] megasas: undo the overwrites of msi user configuration
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (4 preceding siblings ...)
2017-01-13 7:11 ` [Qemu-devel] [PATCH v8 05/10] hcd-xhci: " Cao jin
@ 2017-01-13 7:12 ` Cao jin
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 07/10] vmxnet3: fix reference leak issue Cao jin
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:12 UTC (permalink / raw)
To: qemu-devel
Cc: Hannes Reinecke, Paolo Bonzini, Markus Armbruster,
Marcel Apfelbaum, Michael S. Tsirkin
Commit afea4e14 seems forgetting to undo the overwrites, which is
unsuitable.
CC: Hannes Reinecke <hare@suse.de>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/scsi/megasas.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 14d6e0c6d565..c208d520c4df 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2350,11 +2350,10 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
"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);
}
+ assert(!err || s->msix == ON_OFF_AUTO_AUTO);
+ /* With msi=auto, we fall back to MSI off silently */
+ error_free(err);
}
memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 07/10] vmxnet3: fix reference leak issue
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (5 preceding siblings ...)
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 06/10] megasas: undo the overwrites of msi user configuration Cao jin
@ 2017-01-13 7:12 ` Cao jin
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 08/10] vmxnet3: remove unnecessary internal msix flag Cao jin
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:12 UTC (permalink / raw)
To: qemu-devel
Cc: Dmitry Fleytman, Jason Wang, Markus Armbruster,
Michael S. Tsirkin
On migration target, msix_vector_use() will be called in vmxnet3_post_load()
in second time, without a matching second call to msi_vector_unuse(),
which results in vector reference leak.
CC: Dmitry Fleytman <dmitry@daynix.com>
CC: Jason Wang <jasowang@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/net/vmxnet3.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a433cc017cb1..45e125e92c8a 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2552,21 +2552,11 @@ static void vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size)
static int vmxnet3_post_load(void *opaque, int version_id)
{
VMXNET3State *s = opaque;
- PCIDevice *d = PCI_DEVICE(s);
net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s),
s->max_tx_frags, s->peer_has_vhdr);
net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr);
- if (s->msix_used) {
- if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
- VMW_WRPRN("Failed to re-use MSI-X vectors");
- msix_uninit(d, &s->msix_bar, &s->msix_bar);
- s->msix_used = false;
- return -1;
- }
- }
-
vmxnet3_validate_queues(s);
vmxnet3_validate_interrupts(s);
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 08/10] vmxnet3: remove unnecessary internal msix flag
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (6 preceding siblings ...)
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 07/10] vmxnet3: fix reference leak issue Cao jin
@ 2017-01-13 7:12 ` Cao jin
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 09/10] msi_init: convert assert to return -errno Cao jin
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:12 UTC (permalink / raw)
To: qemu-devel
Cc: Dmitry Fleytman, Jason Wang, Markus Armbruster,
Michael S. Tsirkin
Internal flag msix_used is unnecessary, it has the same effect as
msix_enabled().
The corresponding msi flag is already dropped in commit 1070048e.
CC: Dmitry Fleytman <dmitry@daynix.com>
CC: Jason Wang <jasowang@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/net/vmxnet3.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 45e125e92c8a..af39965c8cc2 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -281,8 +281,6 @@ typedef struct {
Vmxnet3RxqDescr rxq_descr[VMXNET3_DEVICE_MAX_RX_QUEUES];
Vmxnet3TxqDescr txq_descr[VMXNET3_DEVICE_MAX_TX_QUEUES];
- /* Whether MSI-X support was installed successfully */
- bool msix_used;
hwaddr drv_shmem;
hwaddr temp_shared_guest_driver_memory;
@@ -359,7 +357,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State *s, uint32_t int_idx)
{
PCIDevice *d = PCI_DEVICE(s);
- if (s->msix_used && msix_enabled(d)) {
+ if (msix_enabled(d)) {
VMW_IRPRN("Sending MSI-X notification for vector %u", int_idx);
msix_notify(d, int_idx);
return false;
@@ -383,7 +381,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State *s, int lidx)
* This function should never be called for MSI(X) interrupts
* because deassertion never required for message interrupts
*/
- assert(!s->msix_used || !msix_enabled(d));
+ assert(!msix_enabled(d));
/*
* This function should never be called for MSI(X) interrupts
* because deassertion never required for message interrupts
@@ -421,7 +419,7 @@ static void vmxnet3_trigger_interrupt(VMXNET3State *s, int lidx)
s->interrupt_states[lidx].is_pending = true;
vmxnet3_update_interrupt_line_state(s, lidx);
- if (s->msix_used && msix_enabled(d) && s->auto_int_masking) {
+ if (msix_enabled(d) && s->auto_int_masking) {
goto do_automask;
}
@@ -1428,7 +1426,7 @@ static void vmxnet3_update_features(VMXNET3State *s)
static bool vmxnet3_verify_intx(VMXNET3State *s, int intx)
{
- return s->msix_used || msi_enabled(PCI_DEVICE(s))
+ return msix_enabled(PCI_DEVICE(s)) || msi_enabled(PCI_DEVICE(s))
|| intx == pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1;
}
@@ -1445,18 +1443,18 @@ static void vmxnet3_validate_interrupts(VMXNET3State *s)
int i;
VMW_CFPRN("Verifying event interrupt index (%d)", s->event_int_idx);
- vmxnet3_validate_interrupt_idx(s->msix_used, s->event_int_idx);
+ vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)), s->event_int_idx);
for (i = 0; i < s->txq_num; i++) {
int idx = s->txq_descr[i].intr_idx;
VMW_CFPRN("Verifying TX queue %d interrupt index (%d)", i, idx);
- vmxnet3_validate_interrupt_idx(s->msix_used, idx);
+ vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)), idx);
}
for (i = 0; i < s->rxq_num; i++) {
int idx = s->rxq_descr[i].intr_idx;
VMW_CFPRN("Verifying RX queue %d interrupt index (%d)", i, idx);
- vmxnet3_validate_interrupt_idx(s->msix_used, idx);
+ vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)), idx);
}
}
@@ -2185,6 +2183,7 @@ vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors)
static bool
vmxnet3_init_msix(VMXNET3State *s)
{
+ bool msix;
PCIDevice *d = PCI_DEVICE(s);
int res = msix_init(d, VMXNET3_MAX_INTRS,
&s->msix_bar,
@@ -2199,17 +2198,18 @@ vmxnet3_init_msix(VMXNET3State *s)
if (0 > res) {
VMW_WRPRN("Failed to initialize MSI-X, board's MSI support is broken");
- s->msix_used = false;
+ msix = 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;
+ msix = false;
} else {
- s->msix_used = true;
+ msix = true;
}
}
- return s->msix_used;
+
+ return msix;
}
static void
@@ -2217,7 +2217,7 @@ vmxnet3_cleanup_msix(VMXNET3State *s)
{
PCIDevice *d = PCI_DEVICE(s);
- if (s->msix_used) {
+ if (msix_enabled(d)) {
vmxnet3_unuse_msix_vectors(s, VMXNET3_MAX_INTRS);
msix_uninit(d, &s->msix_bar, &s->msix_bar);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 09/10] msi_init: convert assert to return -errno
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (7 preceding siblings ...)
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 08/10] vmxnet3: remove unnecessary internal msix flag Cao jin
@ 2017-01-13 7:12 ` Cao jin
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix() Cao jin
2017-01-13 8:22 ` [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Markus Armbruster
10 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Michael S. Tsirkin, Marcel Apfelbaum
According to the disscussion:
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html
Let leaf function returns reasonable -errno, let caller decide how to
handle the return value.
Suggested-by: Markus Armbruster <armbru@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Acked-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/pci/msi.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b2278a373..af3efbe525ce 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -201,9 +201,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
" 64bit %d mask %d\n",
offset, nr_vectors, msi64bit, msi_per_vector_mask);
- assert(!(nr_vectors & (nr_vectors - 1))); /* power of 2 */
- assert(nr_vectors > 0);
- assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
+ /* vector sanity test: should in range 1 - 32, should be power of 2 */
+ if (!is_power_of_2(nr_vectors) || nr_vectors > PCI_MSI_VECTORS_MAX) {
+ error_setg(errp, "Invalid vector number: %d", nr_vectors);
+ return -EINVAL;
+ }
+
/* the nr of MSI vectors is up to 32 */
vectors_order = ctz32(nr_vectors);
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix()
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (8 preceding siblings ...)
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 09/10] msi_init: convert assert to return -errno Cao jin
@ 2017-01-13 7:12 ` Cao jin
2017-01-13 8:20 ` Hannes Reinecke
2017-01-13 8:26 ` Markus Armbruster
2017-01-13 8:22 ` [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Markus Armbruster
10 siblings, 2 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 7:12 UTC (permalink / raw)
To: qemu-devel
Cc: Hannes Reinecke, Paolo Bonzini, Markus Armbruster,
Marcel Apfelbaum, Michael S. Tsirkin
Also move certain hunk above, to place msix init related code together.
CC: Hannes Reinecke <hare@suse.de>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
hw/scsi/megasas.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
msix_init() doesn't set the MSI-X enable bit, so use msix_enabled()
is not right here, restore the old check without the
megasas_use_msix() wrapper.
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index c208d520c4df..73eab7844ee3 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -155,11 +155,6 @@ static bool megasas_use_queue64(MegasasState *s)
return s->flags & MEGASAS_MASK_USE_QUEUE64;
}
-static bool megasas_use_msix(MegasasState *s)
-{
- return s->msix != ON_OFF_AUTO_OFF;
-}
-
static bool megasas_is_jbod(MegasasState *s)
{
return s->flags & MEGASAS_MASK_USE_JBOD;
@@ -2305,9 +2300,7 @@ static void megasas_scsi_uninit(PCIDevice *d)
{
MegasasState *s = MEGASAS(d);
- if (megasas_use_msix(s)) {
- msix_uninit(d, &s->mmio_io, &s->mmio_io);
- }
+ msix_uninit(d, &s->mmio_io, &s->mmio_io);
msi_uninit(d);
}
@@ -2358,7 +2351,7 @@ 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)) {
+ if (s->msix != ON_OFF_AUTO_OFF) {
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)
@@ -2378,6 +2371,10 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
error_free(err);
}
+ if (s->msix != ON_OFF_AUTO_OFF) {
+ msix_vector_use(dev, 0);
+ }
+
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,
@@ -2393,10 +2390,6 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
pci_register_bar(dev, b->mmio_bar, bar_type, &s->mmio_io);
pci_register_bar(dev, 3, bar_type, &s->queue_io);
- if (megasas_use_msix(s)) {
- msix_vector_use(dev, 0);
- }
-
s->fw_state = MFI_FWSTATE_READY;
if (!s->sas_addr) {
s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix()
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix() Cao jin
@ 2017-01-13 8:20 ` Hannes Reinecke
2017-01-13 8:26 ` Markus Armbruster
1 sibling, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2017-01-13 8:20 UTC (permalink / raw)
To: Cao jin, qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Marcel Apfelbaum,
Michael S. Tsirkin
On 01/13/2017 08:12 AM, Cao jin wrote:
> Also move certain hunk above, to place msix init related code together.
>
> CC: Hannes Reinecke <hare@suse.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
> hw/scsi/megasas.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> msix_init() doesn't set the MSI-X enable bit, so use msix_enabled()
> is not right here, restore the old check without the
> megasas_use_msix() wrapper.
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +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)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix()
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix() Cao jin
2017-01-13 8:20 ` Hannes Reinecke
@ 2017-01-13 8:26 ` Markus Armbruster
1 sibling, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2017-01-13 8:26 UTC (permalink / raw)
To: Cao jin
Cc: qemu-devel, Marcel Apfelbaum, Paolo Bonzini, Michael S. Tsirkin,
Hannes Reinecke
Cao jin <caoj.fnst@cn.fujitsu.com> writes:
> Also move certain hunk above, to place msix init related code together.
>
> CC: Hannes Reinecke <hare@suse.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
> hw/scsi/megasas.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> msix_init() doesn't set the MSI-X enable bit, so use msix_enabled()
> is not right here, restore the old check without the
> megasas_use_msix() wrapper.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error
2017-01-13 7:11 [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Cao jin
` (9 preceding siblings ...)
2017-01-13 7:12 ` [Qemu-devel] [PATCH v8 10/10] megasas: remove unnecessary megasas_use_msix() Cao jin
@ 2017-01-13 8:22 ` Markus Armbruster
2017-01-13 8:34 ` Cao jin
10 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-01-13 8:22 UTC (permalink / raw)
To: Cao jin
Cc: qemu-devel, Jiri Pirko, Michael S. Tsirkin, Jason Wang,
Marcel Apfelbaum, Alex Williamson, Hannes Reinecke,
Dmitry Fleytman, Paolo Bonzini, Gerd Hoffmann
Cao jin <caoj.fnst@cn.fujitsu.com> writes:
> Only a tiny modification in patch "megasas: remove unnecessary
> megasas_use_msix()" to fix a megasas issue.
Please have a look at Michael's review in
Message-ID: <20170112163519-mutt-send-email-mst@kernel.org>
> v8 changelog:
> 1. reorder: place the "megasas: remove unnecessary megasas_use_msix()"
> as the last one. and fix the bug in it, detailed description in it,
> also removed the R-b of it.
> 2. Add the Acked-by from Marcel for first 9 patches; add the R-b from Markus
> to "hcd-xhci: check & correct param before using it".
>
> Test:
> 1. make check ok
> 2. command line test for all affected device, make sure their realization
> is ok.
> 3. detailed test for megasas, hcd-xhci, vmxnet3.
> megasas: install a linux distro is ok
> ./qemu-system-x86_64 --enable-kvm -m 1024
> -device megasas,id=scsi0,bus=pci.0
> -drive file=/xx/scsi-disk.img,if=none,id=drive-scsi0
> -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=4,lun=0,drive=drive-scsi0,id=scsi0-4
> -cdrom /xx/Fedora-Server-DVD-x86_64-23.iso --monitor stdio
>
> hcd-xhci: partition the usbstick.img, mkfs, write to file, is ok
> ./qemu-system-x86_64 -M q35 -m 1024 --enable-kvm
> -drive if=none,id=usbstick,file=/xx/usbstick.img
> -device nec-usb-xhci,id=usb,p2=8,p3=8,bus=pcie.0
> -device usb-storage,bus=usb.0,drive=usbstick --monitor stdio
> /xx/FedoraWorkStatsion23-x86_64.img
>
> vmxnet3: ping another destination belongs to host's network is ok.
> But no migration test, because I don't have a spare machine for now.
> ./qemu-system-x86_64 -M q35 -m 1024 --enable-kvm
> -netdev tap,id=mynet0 -device vmxnet3,netdev=mynet0
> --monitor stdio /home/pino/vm/FedoraWorkStatsion23-x86_64.img
You could migrate to a new QEMU on the same machine.
Or, if you don't want to deal with two instances of QEMU running at the
same time, migrate to file:
(qemu) migrate "exec:cat >zzz"
Start the target with -incoming "exec:cat zzz". Better than nothing.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error
2017-01-13 8:22 ` [Qemu-devel] [PATCH v8 00/10] Convert msix_init() to error Markus Armbruster
@ 2017-01-13 8:34 ` Cao jin
0 siblings, 0 replies; 15+ messages in thread
From: Cao jin @ 2017-01-13 8:34 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Jiri Pirko, Michael S. Tsirkin, Jason Wang,
Marcel Apfelbaum, Alex Williamson, Hannes Reinecke,
Dmitry Fleytman, Paolo Bonzini, Gerd Hoffmann
On 01/13/2017 04:22 PM, Markus Armbruster wrote:
> Cao jin <caoj.fnst@cn.fujitsu.com> writes:
>
>> Only a tiny modification in patch "megasas: remove unnecessary
>> megasas_use_msix()" to fix a megasas issue.
>
> Please have a look at Michael's review in
> Message-ID: <20170112163519-mutt-send-email-mst@kernel.org>
>
Oh...thanks for reminding, I missed that mail...
>> v8 changelog:
>> 1. reorder: place the "megasas: remove unnecessary megasas_use_msix()"
>> as the last one. and fix the bug in it, detailed description in it,
>> also removed the R-b of it.
>> 2. Add the Acked-by from Marcel for first 9 patches; add the R-b from Markus
>> to "hcd-xhci: check & correct param before using it".
>>
>> Test:
>> 1. make check ok
>> 2. command line test for all affected device, make sure their realization
>> is ok.
>> 3. detailed test for megasas, hcd-xhci, vmxnet3.
>> megasas: install a linux distro is ok
>> ./qemu-system-x86_64 --enable-kvm -m 1024
>> -device megasas,id=scsi0,bus=pci.0
>> -drive file=/xx/scsi-disk.img,if=none,id=drive-scsi0
>> -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=4,lun=0,drive=drive-scsi0,id=scsi0-4
>> -cdrom /xx/Fedora-Server-DVD-x86_64-23.iso --monitor stdio
>>
>> hcd-xhci: partition the usbstick.img, mkfs, write to file, is ok
>> ./qemu-system-x86_64 -M q35 -m 1024 --enable-kvm
>> -drive if=none,id=usbstick,file=/xx/usbstick.img
>> -device nec-usb-xhci,id=usb,p2=8,p3=8,bus=pcie.0
>> -device usb-storage,bus=usb.0,drive=usbstick --monitor stdio
>> /xx/FedoraWorkStatsion23-x86_64.img
>>
>> vmxnet3: ping another destination belongs to host's network is ok.
>> But no migration test, because I don't have a spare machine for now.
>> ./qemu-system-x86_64 -M q35 -m 1024 --enable-kvm
>> -netdev tap,id=mynet0 -device vmxnet3,netdev=mynet0
>> --monitor stdio /home/pino/vm/FedoraWorkStatsion23-x86_64.img
>
> You could migrate to a new QEMU on the same machine.
>
> Or, if you don't want to deal with two instances of QEMU running at the
> same time, migrate to file:
>
> (qemu) migrate "exec:cat >zzz"
>
> Start the target with -incoming "exec:cat zzz". Better than nothing.
>
Thanks for the info, I will try
--
Sincerely,
Cao jin
^ permalink raw reply [flat|nested] 15+ messages in thread