Message ID | 20210112124714.220657-1-its@irrelevant.dk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | hw/block/nvme: fix for non-msix machines | expand |
On 1/12/21 1:47 PM, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() > return value") had the unintended effect of breaking support on > several platforms not supporting MSI-X. > > Still check for errors, but only report that MSI-X is unsupported > instead of bailing out. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Fixes: 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() return value") > Fixes: fbf2e5375e33 ("hw/block/nvme: Verify msix_vector_use() returned value") > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/block/nvme.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Jan 12 14:20, Philippe Mathieu-Daudé wrote: > On 1/12/21 1:47 PM, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > > > Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() > > return value") had the unintended effect of breaking support on > > several platforms not supporting MSI-X. > > > > Still check for errors, but only report that MSI-X is unsupported > > instead of bailing out. > > > > Reported-by: Guenter Roeck <linux@roeck-us.net> > > Fixes: 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() return value") > > Fixes: fbf2e5375e33 ("hw/block/nvme: Verify msix_vector_use() returned value") > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > --- > > hw/block/nvme.c | 31 ++++++++++++++++++++++--------- > > 1 file changed, 22 insertions(+), 9 deletions(-) > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Thanks! Applied to nvme-next.
On 1/15/21 8:07 AM, Klaus Jensen wrote: > On Jan 12 14:20, Philippe Mathieu-Daudé wrote: >> On 1/12/21 1:47 PM, Klaus Jensen wrote: >>> From: Klaus Jensen <k.jensen@samsung.com> >>> >>> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() >>> return value") had the unintended effect of breaking support on >>> several platforms not supporting MSI-X. >>> >>> Still check for errors, but only report that MSI-X is unsupported >>> instead of bailing out. >>> BTW maybe worth adding: Cc: qemu-stable@nongnu.org So this patch get backported in the next stable release based on 5.2. >>> Reported-by: Guenter Roeck <linux@roeck-us.net> >>> Fixes: 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() return value") >>> Fixes: fbf2e5375e33 ("hw/block/nvme: Verify msix_vector_use() returned value") >>> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> >>> --- >>> hw/block/nvme.c | 31 ++++++++++++++++++++++--------- >>> 1 file changed, 22 insertions(+), 9 deletions(-) >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> > > Thanks! Applied to nvme-next. >
On Jan 15 10:37, Philippe Mathieu-Daudé wrote: > On 1/15/21 8:07 AM, Klaus Jensen wrote: > > On Jan 12 14:20, Philippe Mathieu-Daudé wrote: > >> On 1/12/21 1:47 PM, Klaus Jensen wrote: > >>> From: Klaus Jensen <k.jensen@samsung.com> > >>> > >>> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() > >>> return value") had the unintended effect of breaking support on > >>> several platforms not supporting MSI-X. > >>> > >>> Still check for errors, but only report that MSI-X is unsupported > >>> instead of bailing out. > >>> > > BTW maybe worth adding: > > Cc: qemu-stable@nongnu.org > > So this patch get backported in the next stable release based on 5.2. > Thanks Philippe, I messed up the bounce, but I sent it on to stable now.
On 1/15/21 10:46 AM, Klaus Jensen wrote: > On Jan 15 10:37, Philippe Mathieu-Daudé wrote: >> On 1/15/21 8:07 AM, Klaus Jensen wrote: >>> On Jan 12 14:20, Philippe Mathieu-Daudé wrote: >>>> On 1/12/21 1:47 PM, Klaus Jensen wrote: >>>>> From: Klaus Jensen <k.jensen@samsung.com> >>>>> >>>>> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() >>>>> return value") had the unintended effect of breaking support on >>>>> several platforms not supporting MSI-X. >>>>> >>>>> Still check for errors, but only report that MSI-X is unsupported >>>>> instead of bailing out. >>>>> >> >> BTW maybe worth adding: >> >> Cc: qemu-stable@nongnu.org >> >> So this patch get backported in the next stable release based on 5.2. >> > > Thanks Philippe, > > I messed up the bounce, but I sent it on to stable now. I meant in the patch description itself before the other tags (qemu-stable scan for commits with the "Cc: qemu-stable@nongnu.org" tag).
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 0854ee307227..cf0fe28fe6eb 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -2590,7 +2590,9 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) n->cq[cq->cqid] = NULL; timer_del(cq->timer); timer_free(cq->timer); - msix_vector_unuse(&n->parent_obj, cq->vector); + if (msix_enabled(&n->parent_obj)) { + msix_vector_unuse(&n->parent_obj, cq->vector); + } if (cq->cqid) { g_free(cq); } @@ -2624,8 +2626,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr, { int ret; - ret = msix_vector_use(&n->parent_obj, vector); - assert(ret == 0); + if (msix_enabled(&n->parent_obj)) { + ret = msix_vector_use(&n->parent_obj, vector); + assert(ret == 0); + } cq->ctrl = n; cq->cqid = cqid; cq->size = size; @@ -4159,9 +4163,12 @@ static void nvme_init_pmr(NvmeCtrl *n, PCIDevice *pci_dev) PCI_BASE_ADDRESS_MEM_PREFETCH, &n->pmrdev->mr); } -static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) +static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) { uint8_t *pci_conf = pci_dev->config; + int ret; + + Error *err = NULL; pci_conf[PCI_INTERRUPT_PIN] = 1; pci_config_set_prog_interface(pci_conf, 0x2); @@ -4181,8 +4188,14 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) n->reg_size); pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem); - if (msix_init_exclusive_bar(pci_dev, n->params.msix_qsize, 4, errp)) { - return; + ret = msix_init_exclusive_bar(pci_dev, n->params.msix_qsize, 4, &err); + if (ret < 0) { + if (ret == -ENOTSUP) { + warn_report_err(err); + } else { + error_propagate(errp, err); + return ret; + } } if (n->params.cmb_size_mb) { @@ -4190,6 +4203,8 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) } else if (n->pmrdev) { nvme_init_pmr(n, pci_dev); } + + return 0; } static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) @@ -4278,9 +4293,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) &pci_dev->qdev, n->parent_obj.qdev.id); nvme_init_state(n); - nvme_init_pci(n, pci_dev, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (nvme_init_pci(n, pci_dev, errp)) { return; }