Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virt_mshv_vtl: Share common processing between poll_apic implementations #630

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

virt_mshv_vtl: Share common processing between poll_apic implementations #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79d8819
virt_mshv_vtl: Share common processing between poll_apic implementations
smalis-msft Jan 8, 2025
1aa8518
fmt
smalis-msft Jan 8, 2025
9e6782e
Fix arm
smalis-msft Jan 8, 2025
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
79 changes: 79 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/apic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#![cfg(guest_arch = "x86_64")]

use super::LapicState;
use super::UhRunVpError;
use hcl::GuestVtl;
use virt::vp::MpState;
use virt_support_apic::ApicWork;

pub(crate) trait ApicBacking {
fn handle_init(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError>;
fn handle_sipi(&mut self, vtl: GuestVtl, vector: u8) -> Result<(), UhRunVpError>;
fn handle_nmi(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError>;
fn handle_interrupt(&mut self, vtl: GuestVtl, vector: u8) -> Result<(), UhRunVpError>;
fn handle_extint(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError>;
}

pub(crate) fn poll_apic_core<T: ApicBacking>(
processor: &mut T,
scan: impl Fn(&mut T) -> ApicWork,
Copy link
Contributor Author

@smalis-msft smalis-msft Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These closures could all move onto ApicBacking if we want, just creates a larger diff.

proxy_irr: impl Fn(&mut T) -> Option<[u32; 8]>,
lapic_access: impl Fn(&mut T) -> &mut LapicState,
vtl1_enabled: impl Fn(&mut T) -> bool,
vtl: GuestVtl,
) -> Result<(), UhRunVpError> {
// Check for interrupt requests from the host and kernel offload.
if vtl == GuestVtl::Vtl0 {
if let Some(irr) = proxy_irr(processor) {
// We can't put the interrupts directly into offload (where supported) because we might need
// to clear the tmr state. This can happen if a vector was previously used for a level
// triggered interrupt, and is now being used for an edge-triggered interrupt.
lapic_access(processor).lapic.request_fixed_interrupts(irr);
}
}

let ApicWork {
init,
extint,
sipi,
nmi,
interrupt,
} = scan(processor);

// An INIT/SIPI targeted at a VP with more than one guest VTL enabled is ignored.
// Check VTL enablement inside each block to avoid taking a lock on the hot path,
// INIT and SIPI are quite cold.
if init {
if !vtl1_enabled(processor) {
processor.handle_init(vtl)?;
}
}

if let Some(vector) = sipi {
if !vtl1_enabled(processor) {
processor.handle_sipi(vtl, vector)?;
}
}

// Interrupts are ignored while waiting for SIPI.
let lapic = lapic_access(processor);
if lapic.activity != MpState::WaitForSipi {
if nmi || lapic.nmi_pending {
lapic.nmi_pending = true;
processor.handle_nmi(vtl)?;
}

if let Some(vector) = interrupt {
processor.handle_interrupt(vtl, vector)?;
}

if extint {
processor.handle_extint(vtl)?;
}
}

Ok(())
}
1 change: 1 addition & 0 deletions openhcl/virt_mshv_vtl/src/processor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! This module contains Underhill specific functionality and implementations of require traits
//! in order to plug into the rest of the common HvLite code.

mod apic;
pub mod mshv;
mod nice;
mod vp_state;
Expand Down
62 changes: 21 additions & 41 deletions openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use super::super::vp_state::UhVpStateAccess;
use super::super::BackingPrivate;
use super::super::UhEmulationState;
use super::super::UhRunVpError;
use crate::processor::apic::ApicBacking;
use crate::processor::from_seg;
use crate::processor::LapicState;
use crate::processor::SidecarExitReason;
Expand Down Expand Up @@ -70,7 +71,6 @@ use virt::StopVp;
use virt::VpHaltReason;
use virt::VpIndex;
use virt_support_apic::ApicClient;
use virt_support_apic::ApicWork;
use virt_support_x86emu::emulate::EmuCheckVtlAccessError;
use virt_support_x86emu::emulate::EmuTranslateError;
use virt_support_x86emu::emulate::EmuTranslateResult;
Expand Down Expand Up @@ -323,48 +323,22 @@ impl BackingPrivate for HypervisorBackedX86 {
vtl: GuestVtl,
scan_irr: bool,
) -> Result<(), UhRunVpError> {
let Some(lapics) = this.backing.lapics.as_mut() else {
if this.backing.lapics.is_none() {
return Ok(());
};

let ApicWork {
init,
extint,
sipi,
nmi,
interrupt,
} = lapics[vtl].lapic.scan(&mut this.vmtime, scan_irr);

// TODO WHP GUEST VSM: An INIT/SIPI targeted at a VP with more than one guest VTL enabled is ignored.
if init {
this.handle_init(vtl)?;
}

if let Some(vector) = sipi {
this.handle_sipi(vtl, vector)?;
}

let Some(lapics) = this.backing.lapics.as_mut() else {
unreachable!()
};

// Interrupts are ignored while waiting for SIPI.
if lapics[vtl].activity != MpState::WaitForSipi {
if nmi || lapics[vtl].nmi_pending {
lapics[vtl].nmi_pending = true;
this.handle_nmi(vtl)?;
}

if let Some(vector) = interrupt {
this.handle_interrupt(vtl, vector)?;
}

if extint {
todo!();
}
}

Ok(())
super::super::apic::poll_apic_core(
this,
|this| {
this.backing.lapics.as_mut().unwrap()[vtl]
.lapic
.scan(&mut this.vmtime, scan_irr)
},
|_| None,
|this| &mut this.backing.lapics.as_mut().unwrap()[vtl],
|_| false, // TODO WHP GUEST VSM
vtl,
)
}

fn halt_in_usermode(this: &mut UhProcessor<'_, Self>, target_vtl: GuestVtl) -> bool {
Expand Down Expand Up @@ -937,7 +911,7 @@ impl<'a, 'b> InterceptHandler<'a, 'b> {
}
}

impl UhProcessor<'_, HypervisorBackedX86> {
impl ApicBacking for UhProcessor<'_, HypervisorBackedX86> {
fn handle_interrupt(&mut self, vtl: GuestVtl, vector: u8) -> Result<(), UhRunVpError> {
const NAMES: &[HvX64RegisterName] = &[
HvX64RegisterName::Rflags,
Expand Down Expand Up @@ -1112,6 +1086,12 @@ impl UhProcessor<'_, HypervisorBackedX86> {
Ok(())
}

fn handle_extint(&mut self, _vtl: GuestVtl) -> Result<(), UhRunVpError> {
todo!()
}
}

impl UhProcessor<'_, HypervisorBackedX86> {
fn set_rip(&mut self, vtl: GuestVtl, rip: u64) -> Result<(), VpHaltReason<UhRunVpError>> {
self.runner
.set_vp_register(vtl, HvX64RegisterName::Rip, rip.into())
Expand Down
82 changes: 26 additions & 56 deletions openhcl/virt_mshv_vtl/src/processor/snp/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

//! Processor support for SNP partitions.

use super::apic::ApicBacking;
use super::from_seg;
use super::hardware_cvm;
use super::private::BackingParams;
Expand Down Expand Up @@ -52,7 +53,6 @@ use virt::Processor;
use virt::VpHaltReason;
use virt::VpIndex;
use virt_support_apic::ApicClient;
use virt_support_apic::ApicWork;
use virt_support_x86emu::emulate::emulate_io;
use virt_support_x86emu::emulate::emulate_translate_gva;
use virt_support_x86emu::emulate::EmulatorSupport as X86EmulatorSupport;
Expand Down Expand Up @@ -392,61 +392,22 @@ impl BackingPrivate for SnpBacked {
vtl: GuestVtl,
scan_irr: bool,
) -> Result<(), UhRunVpError> {
// Check for interrupt requests from the host.
// TODO SNP GUEST VSM supporting VTL 1 proxy irrs requires kernel changes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment (and the corresponding one in TDX) are gone now, as John and I decided that we don't plan on implementing this in the near future. Just doesn't seem to be worthwhile.

if vtl == GuestVtl::Vtl0 {
if let Some(irr) = this.runner.proxy_irr() {
// TODO SNP: filter proxy IRRs.
this.backing.cvm.lapics[vtl]
.lapic
.request_fixed_interrupts(irr);
}
}

// Clear any pending interrupt.
this.runner.vmsa_mut(vtl).v_intr_cntrl_mut().set_irq(false);

let ApicWork {
init,
extint,
sipi,
nmi,
interrupt,
} = this.backing.cvm.lapics[vtl]
.lapic
.scan(&mut this.vmtime, scan_irr);

// An INIT/SIPI targeted at a VP with more than one guest VTL enabled is ignored.
// Check VTL enablement inside each block to avoid taking a lock on the hot path,
// INIT and SIPI are quite cold.
if init {
if !*this.inner.hcvm_vtl1_enabled.lock() {
this.handle_init(vtl)?;
}
}

if let Some(vector) = sipi {
if !*this.inner.hcvm_vtl1_enabled.lock() {
this.handle_sipi(vtl, vector)?;
}
}

// Interrupts are ignored while waiting for SIPI.
if this.backing.cvm.lapics[vtl].activity != MpState::WaitForSipi {
if nmi {
this.handle_nmi(vtl);
}

if let Some(vector) = interrupt {
this.handle_interrupt(vtl, vector);
}

if extint {
tracelimit::warn_ratelimited!("extint not supported");
}
}

Ok(())
// TODO SNP: filter proxy IRRs.
super::apic::poll_apic_core(
this,
|this| {
this.backing.cvm.lapics[vtl]
.lapic
.scan(&mut this.vmtime, scan_irr)
},
|this| this.runner.proxy_irr(),
|this| &mut this.backing.cvm.lapics[vtl],
|this| *this.inner.hcvm_vtl1_enabled.lock(),
vtl,
)
}

fn request_extint_readiness(_this: &mut UhProcessor<'_, Self>) {
Expand Down Expand Up @@ -773,17 +734,18 @@ impl<T> HypercallIo for GhcbEnlightenedHypercall<'_, '_, '_, T> {
}
}

impl UhProcessor<'_, SnpBacked> {
fn handle_interrupt(&mut self, vtl: GuestVtl, vector: u8) {
impl ApicBacking for UhProcessor<'_, SnpBacked> {
fn handle_interrupt(&mut self, vtl: GuestVtl, vector: u8) -> Result<(), UhRunVpError> {
let mut vmsa = self.runner.vmsa_mut(vtl);
vmsa.v_intr_cntrl_mut().set_vector(vector);
vmsa.v_intr_cntrl_mut().set_priority((vector >> 4).into());
vmsa.v_intr_cntrl_mut().set_ignore_tpr(false);
vmsa.v_intr_cntrl_mut().set_irq(true);
self.backing.cvm.lapics[vtl].activity = MpState::Running;
Ok(())
}

fn handle_nmi(&mut self, vtl: GuestVtl) {
fn handle_nmi(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError> {
// 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.
Expand All @@ -795,6 +757,7 @@ impl UhProcessor<'_, SnpBacked> {
.with_valid(true),
);
self.backing.cvm.lapics[vtl].activity = MpState::Running;
Ok(())
}

fn handle_init(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError> {
Expand Down Expand Up @@ -822,6 +785,13 @@ impl UhProcessor<'_, SnpBacked> {
Ok(())
}

fn handle_extint(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError> {
tracelimit::warn_ratelimited!(?vtl, "extint not supported");
Ok(())
}
}

impl UhProcessor<'_, SnpBacked> {
fn handle_synic_deliverable_exit(&mut self) {
let message = hvdef::HvX64SynicSintDeliverableMessage::ref_from_prefix(
self.runner.exit_message().payload(),
Expand Down
Loading
Loading