Skip to content

Commit

Permalink
MFC: r347960: bhyve virtio needs barriers
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed by:	jhb, kib, rgrimes
Approved by:	jhb
Differential Revision:	https://reviews.freebsd.org/D19501
  • Loading branch information
rwgbsd committed May 23, 2019
1 parent 330c653 commit 7532fd5
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions usr.sbin/bhyve/virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* Copyright (c) 2013 Chris Torek <torek @ torek net>
* 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
Expand Down Expand Up @@ -32,6 +33,8 @@ __FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/uio.h>

#include <machine/atomic.h>

#include <stdio.h>
#include <stdint.h>
#include <pthread.h>
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7532fd5

Please sign in to comment.