Page MenuHomeFreeBSD

vmm: fix vcpu atomic load
ClosedPublic

Authored by br on Oct 28 2024, 1:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 27, 12:36 AM
Unknown Object (File)
Jan 29 2025, 10:56 AM
Unknown Object (File)
Jan 27 2025, 7:51 PM
Unknown Object (File)
Jan 27 2025, 7:44 PM
Unknown Object (File)
Jan 27 2025, 5:14 AM
Unknown Object (File)
Jan 18 2025, 3:20 AM
Unknown Object (File)
Jan 8 2025, 9:01 PM
Unknown Object (File)
Jan 8 2025, 12:30 PM

Details

Summary

Load vcpu with acquire semantics as we are making a critical section between creating vcpu and using it.

Test Plan

tested on risc-v only

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

br requested review of this revision.Oct 28 2024, 1:15 PM

In particular, this is paired with an atomic_store_rel_ptr a few lines below. I think in practice this is probably a no-op as any dependent memory accesses are reads of data that vcpu points to, and if you had a mispredicted read it would never dereference a "wrong" pointer, it would just see a NULL pointer and stop once speculation ran into a fault. That said, I do think this is more correct.

This revision is now accepted and ready to land.Oct 28 2024, 1:29 PM
In D47306#1078749, @jhb wrote:

In particular, this is paired with an atomic_store_rel_ptr a few lines below. I think in practice this is probably a no-op as any dependent memory accesses are reads of data that vcpu points to, and if you had a mispredicted read it would never dereference a "wrong" pointer, it would just see a NULL pointer and stop once speculation ran into a fault. That said, I do think this is more correct.

This is perhaps a use-case for atomic_load_consume_ptr()? I think I agree that an acq fence is not formally required, but it looks wrong at first glance.

As an aside, we have #define atomic_load_ptr(p) (*(volatile __typeof(*p) *)(p)) - I wonder why atomic_load_acq_ptr isn't similarly defined.

This revision was automatically updated to reflect the committed changes.