On Tue, Apr 23, 2024, Sean Christopherson wrote:
On Tue, Apr 09, 2024, Peter Gonda wrote:
Add GHCB management functionality similar to the ucall management. Allows for selftest vCPUs to acquire GHCBs for their usage.
Do we actually need a dedicated pool of GHCBs? The conundrum with ucalls is that we have no place in the guest to store the pointer on all architectures. Or rather, we were too lazy to find one. :-)
But for SEV-ES, we have MSR_AMD64_SEV_ES_GHCB, and any test that clobbers that obviously can't use ucalls anyways. Argh, but we can't get a value in that MSR from the host.
...
Anyways, back to selftests. Since we apparently can't stuff MSR_AMD64_SEV_ES_GHCB from host userspace, what if we instead use a trampoline? Instead having vcpu_arch_set_entry_point() point directly at guest_code, point it at a trampoline for SEV-ES guests, and then have the trampoline set MSR_AMD64_SEV_ES_GHCB to the vCPU-specific GHCB before invoking guest_code().
Then we just need a register to stuff the GHCB into. Ah, and the actual guest entry point. GPRs are already part of selftest's "ABI", since they're set by vcpu_args_set(). And this is all 64-bit only, so we can use r10+.
Ugh, the complication is that the trampoline would need to save/restore RAX, RCX, and RDX in order to preserve the values from vcpu_args_set(), but that's just annoying, not hard. And I think it'd be less painful overall than having to create a GHCB pool?
In rough pseudo-asm, something like this?
static void vcpu_sev_es_guest_trampoline(void) { asm volatile(<save rax, rcx, rdx> "mov %%r15d, %%eax\n\t" "shr %%r15, $32\n\t" "mov %%r15d, %%eax\n\t" "mov $MSR_AMD64_SEV_ES_GHCB, %%ecx\n\t" <restore rax, rcx, rdx> "jmp %%r14") }
Scratch using inline asm, it needs to be a proper asm subroutine, as it's possible the compiler could clobber GPRs before emitting the asm. But writing actual assembly code is probably a good thing.
And we need assembly for TDX selftests, which forces vCPUs to start at the RESET vector[*]. Rather than add a TDX specific td_boot.S, we can add a common-ish entry.S to hold all of the CoCo entry points that need to be in assembly.
Then I think we'll eventually end up with something like:
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index fd94a1bd82c9..03818d3c4669 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -597,7 +597,12 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code) struct kvm_regs regs;
vcpu_regs_get(vcpu, ®s); - regs.rip = (unsigned long) guest_code; + if (<is sev-es guest>) + regs.r14 = guest_code; + else if (<is tdx guest>) + <guest_code gets shoved somewhere else> + else + regs.rip = (unsigned long) guest_code; vcpu_regs_set(vcpu, ®s); }
@@ -635,6 +640,10 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id) vcpu_regs_get(vcpu, ®s); regs.rflags = regs.rflags | 0x2; regs.rsp = stack_vaddr; + if (<is sev-es guest>) { + regs.rip = vcpu_sev_es_guest_trampoline; + regs.r15 = <allocate and map ghcb>(); + } vcpu_regs_set(vcpu, ®s);
/* Setup the MP state */ ---
[*] https://lore.kernel.org/all/20231212204647.2170650-6-sagis@google.com