Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions openhcl/underhill_core/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,7 @@ async fn new_underhill_vm(
serial_inputs[0] = Some(Resource::new(
vmbus_serial_guest::OpenVmbusSerialGuestConfig::open(
&vmbus_serial_guest::UART_INTERFACE_INSTANCE_COM1,
dps.general.com1_tx_only,
)
.context("failed to open com1")?,
));
Expand All @@ -2354,6 +2355,7 @@ async fn new_underhill_vm(
serial_inputs[1] = Some(Resource::new(
vmbus_serial_guest::OpenVmbusSerialGuestConfig::open(
&vmbus_serial_guest::UART_INTERFACE_INSTANCE_COM2,
dps.general.com2_tx_only,
)
.context("failed to open com2")?,
));
Expand Down Expand Up @@ -3424,8 +3426,10 @@ fn validate_isolated_configuration(dps: &DevicePlatformSettings) -> Result<(), a
tpm_enabled: _,
com1_enabled: _,
com1_vmbus_redirector: _,
com1_tx_only: _,
com2_enabled: _,
com2_vmbus_redirector: _,
com2_tx_only: _,
suppress_attestation: _,
bios_guid: _,
vpci_boot_enabled: _,
Expand Down
4 changes: 4 additions & 0 deletions openvmm/openvmm_entry/src/cli_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ options:
#[structopt(long, value_name = "SERIAL")]
pub vmbus_com2_serial: Option<SerialConfigCli>,

/// Only allow guest to host serial traffic
#[clap(long)]
pub serial_tx_only: bool,

/// debugcon binding (port:serial, where port is a u16, and serial is (console | stderr | listen=\<path\> | file=\<path\> (overwrites) | listen=tcp:\<ip\>:\<port\> | term[=\<program\>][,name=<windowtitle>] | none))
#[clap(long, value_name = "SERIAL")]
pub debugcon: Option<DebugconSerialConfigCli>,
Expand Down
1 change: 1 addition & 0 deletions openvmm/openvmm_entry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,7 @@ fn vm_config_from_command_line(
},
com1: with_vmbus_com1_serial,
com2: with_vmbus_com2_serial,
serial_tx_only: opt.serial_tx_only,
vtl2_settings: Some(prost::Message::encode_to_vec(&vtl2_settings)),
vmbus_redirection: opt.vmbus_redirect,
vmgs,
Expand Down
1 change: 1 addition & 0 deletions petri/src/vm/openvmm/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,7 @@ impl PetriVmConfigSetupCore<'_> {
},
com1: true,
com2: true,
serial_tx_only: false,
vmbus_redirection: *vmbus_redirect,
vtl2_settings: None, // Will be added at startup to allow tests to modify
vmgs: memdiff_vmgs(self.vmgs)?,
Expand Down
1 change: 1 addition & 0 deletions vm/devices/get/get_protocol/src/dps_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ pub struct HclUartSettings {
pub enable_port: bool,
pub debugger_mode: bool,
pub enable_vmbus_redirector: bool,
pub tx_only: bool,
}

#[derive(Debug, Default, Deserialize, Serialize)]
Expand Down
2 changes: 2 additions & 0 deletions vm/devices/get/get_resources/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub mod ged {
pub com1: bool,
/// Enable COM2 for VTL0 and the VMBUS redirector in VTL2.
pub com2: bool,
/// Only allow guest to host serial traffic
pub serial_tx_only: bool,
/// Enable vmbus redirection.
pub vmbus_redirection: bool,
/// Enable the TPM.
Expand Down
4 changes: 4 additions & 0 deletions vm/devices/get/guest_emulation_device/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ pub struct GuestConfig {
pub com1: bool,
/// Enable COM2 for VTL0 and the VMBUS redirector in VTL2.
pub com2: bool,
/// Only allow guest to host serial traffic
pub serial_tx_only: bool,
/// Enable vmbus redirection.
pub vmbus_redirection: bool,
/// Enable the TPM.
Expand Down Expand Up @@ -1350,11 +1352,13 @@ impl<T: RingMem + Unpin> GedChannel<T> {
enable_port: state.config.com1,
debugger_mode: false,
enable_vmbus_redirector: state.config.com1,
tx_only: state.config.serial_tx_only,
},
com2: get_protocol::dps_json::HclUartSettings {
enable_port: state.config.com2,
debugger_mode: false,
enable_vmbus_redirector: state.config.com2,
tx_only: state.config.serial_tx_only,
},
enable_firmware_debugging,
enable_tpm: state.config.enable_tpm,
Expand Down
1 change: 1 addition & 0 deletions vm/devices/get/guest_emulation_device/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ impl AsyncResolveResource<VmbusDeviceHandleKind, GuestEmulationDeviceHandle>
},
com1: resource.com1,
com2: resource.com2,
serial_tx_only: resource.serial_tx_only,
vmbus_redirection: resource.vmbus_redirection,
enable_tpm: resource.enable_tpm,
vtl2_settings: resource.vtl2_settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ pub fn create_host_channel(
},
com1: true,
com2: true,
serial_tx_only: false,
vmbus_redirection: false,
enable_tpm: false,
vtl2_settings: None,
Expand Down
4 changes: 4 additions & 0 deletions vm/devices/get/guest_emulation_transport/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,16 @@ pub mod platform_settings {
pub battery_enabled: bool,
pub processor_idle_enabled: bool,
pub tpm_enabled: bool,

pub com1_enabled: bool,
pub com1_debugger_mode: bool,
pub com1_vmbus_redirector: bool,
pub com1_tx_only: bool,
pub com2_enabled: bool,
pub com2_debugger_mode: bool,
pub com2_vmbus_redirector: bool,
pub com2_tx_only: bool,

pub firmware_debugging_enabled: bool,
pub hibernation_enabled: bool,

Expand Down
2 changes: 2 additions & 0 deletions vm/devices/get/guest_emulation_transport/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,11 @@ impl GuestEmulationTransportClient {
com1_enabled: json.v1.com1.enable_port,
com1_debugger_mode: json.v1.com1.debugger_mode,
com1_vmbus_redirector: json.v1.com1.enable_vmbus_redirector,
com1_tx_only: json.v1.com1.tx_only,
com2_enabled: json.v1.com2.enable_port,
com2_debugger_mode: json.v1.com2.debugger_mode,
com2_vmbus_redirector: json.v1.com2.enable_vmbus_redirector,
com2_tx_only: json.v1.com2.tx_only,
firmware_debugging_enabled: json.v1.enable_firmware_debugging,
hibernation_enabled: json.v1.enable_hibernation,

Expand Down
2 changes: 2 additions & 0 deletions vm/devices/get/guest_emulation_transport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,13 @@ mod tests {
enable_port: true,
debugger_mode: false,
enable_vmbus_redirector: true,
tx_only: false,
},
com2: get_protocol::dps_json::HclUartSettings {
enable_port: false,
debugger_mode: false,
enable_vmbus_redirector: false,
tx_only: false,
},
enable_firmware_debugging: true,
..Default::default()
Expand Down
63 changes: 51 additions & 12 deletions vm/devices/serial/vmbus_serial_guest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ use zerocopy::IntoBytes;
pub struct OpenVmbusSerialGuestConfig {
/// The open UIO device file.
pub uio_device: File,
/// Only allow guest to host traffic
pub tx_only: bool,
}

impl ResourceId<SerialBackendHandle> for OpenVmbusSerialGuestConfig {
Expand Down Expand Up @@ -79,9 +81,12 @@ mod user_pipe {
impl OpenVmbusSerialGuestConfig {
/// Opens the UIO device for the specified instance GUID and returns the
/// configuration resource.
pub fn open(instance_id: &Guid) -> anyhow::Result<Self> {
pub fn open(instance_id: &Guid, tx_only: bool) -> anyhow::Result<Self> {
let uio_device = vmbus_user_channel::open_uio_device(instance_id)?;
Ok(Self { uio_device })
Ok(Self {
uio_device,
tx_only,
})
}
}

Expand All @@ -106,7 +111,7 @@ mod user_pipe {
let pipe = vmbus_user_channel::message_pipe(input.driver.as_ref(), rsrc.uio_device)
.context("failed to open vmbus serial")?;

let driver = VmbusSerialDriver::new(pipe)
let driver = VmbusSerialDriver::new(pipe, rsrc.tx_only)
.await
.context("failed to create serial transport")?;

Expand All @@ -132,6 +137,7 @@ pub struct VmbusSerialDriver {
failed: bool,
connected: bool,
stats: SerialStats,
tx_only: bool,
}

#[derive(Inspect, Debug, Default)]
Expand Down Expand Up @@ -187,6 +193,7 @@ impl VmbusSerialDriver {
/// Connects to `pipe` and returns a new serial device instance.
pub async fn new(
pipe: impl 'static + AsyncRecv + AsyncSend + Send + Unpin + InspectMut,
tx_only: bool,
) -> Result<Self, Error> {
let mut this = Self {
pipe: Box::new(pipe),
Expand All @@ -199,6 +206,7 @@ impl VmbusSerialDriver {
failed: false,
connected: false,
stats: Default::default(),
tx_only,
};
this.negotiate().await?;
Ok(this)
Expand Down Expand Up @@ -392,11 +400,19 @@ impl AsyncRead for VmbusSerialDriver {
self.rx_waker = Some(cx.waker().clone());
ready!(self.poll_outer(cx))?;
}
let n = buf.len().min(self.rx_buffer.len());
for (s, d) in self.rx_buffer.drain(..n).zip(buf) {
*d = s;

// if one-way serial is enabled, just clear the buffer and pretend we
// didn't get anything
if self.tx_only {
self.rx_buffer.clear();
Poll::Pending
Comment on lines +406 to +408
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tx_only is true, this implementation still requests RX data from the host (via poll_outer -> poll_inner which checks rx_waker.is_some() at line 283), only to discard it here. This wastes CPU cycles and bandwidth. Consider checking tx_only before the loop (line 396) and returning Poll::Pending immediately without setting rx_waker, or checking tx_only in poll_inner (line 282) to avoid requesting RX data in the first place.

Copilot uses AI. Check for mistakes.
} else {
let n = buf.len().min(self.rx_buffer.len());
for (s, d) in self.rx_buffer.drain(..n).zip(buf) {
*d = s;
}
Poll::Ready(Ok(n))
}
Poll::Ready(Ok(n))
}
}

Expand Down Expand Up @@ -445,6 +461,7 @@ mod tests {
use crate::ErrorInner;
use crate::VmbusSerialDriver;
use futures::AsyncWriteExt;
use futures::FutureExt;
use futures::io::AsyncReadExt;
use futures::join;
use pal_async::DefaultDriver;
Expand Down Expand Up @@ -492,7 +509,7 @@ mod tests {
host_vmbus.send(version_response.as_bytes()).await.unwrap();
});

let res = VmbusSerialDriver::new(guest_vmbus).await;
let res = VmbusSerialDriver::new(guest_vmbus, false).await;
match res {
Err(crate::Error(ErrorInner::VersionNotAccepted)) => {}
Err(e) => panic!("Wrong error type returned {e:?}"),
Expand All @@ -505,6 +522,7 @@ mod tests {
/// Creates a new host guest transport pair ready to send data.
async fn new_transport_pair(
driver: &DefaultDriver,
tx_only: bool,
) -> (PolledSocket<UnixStream>, VmbusSerialDriver) {
let (host_vmbus, guest_vmbus) = vmbus_async::pipe::connected_message_pipes(4096);

Expand All @@ -526,14 +544,14 @@ mod tests {
.detach();

// Create the guest serial transport
let guest_transport = VmbusSerialDriver::new(guest_vmbus).await.unwrap();
let guest_transport = VmbusSerialDriver::new(guest_vmbus, tx_only).await.unwrap();

(host_io, guest_transport)
}

#[async_test]
async fn test_basic_read_write(driver: DefaultDriver) {
let (mut host_io, mut guest_io) = new_transport_pair(&driver).await;
let (mut host_io, mut guest_io) = new_transport_pair(&driver, false).await;

let data = vec![1, 2, 3, 4, 5];
let data2 = vec![5, 4, 3, 2, 1];
Expand All @@ -549,7 +567,7 @@ mod tests {

#[async_test]
async fn test_large_read_write(driver: DefaultDriver) {
let (host_io, guest_io) = new_transport_pair(&driver).await;
let (host_io, guest_io) = new_transport_pair(&driver, false).await;

let (mut host_read, mut host_write) = host_io.split();
let (mut guest_read, mut guest_write) = guest_io.split();
Expand Down Expand Up @@ -594,7 +612,7 @@ mod tests {

#[async_test]
async fn test_large_duplex_concurrent_io(driver: DefaultDriver) {
let (host_io, guest_io) = new_transport_pair(&driver).await;
let (host_io, guest_io) = new_transport_pair(&driver, false).await;

let (mut host_read, mut host_write) = host_io.split();
let (mut guest_read, mut guest_write) = guest_io.split();
Expand Down Expand Up @@ -646,4 +664,25 @@ mod tests {

join!(host_write, host_read, guest_write, guest_read);
}

#[async_test]
async fn test_read_write_tx_only(driver: DefaultDriver) {
let (mut host_io, mut guest_io) = new_transport_pair(&driver, true).await;

let data = vec![1, 2, 3, 4, 5];
let data2 = vec![5, 4, 3, 2, 1];
host_io.write_all(&data).await.unwrap();
guest_io.write_all(&data2).await.unwrap();

let mut data_recv = vec![0; 5];
let mut cx = std::task::Context::from_waker(std::task::Waker::noop());
assert!(matches!(
guest_io.read(&mut data_recv).poll_unpin(&mut cx),
std::task::Poll::Pending
));

let mut data_recv2 = vec![0; 5];
host_io.read_exact(&mut data_recv2).await.unwrap();
assert_eq!(data2, data_recv2);
}
}
Loading