TODO TDX comments were removed

openhcl/hcl/src/ioctl.rs

@@ -2819,7 +2819,7 @@ impl Hcl {
             IsolationType::Snp => hvdef::HvRegisterVsmCapabilities::new()
                 .with_deny_lower_vtl_startup(caps.deny_lower_vtl_startup())
                 .with_intercept_page_available(caps.intercept_page_available()),
-            // TODO TDX: Figure out what these values should be.
+            // TODO TDX: Figure out what these values should be. (Issue #560)
             IsolationType::Tdx => hvdef::HvRegisterVsmCapabilities::new()
                 .with_deny_lower_vtl_startup(caps.deny_lower_vtl_startup())
                 .with_intercept_page_available(caps.intercept_page_available()),
@@ -2920,7 +2920,7 @@ impl Hcl {
         target_vtl: HvInputVtl,
     ) -> Result<(), ApplyVtlProtectionsError> {
         if self.isolation.is_hardware_isolated() {
-            // TODO SNP TODO TDX - required for vmbus relay monitor page support
+            // TODO SNP TODO TDX - required for vmbus relay monitor page support (Issue #559)
             todo!();
         }
 

openhcl/openhcl_boot/src/arch/x86_64/tdx.rs

@@ -113,8 +113,7 @@ pub fn get_tdx_tsc_reftime() -> Option<u64> {
     // This is first called by the BSP from openhcl_boot and the frequency
     // is saved in this gloabal variable. Subsequent calls use the global variable.
     if TSC_FREQUENCY.get() == 0 {
-        // TODO TDX: Getting tsc frequency from HV currently. Explore the option
-        // of getting it from more reliable source such as CPUID.
+        // TODO TDX: Explore the option of getting frequency from CPUID. (Issue #432)
         TSC_FREQUENCY.set(read_msr_tdcall(hvdef::HV_X64_MSR_TSC_FREQUENCY));
     }
 

openhcl/openhcl_boot/src/main.rs

@@ -796,14 +796,15 @@ fn validate_vp_hw_ids(partition_info: &PartitionInfo) {
     use hypercall::HwId;
 
     if partition_info.isolation.is_hardware_isolated() {
-        // TODO TDX SNP: we don't have a GHCB/GHCI page set up to communicate
+        // TODO TDX SNP: check and make sure the vp index ordering is in agreement with
+        // the host and hypervisor. We don't have a GHCB/GHCI page set up to communicate
         // with the hypervisor here, so we can't easily perform the check. Since
         // there is no security impact to this check, we can skip it for now; if
         // the VM fails to boot, then this is due to a host contract violation.
         //
         // For TDX, we could use ENUM TOPOLOGY to validate that the TD VCPU
         // indexes correspond to the APIC IDs in the right order. I am not
-        // certain if there are places where we depend on this mapping today.
+        // certain if there are places where we depend on this mapping today. (Issue #572)
         return;
     }
 

openhcl/underhill_mem/src/lib.rs

@@ -1137,11 +1137,11 @@ mod mapping {
                     }
                 }
                 IsolationType::Tdx => {
-                    // TODO TDX GUEST VSM: implement acceptor.vtl_permissions
+                    // TODO TDX: implement acceptor.vtl_permissions
                     // For now, since guest vsm isn't enabled (therefore no VTL
                     // 1), and VTL 0 can't change its own permissions, the
                     // permissions should be the same as when VTL 2 initialized
-                    // guest memory.
+                    // guest memory. (Issue #573)
 
                     GpaVtlPermissions::new(IsolationType::Tdx, vtl, HV_MAP_GPA_PERMISSIONS_ALL)
                 }

openhcl/virt_mshv_vtl/src/cvm_cpuid/mod.rs

@@ -60,7 +60,7 @@ trait CpuidArchInitializer {
     /// for retrieving the result of a given subleaf.
     //
     // (TODO TDX: This will be to populate them, will need to update the
-    // signature to pass CpuidResults as a mutable reference)
+    // signature to pass CpuidResults as a mutable reference. Issue #556)
     fn process_extended_state_subleaves(
         &self,
         results: &mut CpuidSubtable,

openhcl/virt_mshv_vtl/src/cvm_cpuid/tdx.rs

@@ -23,7 +23,7 @@ pub const TDX_REQUIRED_LEAVES: &[(CpuidFunction, Option<u32>)] = &[
     (CpuidFunction::TileInformation, Some(1)),
     (CpuidFunction::TmulInformation, Some(0)),
     // TODO TDX: The following aren't required from AMD. Need to double-check if
-    // they're required for TDX
+    // they're required for TDX (Issue #562)
     (CpuidFunction::CacheAndTlbInformation, None),
     (CpuidFunction::ExtendedFeatures, Some(1)),
     (CpuidFunction::CacheParameters, Some(0)),
@@ -51,7 +51,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
     }
 
     fn extended_max_function(&self) -> u32 {
-        // TODO TDX: Check if this is the same value in the OS repo
+        // TODO TDX: Check if this is the same value in the OS repo (Issue #556)
         CpuidFunction::ExtendedIntelMaximum.0
     }
 
@@ -111,7 +111,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
             }
             CpuidFunction::TmulInformation => {
                 if subleaf == 0 {
-                    // TODO TDX: does this actually have subleaves? the spec says 1+ are reserved
+                    // TODO TDX: does this actually have subleaves? the spec says 1+ are reserved (Issue #556)
                     Some(CpuidResultMask::new(
                         0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, true,
                     ))
@@ -164,7 +164,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
         results: &mut CpuidSubtable,
         extended_state_mask: u64,
     ) -> Result<(), CpuidResultsError> {
-        // TODO TDX: See HvlpPopulateExtendedStateCpuid
+        // TODO TDX: See HvlpPopulateExtendedStateCpuid (Issue #556)
         let xfd_supported = if let Some(support) = results.get(&1).map(
             |CpuidResult {
                  eax,
@@ -188,7 +188,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
                 let result = Self::cpuid(CpuidFunction::ExtendedStateEnumeration.0, i);
                 let result_xfd = cpuid::ExtendedStateEnumerationSubleafNEcx::from(result.ecx).xfd();
                 if xfd_supported && result_xfd {
-                    // TODO TDX: update some maximum xfd value; see HvlpMaximumXfd
+                    // TODO TDX: update some maximum xfd value; see HvlpMaximumXfd (Issue #556)
                 }
 
                 results.insert(i, result);
@@ -205,8 +205,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
         _address_space_sizes_ecx: cpuid::ExtendedAddressSpaceSizesEcx,
         _processor_topology_ebx: Option<cpuid::ProcessorTopologyDefinitionEbx>, // Will be None for Intel
     ) -> Result<super::ExtendedTopologyResult, CpuidResultsError> {
-        // TODO TDX: see HvlpInitializeCpuidTopologyIntel
-        // TODO TDX: fix returned errors
+        // TODO TDX: see HvlpInitializeCpuidTopologyIntel and fix returned errors (Issue #556)
         let vps_per_socket;
         if !version_and_features_edx.mt_per_socket() {
             if version_and_features_ebx.lps_per_package() > 1 {
@@ -220,8 +219,7 @@ impl CpuidArchInitializer for TdxCpuidInitializer {
             vps_per_socket = version_and_features_ebx.lps_per_package();
         }
 
-        // TODO TDX: validation of leaf 0xB
-
+        // TODO TDX: validation of leaf 0xB (Issue #556)
         Ok(super::ExtendedTopologyResult {
             subleaf0: None,
             subleaf1: None,

openhcl/virt_mshv_vtl/src/lib.rs

@@ -866,7 +866,7 @@ impl virt::Synic for UhPartition {
     fn monitor_support(&self) -> Option<&dyn virt::SynicMonitor> {
         // TODO TDX TODO SNP: Disable monitor support for TDX and SNP as support
         // for VTL2 protections is needed to emulate this page, which is not
-        // implemented yet.
+        // implemented yet. (Issue #575)
         if self.inner.isolation.is_hardware_isolated() {
             None
         } else {
@@ -1391,7 +1391,7 @@ impl<'a> UhProtoPartition<'a> {
         // TODO TDX: This currently works on TDX because all Underhill TDs today
         // have the debug policy bit set, allowing the hypervisor to install the
         // intercept on behalf of the guest. In the future, Underhill should
-        // register for these intercepts itself.
+        // register for these intercepts itself. (Issue #558)
         if params.intercept_debug_exceptions {
             if !cfg!(feature = "gdb") {
                 return Err(Error::InvalidDebugConfiguration);
@@ -1745,7 +1745,7 @@ impl UhProtoPartition<'_> {
         match params.isolation {
             IsolationType::None | IsolationType::Vbs => {}
             #[cfg(guest_arch = "x86_64")]
-            IsolationType::Tdx => return false, // TODO TDX GUEST_VSM
+            IsolationType::Tdx => return false, // TODO TDX GUEST_VSM (Issue #557)
             #[cfg(guest_arch = "x86_64")]
             IsolationType::Snp => {
                 if !params.env_cvm_guest_vsm {

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

@@ -709,7 +709,7 @@ impl<T, B: HardwareIsolatedBacking> UhHypercallHandler<'_, '_, T, B> {
         }
 
         // TODO GUEST VSM: interrupt rewinding
-        // TODO TDX GUEST VSM: update execution mode
+        // TODO TDX GUEST VSM: update execution mode (Issue #687)
     }
 }
 

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

@@ -500,7 +500,7 @@ impl HardwareIsolatedBacking for TdxBacked {
         _source_vtl: GuestVtl,
         _target_vtl: GuestVtl,
     ) {
-        // TODO TDX GUEST VSM
+        // TODO TDX GUEST VSM (Issue #685)
         todo!()
     }
 
@@ -564,7 +564,7 @@ impl BackingPrivate for TdxBacked {
         _shared: &TdxBackedShared,
     ) -> Result<Self, crate::Error> {
         // TODO TDX: TDX shares the vp context page for xmm registers only. It
-        // should probably move to its own page.
+        // should probably move to its own page. (Issue #555)
         //
         // FX regs and XMM registers are zero-initialized by the kernel. Set
         // them to the arch default.
@@ -590,15 +590,13 @@ impl BackingPrivate for TdxBacked {
         *rflags = regs.rflags;
         *rip = regs.rip;
 
-        // TODO TDX: ssp is for shadow stack
-
-        // TODO TDX: direct overlay like snp?
-        // TODO TDX: lapic / APIC setup?
-
-        // TODO TDX: see ValInitializeVplc
-        // TODO TDX: XCR_XFMEM setup?
-
-        // TODO TDX GUEST VSM: Presumably we need to duplicate much of this work
+        // TODO TDX: Implement the following for vp context (Issue #555)
+        // ssp is for shadow stack
+        // direct overlay like snp?
+        // lapic / APIC setup?
+        // see ValInitializeVplc
+        // XCR_XFMEM setup?
+        // GUEST VSM: Presumably we need to duplicate much of this work
         // when VTL 1 is enabled.
 
         // Configure L2 controls to permit shared memory.
@@ -649,7 +647,7 @@ impl BackingPrivate for TdxBacked {
         // Allowed cr4 bits depend on the values allowed by the SEAM.
         //
         // TODO TDX: Consider just using MSR kernel module instead of explicit
-        // ioctl.
+        // ioctl. (Issue #555)
         let read_cr4 = hcl.read_vmx_cr4_fixed1();
         let allowed_cr4_bits = (ShadowedRegister::Cr4.guest_owned_mask() | X64_CR4_MCE) & read_cr4;
 
@@ -689,7 +687,7 @@ impl BackingPrivate for TdxBacked {
         let pfns = pfns_handle.base_pfn()..pfns_handle.base_pfn() + pfns_handle.size_pages();
         let overlays: Vec<_> = pfns.collect();
 
-        // TODO TDX: This needs to come from a private pool
+        // TODO TDX: This needs to come from a private pool (Issue #555)
         let flush_page = params
             .partition
             .shared_vis_pages_pool
@@ -765,7 +763,7 @@ impl BackingPrivate for TdxBacked {
 
     fn init(this: &mut UhProcessor<'_, Self>) {
         // TODO TDX GUEST VSM: Presumably we need to duplicate much of this work
-        // when VTL 1 is enabled.
+        // when VTL 1 is enabled. (Issue #555)
 
         // Configure the synic overlays.
         let pfns = &this.backing.direct_overlays_pfns;
@@ -862,7 +860,7 @@ impl BackingPrivate for TdxBacked {
         this: &mut UhProcessor<'_, Self>,
         _dev: &impl CpuIo,
     ) -> Result<bool, UhRunVpError> {
-        // TODO TDX GUEST VSM
+        // TODO TDX GUEST VSM (Issue #686)
         this.hcvm_handle_cross_vtl_interrupts(|_this, _vtl, _check_rflags| false)
     }
 
@@ -888,7 +886,7 @@ impl UhProcessor<'_, TdxBacked> {
     /// poll retried.
     fn try_poll_apic(&mut self, vtl: GuestVtl, scan_irr: bool) -> Result<bool, UhRunVpError> {
         // Check for interrupt requests from the host and kernel IPI offload.
-        // TODO TDX GUEST VSM supporting VTL 1 proxy irrs requires kernel changes
+        // TODO TDX GUEST VSM supporting VTL 1 proxy irrs requires kernel changes (Issue #691)
         if vtl == GuestVtl::Vtl0 {
             if let Some(irr) = self.runner.proxy_irr() {
                 // We can't put the interrupts directly on the APIC page because we might need
@@ -1463,7 +1461,7 @@ impl UhProcessor<'_, TdxBacked> {
             // should be spurious EPT exits. Can we validate or assert that
             // somehow? If we were to somehow call some other path which would
             // set interruption_information before we inject this one, we would
-            // lose this interrupt.
+            // lose this interrupt. (Issue #553)
             if next_interruption.valid() {
                 tracing::debug!(
                     ?next_interruption,
@@ -1692,7 +1690,7 @@ impl UhProcessor<'_, TdxBacked> {
 
                 // TODO TDX: see lots of these exits while waiting at frontpage.
                 // Probably expected, given we will still get L1 timer
-                // interrupts?
+                // interrupts? (Issue #688)
                 self.clear_interrupt_shadow(intercepted_vtl);
                 self.advance_to_next_instruction();
                 &mut self.backing.vtls[intercepted_vtl].exit_stats.hlt
@@ -1768,10 +1766,9 @@ impl UhProcessor<'_, TdxBacked> {
                 &mut self.backing.vtls[intercepted_vtl].exit_stats.wbinvd
             }
             VmxExit::EPT_VIOLATION => {
-                // TODO TDX: If this is an access to a shared gpa, we need to
-                // check the intercept page to see if this is a real exit or
-                // spurious. This exit is only real if the hypervisor has
-                // delivered an intercept message for this GPA.
+                // TODO TDX: If this is an access to a shared gpa, then this
+                // is the result of hypervisor unable to handle it and
+                // delivered an intercept message for this GPA. (Issue #689)
                 //
                 // However, at this point the kernel has cleared that
                 // information so some kind of redesign is required to figure
@@ -2042,14 +2039,14 @@ impl UhProcessor<'_, TdxBacked> {
     }
 
     fn read_msr_cvm(&self, msr: u32, vtl: GuestVtl) -> Result<u64, MsrError> {
-        // TODO TDX: port remaining tdx and common values
+        // TODO TDX: port remaining tdx and common values (Issue #551)
         //
-        // TODO TDX: consider if this can be shared with SnpBacked's
+        // Consider if this can be shared with SnpBacked's
         // implementation. For the most part other than Intel/TDX specific
         // registers, MSR handling should be the same.
 
         match msr {
-            // TODO TDX: LIFTED FROM WHP
+            // TODO TDX: LIFTED FROM WHP (Issue #551)
             x86defs::X86X_IA32_MSR_PLATFORM_ID => {
                 // Windows requires accessing this to boot. WHP
                 // used to pass this through to the hardware,
@@ -2206,7 +2203,7 @@ impl UhProcessor<'_, TdxBacked> {
         self.runner
             .write_vmcs32(vtl, seg.attributes(), !0, attributes.into());
 
-        // TODO TDX: cache CS into last exit because last exit contains CS optionally?
+        // TODO TDX: cache CS into last exit because last exit contains CS optionally? (Issue #550)
 
         Ok(())
     }
@@ -2361,7 +2358,7 @@ impl<T: CpuIo> X86EmulatorSupport for UhEmulationState<'_, '_, T, TdxBacked> {
         _mode: TranslateMode,
     ) -> Result<(), virt_support_x86emu::emulate::EmuCheckVtlAccessError<Self::Error>> {
         // Lock Vtl TLB
-        // TODO TDX GUEST VSM: VTL1 not yet supported
+        // TODO TDX GUEST VSM: VTL1 not yet supported (Issue #549)
         Ok(())
     }
 
@@ -2744,7 +2741,7 @@ impl<T: CpuIo> UhHypercallHandler<'_, '_, T, TdxBacked> {
             hv1_hypercall::HvPostMessage,
             hv1_hypercall::HvSignalEvent,
             hv1_hypercall::HvExtQueryCapabilities,
-            // TODO TDX: copied from SNP, enable individually as needed.
+            // TODO TDX: copied from SNP, enable individually as needed. (Issue #548)
             // hv1_hypercall::HvGetVpRegisters,
             // hv1_hypercall::HvEnablePartitionVtl,
         ]
@@ -2949,8 +2946,8 @@ impl AccessVpState for UhVpStateAccess<'_, '_, TdxBacked> {
             nmi_masked: interruptibility.blocked_by_nmi(),
             interrupt_shadow: interruptibility.blocked_by_sti()
                 || interruptibility.blocked_by_movss(),
-            pending_event: None,        // TODO TDX
-            pending_interruption: None, // TODO TDX
+            pending_event: None,        // TODO TDX (Issue #547)
+            pending_interruption: None, // TODO TDX (Issue #547)
         })
     }
 
@@ -2960,8 +2957,8 @@ impl AccessVpState for UhVpStateAccess<'_, '_, TdxBacked> {
             nmi_pending,
             nmi_masked,
             interrupt_shadow,
-            pending_event: _,        // TODO TDX
-            pending_interruption: _, // TODO TDX
+            pending_event: _,        // TODO TDX (Issue #547)
+            pending_interruption: _, // TODO TDX (Issue #547)
         } = value;
         self.vp.backing.cvm.lapics[self.vtl].activity = mp_state;
         self.vp.backing.cvm.lapics[self.vtl].nmi_pending = nmi_pending;
@@ -3031,14 +3028,14 @@ impl AccessVpState for UhVpStateAccess<'_, '_, TdxBacked> {
 
     fn mtrrs(&mut self) -> Result<vp::Mtrrs, Self::Error> {
         Ok(vp::Mtrrs {
-            msr_mtrr_def_type: 0, // TODO TDX: MTRRs
-            fixed: [0; 11],       // TODO TDX: MTRRs
-            variable: [0; 16],    // TODO TDX: MTRRs
+            msr_mtrr_def_type: 0, // TODO TDX: MTRRs (Issue #547)
+            fixed: [0; 11],       // TODO TDX: MTRRs (Issue #547)
+            variable: [0; 16],    // TODO TDX: MTRRs (Issue #547)
         })
     }
 
     fn set_mtrrs(&mut self, _value: &vp::Mtrrs) -> Result<(), Self::Error> {
-        // TODO TDX: MTRRs
+        // TODO TDX: MTRRs (Issue #547)
         Ok(())
     }
 

vm/hv1/hv1_emulator/src/cpuid.rs

@@ -129,7 +129,7 @@ pub fn hv_cpuid_leaves(
                 .with_use_apic_msrs(use_apic_msrs);
 
             if hardware_isolated {
-                // TODO TDX too when it's ready
+                // TODO TDX: Process TDX isolation type when it's ready (Issue #546)
                 if isolation == IsolationType::Snp {
                     enlightenments = enlightenments
                         .with_use_hypercall_for_remote_flush_and_local_flush_entire(true);