Re: [PATCH 4/6] Add GHCB allocations and helpers

Sean Christopherson
2024-04-24T14:39:39Z

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, &regs);
- 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, &regs);
}

@@ -635,6 +640,10 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
vcpu_regs_get(vcpu, &regs);
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, &regs);

/* Setup the MP state */
---

[*] https://lore.kernel.org/all/20231212204647.2170650-6-sagis@google.com