Skip to content

Commit 89ba9c3

Browse files
authored
vmbus_server: allow unstick to work without full memory mapping (#1857)
This change updates how the "unstick" functionality accesses channel ring buffers by only mapping the control page, and using `lockable_subrange` so that a `GuestMemory` implementation that doesn't have a full mapping has the opportunity to create a partial mapping. This allows unsticking a channel to work with alternative `GuestMemory` implementations used in Hyper-V.
1 parent 99bd76c commit 89ba9c3

File tree

2 files changed

+57
-36
lines changed

2 files changed

+57
-36
lines changed

vm/devices/vmbus/vmbus_ring/src/lib.rs

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ mod pipe_protocol {
7272
mod protocol {
7373
use crate::CONTROL_WORD_COUNT;
7474
use inspect::Inspect;
75+
use safeatomic::AtomicSliceOps;
7576
use std::fmt::Debug;
7677
use std::sync::atomic::AtomicU32;
7778
use std::sync::atomic::Ordering;
@@ -97,7 +98,14 @@ mod protocol {
9798
/// A control page accessor.
9899
pub struct Control<'a>(pub &'a [AtomicU32; CONTROL_WORD_COUNT]);
99100

100-
impl Control<'_> {
101+
impl<'a> Control<'a> {
102+
pub fn from_page(page: &'a guestmem::Page) -> Option<Self> {
103+
let slice = page.as_atomic_slice()?[..CONTROL_WORD_COUNT]
104+
.try_into()
105+
.unwrap();
106+
Some(Self(slice))
107+
}
108+
101109
pub fn inp(&self) -> &AtomicU32 {
102110
&self.0[0]
103111
}
@@ -1208,31 +1216,27 @@ impl<M: RingMem> Inspect for InnerRing<M> {
12081216
///
12091217
/// Panics if control_page is not aligned.
12101218
pub fn inspect_ring(control_page: &guestmem::Page, response: &mut inspect::Response<'_>) {
1211-
let control = control_page.as_atomic_slice().unwrap()[..CONTROL_WORD_COUNT]
1212-
.try_into()
1213-
.unwrap();
1214-
1215-
response.field("control", Control(control));
1219+
let control = Control::from_page(control_page).expect("control page is not aligned");
1220+
response.field("control", control);
12161221
}
12171222

12181223
/// Returns whether a ring buffer is in a state where the receiving end might
12191224
/// need a signal.
1220-
pub fn reader_needs_signal<M: RingMem>(mem: M) -> bool {
1221-
InnerRing::new(mem).is_ok_and(|ring| {
1222-
let control = ring.control();
1225+
pub fn reader_needs_signal(control_page: &guestmem::Page) -> bool {
1226+
Control::from_page(control_page).is_some_and(|control| {
12231227
control.interrupt_mask().load(Ordering::Relaxed) == 0
12241228
&& (control.inp().load(Ordering::Relaxed) != control.outp().load(Ordering::Relaxed))
12251229
})
12261230
}
12271231

12281232
/// Returns whether a ring buffer is in a state where the sending end might need
12291233
/// a signal.
1230-
pub fn writer_needs_signal<M: RingMem>(mem: M) -> bool {
1231-
InnerRing::new(mem).is_ok_and(|ring| {
1232-
let control = ring.control();
1234+
pub fn writer_needs_signal(control_page: &guestmem::Page, ring_size: u32) -> bool {
1235+
Control::from_page(control_page).is_some_and(|control| {
12331236
let pending_size = control.pending_send_size().load(Ordering::Relaxed);
12341237
pending_size != 0
1235-
&& ring.free(
1238+
&& ring_free(
1239+
ring_size,
12361240
control.inp().load(Ordering::Relaxed),
12371241
control.outp().load(Ordering::Relaxed),
12381242
) >= pending_size
@@ -1298,16 +1302,20 @@ impl<M: RingMem> InnerRing<M> {
12981302
}
12991303

13001304
fn free(&self, inp: u32, outp: u32) -> u32 {
1301-
// It's not possible to fully fill the ring since that state would be
1302-
// indistinguishable from the empty ring. So subtract 8 bytes from the
1303-
// result.
1304-
if outp > inp {
1305-
// |....inp____outp.....|
1306-
outp - inp - 8
1307-
} else {
1308-
// |____outp....inp_____|
1309-
self.size - (inp - outp) - 8
1310-
}
1305+
ring_free(self.size, inp, outp)
1306+
}
1307+
}
1308+
1309+
fn ring_free(size: u32, inp: u32, outp: u32) -> u32 {
1310+
// It's not possible to fully fill the ring since that state would be
1311+
// indistinguishable from the empty ring. So subtract 8 bytes from the
1312+
// result.
1313+
if outp > inp {
1314+
// |....inp____outp.....|
1315+
outp - inp - 8
1316+
} else {
1317+
// |____outp....inp_____|
1318+
size - (inp - outp) - 8
13111319
}
13121320
}
13131321

vm/devices/vmbus/vmbus_server/src/lib.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ use vmbus_channel::bus::ParentBus;
7777
use vmbus_channel::bus::RestoreResult;
7878
use vmbus_channel::gpadl::GpadlMap;
7979
use vmbus_channel::gpadl_ring::AlignedGpadlView;
80-
use vmbus_channel::gpadl_ring::GpadlRingMem;
8180
use vmbus_core::HvsockConnectRequest;
8281
use vmbus_core::HvsockConnectResult;
8382
use vmbus_core::MaxVersionInfo;
@@ -1396,16 +1395,17 @@ impl ServerTask {
13961395
in_gpadl: AlignedGpadlView,
13971396
guest_to_host_event: Option<&ChannelEvent>,
13981397
host_to_guest_interrupt: &Interrupt,
1399-
) -> Result<(), anyhow::Error> {
1400-
let incoming_mem = GpadlRingMem::new(in_gpadl, gm)?;
1398+
) -> anyhow::Result<()> {
1399+
let control_page = lock_gpn_with_subrange(gm, in_gpadl.gpns()[0])?;
14011400
if let Some(guest_to_host_event) = guest_to_host_event {
1402-
if ring::reader_needs_signal(&incoming_mem) {
1401+
if ring::reader_needs_signal(control_page.pages()[0]) {
14031402
tracelimit::info_ratelimited!(channel = %channel.key, "waking host for incoming ring");
14041403
guest_to_host_event.0.deliver();
14051404
}
14061405
}
14071406

1408-
if ring::writer_needs_signal(&incoming_mem) {
1407+
let ring_size = gpadl_ring_size(&in_gpadl).try_into()?;
1408+
if ring::writer_needs_signal(control_page.pages()[0], ring_size) {
14091409
tracelimit::info_ratelimited!(channel = %channel.key, "waking guest for incoming ring");
14101410
host_to_guest_interrupt.deliver();
14111411
}
@@ -1418,14 +1418,16 @@ impl ServerTask {
14181418
out_gpadl: AlignedGpadlView,
14191419
guest_to_host_event: Option<&ChannelEvent>,
14201420
host_to_guest_interrupt: &Interrupt,
1421-
) -> Result<(), anyhow::Error> {
1422-
let outgoing_mem = GpadlRingMem::new(out_gpadl, gm)?;
1423-
if ring::reader_needs_signal(&outgoing_mem) {
1421+
) -> anyhow::Result<()> {
1422+
let control_page = lock_gpn_with_subrange(gm, out_gpadl.gpns()[0])?;
1423+
if ring::reader_needs_signal(control_page.pages()[0]) {
14241424
tracelimit::info_ratelimited!(channel = %channel.key, "waking guest for outgoing ring");
14251425
host_to_guest_interrupt.deliver();
14261426
}
1427+
14271428
if let Some(guest_to_host_event) = guest_to_host_event {
1428-
if ring::writer_needs_signal(&outgoing_mem) {
1429+
let ring_size = gpadl_ring_size(&out_gpadl).try_into()?;
1430+
if ring::writer_needs_signal(control_page.pages()[0], ring_size) {
14291431
tracelimit::info_ratelimited!(channel = %channel.key, "waking host for outgoing ring");
14301432
guest_to_host_event.0.deliver();
14311433
}
@@ -2077,17 +2079,20 @@ fn inspect_rings(
20772079
fn inspect_ring(req: inspect::Request<'_>, gpadl: &AlignedGpadlView, gm: &GuestMemory) {
20782080
let mut resp = req.respond();
20792081

2080-
// Data size excluding the control page.
2081-
let ring_size = (gpadl.gpns().len() - 1) * PAGE_SIZE;
2082-
resp.hex("ring_size", ring_size);
2082+
resp.hex("ring_size", gpadl_ring_size(gpadl));
20832083

20842084
// Lock just the control page. Use a subrange to allow a GuestMemory without a full mapping to
20852085
// create one.
2086-
if let Ok(pages) = lock_page_with_subrange(gm, gpadl.gpns()[0] * PAGE_SIZE as u64) {
2086+
if let Ok(pages) = lock_gpn_with_subrange(gm, gpadl.gpns()[0]) {
20872087
ring::inspect_ring(pages.pages()[0], &mut resp);
20882088
}
20892089
}
20902090

2091+
fn gpadl_ring_size(gpadl: &AlignedGpadlView) -> usize {
2092+
// Data size excluding the control page.
2093+
(gpadl.gpns().len() - 1) * PAGE_SIZE
2094+
}
2095+
20912096
/// Helper to create a subrange before locking a single page.
20922097
///
20932098
/// This allows us to lock a page in a `GuestMemory` that doesn't have a full mapping, but can
@@ -2098,6 +2103,14 @@ fn lock_page_with_subrange(gm: &GuestMemory, offset: u64) -> anyhow::Result<gues
20982103
.lock_gpns(false, &[0])?)
20992104
}
21002105

2106+
/// Helper to create a subrange before locking a single page from a gpn.
2107+
///
2108+
/// This allows us to lock a page in a `GuestMemory` that doesn't have a full mapping, but can
2109+
/// create one for a subrange.
2110+
fn lock_gpn_with_subrange(gm: &GuestMemory, gpn: u64) -> anyhow::Result<guestmem::LockedPages> {
2111+
lock_page_with_subrange(gm, gpn * PAGE_SIZE as u64)
2112+
}
2113+
21012114
pub(crate) struct MessageSender {
21022115
send: mpsc::Sender<SynicMessage>,
21032116
multiclient: bool,

0 commit comments

Comments
 (0)