From 93db90f7f3231fde706cbb7ca0c2be9ed8d1abcf Mon Sep 17 00:00:00 2001 From: Andreas Economides Date: Wed, 15 Sep 2021 16:59:03 +0000 Subject: [PATCH] nvmf/vfio-user: fix SQ race condition The host driver should do a wmb() before it updates the SQ tail doorbell to ensure that any writes to the SQ are guaranteed to be visible if a doorbell update is visible (a store-release) - and indeed, this is what the Linux NVMe driver does. Therefore, we require a rmb() after we read the tail doorbell in order to synchronise properly with the host driver (we need a load-acquire), and guarantee that the updates to the SQ are visisble to us. Signed-off-by: Andreas Economides Change-Id: I57eb1e1f0dfb1091a8f10f40f8ee0e2604d9268c Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9514 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Ben Walker --- lib/nvmf/vfio_user.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/nvmf/vfio_user.c b/lib/nvmf/vfio_user.c index a7ca63251..35cf3e9d0 100644 --- a/lib/nvmf/vfio_user.c +++ b/lib/nvmf/vfio_user.c @@ -2746,7 +2746,16 @@ nvmf_vfio_user_qpair_poll(struct nvmf_vfio_user_qpair *qpair) ctrlr = qpair->ctrlr; + /* Load-Acquire. */ new_tail = *tdbl(ctrlr, &qpair->sq); + + /* + * Ensure that changes to the queue are visible to us. + * The host driver should write the queue first, do a wmb(), and then + * update the SQ tail doorbell (their Store-Release). + */ + spdk_rmb(); + if (sq_head(qpair) == new_tail) { return 0; }