From a2da737729efbd9e35eded30a5b8966423bde62d Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Thu, 28 Jul 2022 08:36:07 +0200 Subject: [PATCH 1/3] hw/nvme: skip queue processing if notifier is cleared While it is safe to process the queues when they are empty, skip it if the event notifier callback was invoked spuriously. Reviewed-by: Keith Busch Reviewed-by: Jinhao Fan Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 533ad14e7a..8aa73b048d 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4238,7 +4238,9 @@ static void nvme_cq_notifier(EventNotifier *e) NvmeCQueue *cq = container_of(e, NvmeCQueue, notifier); NvmeCtrl *n = cq->ctrl; - event_notifier_test_and_clear(&cq->notifier); + if (!event_notifier_test_and_clear(e)) { + return; + } nvme_update_cq_head(cq); @@ -4275,7 +4277,9 @@ static void nvme_sq_notifier(EventNotifier *e) { NvmeSQueue *sq = container_of(e, NvmeSQueue, notifier); - event_notifier_test_and_clear(&sq->notifier); + if (!event_notifier_test_and_clear(e)) { + return; + } nvme_process_sq(sq); } From 04e8da889016e77a3b9fea7e11569f61aadafbcb Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Thu, 28 Jul 2022 08:48:51 +0200 Subject: [PATCH 2/3] hw/nvme: unregister the event notifier handler on the main loop Make sure the notifier handler is unregistered in the main loop prior to cleaning it up. Fixes: 2e53b0b45024 ("hw/nvme: Use ioeventfd to handle doorbell updates") Reviewed-by: Keith Busch Reviewed-by: Jinhao Fan Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 8aa73b048d..70b454eedb 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4311,6 +4311,7 @@ static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n) if (sq->ioeventfd_enabled) { memory_region_del_eventfd(&n->iomem, 0x1000 + offset, 4, false, 0, &sq->notifier); + event_notifier_set_handler(&sq->notifier, NULL); event_notifier_cleanup(&sq->notifier); } g_free(sq->io_req); @@ -4701,6 +4702,7 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) if (cq->ioeventfd_enabled) { memory_region_del_eventfd(&n->iomem, 0x1000 + offset, 4, false, 0, &cq->notifier); + event_notifier_set_handler(&cq->notifier, NULL); event_notifier_cleanup(&cq->notifier); } if (msix_enabled(&n->parent_obj)) { From e2e137f64282a2ee2f359b6df4cd93c83a308e7b Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Thu, 28 Jul 2022 08:34:21 +0200 Subject: [PATCH 3/3] hw/nvme: do not enable ioeventfd by default Do not enable ioeventfd by default. Let the feature mature a bit before we consider enabling it by default. Fixes: 2e53b0b45024 ("hw/nvme: Use ioeventfd to handle doorbell updates") Reviewed-by: Keith Busch Reviewed-by: Jinhao Fan Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 70b454eedb..87aeba0564 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -7670,7 +7670,7 @@ static Property nvme_props[] = { DEFINE_PROP_UINT8("vsl", NvmeCtrl, params.vsl, 7), DEFINE_PROP_BOOL("use-intel-id", NvmeCtrl, params.use_intel_id, false), DEFINE_PROP_BOOL("legacy-cmb", NvmeCtrl, params.legacy_cmb, false), - DEFINE_PROP_BOOL("ioeventfd", NvmeCtrl, params.ioeventfd, true), + DEFINE_PROP_BOOL("ioeventfd", NvmeCtrl, params.ioeventfd, false), DEFINE_PROP_UINT8("zoned.zasl", NvmeCtrl, params.zasl, 0), DEFINE_PROP_BOOL("zoned.auto_transition", NvmeCtrl, params.auto_transition_zones, true),