The example code in "Receiving Used Buffers From The Device" goes as
follows:
vring_disable_interrupts(vq);
for (;;) {
if (vq->last_seen_used == vring->used.idx) {
vring_enable_interrupts(vq);
mb();
if (vq->last_seen_used == vring->used.idx)
break;
vring_disable_interrupts(vq);
}
struct vring_used_elem *e =
&vring->used.ring[vq->last_seen_used % qsz];
process_buffer(e);
vq->last_seen_used++;
}
Suppose that process_buffer() does something like this:
process_buffer(struct vring_used_elem *e)
{
u32 id;
u32 len;
id = e->id;
len = e->len;
/* go on to process the descriptor chain starting at "id" */
process_desc_chain(id, len);
}
On an architecture that is not sequentially consistent, the "e->id"
load could be executed out of order, speculatively; for example:
vring_disable_interrupts(vq);
for (;;) {
struct vring_used_elem *e =
&vring->used.ring[vq->last_seen_used % qsz];
u32 id = e->id;
u32 len = e->len;
if (vq->last_seen_used == vring->used.idx) {
vring_enable_interrupts(vq);
mb();
if (vq->last_seen_used == vring->used.idx)
break;
vring_disable_interrupts(vq);
}
process_desc_chain(id, len);
vq->last_seen_used++;
}
The (*e) pointer dereferences are OK, but the pointer targets could be
stale at the time of the speculative load (just adding comments):
vring_disable_interrupts(vq);
for (;;) {
/* ring is empty, load stale data into "id" and "len" */
struct vring_used_elem *e =
&vring->used.ring[vq->last_seen_used % qsz];
u32 id = e->id;
u32 len = e->len;
/* at this point the host goes through all of the following:
* - fill in a descriptor chain
* - place the index of its head descriptor on the used ring
* - store the number of processed bytes too
* - mb()
* - bump used index
* - mb()
*
* and the used index happens to become visible to the guest,
* enabling it to move past the next check:
*/
if (vq->last_seen_used == vring->used.idx) {
vring_enable_interrupts(vq);
mb();
if (vq->last_seen_used == vring->used.idx)
break;
vring_disable_interrupts(vq);
}
/* use stale "id" and "len" */
process_desc_chain(id, len);
vq->last_seen_used++;
}
Prevent this by adding a barrier between the ring emptiness check and the
access to the used element.
Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
---
virtio-spec.lyx | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 51f41dc..2186d35 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -2982,6 +2982,11 @@ for (;;) {
\begin_layout Plain Layout
+ mb();
+\end_layout
+
+\begin_layout Plain Layout
+
process_buffer(e);
\end_layout
--
1.7.1
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
