From patchwork Wed Aug 17 14:39:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cao jin X-Patchwork-Id: 9286061 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2873060574 for ; Wed, 17 Aug 2016 14:39:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 19579298F6 for ; Wed, 17 Aug 2016 14:39:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0D4C7298FF; Wed, 17 Aug 2016 14:39:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A4B95298F6 for ; Wed, 17 Aug 2016 14:39:30 +0000 (UTC) Received: from localhost ([::1]:47841 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ba209-00056S-MY for patchwork-qemu-devel@patchwork.kernel.org; Wed, 17 Aug 2016 10:39:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37042) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ba1sF-0007kr-FK for qemu-devel@nongnu.org; Wed, 17 Aug 2016 10:31:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ba1sC-00071F-93 for qemu-devel@nongnu.org; Wed, 17 Aug 2016 10:31:18 -0400 Received: from [59.151.112.132] (port=48281 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ba1s9-0006x8-JT for qemu-devel@nongnu.org; Wed, 17 Aug 2016 10:31:16 -0400 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="10014912" Received: from unknown (HELO cn.fujitsu.com) ([10.167.33.5]) by heian.cn.fujitsu.com with ESMTP; 17 Aug 2016 22:30:47 +0800 Received: from G08CNEXCHPEKD03.g08.fujitsu.local (unknown [10.167.33.85]) by cn.fujitsu.com (Postfix) with ESMTP id 92CB542CD102; Wed, 17 Aug 2016 22:30:43 +0800 (CST) Received: from G08FNSTD140223.g08.fujitsu.local (10.167.226.69) by G08CNEXCHPEKD03.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP Server (TLS) id 14.3.279.2; Wed, 17 Aug 2016 22:30:42 +0800 From: Cao jin To: Date: Wed, 17 Aug 2016 22:39:03 +0800 Message-ID: <1471444747-6277-3-git-send-email-caoj.fnst@cn.fujitsu.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1471444747-6277-1-git-send-email-caoj.fnst@cn.fujitsu.com> References: <1471444747-6277-1-git-send-email-caoj.fnst@cn.fujitsu.com> MIME-Version: 1.0 X-Originating-IP: [10.167.226.69] X-yoursite-MailScanner-ID: 92CB542CD102.AE208 X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: caoj.fnst@cn.fujitsu.com X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 59.151.112.132 Subject: [Qemu-devel] [PATCH 2/6] pci: Convert msix_init() to Error and fix callers to check it X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jiri Pirko , "Michael S. Tsirkin" , Jason Wang , Markus Armbruster , Marcel Apfelbaum , Alex Williamson , Hannes Reinecke , Dmitry Fleytman , Paolo Bonzini , Gerd Hoffmann Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 CC: Gerd Hoffmann CC: Dmitry Fleytman CC: Jason Wang CC: Michael S. Tsirkin CC: Hannes Reinecke CC: Paolo Bonzini CC: Alex Williamson CC: Markus Armbruster CC: Marcel Apfelbaum Signed-off-by: Cao jin --- 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);