From 9162f101257639cc4c7e20f72f77268b1256dd79 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Sun, 10 Nov 2024 14:04:27 +0100 Subject: [PATCH 1/4] hw/nvme: fix msix_uninit with exclusive bar Commit fa905f65c554 introduced a machine compatibility parameter to enable an exclusive bar for msix. It failed to account for this when cleaning up. Make sure that if an exclusive bar is enabled, we use the proper cleanup routine. Cc: qemu-stable@nongnu.org Fixes: fa905f65c554 ("hw/nvme: add machine compatibility parameter to enable msix exclusive bar") Reviewed-by: Jesper Wendel Devantier Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 69bce20f66..13898d5827 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -8904,7 +8904,12 @@ static void nvme_exit(PCIDevice *pci_dev) pcie_sriov_pf_exit(pci_dev); } - msix_uninit(pci_dev, &n->bar0, &n->bar0); + if (n->params.msix_exclusive_bar && !pci_is_vf(pci_dev)) { + msix_uninit_exclusive_bar(pci_dev); + } else { + msix_uninit(pci_dev, &n->bar0, &n->bar0); + } + memory_region_del_subregion(&n->bar0, &n->iomem); } From 149f6e90b5b5689a4089a3432d83247d9f659684 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Sun, 10 Nov 2024 20:26:23 +0100 Subject: [PATCH 2/4] hw/nvme: fix use/unuse of msix vectors Only call msix_{un,}use_vector() when interrupts are actually enabled for a completion queue. Reviewed-by: Jesper Wendel Devantier Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 13898d5827..a38f460a78 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -5423,7 +5423,7 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) event_notifier_set_handler(&cq->notifier, NULL); event_notifier_cleanup(&cq->notifier); } - if (msix_enabled(pci)) { + if (msix_enabled(pci) && cq->irq_enabled) { msix_vector_unuse(pci, cq->vector); } if (cq->cqid) { @@ -5464,9 +5464,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr, { PCIDevice *pci = PCI_DEVICE(n); - if (msix_enabled(pci)) { + if (msix_enabled(pci) && irq_enabled) { msix_vector_use(pci, vector); } + cq->ctrl = n; cq->cqid = cqid; cq->size = size; From e85987786d248dd2792944e703ed4d31edbfbc54 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Mon, 11 Nov 2024 12:15:10 +0100 Subject: [PATCH 3/4] hw/nvme: SR-IOV VFs must hardwire pci interrupt pin register to zero The PCI Interrupt Pin Register does not apply to VFs and MUST be hardwired to zero. Fixes: 44c2c09488db ("hw/nvme: Add support for SR-IOV") Reviewed-by: Jesper Wendel Devantier Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index a38f460a78..61c114c66d 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -656,6 +656,12 @@ static void nvme_irq_check(NvmeCtrl *n) if (msix_enabled(pci)) { return; } + + /* vfs does not implement intx */ + if (pci_is_vf(pci)) { + return; + } + if (~intms & n->irq_status) { pci_irq_assert(pci); } else { @@ -8544,7 +8550,7 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) unsigned nr_vectors; int ret; - pci_conf[PCI_INTERRUPT_PIN] = 1; + pci_conf[PCI_INTERRUPT_PIN] = pci_is_vf(pci_dev) ? 0 : 1; pci_config_set_prog_interface(pci_conf, 0x2); if (n->params.use_intel_id) { From 6651f8f2e5051f6750c2534ab3151339b3c476a2 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Mon, 11 Nov 2024 12:14:49 +0100 Subject: [PATCH 4/4] hw/nvme: take a reference on the subsystem on vf realization Make sure we grab a reference on the subsystem when a VF is realized. Otherwise, the subsytem will be unrealized automatically when the VFs are unregistered and unreffed. This fixes a latent bug but was not exposed until commit 08f632848008 ("pcie: Release references of virtual functions"). This was then fixed (or rather, hidden) by commit c613ad25125b ("pcie_sriov: Do not manually unrealize"), but that was then reverted (due to other issues) in commit b0fdaee5d1ed, exposing the bug yet again. Cc: qemu-stable@nongnu.org Fixes: 08f632848008 ("pcie: Release references of virtual functions") Reviewed-by: Jesper Wendel Devantier Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 61c114c66d..ec75419566 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -8841,6 +8841,13 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) */ n->params.serial = g_strdup(pn->params.serial); n->subsys = pn->subsys; + + /* + * Assigning this link (strong link) causes an `object_unref` later in + * `object_release_link_property`. Increment the refcount to balance + * this out. + */ + object_ref(OBJECT(pn->subsys)); } if (!nvme_check_params(n, errp)) {