oxidecomputer / oxidecomputer/propolis

Doorbell Buffer fences are inappropriately lax

Open
#1,025 0 comments 0 reactions 1 assignee View on GitHub

@iximeow is already working on this.

Since Jan 28, 2026.

bug
Dominant language
Rust
Stars
270
Forks
42
Avg merge
4d 5h
Merged PRs (30d)
6

Description

std::sync::atomic::fence talks about the general pattern we have in queue.rs where we fence(Release); write() and read(); fence(Acquire). but we're not actually doing either of

* followed by an atomic write ‘X’ with any ordering on some atomic object ‘m’,
...
* an atomic read ‘Y’ with any ordering on ‘m’,

since the mem.write and mem.read are through a *u32 and normal non-atomic pointer write/read. and for the same reasons I'd described in #1005, it would be inappropriate to convert these to a &AtomicU32 and do reads/writes anyway. more "UB by the spec, probably fine in practice": from https://doc.rust-lang.org/std/sync/atomic/#memory-model-for-atomic-accesses, a guest could do a conflicting write or access the atomic with any other size concurrent to our operation in Propolis.

and worse, the guest OS might not care about C++ atomic semantics anyway; even if everything were on the up and up about atomic accesses, we don't know the guest has fulfilled its end of a fence(Release)-like operation, so we don't really know that we've established a happens-before relationship with guest memory.

I'm not yet sure exactly what kind of fence (mfence? or nothing?) we do need or where we should do that fencing, just that what we're doing now is definitely not what we'd want.. just making these mfence seems simple, but makes me wonder what the expected fencing is between a shadow doorbell read and, say, reading a submission queue entry from the indicated queue?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.