Skip to content

SNP: Secure AVIC support #1172

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.

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9734,6 +9734,7 @@ dependencies = [
"arbitrary",
"bitfield-struct 0.10.1",
"open_enum",
"static_assertions",
"zerocopy 0.8.24",
]

Expand Down
41 changes: 37 additions & 4 deletions openhcl/hcl/src/ioctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ use std::sync::atomic::Ordering;
use thiserror::Error;
use user_driver::DmaClient;
use user_driver::memory::MemoryBlock;
use x86defs::snp::SevAvicPage;
use x86defs::snp::SevVmsa;
use x86defs::tdx::TdCallResultCode;
use x86defs::vmx::ApicPage;
use x86defs::vmx::VmxApicPage;
use zerocopy::FromBytes;
use zerocopy::FromZeros;
use zerocopy::Immutable;
Expand Down Expand Up @@ -395,6 +396,7 @@ mod ioctls {
const MSHV_INVLPGB: u16 = 0x36;
const MSHV_TLBSYNC: u16 = 0x37;
const MSHV_KICKCPUS: u16 = 0x38;
const MSHV_VTL_SECURE_AVIC_VTL0_PFN: u16 = 0x39;

#[repr(C)]
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -572,6 +574,13 @@ mod ioctls {
u64
);

ioctl_readwrite!(
hcl_read_secure_avic_vtl0_pfn,
MSHV_IOCTL,
MSHV_VTL_SECURE_AVIC_VTL0_PFN,
u64
);

pub const HCL_CAP_REGISTER_PAGE: u32 = 1;
pub const HCL_CAP_VTL_RETURN_ACTION: u32 = 2;
pub const HCL_CAP_DR6_SHARED: u32 = 3;
Expand Down Expand Up @@ -1627,9 +1636,12 @@ enum BackingState {
},
Snp {
vmsa: VtlArray<MappedPage<SevVmsa>, 2>,
vtl0_apic_page: MappedPage<SevAvicPage>,
/// VTL1 runs with the alternate interrupt injection.
vtl1_apic_page: MemoryBlock,
},
Tdx {
vtl0_apic_page: MappedPage<ApicPage>,
vtl0_apic_page: MappedPage<VmxApicPage>,
vtl1_apic_page: MemoryBlock,
},
}
Expand Down Expand Up @@ -1693,6 +1705,12 @@ impl HclVp {
.map_err(|e| Error::MmapVp(e, Some(Vtl::Vtl1)))?;
BackingState::Snp {
vmsa: [vmsa_vtl0, vmsa_vtl1].into(),
vtl0_apic_page: MappedPage::new(fd, MSHV_APIC_PAGE_OFFSET | vp as i64)
.map_err(|e| Error::MmapVp(e, Some(Vtl::Vtl0)))?,
vtl1_apic_page: private_dma_client
.ok_or(Error::MissingPrivateMemory)?
.allocate_dma_buffer(HV_PAGE_SIZE as usize)
.map_err(Error::AllocVp)?,
}
}
IsolationType::Tdx => BackingState::Tdx {
Expand Down Expand Up @@ -2205,6 +2223,8 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
| HvX64RegisterName::VsmVpSecureConfigVtl0
| HvX64RegisterName::VsmVpSecureConfigVtl1
| HvX64RegisterName::CrInterceptControl
| HvX64RegisterName::SevAvicGpa
| HvX64RegisterName::GuestVsmPartitionConfig
)
));
self.set_vp_registers_hvcall_inner(vtl, &registers)
Expand Down Expand Up @@ -2510,8 +2530,6 @@ impl Hcl {
sint: u8,
message: &HvMessage,
) -> Result<(), HvError> {
tracing::trace!(vp, sint, "posting message");

let post_message = hvdef::hypercall::PostMessageDirect {
partition_id: HV_PARTITION_ID_SELF,
vp_index: vp,
Expand Down Expand Up @@ -3128,6 +3146,21 @@ impl Hcl {
vp_pfn
}

/// Gets the PFN for the VTL 0 secure AVIC
pub fn secure_avic_vtl0_pfn(&self, cpu_index: u32) -> u64 {
let mut savic_pfn = cpu_index as u64; // input vp, output pfn

// SAFETY: The ioctl requires no prerequisites other than the Secure AVIC VTL 0
// should be mapped. This ioctl should never fail as long as the VTL 0
// Secure AVIC was mapped.
unsafe {
hcl_read_secure_avic_vtl0_pfn(self.mshv_vtl.file.as_raw_fd(), &mut savic_pfn)
.expect("should always succeed");
}

savic_pfn
}

/// Returns the isolation type for the partition.
pub fn isolation(&self) -> IsolationType {
self.isolation
Expand Down
47 changes: 46 additions & 1 deletion openhcl/hcl/src/ioctl/snp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ use sidecar_client::SidecarVp;
use std::cell::UnsafeCell;
use std::os::fd::AsRawFd;
use thiserror::Error;
use x86defs::snp::SevAvicPage;
use x86defs::snp::SevRmpAdjust;
use x86defs::snp::SevVmsa;

/// Runner backing for SNP partitions.
pub struct Snp<'a> {
vmsa: VtlArray<&'a UnsafeCell<SevVmsa>, 2>,
avic_pages: VtlArray<&'a UnsafeCell<SevAvicPage>, 2>,
}

/// Error returned by failing SNP operations.
Expand Down Expand Up @@ -176,11 +178,21 @@ impl MshvVtl {
impl<'a> super::private::BackingPrivate<'a> for Snp<'a> {
fn new(vp: &'a HclVp, sidecar: Option<&SidecarVp<'_>>, _hcl: &Hcl) -> Result<Self, NoRunner> {
assert!(sidecar.is_none());
let super::BackingState::Snp { vmsa } = &vp.backing else {
let super::BackingState::Snp {
vtl0_apic_page,
vtl1_apic_page,
vmsa,
} = &vp.backing
else {
return Err(NoRunner::MismatchedIsolation);
};

// SAFETY: The mapping is held for the appropriate lifetime, and the
// APIC page is never accessed as any other type, or by any other location.
let vtl1_apic_page = unsafe { &*vtl1_apic_page.base().cast() };

Ok(Self {
avic_pages: [vtl0_apic_page.as_ref(), vtl1_apic_page].into(),
vmsa: vmsa.each_ref().map(|mp| mp.as_ref()),
})
}
Expand Down Expand Up @@ -242,4 +254,37 @@ impl<'a> ProcessorRunner<'a, Snp<'a>> {
})
.into_inner()
}

/// Gets a PFN of the VTL0 secure AVIC page.
/// TODO: Maybe there is a better way other than passing `cpu_index`` here.
pub fn secure_avic_vtl0_pfn(&self, cpu_index: u32) -> u64 {
self.hcl.secure_avic_vtl0_pfn(cpu_index)
}

/// Gets a reference to the secure AVIC page for the given VTL.
pub fn secure_avic_page(&self, vtl: GuestVtl) -> &SevAvicPage {
// SAFETY: the APIC pages will not be concurrently accessed by the processor
// while this VP is in VTL2.
unsafe { &*self.state.avic_pages[vtl].get() }
}

/// Gets a mutable reference to the secure AVIC page for the given VTL.
pub fn secure_avic_page_mut(&mut self, vtl: GuestVtl) -> &mut SevAvicPage {
// SAFETY: the AVIC pages will not be concurrently accessed by the processor
// while this VP is in VTL2.
unsafe { &mut *self.state.avic_pages[vtl].get() }
}

/// Gets a mutable reference to the secure AVIC page for the given VTL.
pub fn secure_avic_page_vmsa_mut(
&mut self,
vtl: GuestVtl,
) -> (&mut SevAvicPage, VmsaWrapper<'_, &mut SevVmsa>) {
// SAFETY: the AVIC pages will not be concurrently accessed by the processor
// while this VP is in VTL2.
let avic_page = unsafe { &mut *self.state.avic_pages[vtl].get() };
let vmsa = self.vmsa_mut(vtl);

(avic_page, vmsa)
}
}
8 changes: 4 additions & 4 deletions openhcl/hcl/src/ioctl/tdx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ use x86defs::tdx::TdxGp;
use x86defs::tdx::TdxL2Ctls;
use x86defs::tdx::TdxL2EnterGuestState;
use x86defs::tdx::TdxVmFlags;
use x86defs::vmx::ApicPage;
use x86defs::vmx::VmcsField;
use x86defs::vmx::VmxApicPage;

/// Runner backing for TDX partitions.
pub struct Tdx<'a> {
apic_pages: VtlArray<&'a UnsafeCell<ApicPage>, 2>,
apic_pages: VtlArray<&'a UnsafeCell<VmxApicPage>, 2>,
}

impl MshvVtl {
Expand Down Expand Up @@ -123,14 +123,14 @@ impl<'a> ProcessorRunner<'a, Tdx<'a>> {
}

/// Gets a reference to the tdx APIC page for the given VTL.
pub fn tdx_apic_page(&self, vtl: GuestVtl) -> &ApicPage {
pub fn tdx_apic_page(&self, vtl: GuestVtl) -> &VmxApicPage {
// SAFETY: the APIC pages will not be concurrently accessed by the processor
// while this VP is in VTL2.
unsafe { &*self.state.apic_pages[vtl].get() }
}

/// Gets a mutable reference to the tdx APIC page for the given VTL.
pub fn tdx_apic_page_mut(&mut self, vtl: GuestVtl) -> &mut ApicPage {
pub fn tdx_apic_page_mut(&mut self, vtl: GuestVtl) -> &mut VmxApicPage {
// SAFETY: the APIC pages will not be concurrently accessed by the processor
// while this VP is in VTL2.
unsafe { &mut *self.state.apic_pages[vtl].get() }
Expand Down
6 changes: 6 additions & 0 deletions openhcl/hcl/src/vmsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use std::array;
use std::ops::Deref;
use std::ops::DerefMut;
use x86defs::snp::SecureAvicControl;
use x86defs::snp::SevEventInjectInfo;
use x86defs::snp::SevFeatures;
use x86defs::snp::SevSelector;
Expand Down Expand Up @@ -267,6 +268,11 @@ reg_direct_mut!(v_intr_cntrl, v_intr_cntrl_mut, SevVirtualInterruptControl);
reg_direct!(virtual_tom, set_virtual_tom, u64);
reg_direct!(event_inject, set_event_inject, SevEventInjectInfo);
reg_direct!(guest_error_code, set_guest_error_code, u64);
reg_direct_mut!(
secure_avic_control,
secure_avic_control_mut,
SecureAvicControl
);
regss!(es, set_es);
regss!(cs, set_cs);
regss!(ss, set_ss);
Expand Down
12 changes: 11 additions & 1 deletion openhcl/virt_mshv_vtl/src/cvm_cpuid/snp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ impl CpuidArchInitializer for SnpCpuidInitializer {
.with_vmpl(true)
.with_rmp_query(true)
.with_tsc_aux_virtualization(true)
.with_secure_avic(true)
.into(),
cpuid::ExtendedSevFeaturesEbx::new()
.with_cbit_position(0x3f)
Expand Down Expand Up @@ -430,12 +431,21 @@ impl CpuidArchInitializer for SnpCpuidInitializer {
.with_use_ex_processor_masks(true)
// If only xAPIC is supported, then the Hyper-V MSRs are
// more efficient for EOIs.
//
// If X2APIC is supported, then we can use the X2APIC MSRs. These
// are as efficient as the Hyper-V MSRs, and they are
// compatible with APIC hardware offloads.
// However, Lazy EOI on SNP is beneficial and requires the
// Hyper-V MSRs to function. Enable it here always.
.with_use_apic_msrs(true)
//
// When Secure AVIC is enabled, x2APIC MSR accesses are
// not intercepted and are always intercepted when Secure AVIC is
// disabled (15.36.21.5 Guest APIC Accesses). As the production
// scenario is to have Secure AVIC enabled, we can disable the use
// of Hyper-V MSRs to save some performance overhead of intercept processing.
//
// Secure AVIC accelerates EOIs (15.36.21.5 Guest APIC Accesses).
.with_use_apic_msrs(false)
.with_long_spin_wait_count(!0)
.with_use_hypercall_for_remote_flush_and_local_flush_entire(true)
.with_use_synthetic_cluster_ipi(true);
Expand Down
3 changes: 3 additions & 0 deletions openhcl/virt_mshv_vtl/src/processor/hardware_cvm/apic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ pub(crate) fn poll_apic_core<'b, B: HardwareIsolatedBacking, T: ApicBacking<'b,
// 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.

// tracing::warn!("VTL0 interrupts: {:x?}", irr);

apic_backing.vp().backing.cvm_state_mut().lapics[vtl]
.lapic
.request_fixed_interrupts(irr);
Expand Down
Loading