Skip to content

Commit e12caa0

Browse files
authored
openhcl/tdx: fix bytecode issue in reset vector s.t. mailbox is gated by kernel (#2341)
## Context OpenHCL implements the ACPI Mailbox protocol for starting APs in TDX CVMs. The implementation involves spinning in the reset vector, until we receive a message to jump to a wakeup vector in the kernel. The kernel communicates that we should start by sending an apic_id, and then a start command. There is an intermittent issue where APs will fail to start, and the kernel failure log suggests that we are never exiting the reset vector (#2334) ## Bug There is a spinloop in the reset vector, where we wait until the kernel writes the APIC_ID of the AP into the `id` field of the mailbox page, and then a `0x1` to the `command` field. This spinloop works by reading the `command` field into `ebx`, and comparing it to a fixed value of `1` in `dx`. However, the instruction that moves the fixed WORD into `dx` has the wrong bytecode, it is instead writing a fixed DWORD into `edx`. The instruction thus considers the next WORD in the reset vector, which is a cmp instruction, to be part of the operand to the mov, and never executes it. Normally this type of issue would be obvious and lead to a crash, but there are two reasons this code worked most of the time: 1. The cmp instruction after the mov was exactly two bytes, and those two bytes were read as part of a constant. During execution, the reset vector smoothly moves to the instruction after the cmp, and the value in `edx` is not used again. 2. The kernel is still gating the reset vector with the APIC_ID; the APs still start serially when the kernel selects them, just not at the timing expected by the kernel. ## Fix Explicitly compare to a constant DWORD, as that is the size of the `command` register written by the kernel.
1 parent 096c3a4 commit e12caa0

File tree

2 files changed

+9
-5
lines changed

2 files changed

+9
-5
lines changed

vm/loader/igvmfilegen/src/file_loader.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,9 +1287,9 @@ mod tests {
12871287
#[test]
12881288
fn test_tdx_measurement() {
12891289
let ref_mrtd: [u8; 48] = [
1290-
142, 235, 138, 197, 222, 36, 154, 25, 110, 198, 217, 131, 116, 129, 48, 146, 248, 99,
1291-
83, 133, 144, 128, 224, 130, 253, 243, 85, 35, 71, 246, 197, 202, 172, 193, 152, 184,
1292-
115, 127, 55, 3, 169, 107, 164, 126, 128, 145, 203, 28,
1290+
214, 88, 54, 138, 48, 41, 223, 124, 152, 113, 236, 159, 157, 24, 134, 87, 36, 12, 163,
1291+
162, 115, 128, 222, 247, 130, 13, 114, 103, 87, 67, 73, 89, 166, 251, 86, 245, 63, 209,
1292+
246, 246, 164, 240, 96, 164, 22, 183, 142, 219,
12931293
];
12941294

12951295
let mut loader = IgvmLoader::<X86Register>::new(

vm/loader/igvmfilegen/src/vp_context_builder/tdx.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,12 @@ impl VpContextBuilder for TdxHardwareContext {
380380
.wrapping_sub((byte_offset + 4) as u32);
381381
byte_offset = copy_instr(&mut reset_page, byte_offset, relative_offset.as_bytes());
382382

383-
// mov dx, 01h
384-
byte_offset = copy_instr(&mut reset_page, byte_offset, &[0xBA, 0x01, 0x00]);
383+
// mov edx, 01h
384+
byte_offset = copy_instr(
385+
&mut reset_page,
386+
byte_offset,
387+
&[0xBA, 0x01, 0x00, 0x00, 0x00],
388+
);
385389

386390
// cmp ebx, edx
387391
byte_offset = copy_instr(&mut reset_page, byte_offset, &[0x39, 0xD3]);

0 commit comments

Comments
 (0)