| Message ID | 20240409133959.2888018-7-pgonda@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add initial GHCB support for SEV-ES selftests | expand |
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, ®s); > + Spurious whitespace.
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, ®s); + 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;
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(-)