Date: Thu, 23 May 2019 18:48:47 +0000 (UTC) From: "Rodney W. Grimes" <rgrimes@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r348186 - stable/12/usr.sbin/bhyve Message-ID: <201905231848.x4NImljq074575@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rgrimes Date: Thu May 23 18:48:46 2019 New Revision: 348186 URL: https://svnweb.freebsd.org/changeset/base/348186 Log: MFC: r347960: 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 <patrick.mooney@joyent.com> Reviewed by: jhb, kib, rgrimes Approved by: jhb Differential Revision: https://reviews.freebsd.org/D19501 Modified: stable/12/usr.sbin/bhyve/virtio.c Directory Properties: stable/12/ (props changed) Modified: stable/12/usr.sbin/bhyve/virtio.c ============================================================================== --- stable/12/usr.sbin/bhyve/virtio.c Thu May 23 18:37:05 2019 (r348185) +++ stable/12/usr.sbin/bhyve/virtio.c Thu May 23 18:48:46 2019 (r348186) @@ -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 @@ -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> @@ -422,6 +425,12 @@ vq_relchain(struct vqueue_info *vq, uint16_t idx, uint 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_avai 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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905231848.x4NImljq074575>