TDX: LDTR and TR access emulation (#1237)

This fills in the todo for emulating these 4 instructions, as well as
including some small refactors to the previous 4, and adding a missing
security check for load instructions. I don't have a way to test this
end-to-end currently, but the refactors at least don't break the code
that I can test.

Author: smalis-msft

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

@@ -1138,7 +1138,7 @@ impl UhProcessor<'_, TdxBacked> {
         }
 
         if self.backing.vtls[vtl].processor_controls != new_processor_controls {
-            tracing::debug!(?new_processor_controls, ?vtl, "requesting window change");
+            tracing::trace!(?new_processor_controls, ?vtl, "requesting window change");
             self.runner.write_vmcs32(
                 vtl,
                 VmcsField::VMX_VMCS_PROCESSOR_CONTROLS,
@@ -2071,10 +2071,10 @@ impl UhProcessor<'_, TdxBacked> {
                         HvX64RegisterName::Gdtr
                     }
                 };
-                // We only support fowarding intercepts for descriptor table writes today.
-                if (info.instruction().is_write()
+                // We only support fowarding intercepts for descriptor table loads today.
+                if (info.instruction().is_load()
                     && !self.cvm_try_protect_secure_register_write(intercepted_vtl, reg, 0))
-                    || !info.instruction().is_write()
+                    || !info.instruction().is_load()
                 {
                     self.emulate_gdtr_or_idtr(intercepted_vtl, dev).await?;
                 }
@@ -2091,10 +2091,10 @@ impl UhProcessor<'_, TdxBacked> {
                     }
                     LdtrOrTrInstruction::Str | LdtrOrTrInstruction::Ltr => HvX64RegisterName::Tr,
                 };
-                // We only support fowarding intercepts for descriptor table writes today.
-                if (info.instruction().is_write()
+                // We only support fowarding intercepts for descriptor table loads today.
+                if (info.instruction().is_load()
                     && !self.cvm_try_protect_secure_register_write(intercepted_vtl, reg, 0))
-                    || !info.instruction().is_write()
+                    || !info.instruction().is_load()
                 {
                     self.emulate_ldtr_or_tr(intercepted_vtl, dev).await?;
                 }
@@ -2752,7 +2752,9 @@ impl<T: CpuIo> X86EmulatorSupport for UhEmulationState<'_, '_, T, TdxBacked> {
         let exit_info = TdxExit(self.vp.runner.tdx_vp_enter_exit_info());
         let ept_info = VmxEptExitQualification::from(exit_info.qualification());
 
-        if ept_info.gva_valid() {
+        if exit_info.code().vmx_exit().basic_reason() == VmxExitBasic::EPT_VIOLATION
+            && ept_info.gva_valid()
+        {
             Some(virt_support_x86emu::emulate::InitialTranslation {
                 gva: exit_info.gla(),
                 gpa: exit_info.gpa(),
@@ -3061,44 +3063,26 @@ impl UhProcessor<'_, TdxBacked> {
             VmxExitBasic::GDTR_OR_IDTR
         );
         let instr_info = GdtrOrIdtrInstructionInfo::from(exit_info.instr_info().info());
-        let gps = self.runner.tdx_enter_guest_gps();
 
-        // Check if read instructions are blocked by UMIP.
-        if !instr_info.instruction().is_write()
-            && exit_info.cpl() > 0
-            && self.read_cr4(vtl) & X64_CR4_UMIP != 0
+        // Check if load instructions are executed outside of kernel mode.
+        // Check if store instructions are blocked by UMIP.
+        if (instr_info.instruction().is_load() && exit_info.cpl() != 0)
+            || (!instr_info.instruction().is_load()
+                && exit_info.cpl() > 0
+                && self.read_cr4(vtl) & X64_CR4_UMIP != 0)
         {
             self.inject_gpf(vtl);
             return Ok(());
         }
 
-        // Displacement is stored in the qualification field for these instructions.
-        let mut gva = exit_info.qualification();
-        if !instr_info.base_register_invalid() {
-            gva += gps[instr_info.base_register() as usize];
-        }
-        if !instr_info.index_register_invalid() {
-            gva += gps[instr_info.index_register() as usize] << instr_info.scaling();
-        }
-        match instr_info.address_size() {
-            // 16-bit address size
-            0 => gva &= 0xFFFF,
-            // 32-bit address size
-            1 => gva &= 0xFFFFFFFF,
-            // 64-bit address size
-            2 => {}
-            _ => unreachable!(),
-        }
-
-        let segment = match instr_info.segment_register() {
-            0 => Segment::ES,
-            1 => Segment::CS,
-            2 => Segment::SS,
-            3 => Segment::DS,
-            4 => Segment::FS,
-            5 => Segment::GS,
-            _ => unreachable!(),
-        };
+        let (gva, segment) = self.compute_gva_for_table_access_emulation(
+            exit_info.qualification(),
+            (!instr_info.base_register_invalid()).then_some(instr_info.base_register()),
+            (!instr_info.index_register_invalid()).then_some(instr_info.index_register()),
+            instr_info.scaling(),
+            instr_info.address_size(),
+            instr_info.segment_register(),
+        );
 
         let gm = &self.partition.gm[vtl];
         let interruption_pending = self.backing.vtls[vtl].interruption_information.valid();
@@ -3176,10 +3160,167 @@ impl UhProcessor<'_, TdxBacked> {
 
     async fn emulate_ldtr_or_tr(
         &mut self,
-        _vtl: GuestVtl,
-        _dev: &impl CpuIo,
+        vtl: GuestVtl,
+        dev: &impl CpuIo,
     ) -> Result<(), VpHaltReason<UhRunVpError>> {
-        todo!()
+        let exit_info = TdxExit(self.runner.tdx_vp_enter_exit_info());
+        assert_eq!(
+            exit_info.code().vmx_exit().basic_reason(),
+            VmxExitBasic::LDTR_OR_TR
+        );
+        let instr_info = LdtrOrTrInstructionInfo::from(exit_info.instr_info().info());
+
+        // Check if load instructions are executed outside of kernel mode.
+        // Check if store instructions are blocked by UMIP.
+        if (instr_info.instruction().is_load() && exit_info.cpl() != 0)
+            || (!instr_info.instruction().is_load()
+                && exit_info.cpl() > 0
+                && self.read_cr4(vtl) & X64_CR4_UMIP != 0)
+        {
+            self.inject_gpf(vtl);
+            return Ok(());
+        }
+
+        let gm = &self.partition.gm[vtl];
+        let interruption_pending = self.backing.vtls[vtl].interruption_information.valid();
+
+        match instr_info.instruction() {
+            LdtrOrTrInstruction::Sldt | LdtrOrTrInstruction::Str => {
+                let value = self.runner.read_vmcs16(
+                    vtl,
+                    if matches!(instr_info.instruction(), LdtrOrTrInstruction::Sldt) {
+                        TdxSegmentReg::Ldtr
+                    } else {
+                        TdxSegmentReg::Tr
+                    }
+                    .selector(),
+                );
+
+                if instr_info.memory_or_register() {
+                    let gps = self.runner.tdx_enter_guest_gps_mut();
+                    gps[instr_info.register_1() as usize] = value.into();
+                } else {
+                    let (gva, segment) = self.compute_gva_for_table_access_emulation(
+                        exit_info.qualification(),
+                        (!instr_info.base_register_invalid()).then_some(instr_info.base_register()),
+                        (!instr_info.index_register_invalid())
+                            .then_some(instr_info.index_register()),
+                        instr_info.scaling(),
+                        instr_info.address_size(),
+                        instr_info.segment_register(),
+                    );
+                    let mut emulation_state = UhEmulationState {
+                        vp: &mut *self,
+                        interruption_pending,
+                        devices: dev,
+                        vtl,
+                        cache: TdxEmulationCache::default(),
+                    };
+                    emulate_insn_memory_op(
+                        &mut emulation_state,
+                        gm,
+                        dev,
+                        gva,
+                        segment,
+                        x86emu::AlignmentMode::Standard,
+                        EmulatedMemoryOperation::Write(&value.to_le_bytes()),
+                    )
+                    .await?;
+                }
+            }
+
+            LdtrOrTrInstruction::Lldt | LdtrOrTrInstruction::Ltr => {
+                let value = if instr_info.memory_or_register() {
+                    let gps = self.runner.tdx_enter_guest_gps();
+                    gps[instr_info.register_1() as usize] as u16
+                } else {
+                    let (gva, segment) = self.compute_gva_for_table_access_emulation(
+                        exit_info.qualification(),
+                        (!instr_info.base_register_invalid()).then_some(instr_info.base_register()),
+                        (!instr_info.index_register_invalid())
+                            .then_some(instr_info.index_register()),
+                        instr_info.scaling(),
+                        instr_info.address_size(),
+                        instr_info.segment_register(),
+                    );
+                    let mut emulation_state = UhEmulationState {
+                        vp: &mut *self,
+                        interruption_pending,
+                        devices: dev,
+                        vtl,
+                        cache: TdxEmulationCache::default(),
+                    };
+                    let mut buf = [0u8; 2];
+                    emulate_insn_memory_op(
+                        &mut emulation_state,
+                        gm,
+                        dev,
+                        gva,
+                        segment,
+                        x86emu::AlignmentMode::Standard,
+                        EmulatedMemoryOperation::Read(&mut buf),
+                    )
+                    .await?;
+                    u16::from_le_bytes(buf)
+                };
+                self.runner.write_vmcs16(
+                    vtl,
+                    if matches!(instr_info.instruction(), LdtrOrTrInstruction::Lldt) {
+                        TdxSegmentReg::Ldtr
+                    } else {
+                        TdxSegmentReg::Tr
+                    }
+                    .selector(),
+                    !0,
+                    value,
+                );
+            }
+        }
+
+        self.advance_to_next_instruction(vtl);
+        Ok(())
+    }
+
+    fn compute_gva_for_table_access_emulation(
+        &self,
+        qualification: u64,
+        base_reg: Option<u8>,
+        index_reg: Option<u8>,
+        scaling: u8,
+        address_size: u8,
+        segment_register: u8,
+    ) -> (u64, Segment) {
+        let gps = self.runner.tdx_enter_guest_gps();
+
+        // Displacement is stored in the qualification field for these instructions.
+        let mut gva = qualification;
+        if let Some(base_register) = base_reg {
+            gva += gps[base_register as usize];
+        }
+        if let Some(index_register) = index_reg {
+            gva += gps[index_register as usize] << scaling;
+        }
+        match address_size {
+            // 16-bit address size
+            0 => gva &= 0xFFFF,
+            // 32-bit address size
+            1 => gva &= 0xFFFFFFFF,
+            // 64-bit address size
+            2 => {}
+            _ => unreachable!(),
+        }
+
+        let segment = match segment_register {
+            0 => Segment::ES,
+            1 => Segment::CS,
+            2 => Segment::SS,
+            3 => Segment::DS,
+            4 => Segment::FS,
+            5 => Segment::GS,
+            _ => unreachable!(),
+        };
+
+        (gva, segment)
     }
 }
 

vm/x86/x86defs/src/vmx.rs

@@ -341,7 +341,7 @@ impl GdtrOrIdtrInstruction {
         self as u8
     }
 
-    pub const fn is_write(self) -> bool {
+    pub const fn is_load(self) -> bool {
         match self {
             GdtrOrIdtrInstruction::Lgdt | GdtrOrIdtrInstruction::Lidt => true,
             GdtrOrIdtrInstruction::Sgdt | GdtrOrIdtrInstruction::Sidt => false,
@@ -358,6 +358,7 @@ pub struct LdtrOrTrInstructionInfo {
     pub register_1: u8,
     #[bits(3)]
     pub address_size: u8,
+    /// 0 - Memory, 1 - Register
     pub memory_or_register: bool,
     #[bits(4)]
     _reserved2: u8,
@@ -398,7 +399,7 @@ impl LdtrOrTrInstruction {
         self as u8
     }
 
-    pub const fn is_write(self) -> bool {
+    pub const fn is_load(self) -> bool {
         match self {
             LdtrOrTrInstruction::Lldt | LdtrOrTrInstruction::Ltr => true,
             LdtrOrTrInstruction::Sldt | LdtrOrTrInstruction::Str => false,