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

[Sean Christopherson <seanjc@xxxxxxxxxx>]

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@xxxxxxxxxx