From 10a0118cf7ab1d781369d7389450956cc33bf56f Mon Sep 17 00:00:00 2001 From: Steven Malis <137308034+smalis-msft@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:32:25 -0400 Subject: [PATCH] Move usermode halt logic into backing-specific logic (#114) Supersedes https://github.com/microsoft/openvmm/pull/111. Since WHP is the only case in which we can hit this path, make it a bit clearer with a new per-backing method. Open for bikeshedding of course --- openhcl/virt_mshv_vtl/src/processor/mod.rs | 22 ++++++++++++------- .../virt_mshv_vtl/src/processor/mshv/apic.rs | 15 +++---------- .../virt_mshv_vtl/src/processor/mshv/arm64.rs | 4 ++-- .../virt_mshv_vtl/src/processor/mshv/x64.rs | 11 +++++++++- .../virt_mshv_vtl/src/processor/snp/mod.rs | 7 ++---- .../virt_mshv_vtl/src/processor/tdx/mod.rs | 7 ++---- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/openhcl/virt_mshv_vtl/src/processor/mod.rs b/openhcl/virt_mshv_vtl/src/processor/mod.rs index cfb3f0b0ed..f26ded6b61 100644 --- a/openhcl/virt_mshv_vtl/src/processor/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/mod.rs @@ -207,12 +207,12 @@ mod private { stop: &mut StopVp<'_>, ) -> impl Future>>; - /// Returns true if the VP is ready to run the given VTL, false if it is halted. + /// Process any pending APIC work. fn poll_apic( this: &mut UhProcessor<'_, Self>, vtl: GuestVtl, scan_irr: bool, - ) -> Result; + ) -> Result<(), UhRunVpError>; /// Requests the VP to exit when an external interrupt is ready to be /// delivered. @@ -235,6 +235,13 @@ mod private { /// the target VTL that will become active. fn switch_vtl_state(this: &mut UhProcessor<'_, Self>, target_vtl: GuestVtl); + /// Returns whether this VP should be put to sleep in usermode, or + /// whether it's ready to proceed into the kernel. + fn halt_in_usermode(this: &mut UhProcessor<'_, Self>, target_vtl: GuestVtl) -> bool { + let _ = (this, target_vtl); + false + } + fn inspect_extra(_this: &mut UhProcessor<'_, Self>, _resp: &mut inspect::Response<'_>) {} } } @@ -654,15 +661,13 @@ impl<'p, T: Backing> Processor for UhProcessor<'p, T> { self.update_synic(GuestVtl::Vtl0, true); } - // TODO CVM GUEST VSM: Split ready into two to track per-vtl - let mut ready = false; for vtl in [GuestVtl::Vtl1, GuestVtl::Vtl0] { // Process interrupts. if self.hv(vtl).is_some() { self.update_synic(vtl, false); } - ready |= T::poll_apic(self, vtl, scan_irr[vtl] || first_scan_irr) + T::poll_apic(self, vtl, scan_irr[vtl] || first_scan_irr) .map_err(VpHaltReason::Hypervisor)?; } first_scan_irr = false; @@ -675,11 +680,12 @@ impl<'p, T: Backing> Processor for UhProcessor<'p, T> { } } - if ready { + // TODO WHP GUEST VSM: This should be next_vtl + if T::halt_in_usermode(self, GuestVtl::Vtl0) { + break Poll::Pending; + } else { return >>::Ok(()).into(); } - - break Poll::Pending; }) .await?; diff --git a/openhcl/virt_mshv_vtl/src/processor/mshv/apic.rs b/openhcl/virt_mshv_vtl/src/processor/mshv/apic.rs index d7a7b74faf..113fed4898 100644 --- a/openhcl/virt_mshv_vtl/src/processor/mshv/apic.rs +++ b/openhcl/virt_mshv_vtl/src/processor/mshv/apic.rs @@ -262,13 +262,9 @@ impl UhApicState { impl UhProcessor<'_, HypervisorBackedX86> { /// Returns true if the VP is ready to run, false if it is halted. - pub(super) fn poll_apic( - &mut self, - vtl: GuestVtl, - scan_irr: bool, - ) -> Result { + pub(super) fn poll_apic(&mut self, vtl: GuestVtl, scan_irr: bool) -> Result<(), UhRunVpError> { let Some(lapics) = self.backing.lapics.as_mut() else { - return Ok(true); + return Ok(()); }; let lapic = &mut lapics[vtl]; @@ -309,12 +305,7 @@ impl UhProcessor<'_, HypervisorBackedX86> { self.handle_sipi(vtl, vector)?; } - let lapic = &self.backing.lapics.as_ref().unwrap()[vtl]; - if lapic.halted || lapic.startup_suspend { - return Ok(false); - } - - Ok(true) + Ok(()) } fn handle_init(&mut self, vtl: GuestVtl) -> Result<(), UhRunVpError> { diff --git a/openhcl/virt_mshv_vtl/src/processor/mshv/arm64.rs b/openhcl/virt_mshv_vtl/src/processor/mshv/arm64.rs index 757aac4014..59cee72030 100644 --- a/openhcl/virt_mshv_vtl/src/processor/mshv/arm64.rs +++ b/openhcl/virt_mshv_vtl/src/processor/mshv/arm64.rs @@ -200,8 +200,8 @@ impl BackingPrivate for HypervisorBackedArm64 { _this: &mut UhProcessor<'_, Self>, _vtl: GuestVtl, _scan_irr: bool, - ) -> Result { - Ok(true) + ) -> Result<(), UhRunVpError> { + Ok(()) } fn request_extint_readiness(this: &mut UhProcessor<'_, Self>) { diff --git a/openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs b/openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs index 86788213a2..031395d596 100644 --- a/openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs +++ b/openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs @@ -292,10 +292,19 @@ impl BackingPrivate for HypervisorBackedX86 { this: &mut UhProcessor<'_, Self>, vtl: GuestVtl, scan_irr: bool, - ) -> Result { + ) -> Result<(), UhRunVpError> { this.poll_apic(vtl, scan_irr) } + fn halt_in_usermode(this: &mut UhProcessor<'_, Self>, target_vtl: GuestVtl) -> bool { + if let Some(lapics) = this.backing.lapics.as_ref() { + if lapics[target_vtl].halted || lapics[target_vtl].startup_suspend { + return true; + } + } + false + } + fn request_extint_readiness(this: &mut UhProcessor<'_, Self>) { this.backing .next_deliverability_notifications diff --git a/openhcl/virt_mshv_vtl/src/processor/snp/mod.rs b/openhcl/virt_mshv_vtl/src/processor/snp/mod.rs index 72d0412aaf..70775f2374 100644 --- a/openhcl/virt_mshv_vtl/src/processor/snp/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/snp/mod.rs @@ -340,7 +340,7 @@ impl BackingPrivate for SnpBacked { this: &mut UhProcessor<'_, Self>, vtl: GuestVtl, scan_irr: bool, - ) -> Result { + ) -> Result<(), UhRunVpError> { // Check for interrupt requests from the host. // TODO SNP GUEST VSM supporting VTL 1 proxy irrs requires kernel changes if vtl == GuestVtl::Vtl0 { @@ -390,10 +390,7 @@ impl BackingPrivate for SnpBacked { } } - // Return ready even if halted. `run_vp` will wait in the kernel when - // halted, which is necessary so that we are efficiently notified when - // more interrupts arrive. - Ok(true) + Ok(()) } fn request_extint_readiness(_this: &mut UhProcessor<'_, Self>) { diff --git a/openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs b/openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs index 0baaeadc06..91b75862a4 100644 --- a/openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs @@ -736,7 +736,7 @@ impl BackingPrivate for TdxBacked { this: &mut UhProcessor<'_, Self>, _vtl: GuestVtl, scan_irr: bool, - ) -> Result { + ) -> Result<(), UhRunVpError> { if !this.try_poll_apic(scan_irr)? { tracing::info!("disabling APIC offload due to auto EOI"); let page = zerocopy::transmute_mut!(this.runner.tdx_apic_page_mut()); @@ -747,10 +747,7 @@ impl BackingPrivate for TdxBacked { this.try_poll_apic(false)?; } - // Return ready even if halted. `run_vp` will wait in the kernel when - // halted, which is necessary so that we are efficiently notified when - // more interrupts arrive. - Ok(true) + Ok(()) } fn request_extint_readiness(_this: &mut UhProcessor<'_, Self>) {