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
5 changes: 5 additions & 0 deletions openhcl/underhill_core/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2266,6 +2266,11 @@ async fn new_underhill_vm(
);
}

// Set the callback in GET to trigger the debug interrupt.
let p = partition.clone();
let debug_interrupt_callback = move |vtl: u8| p.assert_debug_interrupt(vtl);
get_client.set_debug_interrupt_callback(Box::new(debug_interrupt_callback));

let mut input_distributor = InputDistributor::new(remote_console_cfg.input);
resolver.add_async_resolver::<KeyboardInputHandleKind, _, MultiplexedInputHandle, _>(
input_distributor.client().clone(),
Expand Down
10 changes: 9 additions & 1 deletion openhcl/virt_mshv_vtl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ use user_driver::DmaClient;
use virt::IsolationType;
use virt::PartitionCapabilities;
use virt::VpIndex;
use virt::X86Partition;
use virt::irqcon::IoApicRouting;
use virt::irqcon::MsiRequest;
use virt::x86::apic_software_device::ApicSoftwareDevices;
Expand Down Expand Up @@ -858,7 +859,7 @@ impl virt::Partition for UhPartition {
}
}

impl virt::X86Partition for UhPartition {
impl X86Partition for UhPartition {
fn ioapic_routing(&self) -> Arc<dyn IoApicRouting> {
self.inner.clone()
}
Expand Down Expand Up @@ -1943,6 +1944,13 @@ impl UhPartition {
}
}

/// Trigger the LINT1 interrupt vector on the LAPIC of the BSP.
#[cfg(guest_arch = "x86_64")]
pub fn assert_debug_interrupt(&self, vtl: u8) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The vtl param should probably be a GuestVtl, and the conversion should happen at a higher level. I guess in the callback.

let bsp_index = VpIndex::new(0);
self.pulse_lint(bsp_index, Vtl::try_from(vtl).unwrap(), 1)
}

/// Enables or disables the PM timer assist.
pub fn set_pm_timer_assist(&self, port: Option<u16>) -> Result<(), HvError> {
self.inner.hcl.set_pm_timer_assist(port)
Expand Down
17 changes: 16 additions & 1 deletion openhcl/virt_mshv_vtl/src/processor/hardware_cvm/apic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use crate::UhProcessor;
use crate::processor::HardwareIsolatedBacking;
use crate::processor::NMI_SUPPRESS_LINT1_REQUESTED;
use cvm_tracing::CVM_ALLOWED;
use hcl::GuestVtl;
use virt::Processor;
Expand All @@ -28,6 +29,8 @@ pub(crate) trait ApicBacking<'b, B: HardwareIsolatedBacking> {
fn handle_extint(&mut self, vtl: GuestVtl) {
tracelimit::warn_ratelimited!(CVM_ALLOWED, ?vtl, "extint not supported");
}

fn supports_nmi_masking(&mut self) -> bool;
}

pub(crate) fn poll_apic_core<'b, B: HardwareIsolatedBacking, T: ApicBacking<'b, B>>(
Expand All @@ -53,6 +56,7 @@ pub(crate) fn poll_apic_core<'b, B: HardwareIsolatedBacking, T: ApicBacking<'b,
extint,
sipi,
nmi,
lint1,
interrupt,
} = vp.backing.cvm_state_mut().lapics[vtl]
.lapic
Expand Down Expand Up @@ -93,10 +97,21 @@ pub(crate) fn poll_apic_core<'b, B: HardwareIsolatedBacking, T: ApicBacking<'b,
}

// Interrupts are ignored while waiting for SIPI.
let supports_nmi_masking = apic_backing.supports_nmi_masking();
let lapic = &mut apic_backing.vp().backing.cvm_state_mut().lapics[vtl];
if lapic.activity != MpState::WaitForSipi {
if nmi || lapic.nmi_pending {
if lint1 {
if supports_nmi_masking || !lapic.cross_vtl_nmi_requested {
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate on this cross_vtl_nmi_requested thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VTL0-generated NMIs may occur during normal execution e.g. due to KD interaction, and so we don't want these to block crash dumps. But if VTL1 injected an NMI that only occurs due to bug check so don't allow other NMI sources

lapic.nmi_suppression |= NMI_SUPPRESS_LINT1_REQUESTED;
lapic.nmi_pending = true;
}
}

if nmi {
lapic.nmi_pending = true;
}

if lapic.nmi_pending {
apic_backing.handle_nmi(vtl);
}

Expand Down
11 changes: 11 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/hardware_cvm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2766,6 +2766,17 @@ impl<T, B: HardwareIsolatedBacking> hv1_hypercall::AssertVirtualInterrupt
return Err(HvError::InvalidParameter);
}

if matches!(
interrupt_control.interrupt_type(),
HvInterruptType::HvX64InterruptTypeNmi
) {
// Note whether this is a cross-VTL NMI request. If so, this must be
// visible before any NMIs are requested.
if self.intercepted_vtl == GuestVtl::Vtl1 && target_vtl == GuestVtl::Vtl0 {
self.vp.backing.cvm_state_mut().lapics[target_vtl].cross_vtl_nmi_requested = true;
}
}

self.vp.partition.request_msi(
target_vtl,
MsiRequest::new_x86(
Expand Down
12 changes: 12 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,21 @@ impl VtlsTlbLocked {
}
}

// NMI suppression state to prevent duplicate NMI
#[cfg(guest_arch = "x86_64")]
const NMI_SUPPRESS_LINT1_DELIVERED: u32 = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

These and nmi_suppression should be combined into a bitfield type

#[cfg(guest_arch = "x86_64")]
const NMI_SUPPRESS_LINT1_REQUESTED: u32 = 1 << 1;

#[cfg(guest_arch = "x86_64")]
#[derive(Inspect)]
pub(crate) struct LapicState {
lapic: LocalApic,
activity: MpState,
nmi_pending: bool,
lint1_pending: bool,
nmi_suppression: u32,
cross_vtl_nmi_requested: bool,
}

#[cfg(guest_arch = "x86_64")]
Expand All @@ -169,6 +178,9 @@ impl LapicState {
lapic,
activity,
nmi_pending: false,
lint1_pending: false,
nmi_suppression: 0,
cross_vtl_nmi_requested: false,
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/snp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ use crate::UhCvmVpState;
use crate::UhPartitionInner;
use crate::UhPartitionNewParams;
use crate::WakeReason;
use crate::processor::NMI_SUPPRESS_LINT1_DELIVERED;
use crate::processor::NMI_SUPPRESS_LINT1_REQUESTED;
use crate::processor::UhHypercallHandler;
use crate::processor::UhProcessor;
use crate::processor::hardware_cvm::apic::ApicBacking;
Expand Down Expand Up @@ -884,6 +886,20 @@ impl<'b> ApicBacking<'b, SnpBacked> for UhProcessor<'b, SnpBacked> {
// TODO SNP: support virtual NMI injection
// For now, just inject an NMI and hope for the best.
// Don't forget to update handle_cross_vtl_interrupts if this code changes.

if (self.backing.cvm.lapics[vtl].nmi_suppression & NMI_SUPPRESS_LINT1_DELIVERED) != 0 {
// Cancel this NMI request since it cannot be delivered.
self.backing.cvm.lapics[vtl].nmi_pending = false;
return;
}

if (self.backing.cvm.lapics[vtl].nmi_suppression & NMI_SUPPRESS_LINT1_REQUESTED) != 0 {
// If a LINT1 NMI has been requested, then it is being delivered now,
// so no further NMIs can be delivered.
self.backing.cvm.lapics[vtl].nmi_suppression &= !NMI_SUPPRESS_LINT1_REQUESTED;
self.backing.cvm.lapics[vtl].nmi_suppression |= NMI_SUPPRESS_LINT1_DELIVERED;
Copy link
Contributor

@smalis-msft smalis-msft Oct 7, 2025

Choose a reason for hiding this comment

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

Maybe I missed it, but where do we clear DELIVERED? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DELIVERED isn't cleared. Once LINT1 debug interrupt has been delivered you can't deliver another LINT1 or NMI. The HCL doesn't know when the NMI is complete so its not safe to signal twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we trace a warning or something then if we get one of these and delivered is already set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the guest is already crashing and hopefully writing a crash dump so no need to add another write at that time

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about tracing being low cost

}

let mut vmsa = self.runner.vmsa_mut(vtl);

// TODO GUEST VSM: Don't inject the NMI if there's already an event
Expand All @@ -905,6 +921,10 @@ impl<'b> ApicBacking<'b, SnpBacked> for UhProcessor<'b, SnpBacked> {
vmsa.set_rip(0);
self.backing.cvm.lapics[vtl].activity = MpState::Running;
}

fn supports_nmi_masking(&mut self) -> bool {
false
}
}

impl UhProcessor<'_, SnpBacked> {
Expand Down
4 changes: 4 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,10 @@ impl<'b> hardware_cvm::apic::ApicBacking<'b, TdxBacked> for TdxApicScanner<'_, '
self.vp.backing.vtls[vtl].private_regs.rip = 0;
self.vp.backing.cvm.lapics[vtl].activity = MpState::Running;
}

fn supports_nmi_masking(&mut self) -> bool {
true
}
}

impl UhProcessor<'_, TdxBacked> {
Expand Down
11 changes: 11 additions & 0 deletions vm/devices/get/get_protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ open_enum! {
MODIFY_VTL2_SETTINGS_REV1 = 6,
// --- GE ---
BATTERY_STATUS = 7,
INJECT_DEBUG_INTERRUPT = 8,
}
}

Expand Down Expand Up @@ -1759,6 +1760,16 @@ impl BatteryStatusNotification {
}
}

#[repr(C)]
#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout)]
pub struct InjectDebugInterruptNotification {
pub message_header: HeaderGuestNotification,
pub vtl: u8,
pub _pad: u8,
}

const_assert_eq!(6, size_of::<InjectDebugInterruptNotification>());

#[bitfield(u64)]
#[derive(IntoBytes, FromBytes, Immutable, KnownLayout)]
pub struct CreateRamGpaRangeFlags {
Expand Down
6 changes: 6 additions & 0 deletions vm/devices/get/guest_emulation_transport/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ impl GuestEmulationTransportClient {
.notify(msg::Msg::SetGpaAllocator(gpa_allocator));
}

/// Set the the callback to trigger the debug interrupt.
pub fn set_debug_interrupt_callback(&mut self, callback: Box<dyn Fn(u8) + Send + Sync>) {
self.control
.notify(msg::Msg::SetDebugInterruptCallback(callback));
}

/// Send the attestation request to the IGVM agent on the host.
pub async fn igvm_attest(
&self,
Expand Down
39 changes: 37 additions & 2 deletions vm/devices/get/guest_emulation_transport/src/process_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ pub(crate) mod msg {
Inspect(inspect::Deferred),
/// Store the gpa allocator to be used for attestation.
SetGpaAllocator(Arc<dyn DmaClient>),
/// Store the callback to trigger the debug interrupt.
SetDebugInterruptCallback(Box<dyn Fn(u8) + Send + Sync>),

// Late bound receivers for Guest Notifications
/// Take the late-bound GuestRequest receiver for Generation Id updates.
Expand Down Expand Up @@ -493,6 +495,8 @@ pub(crate) struct ProcessLoop<T: RingMem> {
#[inspect(skip)]
secondary_host_requests_read_send: mesh::Sender<Vec<u8>>,
gpa_allocator: Option<Arc<dyn DmaClient>>,
#[inspect(skip)]
set_debug_interrupt: Option<Box<dyn Fn(u8) + Send + Sync>>,
stats: Stats,

guest_notification_listeners: GuestNotificationListeners,
Expand Down Expand Up @@ -700,6 +704,7 @@ impl<T: RingMem> ProcessLoop<T> {
battery_status: GuestNotificationSender::new(),
},
gpa_allocator: None,
set_debug_interrupt: None,
}
}

Expand Down Expand Up @@ -1029,6 +1034,9 @@ impl<T: RingMem> ProcessLoop<T> {
Msg::SetGpaAllocator(gpa_allocator) => {
self.gpa_allocator = Some(gpa_allocator);
}
Msg::SetDebugInterruptCallback(callback) => {
self.set_debug_interrupt = Some(callback);
}

// Late bound receivers for Guest Notifications
Msg::TakeVtl2SettingsReceiver(req) => req.handle_sync(|()| {
Expand Down Expand Up @@ -1244,8 +1252,8 @@ impl<T: RingMem> ProcessLoop<T> {
) -> Result<(), FatalError> {
use get_protocol::GuestNotifications;

// Version must be latest. Give up if not.
if header.message_version != get_protocol::MessageVersions::HEADER_VERSION_1 {
// Version must be at least version 1.
if header.message_version < get_protocol::MessageVersions::HEADER_VERSION_1 {
tracing::error!(
msg = ?buf,
version = ?header.message_version,
Expand Down Expand Up @@ -1287,6 +1295,9 @@ impl<T: RingMem> ProcessLoop<T> {
GuestNotifications::BATTERY_STATUS => {
self.handle_battery_status_notification(read_guest_notification(id, buf)?)?;
}
GuestNotifications::INJECT_DEBUG_INTERRUPT => {
self.handle_debug_interrupt_notification(read_guest_notification(id, buf)?)?;
}
invalid_notification => {
tracing::error!(
?invalid_notification,
Expand Down Expand Up @@ -1489,6 +1500,30 @@ impl<T: RingMem> ProcessLoop<T> {
})
}

fn handle_debug_interrupt_notification(
&mut self,
notification: get_protocol::InjectDebugInterruptNotification,
) -> Result<(), FatalError> {
tracing::debug!(
"Received inject debug interrupt notification, vtl = {}",
notification.vtl,
);

if notification.vtl != 0 {
tracing::error!(
?notification.vtl,
"Debug interrupts can only be injected into VTL 0",
);
} else {
// Trigger the LINT1 interrupt vector on the LAPIC of the BSP.
self.set_debug_interrupt
.as_ref()
.map(|callback| callback(notification.vtl));
}

Ok(())
}

fn complete_modify_vtl2_settings(
&mut self,
result: Result<(), RpcError<Vec<Vtl2SettingsErrorInfo>>>,
Expand Down
Loading
Loading