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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZikW-NjgaSMVm9uj@google.com>
Date: Wed, 24 Apr 2024 07:39:37 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Peter Gonda <pgonda@...gle.com>
Cc: linux-kernel@...r.kernel.org, Vishal Annapurve <vannapurve@...gle.com>, 
	Ackerley Tng <ackerleytng@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, 
	Claudio Imbrenda <imbrenda@...ux.ibm.com>, Carlos Bilbao <carlos.bilbao@....com>, 
	Tom Lendacky <thomas.lendacky@....com>, Michael Roth <michael.roth@....com>, kvm@...r.kernel.org, 
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 4/6] Add GHCB allocations and helpers

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