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

underhill_mem: Wire up TLB access for flushing and locking #841

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+277 −115
Open

underhill_mem: Wire up TLB access for flushing and locking #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb37fbd
Add TLB lock & flush access to underhill_mem
smalis-msft Feb 11, 2025
d91d4a3
feedback comment
smalis-msft Feb 13, 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
64 changes: 52 additions & 12 deletions openhcl/underhill_mem/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use std::sync::Arc;
use thiserror::Error;
use virt::IsolationType;
use virt_mshv_vtl::ProtectIsolatedMemory;
use virt_mshv_vtl::TlbFlushLockAccess;
use vm_topology::memory::MemoryLayout;
use x86defs::snp::SevRmpAdjust;
use x86defs::tdx::GpaVmAttributes;
Expand Down Expand Up @@ -265,7 +266,7 @@ impl MemoryAcceptor {

/// Apply the initial protections on lower-vtl memory.
///
/// After initialization, the default protections should be applied.
/// After initialization, the default protections should be applied.
pub fn apply_initial_lower_vtl_protections(
&self,
range: MemoryRange,
Expand Down Expand Up @@ -337,7 +338,6 @@ impl MemoryAcceptor {
permissions: rmpadjust,
vtl: vtl.into(),
})
// TODO SNP: Flush TLB
}
GpaVtlPermissions::Tdx((attributes, mask)) => {
// For TDX VMs, the permissions apply to the specified VTL.
Expand Down Expand Up @@ -375,13 +375,24 @@ struct HypercallOverlay {
permissions: GpaVtlPermissions,
}

// TODO CVM GUEST VSM: This type needs to go away, and proper functionality needs
// to be added here, but resolving the layering with hv1_emulator is complicated.
struct NoOpTlbFlushLockAccess;
impl TlbFlushLockAccess for NoOpTlbFlushLockAccess {
fn flush(&mut self, _vtl: GuestVtl) {}
fn flush_entire(&mut self) {}
fn set_wait_for_tlb_locks(&mut self, _vtl: GuestVtl) {}
}

impl VtlProtectHypercallOverlay for HypercallOverlayProtector {
fn change_overlay(&self, gpn: u64) {
self.protector.change_hypercall_overlay(self.vtl, gpn)
self.protector
.change_hypercall_overlay(self.vtl, gpn, &mut NoOpTlbFlushLockAccess)
smalis-msft marked this conversation as resolved.
Show resolved Hide resolved
}

fn disable_overlay(&self) {
self.protector.disable_hypercall_overlay(self.vtl)
self.protector
.disable_hypercall_overlay(self.vtl, &mut NoOpTlbFlushLockAccess)
}
}

Expand Down Expand Up @@ -471,7 +482,12 @@ impl HardwareIsolatedMemoryProtector {
}

impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
fn change_host_visibility(&self, shared: bool, gpns: &[u64]) -> Result<(), (HvError, usize)> {
fn change_host_visibility(
&self,
shared: bool,
gpns: &[u64],
tlb_access: &mut dyn TlbFlushLockAccess,
sluck-msft marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<(), (HvError, usize)> {
// Validate the ranges are RAM.
for &gpn in gpns {
if !self
Expand Down Expand Up @@ -531,7 +547,13 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
clear_bitmap.update_bitmap(range, false);
}

// TODO SNP: flush concurrent accessors and TLB.
// TODO SNP: flush concurrent accessors.
if let IsolationType::Snp = self.acceptor.isolation {
// We need to ensure that the guest TLB has been fully flushed since
// the unaccept operation is not guaranteed to do so in hardware,
// and the hypervisor is also not trusted with TLB hygiene.
tlb_access.flush_entire();
}

// TODO SNP: check list of locks, roll back bitmap changes if there was one.

Expand Down Expand Up @@ -646,6 +668,7 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
&self,
vtl: GuestVtl,
vtl_protections: HvMapGpaFlags,
tlb_access: &mut dyn TlbFlushLockAccess,
) -> Result<(), HvError> {
// Prevent visibility changes while VTL protections are being
// applied.
Expand Down Expand Up @@ -693,6 +716,10 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
self.apply_protections_with_overlay_handling(vtl, &ranges, vtl_protections)
.expect("applying vtl protections should succeed");

// Invalidate the entire VTL 0 TLB to ensure that the new permissions
// are observed.
tlb_access.flush(GuestVtl::Vtl0);

Ok(())
}

Expand All @@ -701,6 +728,7 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
vtl: GuestVtl,
gpns: &[u64],
protections: HvMapGpaFlags,
tlb_access: &mut dyn TlbFlushLockAccess,
) -> Result<(), (HvError, usize)> {
// Validate the ranges are RAM.
for &gpn in gpns {
Expand Down Expand Up @@ -737,7 +765,13 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
self.apply_protections_with_overlay_handling(vtl, &ranges, protections)
.expect("applying vtl protections should succeed");

// TODO CVM GUEST VSM: flush TLB and wait for the tlb lock
// Since page protections were modified, we must invalidate the entire
// VTL 0 TLB to ensure that the new permissions are observed, and wait for
// other CPUs to release all guest mappings before declaring that the VTL
// protection change has completed.
tlb_access.flush(GuestVtl::Vtl0);
tlb_access.set_wait_for_tlb_locks(vtl);

Ok(())
}

Expand All @@ -751,7 +785,12 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
})
}

fn change_hypercall_overlay(&self, vtl: GuestVtl, gpn: u64) {
fn change_hypercall_overlay(
&self,
vtl: GuestVtl,
gpn: u64,
tlb_access: &mut dyn TlbFlushLockAccess,
) {
// Should already have written contents to the page via the guest
// memory object, confirming that this is a guest page
assert!(self
Expand Down Expand Up @@ -806,14 +845,15 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {
.apply_protections_from_flags(
MemoryRange::new(gpn * HV_PAGE_SIZE..(gpn + 1) * HV_PAGE_SIZE),
vtl,
HV_MAP_GPA_PERMISSIONS_ALL,
HV_MAP_GPA_PERMISSIONS_ALL.with_writable(false),
)
.expect("applying vtl protections should succeed");

// TODO CVM GUEST VSM: flush TLB
// Flush the guest TLB to ensure that the new permissions are observed.
tlb_access.flush(vtl);
}

fn disable_hypercall_overlay(&self, vtl: GuestVtl) {
fn disable_hypercall_overlay(&self, vtl: GuestVtl, tlb_access: &mut dyn TlbFlushLockAccess) {
let _lock = self.inner.lock();

let mut overlay = self.hypercall_overlay[vtl].lock();
Expand All @@ -825,7 +865,7 @@ impl ProtectIsolatedMemory for HardwareIsolatedMemoryProtector {

*overlay = None;

// TODO CVM GUEST VSM: flush TLB
tlb_access.flush(vtl);
}

fn set_vtl1_protections_enabled(&self) {
Expand Down
30 changes: 27 additions & 3 deletions openhcl/virt_mshv_vtl/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,12 @@ pub struct UhLateParams<'a> {
/// Trait for CVM-related protections on guest memory.
pub trait ProtectIsolatedMemory: Send + Sync {
/// Changes host visibility on guest memory.
fn change_host_visibility(&self, shared: bool, gpns: &[u64]) -> Result<(), (HvError, usize)>;
fn change_host_visibility(
&self,
shared: bool,
gpns: &[u64],
tlb_access: &mut dyn TlbFlushLockAccess,
) -> Result<(), (HvError, usize)>;

/// Queries host visibility on guest memory.
fn query_host_visibility(
Expand All @@ -1274,6 +1279,7 @@ pub trait ProtectIsolatedMemory: Send + Sync {
&self,
vtl: GuestVtl,
protections: HvMapGpaFlags,
tlb_access: &mut dyn TlbFlushLockAccess,
) -> Result<(), HvError>;

/// Changes the vtl protections on a range of guest memory.
Expand All @@ -1282,6 +1288,7 @@ pub trait ProtectIsolatedMemory: Send + Sync {
vtl: GuestVtl,
gpns: &[u64],
protections: HvMapGpaFlags,
tlb_access: &mut dyn TlbFlushLockAccess,
) -> Result<(), (HvError, usize)>;

/// Retrieves a protector for the hypercall code page overlay for a target
Expand All @@ -1292,10 +1299,15 @@ pub trait ProtectIsolatedMemory: Send + Sync {
) -> Box<dyn VtlProtectHypercallOverlay>;

/// Changes the overlay for the hypercall code page for a target VTL.
fn change_hypercall_overlay(&self, vtl: GuestVtl, gpn: u64);
fn change_hypercall_overlay(
&self,
vtl: GuestVtl,
gpn: u64,
tlb_access: &mut dyn TlbFlushLockAccess,
);

/// Disables the overlay for the hypercall code page for a target VTL.
fn disable_hypercall_overlay(&self, vtl: GuestVtl);
fn disable_hypercall_overlay(&self, vtl: GuestVtl, tlb_access: &mut dyn TlbFlushLockAccess);

/// Alerts the memory protector that vtl 1 is ready to set vtl protections
/// on lower-vtl memory, and that these protections should be enforced.
Expand All @@ -1306,6 +1318,18 @@ pub trait ProtectIsolatedMemory: Send + Sync {
fn vtl1_protections_enabled(&self) -> bool;
}

/// Trait for access to TLB flush and lock machinery.
pub trait TlbFlushLockAccess {
/// Flush the entire TLB for all VPs for the given VTL.
fn flush(&mut self, vtl: GuestVtl);

/// Flush the entire TLB for all VPs for all VTLs.
fn flush_entire(&mut self);

/// Causes the specified VTL on the current VP to wait on all TLB locks.
fn set_wait_for_tlb_locks(&mut self, vtl: GuestVtl);
}

/// A partially built partition. Used to allow querying partition capabilities
/// before fully instantiating the partition.
pub struct UhProtoPartition<'a> {
Expand Down
Loading
Loading