Secure AVIC draft

Author: romank-msft

Cargo.lock

@@ -9520,6 +9520,7 @@ dependencies = [
  "arbitrary",
  "bitfield-struct 0.10.1",
  "open_enum",
+ "static_assertions",
  "zerocopy 0.8.23",
 ]
 

openhcl/hcl/src/ioctl.rs

@@ -77,9 +77,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;
@@ -1548,9 +1549,12 @@ enum BackingState {
     },
     Snp {
         vmsa: VtlArray<MappedPage<SevVmsa>, 2>,
+        vtl0_apic_page: MappedPage<SevAvicPage>,
+        vtl1_apic_page: MemoryBlock,
     },
     Tdx {
-        vtl0_apic_page: MappedPage<ApicPage>,
+        vtl0_apic_page: MappedPage<VmxApicPage>,
+        // TODO: Once VTL0 works, add plumbing for VTL1.
         vtl1_apic_page: MemoryBlock,
     },
 }
@@ -1602,6 +1606,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 {

openhcl/hcl/src/ioctl/snp.rs

@@ -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>,
+    apic_pages: VtlArray<&'a UnsafeCell<SevAvicPage>, 2>,
 }
 
 /// Error returned by failing SNP operations.
@@ -176,11 +178,28 @@ 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);
         };
 
+        // TODO: Register the VTL 1 AVIC page with the hypervisor.
+        // Specification: "SEV-ES Guest-Hypervisor Communication Block Standartization",
+        // 4.1.16.1 "Backing page support".
+        //
+        // The VTL 0 APIC page is registered by the kernel.
+        let vtl1_apic_page_addr = vtl1_apic_page.pfns()[0] * user_driver::memory::PAGE_SIZE64;
+
+        // 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 {
+            apic_pages: [vtl0_apic_page.as_ref(), vtl1_apic_page].into(),
             vmsa: vmsa.each_ref().map(|mp| mp.as_ref()),
         })
     }

openhcl/hcl/src/ioctl/tdx.rs

@@ -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 {
@@ -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() }

openhcl/hcl/src/protocol.rs

@@ -175,6 +175,9 @@ pub struct hcl_hvcall {
 
 pub const HCL_REG_PAGE_OFFSET: i64 = 1 << 16;
 pub const HCL_VMSA_PAGE_OFFSET: i64 = 2 << 16;
+// TODO: may need another constant if the kernel needs to allocate a page for
+//      the VTL1 secure AVICs. Currently the kernel figures out the type of the
+//      hardware isolation and allocates the page for VTL.
 pub const MSHV_APIC_PAGE_OFFSET: i64 = 3 << 16;
 pub const HCL_VMSA_GUEST_VSM_PAGE_OFFSET: i64 = 4 << 16;
 

openhcl/openhcl_boot/src/host_params/shim_params.rs

@@ -107,6 +107,8 @@ pub struct ShimParams {
     /// Memory used by the shim.
     pub used: MemoryRange,
     pub bounce_buffer: Option<MemoryRange>,
+    /// Enable secure AVIC if supported.
+    pub auto_enable_secure_avic: bool,
 }
 
 impl ShimParams {
@@ -133,6 +135,7 @@ impl ShimParams {
             used_end,
             bounce_buffer_start,
             bounce_buffer_size,
+            auto_enable_secure_avic,
         } = raw;
 
         let isolation_type = get_isolation_type(supported_isolation_type);
@@ -166,6 +169,7 @@ impl ShimParams {
                     ..shim_base_address.wrapping_add_signed(used_end),
             ),
             bounce_buffer,
+            auto_enable_secure_avic: auto_enable_secure_avic != 0,
         }
     }
 

openhcl/openhcl_boot/src/main.rs

@@ -720,6 +720,9 @@ fn shim_main(shim_params_raw_offset: isize) -> ! {
         // Chain in the setup data.
         setup_data_tail.next = core::ptr::from_ref(&*cc_data) as u64;
         setup_data_tail = &mut cc_data.header;
+
+        // TODO: auto enable secure AVIC if CPUID indicates it is supported
+        // and the guest interrupt control is supported.
     }
 
     let reserved_memory = reserved_memory_regions(partition_info, sidecar.as_ref());

openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs

@@ -75,6 +75,7 @@ use virt_support_x86emu::emulate::emulate_io;
 use virt_support_x86emu::emulate::emulate_translate_gva;
 use virt_support_x86emu::translate::TranslationRegisters;
 use vmcore::vmtime::VmTimeAccess;
+use x86defs::ApicRegister;
 use x86defs::RFlags;
 use x86defs::X64_CR0_ET;
 use x86defs::X64_CR0_NE;
@@ -95,8 +96,6 @@ use x86defs::tdx::TdxGp;
 use x86defs::tdx::TdxInstructionInfo;
 use x86defs::tdx::TdxL2Ctls;
 use x86defs::tdx::TdxVpEnterRaxResult;
-use x86defs::vmx::ApicPage;
-use x86defs::vmx::ApicRegister;
 use x86defs::vmx::CR_ACCESS_TYPE_LMSW;
 use x86defs::vmx::CR_ACCESS_TYPE_MOV_TO_CR;
 use x86defs::vmx::CrAccessQualification;
@@ -114,6 +113,7 @@ use x86defs::vmx::SecondaryProcessorControls;
 use x86defs::vmx::VMX_ENTRY_CONTROL_LONG_MODE_GUEST;
 use x86defs::vmx::VMX_FEATURE_CONTROL_LOCKED;
 use x86defs::vmx::VmcsField;
+use x86defs::vmx::VmxApicPage;
 use x86defs::vmx::VmxEptExitQualification;
 use x86defs::vmx::VmxExit;
 use x86defs::vmx::VmxExitBasic;
@@ -2917,7 +2917,7 @@ impl UhProcessor<'_, TdxBacked> {
 
 struct TdxApicClient<'a, T> {
     partition: &'a UhPartitionInner,
-    apic_page: &'a mut ApicPage,
+    apic_page: &'a mut VmxApicPage,
     dev: &'a T,
     vmtime: &'a VmTimeAccess,
     vtl: GuestVtl,
@@ -2954,7 +2954,7 @@ impl<T: CpuIo> ApicClient for TdxApicClient<'_, T> {
     }
 }
 
-fn pull_apic_offload(page: &mut ApicPage) -> ([u32; 8], [u32; 8]) {
+fn pull_apic_offload(page: &mut VmxApicPage) -> ([u32; 8], [u32; 8]) {
     let mut irr = [0; 8];
     let mut isr = [0; 8];
     for (((irr, page_irr), isr), page_isr) in irr

vm/loader/igvmfilegen/src/file_loader.rs

@@ -14,6 +14,7 @@ use crate::vp_context_builder::VpContextBuilder;
 use crate::vp_context_builder::VpContextPageState;
 use crate::vp_context_builder::VpContextState;
 use crate::vp_context_builder::snp::InjectionType;
+use crate::vp_context_builder::snp::SecureAvic;
 use crate::vp_context_builder::snp::SnpHardwareContext;
 use crate::vp_context_builder::tdx::TdxHardwareContext;
 use crate::vp_context_builder::vbs::VbsRegister;
@@ -149,6 +150,7 @@ pub enum LoaderIsolationType {
         shared_gpa_boundary_bits: Option<u8>,
         policy: SnpPolicy,
         injection_type: InjectionType,
+        secure_avic: SecureAvic,
         // TODO SNP: SNP Keys? Other data?
     },
     Tdx {
@@ -201,6 +203,7 @@ impl IgvmLoaderRegister for X86Register {
                 shared_gpa_boundary_bits,
                 policy,
                 injection_type,
+                secure_avic,
             } => {
                 // TODO SNP: assumed that shared_gpa_boundary is always available.
                 let shared_gpa_boundary =
@@ -227,6 +230,7 @@ impl IgvmLoaderRegister for X86Register {
                     !with_paravisor,
                     shared_gpa_boundary,
                     injection_type,
+                    secure_avic,
                 ));
 
                 (platform_header, vec![init_header], vp_context_builder)
@@ -890,25 +894,30 @@ impl<R: IgvmLoaderRegister + GuestArch + 'static> ImageLoad<R> for IgvmVtlLoader
                 paravisor_present: self.loader.paravisor_present,
                 isolation_type: IsolationType::None,
                 shared_gpa_boundary_bits: None,
+                auto_enable_secure_apic: false,
             },
             LoaderIsolationType::Vbs { .. } => IsolationConfig {
                 paravisor_present: self.loader.paravisor_present,
                 isolation_type: IsolationType::Vbs,
                 shared_gpa_boundary_bits: None,
+                auto_enable_secure_apic: false,
             },
             LoaderIsolationType::Snp {
                 shared_gpa_boundary_bits,
                 policy: _,
                 injection_type: _,
+                secure_avic,
             } => IsolationConfig {
                 paravisor_present: self.loader.paravisor_present,
                 isolation_type: IsolationType::Snp,
                 shared_gpa_boundary_bits,
+                auto_enable_secure_apic: matches!(secure_avic, SecureAvic::Auto),
             },
             LoaderIsolationType::Tdx { .. } => IsolationConfig {
                 paravisor_present: self.loader.paravisor_present,
                 isolation_type: IsolationType::Tdx,
                 shared_gpa_boundary_bits: Some(TDX_SHARED_GPA_BOUNDARY_BITS),
+                auto_enable_secure_apic: false,
             },
         }
     }
@@ -1260,6 +1269,7 @@ mod tests {
                 shared_gpa_boundary_bits: Some(39),
                 policy: SnpPolicy::from((0x1 << 17) | (0x1 << 16) | (0x1f)),
                 injection_type: InjectionType::Restricted,
+                secure_avic: SecureAvic::Enabled,
             },
         );
         let data = vec![0, 5];

vm/loader/igvmfilegen/src/main.rs

@@ -25,6 +25,7 @@ use igvmfilegen_config::Image;
 use igvmfilegen_config::LinuxImage;
 use igvmfilegen_config::ResourceType;
 use igvmfilegen_config::Resources;
+use igvmfilegen_config::SecureAvicType;
 use igvmfilegen_config::SnpInjectionType;
 use igvmfilegen_config::UefiConfigType;
 use loader::importer::Aarch64Register;
@@ -182,6 +183,7 @@ fn create_igvm_file<R: IgvmfilegenRegister + GuestArch + 'static>(
                 policy,
                 enable_debug,
                 injection_type,
+                secure_avic,
             } => LoaderIsolationType::Snp {
                 shared_gpa_boundary_bits,
                 policy: SnpPolicy::from(policy).with_debug(enable_debug as u8),
@@ -191,6 +193,11 @@ fn create_igvm_file<R: IgvmfilegenRegister + GuestArch + 'static>(
                         vp_context_builder::snp::InjectionType::Restricted
                     }
                 },
+                secure_avic: match secure_avic {
+                    SecureAvicType::Auto => vp_context_builder::snp::SecureAvic::Auto,
+                    SecureAvicType::Enabled => vp_context_builder::snp::SecureAvic::Enabled,
+                    SecureAvicType::Disabled => vp_context_builder::snp::SecureAvic::Disabled,
+                },
             },
             ConfigIsolationType::Tdx {
                 enable_debug,