diff mbox series

[6/6] Add ability for SEV-ES guests to use ucalls via GHCB

Message ID 20240409133959.2888018-7-pgonda@google.com (mailing list archive)
State New
Headers show
Series Add initial GHCB support for SEV-ES selftests | expand

Commit Message

Peter Gonda April 9, 2024, 1:39 p.m. UTC
Modifies ucall handling for SEV-ES VMs. Instead of using an out
instruction and storing the ucall pointer in RDI, SEV-ES guests use a
outsb VMGEXIT to move the ucall pointer as the data. Allows for SEV-ES
to use ucalls instead of relying the SEV-ES MSR based termination protocol.

Cc: Vishal Annapurve <vannapurve@google.com>
Cc: Ackerley Tng <ackerleytng@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Carlos Bilbao <carlos.bilbao@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: kvm@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: Peter Gonda <pgonda@google.com>
---
 .../selftests/kvm/include/x86_64/sev.h        |  2 +
 tools/testing/selftests/kvm/lib/x86_64/sev.c  | 67 ++++++++++++++++++-
 .../testing/selftests/kvm/lib/x86_64/ucall.c  | 17 +++++
 .../selftests/kvm/x86_64/sev_smoke_test.c     | 17 +----
 4 files changed, 84 insertions(+), 19 deletions(-)

Comments

Sean Christopherson April 23, 2024, 11:50 p.m. UTC | #1
On Tue, Apr 09, 2024, Peter Gonda wrote:
> Modifies ucall handling for SEV-ES VMs. 

Please follow the preferred changelog style as described in
Documentation/process/maintainer-kvm-x86.rst

> Instead of using an out instruction and storing the ucall pointer in RDI,
> SEV-ES guests use a outsb VMGEXIT to move the ucall pointer as the data.

Explain _why_.  After poking around, I think I agree that string I/O is the least
awful choice, but string I/O is generally unpleasant.  E.g. my initial reaction
to this
		addr = *(uint64_t *)((uint8_t *)(run) + run->io.data_offset);

was quite literally, "LOL, what?".

We could use MMIO, because there is no *real* instruction in the guest, it's all
make believe, i.e. there doesn't actually need to be MMIO anywhere.  But then we
need to define an address; it could simply be the ucall address, but then SEV-ES
ends up with a completely different flow then the regular magic I/O port.

The changelog should capture explain why string I/O was chosen over the "obvious"
alternatives so that readers and reviewers aren't left wondering why on earth
we *chose* to use string I/O.

> Allows for SEV-ES to use ucalls instead of relying the SEV-ES MSR based
> termination protocol.
> 
> Cc: Vishal Annapurve <vannapurve@google.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Carlos Bilbao <carlos.bilbao@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Signed-off-by: Peter Gonda <pgonda@google.com>
> ---
>  .../selftests/kvm/include/x86_64/sev.h        |  2 +
>  tools/testing/selftests/kvm/lib/x86_64/sev.c  | 67 ++++++++++++++++++-
>  .../testing/selftests/kvm/lib/x86_64/ucall.c  | 17 +++++
>  .../selftests/kvm/x86_64/sev_smoke_test.c     | 17 +----
>  4 files changed, 84 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
> index 691dc005e2a1..26447caccd40 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
> @@ -109,4 +109,6 @@ static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
>  bool is_sev_enabled(void);
>  bool is_sev_es_enabled(void);
>  
> +void sev_es_ucall_port_write(uint32_t port, uint64_t data);
> +
>  #endif /* SELFTEST_KVM_SEV_H */
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> index 5b3f0a8a931a..276477f2c2cf 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> @@ -8,11 +8,18 @@
>  #include "svm.h"
>  #include "svm_util.h"
>  
> +#define IOIO_TYPE_STR (1 << 2)
> +#define IOIO_SEG_DS (1 << 11 | 1 << 10)
> +#define IOIO_DATA_8 (1 << 4)
> +#define IOIO_REP (1 << 3)
> +
> +#define SW_EXIT_CODE_IOIO 0x7b
> +
>  struct ghcb_entry {
>  	struct ghcb ghcb;
>  
>  	/* Guest physical address of this GHCB. */
> -	void *gpa;
> +	uint64_t gpa;
>  
>  	/* Host virtual address of this struct. */
>  	struct ghcb_entry *hva;
> @@ -45,16 +52,22 @@ void ghcb_init(struct kvm_vm *vm)
>  	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
>  		entry = &hdr->ghcbs[i];
>  		entry->hva = entry;
> -		entry->gpa = addr_hva2gpa(vm, &entry->ghcb);
> +		entry->gpa = (uint64_t)addr_hva2gpa(vm, &entry->ghcb);
>  	}
>  
>  	write_guest_global(vm, ghcb_pool, (struct ghcb_header *)vaddr);
>  }
>  
> +static void sev_es_terminate(void)
> +{
> +	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
> +}
> +
>  static struct ghcb_entry *ghcb_alloc(void)
>  {
>  	return &ghcb_pool->ghcbs[0];
>  	struct ghcb_entry *entry;
> +	struct ghcb *ghcb;
>  	int i;
>  
>  	if (!ghcb_pool)
> @@ -63,12 +76,18 @@ static struct ghcb_entry *ghcb_alloc(void)
>  	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
>  		if (!test_and_set_bit(i, ghcb_pool->in_use)) {
>  			entry = &ghcb_pool->ghcbs[i];
> -			memset(&entry->ghcb, 0, sizeof(entry->ghcb));
> +			ghcb = &entry->ghcb;
> +
> +			memset(&ghcb, 0, sizeof(*ghcb));
> +			ghcb->ghcb_usage = 0;
> +			ghcb->protocol_version = 1;
> +
>  			return entry;
>  		}
>  	}
>  
>  ucall_failed:
> +	sev_es_terminate();
>  	return NULL;
>  }
>  
> @@ -200,3 +219,45 @@ bool is_sev_es_enabled(void)
>  	return is_sev_enabled() &&
>  	       rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ES_ENABLED;
>  }
> +
> +static uint64_t setup_exitinfo1_portio(uint32_t port)
> +{
> +	uint64_t exitinfo1 = 0;
> +
> +	exitinfo1 |= IOIO_TYPE_STR;
> +	exitinfo1 |= ((port & 0xffff) << 16);
> +	exitinfo1 |= IOIO_SEG_DS;
> +	exitinfo1 |= IOIO_DATA_8;
> +	exitinfo1 |= IOIO_REP;
> +
> +	return exitinfo1;
> +}
> +
> +static void do_vmg_exit(uint64_t ghcb_gpa)
> +{
> +	wrmsr(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa);
> +	__asm__ __volatile__("rep; vmmcall");
> +}
> +
> +void sev_es_ucall_port_write(uint32_t port, uint64_t data)
> +{
> +	struct ghcb_entry *entry;
> +	struct ghcb *ghcb;
> +	const uint64_t exitinfo1 = setup_exitinfo1_portio(port);
> +
> +	entry = ghcb_alloc();
> +	ghcb = &entry->ghcb;
> +
> +	ghcb_set_sw_exit_code(ghcb, SW_EXIT_CODE_IOIO);
> +	ghcb_set_sw_exit_info_1(ghcb, exitinfo1);
> +	ghcb_set_sw_exit_info_2(ghcb, sizeof(data));
> +
> +	// Setup the SW Stratch buffer pointer.
> +	ghcb_set_sw_scratch(ghcb,
> +			    entry->gpa + offsetof(struct ghcb, shared_buffer));
> +	memcpy(&ghcb->shared_buffer, &data, sizeof(data));
> +
> +	do_vmg_exit(entry->gpa);
> +
> +	ghcb_free(entry);
> +}
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
> index 1265cecc7dd1..24da2f4316d8 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
> @@ -5,6 +5,8 @@
>   * Copyright (C) 2018, Red Hat, Inc.
>   */
>  #include "kvm_util.h"
> +#include "processor.h"
> +#include "sev.h"
>  
>  #define UCALL_PIO_PORT ((uint16_t)0x1000)
>  
> @@ -21,6 +23,10 @@ void ucall_arch_do_ucall(vm_vaddr_t uc)
>  #define HORRIFIC_L2_UCALL_CLOBBER_HACK	\
>  	"rcx", "rsi", "r8", "r9", "r10", "r11"
>  
> +	if (is_sev_es_enabled()) {

No curly braces needed.

> +		sev_es_ucall_port_write(UCALL_PIO_PORT, uc);
> +	}

This will clearly fall through to the standard IN, which I suspect is wrong and
only "works" because the only usage is a single GUEST_DONE(), i.e. no test
actually resumes to this point.

> +
>  	asm volatile("push %%rbp\n\t"
>  		     "push %%r15\n\t"
>  		     "push %%r14\n\t"
> @@ -48,8 +54,19 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
>  
>  	if (run->exit_reason == KVM_EXIT_IO && run->io.port == UCALL_PIO_PORT) {
>  		struct kvm_regs regs;
> +		uint64_t addr;
> +
> +		if (vcpu->vm->subtype == VM_SUBTYPE_SEV_ES) {
> +			TEST_ASSERT(

No Google3 style please.  I'm going to start charging folks for these violations.
I don't know _how_, but darn it, I'll find a way :-)

> +				run->io.count == 8 && run->io.size == 1,
> +				"SEV-ES ucall exit requires 8 byte string out\n");
> +
> +			addr = *(uint64_t *)((uint8_t *)(run) + run->io.data_offset);

Rather than this amazing bit of casting, I'm tempted to say we should add
kvm_vcpu_arch{} and then map the PIO page in vm_arch_vcpu_add().  Then this
is more sanely:

			return *(uint64_t *)vcpu->arch.pio);

where vcpu->arch.pio is a "void *".  At least, I think that would work?


> +			return (void *)addr;
> +		}
>  
>  		vcpu_regs_get(vcpu, &regs);
> +

Spurious whitespace.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
index 691dc005e2a1..26447caccd40 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -109,4 +109,6 @@  static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
 bool is_sev_enabled(void);
 bool is_sev_es_enabled(void);
 
+void sev_es_ucall_port_write(uint32_t port, uint64_t data);
+
 #endif /* SELFTEST_KVM_SEV_H */
diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
index 5b3f0a8a931a..276477f2c2cf 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
@@ -8,11 +8,18 @@ 
 #include "svm.h"
 #include "svm_util.h"
 
+#define IOIO_TYPE_STR (1 << 2)
+#define IOIO_SEG_DS (1 << 11 | 1 << 10)
+#define IOIO_DATA_8 (1 << 4)
+#define IOIO_REP (1 << 3)
+
+#define SW_EXIT_CODE_IOIO 0x7b
+
 struct ghcb_entry {
 	struct ghcb ghcb;
 
 	/* Guest physical address of this GHCB. */
-	void *gpa;
+	uint64_t gpa;
 
 	/* Host virtual address of this struct. */
 	struct ghcb_entry *hva;
@@ -45,16 +52,22 @@  void ghcb_init(struct kvm_vm *vm)
 	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
 		entry = &hdr->ghcbs[i];
 		entry->hva = entry;
-		entry->gpa = addr_hva2gpa(vm, &entry->ghcb);
+		entry->gpa = (uint64_t)addr_hva2gpa(vm, &entry->ghcb);
 	}
 
 	write_guest_global(vm, ghcb_pool, (struct ghcb_header *)vaddr);
 }
 
+static void sev_es_terminate(void)
+{
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+}
+
 static struct ghcb_entry *ghcb_alloc(void)
 {
 	return &ghcb_pool->ghcbs[0];
 	struct ghcb_entry *entry;
+	struct ghcb *ghcb;
 	int i;
 
 	if (!ghcb_pool)
@@ -63,12 +76,18 @@  static struct ghcb_entry *ghcb_alloc(void)
 	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
 		if (!test_and_set_bit(i, ghcb_pool->in_use)) {
 			entry = &ghcb_pool->ghcbs[i];
-			memset(&entry->ghcb, 0, sizeof(entry->ghcb));
+			ghcb = &entry->ghcb;
+
+			memset(&ghcb, 0, sizeof(*ghcb));
+			ghcb->ghcb_usage = 0;
+			ghcb->protocol_version = 1;
+
 			return entry;
 		}
 	}
 
 ucall_failed:
+	sev_es_terminate();
 	return NULL;
 }
 
@@ -200,3 +219,45 @@  bool is_sev_es_enabled(void)
 	return is_sev_enabled() &&
 	       rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ES_ENABLED;
 }
+
+static uint64_t setup_exitinfo1_portio(uint32_t port)
+{
+	uint64_t exitinfo1 = 0;
+
+	exitinfo1 |= IOIO_TYPE_STR;
+	exitinfo1 |= ((port & 0xffff) << 16);
+	exitinfo1 |= IOIO_SEG_DS;
+	exitinfo1 |= IOIO_DATA_8;
+	exitinfo1 |= IOIO_REP;
+
+	return exitinfo1;
+}
+
+static void do_vmg_exit(uint64_t ghcb_gpa)
+{
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa);
+	__asm__ __volatile__("rep; vmmcall");
+}
+
+void sev_es_ucall_port_write(uint32_t port, uint64_t data)
+{
+	struct ghcb_entry *entry;
+	struct ghcb *ghcb;
+	const uint64_t exitinfo1 = setup_exitinfo1_portio(port);
+
+	entry = ghcb_alloc();
+	ghcb = &entry->ghcb;
+
+	ghcb_set_sw_exit_code(ghcb, SW_EXIT_CODE_IOIO);
+	ghcb_set_sw_exit_info_1(ghcb, exitinfo1);
+	ghcb_set_sw_exit_info_2(ghcb, sizeof(data));
+
+	// Setup the SW Stratch buffer pointer.
+	ghcb_set_sw_scratch(ghcb,
+			    entry->gpa + offsetof(struct ghcb, shared_buffer));
+	memcpy(&ghcb->shared_buffer, &data, sizeof(data));
+
+	do_vmg_exit(entry->gpa);
+
+	ghcb_free(entry);
+}
diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
index 1265cecc7dd1..24da2f4316d8 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
@@ -5,6 +5,8 @@ 
  * Copyright (C) 2018, Red Hat, Inc.
  */
 #include "kvm_util.h"
+#include "processor.h"
+#include "sev.h"
 
 #define UCALL_PIO_PORT ((uint16_t)0x1000)
 
@@ -21,6 +23,10 @@  void ucall_arch_do_ucall(vm_vaddr_t uc)
 #define HORRIFIC_L2_UCALL_CLOBBER_HACK	\
 	"rcx", "rsi", "r8", "r9", "r10", "r11"
 
+	if (is_sev_es_enabled()) {
+		sev_es_ucall_port_write(UCALL_PIO_PORT, uc);
+	}
+
 	asm volatile("push %%rbp\n\t"
 		     "push %%r15\n\t"
 		     "push %%r14\n\t"
@@ -48,8 +54,19 @@  void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
 
 	if (run->exit_reason == KVM_EXIT_IO && run->io.port == UCALL_PIO_PORT) {
 		struct kvm_regs regs;
+		uint64_t addr;
+
+		if (vcpu->vm->subtype == VM_SUBTYPE_SEV_ES) {
+			TEST_ASSERT(
+				run->io.count == 8 && run->io.size == 1,
+				"SEV-ES ucall exit requires 8 byte string out\n");
+
+			addr = *(uint64_t *)((uint8_t *)(run) + run->io.data_offset);
+			return (void *)addr;
+		}
 
 		vcpu_regs_get(vcpu, &regs);
+
 		return (void *)regs.rdi;
 	}
 	return NULL;
diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
index 1d84e78e7ae2..2448533a9a41 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
@@ -18,12 +18,7 @@  static void guest_sev_es_code(void)
 	/* TODO: Check CPUID after GHCB-based hypercall support is added. */
 	GUEST_ASSERT(is_sev_es_enabled());
 
-	/*
-	 * TODO: Add GHCB and ucall support for SEV-ES guests.  For now, simply
-	 * force "termination" to signal "done" via the GHCB MSR protocol.
-	 */
-	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
-	__asm__ __volatile__("rep; vmmcall");
+	GUEST_DONE();
 }
 
 static void guest_sev_code(void)
@@ -45,16 +40,6 @@  static void test_sev(void *guest_code, uint64_t policy)
 	for (;;) {
 		vcpu_run(vcpu);
 
-		if (policy & SEV_POLICY_ES) {
-			TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_SYSTEM_EVENT,
-				    "Wanted SYSTEM_EVENT, got %s",
-				    exit_reason_str(vcpu->run->exit_reason));
-			TEST_ASSERT_EQ(vcpu->run->system_event.type, KVM_SYSTEM_EVENT_SEV_TERM);
-			TEST_ASSERT_EQ(vcpu->run->system_event.ndata, 1);
-			TEST_ASSERT_EQ(vcpu->run->system_event.data[0], GHCB_MSR_TERM_REQ);
-			break;
-		}
-
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_SYNC:
 			continue;