From efc25f65e2f11a27a2f5ea289856f14e2236cd26 Mon Sep 17 00:00:00 2001 From: "Rodney W. Grimes" Date: Sat, 18 May 2019 19:32:38 +0000 Subject: [PATCH] bhyve virtio needs barriers Under certain tight race conditions, we found that the lack of a memory barrier in bhyve's virtio handling causes it to miss a NO_NOTIFY state transition on block devices, resulting in guest stall. The investigation is recorded in OS-7613. As part of the examination into bhyve's use of barriers, one other section was found to be problematic, but only on non-x86 ISAs with less strict memory ordering. That was addressed in this patch as well, although it was not at all a problem on x86. PR: 231117 Submitted by: Patrick Mooney Reviewed by: jhb, kib, rgrimes Approved by: jhb MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D19501 --- usr.sbin/bhyve/virtio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/usr.sbin/bhyve/virtio.c b/usr.sbin/bhyve/virtio.c index 4c85000796e2..47a3ed29bac0 100644 --- a/usr.sbin/bhyve/virtio.c +++ b/usr.sbin/bhyve/virtio.c @@ -3,6 +3,7 @@ * * Copyright (c) 2013 Chris Torek * All rights reserved. + * Copyright (c) 2019 Joyent, Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -32,6 +33,8 @@ __FBSDID("$FreeBSD$"); #include #include +#include + #include #include #include @@ -422,6 +425,12 @@ vq_relchain(struct vqueue_info *vq, uint16_t idx, uint32_t iolen) vue = &vuh->vu_ring[uidx++ & mask]; vue->vu_idx = idx; vue->vu_tlen = iolen; + + /* + * Ensure the used descriptor is visible before updating the index. + * This is necessary on ISAs with memory ordering less strict than x86. + */ + atomic_thread_fence_rel(); vuh->vu_idx = uidx; } @@ -459,6 +468,13 @@ vq_endchains(struct vqueue_info *vq, int used_all_avail) vs = vq->vq_vs; old_idx = vq->vq_save_used; vq->vq_save_used = new_idx = vq->vq_used->vu_idx; + + /* + * Use full memory barrier between vu_idx store from preceding + * vq_relchain() call and the loads from VQ_USED_EVENT_IDX() or + * va_flags below. + */ + atomic_thread_fence_seq_cst(); if (used_all_avail && (vs->vs_negotiated_caps & VIRTIO_F_NOTIFY_ON_EMPTY)) intr = 1;