From: Sean Christopherson <seanjc@google.com> To: Michael Roth <michael.roth@amd.com> Cc: linux-kselftest@vger.kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, Nathan Tempelman <natet@google.com>, Marc Orr <marcorr@google.com>, Steve Rutherford <srutherford@google.com>, Mingwei Zhang <mizhang@google.com>, Brijesh Singh <brijesh.singh@amd.com>, Tom Lendacky <thomas.lendacky@amd.com>, Varad Gautam <varad.gautam@suse.com>, Shuah Khan <shuah@kernel.org>, Vitaly Kuznetsov <vkuznets@redhat.com>, David Woodhouse <dwmw@amazon.co.uk>, Ricardo Koller <ricarkol@google.com>, Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>, Alexandru Elisei <alexandru.elisei@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, kvmarm@lists.cs.columbia.edu Subject: Re: [RFC PATCH 00/10] KVM: selftests: Add support for test-selectable ucall implementations Date: Thu, 30 Dec 2021 21:11:12 +0000 [thread overview] Message-ID: <Yc4gcJdhxthBKUUd@google.com> (raw) In-Reply-To: <20211210164620.11636-1-michael.roth@amd.com> On Fri, Dec 10, 2021, Michael Roth wrote: > To summarize, x86 relies on a ucall based on using PIO intructions to generate > an exit to userspace and provide the GVA of a dynamically-allocated ucall > struct that resides in guest memory and contains information about how to > handle/interpret the exit. This doesn't work for SEV guests for 3 main reasons: > > 1) The guest memory is generally encrypted during run-time, so the guest > needs to ensure the ucall struct is allocated in shared memory. > 2) The guest page table is also encrypted, so the address would need to be a > GPA instead of a GVA. > 3) The guest vCPU register may also be encrypted in the case of > SEV-ES/SEV-SNP, so the approach of examining vCPU register state has > additional requirements such as requiring guest code to implement a #VC > handler that can provide the appropriate registers via a vmgexit. > > To address these issues, the SEV selftest RFC1 patchset introduced a set of new > SEV-specific interfaces that closely mirrored the functionality of > ucall()/get_ucall(), but relied on a pre-allocated/static ucall buffer in > shared guest memory so it that guest code could pass messages/state to the host > by simply writing to this pre-arranged shared memory region and then generating > an exit to userspace (via a halt instruction). > > Paolo suggested instead implementing support for test/guest-specific ucall > implementations that could be used as an alternative to the default PIO-based > ucall implementations as-needed based on test/guest requirements, while still > allowing for tests to use a common set interfaces like ucall()/get_ucall(). This all seems way more complicated than it needs to be. HLT is _worse_ than PIO on x86 because it triggers a userspace exit if and only if the local APIC is not in-kernel. That is bound to bite someone. The only issue with SEV is the address, not the VM-Exit mechanism. That doesn't change with SEV-ES, SEV-SNP, or TDX, as PIO and HLT will both get reflected as #VC/#VE, i.e. the guest side needs to be updated to use VMGEXIT/TDCALL no matter what, at which point having the hypercall request PIO emulation is just as easy as requesting HLT. I also don't like having to differentiate between a "shared" and "regular" ucall. I kind of like having to explicitly pass the ucall object being used, but that puts undue burden on simple single-vCPU tests. The inability to read guest private memory is really the only issue, and that can be easily solved without completely revamping the ucall framework, and without having to update a huge pile of tests to make them place nice with private memory. This would also be a good opportunity to clean up the stupidity of tests having to manually call ucall_init(), drop the unused/pointless @arg from ucall_init(), and maybe even fix arm64's lurking landmine of not being SMP safe (the address is shared by all vCPUs). To reduce the burden on tests and avoid ordering issues with creating vCPUs, allocate a ucall struct for every possible vCPU when the VM is created and stuff the GPA of the struct in the struct itself so that the guest can communicate the GPA instead of the GVA. Then confidential VMs just need to make all structs shared. If all architectures have a way to access a vCPU ID, the ucall structs could be stored as a simple array. If not, a list based allocator would probably suffice. E.g. something like this, except the list management is in common code instead of x86, and also delete all the per-test ucall_init() calls. diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c index a3489973e290..9aab6407bd42 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c +++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c @@ -8,19 +8,59 @@ #define UCALL_PIO_PORT ((uint16_t)0x1000) -void ucall_init(struct kvm_vm *vm, void *arg) +static struct list_head *ucall_list; + +void ucall_init(struct kvm_vm *vm) { + struct ucall *ucalls; + int nr_cpus = kvm_check_cap(KVM_CAP_MAX_VCPUS); + int i; + + TEST_ASSERT(!ucall_list, "ucall() can only be used by one VM at a time"); + + INIT_LIST_HEAD(&vm->ucall_list); + + ucalls = vm_vaddr_alloc(nr_cpus * sizeof(struct ucall)); + ucall_make_shared(ucall_list, <size>); + + for (i = 0; i < nr_cpus; i++) { + ucalls[i].gpa = addr_gva2gpa(vm, &ucalls[i]); + + list_add(&vm->ucall_list, &ucalls[i].list) + } + + ucall_list = &vm->ucall_list; + sync_global_to_guest(vm, ucall_list); } void ucall_uninit(struct kvm_vm *vm) { + ucall_list = NULL; + sync_global_to_guest(vm, ucall_list); +} + +static struct ucall *ucall_alloc(void) +{ + struct ucall *uc; + + /* Is there a lock primitive for the guest? */ + lock_something(&ucall_lock); + uc = list_first_entry(ucall_list, struct ucall, list); + + list_del(&uc->list); + unlock_something(&ucall_lock); +} + +static void ucall_free(struct ucall *uc) +{ + lock_something(&ucall_lock); + list_add(&uc->list, ucall_list); + unlock_something(&ucall_lock); } void ucall(uint64_t cmd, int nargs, ...) { - struct ucall uc = { - .cmd = cmd, - }; + struct ucall *uc = ucall_alloc(); va_list va; int i; @@ -32,7 +72,9 @@ void ucall(uint64_t cmd, int nargs, ...) va_end(va); asm volatile("in %[port], %%al" - : : [port] "d" (UCALL_PIO_PORT), "D" (&uc) : "rax", "memory"); + : : [port] "d" (UCALL_PIO_PORT), "D" (uc->gpa) : "rax", "memory"); + + ucall_free(uc); } uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc) @@ -47,7 +89,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc) struct kvm_regs regs; vcpu_regs_get(vm, vcpu_id, ®s); - memcpy(&ucall, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi), + memcpy(&ucall, addr_gpa2hva(vm, (vm_paddr_t)regs.rdi), sizeof(ucall)); vcpu_run_complete_io(vm, vcpu_id);
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com> To: Michael Roth <michael.roth@amd.com> Cc: Brijesh Singh <brijesh.singh@amd.com>, kvm@vger.kernel.org, David Hildenbrand <david@redhat.com>, Marc Orr <marcorr@google.com>, linux-kselftest@vger.kernel.org, "H . Peter Anvin" <hpa@zytor.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, Shuah Khan <shuah@kernel.org>, kvmarm@lists.cs.columbia.edu, Nathan Tempelman <natet@google.com>, Janosch Frank <frankja@linux.ibm.com>, Marc Zyngier <maz@kernel.org>, Joerg Roedel <joro@8bytes.org>, x86@kernel.org, Ingo Molnar <mingo@redhat.com>, Mingwei Zhang <mizhang@google.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Tom Lendacky <thomas.lendacky@amd.com>, Borislav Petkov <bp@alien8.de>, Thomas Gleixner <tglx@linutronix.de>, Varad Gautam <varad.gautam@suse.com>, Jim Mattson <jmattson@google.com>, Steve Rutherford <srutherford@google.com>, linux-kernel@vger.kernel.org, Vitaly Kuznetsov <vkuznets@redhat.com>, David Woodhouse <dwmw@amazon.co.uk> Subject: Re: [RFC PATCH 00/10] KVM: selftests: Add support for test-selectable ucall implementations Date: Thu, 30 Dec 2021 21:11:12 +0000 [thread overview] Message-ID: <Yc4gcJdhxthBKUUd@google.com> (raw) In-Reply-To: <20211210164620.11636-1-michael.roth@amd.com> On Fri, Dec 10, 2021, Michael Roth wrote: > To summarize, x86 relies on a ucall based on using PIO intructions to generate > an exit to userspace and provide the GVA of a dynamically-allocated ucall > struct that resides in guest memory and contains information about how to > handle/interpret the exit. This doesn't work for SEV guests for 3 main reasons: > > 1) The guest memory is generally encrypted during run-time, so the guest > needs to ensure the ucall struct is allocated in shared memory. > 2) The guest page table is also encrypted, so the address would need to be a > GPA instead of a GVA. > 3) The guest vCPU register may also be encrypted in the case of > SEV-ES/SEV-SNP, so the approach of examining vCPU register state has > additional requirements such as requiring guest code to implement a #VC > handler that can provide the appropriate registers via a vmgexit. > > To address these issues, the SEV selftest RFC1 patchset introduced a set of new > SEV-specific interfaces that closely mirrored the functionality of > ucall()/get_ucall(), but relied on a pre-allocated/static ucall buffer in > shared guest memory so it that guest code could pass messages/state to the host > by simply writing to this pre-arranged shared memory region and then generating > an exit to userspace (via a halt instruction). > > Paolo suggested instead implementing support for test/guest-specific ucall > implementations that could be used as an alternative to the default PIO-based > ucall implementations as-needed based on test/guest requirements, while still > allowing for tests to use a common set interfaces like ucall()/get_ucall(). This all seems way more complicated than it needs to be. HLT is _worse_ than PIO on x86 because it triggers a userspace exit if and only if the local APIC is not in-kernel. That is bound to bite someone. The only issue with SEV is the address, not the VM-Exit mechanism. That doesn't change with SEV-ES, SEV-SNP, or TDX, as PIO and HLT will both get reflected as #VC/#VE, i.e. the guest side needs to be updated to use VMGEXIT/TDCALL no matter what, at which point having the hypercall request PIO emulation is just as easy as requesting HLT. I also don't like having to differentiate between a "shared" and "regular" ucall. I kind of like having to explicitly pass the ucall object being used, but that puts undue burden on simple single-vCPU tests. The inability to read guest private memory is really the only issue, and that can be easily solved without completely revamping the ucall framework, and without having to update a huge pile of tests to make them place nice with private memory. This would also be a good opportunity to clean up the stupidity of tests having to manually call ucall_init(), drop the unused/pointless @arg from ucall_init(), and maybe even fix arm64's lurking landmine of not being SMP safe (the address is shared by all vCPUs). To reduce the burden on tests and avoid ordering issues with creating vCPUs, allocate a ucall struct for every possible vCPU when the VM is created and stuff the GPA of the struct in the struct itself so that the guest can communicate the GPA instead of the GVA. Then confidential VMs just need to make all structs shared. If all architectures have a way to access a vCPU ID, the ucall structs could be stored as a simple array. If not, a list based allocator would probably suffice. E.g. something like this, except the list management is in common code instead of x86, and also delete all the per-test ucall_init() calls. diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c index a3489973e290..9aab6407bd42 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c +++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c @@ -8,19 +8,59 @@ #define UCALL_PIO_PORT ((uint16_t)0x1000) -void ucall_init(struct kvm_vm *vm, void *arg) +static struct list_head *ucall_list; + +void ucall_init(struct kvm_vm *vm) { + struct ucall *ucalls; + int nr_cpus = kvm_check_cap(KVM_CAP_MAX_VCPUS); + int i; + + TEST_ASSERT(!ucall_list, "ucall() can only be used by one VM at a time"); + + INIT_LIST_HEAD(&vm->ucall_list); + + ucalls = vm_vaddr_alloc(nr_cpus * sizeof(struct ucall)); + ucall_make_shared(ucall_list, <size>); + + for (i = 0; i < nr_cpus; i++) { + ucalls[i].gpa = addr_gva2gpa(vm, &ucalls[i]); + + list_add(&vm->ucall_list, &ucalls[i].list) + } + + ucall_list = &vm->ucall_list; + sync_global_to_guest(vm, ucall_list); } void ucall_uninit(struct kvm_vm *vm) { + ucall_list = NULL; + sync_global_to_guest(vm, ucall_list); +} + +static struct ucall *ucall_alloc(void) +{ + struct ucall *uc; + + /* Is there a lock primitive for the guest? */ + lock_something(&ucall_lock); + uc = list_first_entry(ucall_list, struct ucall, list); + + list_del(&uc->list); + unlock_something(&ucall_lock); +} + +static void ucall_free(struct ucall *uc) +{ + lock_something(&ucall_lock); + list_add(&uc->list, ucall_list); + unlock_something(&ucall_lock); } void ucall(uint64_t cmd, int nargs, ...) { - struct ucall uc = { - .cmd = cmd, - }; + struct ucall *uc = ucall_alloc(); va_list va; int i; @@ -32,7 +72,9 @@ void ucall(uint64_t cmd, int nargs, ...) va_end(va); asm volatile("in %[port], %%al" - : : [port] "d" (UCALL_PIO_PORT), "D" (&uc) : "rax", "memory"); + : : [port] "d" (UCALL_PIO_PORT), "D" (uc->gpa) : "rax", "memory"); + + ucall_free(uc); } uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc) @@ -47,7 +89,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc) struct kvm_regs regs; vcpu_regs_get(vm, vcpu_id, ®s); - memcpy(&ucall, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi), + memcpy(&ucall, addr_gpa2hva(vm, (vm_paddr_t)regs.rdi), sizeof(ucall)); vcpu_run_complete_io(vm, vcpu_id); _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply other threads:[~2021-12-30 21:11 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-10 16:46 [RFC PATCH 00/10] KVM: selftests: Add support for test-selectable ucall implementations Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 01/10] kvm: selftests: move base kvm_util.h declarations to kvm_util_base.h Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 02/10] kvm: selftests: move ucall declarations into ucall_common.h Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-25 9:11 ` Andrew Jones 2021-12-25 9:11 ` Andrew Jones 2021-12-10 16:46 ` [PATCH RFC 03/10] kvm: selftests: introduce ucall_ops for test/arch-specific ucall implementations Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 04/10] kvm: arm64: selftests: use ucall_ops to define default ucall implementation Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 05/10] kvm: s390: " Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 06/10] kvm: selftests: add ucall interfaces based around shared memory Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 07/10] kvm: selftests: add ucall_shared ops for PIO Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 08/10] kvm: selftests: introduce ucall implementation based on halt instructions Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 09/10] kvm: selftests: add GUEST_SHARED_* macros for shared ucall implementations Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-10 16:46 ` [PATCH RFC 10/10] kvm: selftests: add ucall_test to test various ucall functionality Michael Roth 2021-12-10 16:46 ` Michael Roth 2021-12-22 14:46 ` [RFC PATCH 00/10] KVM: selftests: Add support for test-selectable ucall implementations Paolo Bonzini 2021-12-22 14:46 ` Paolo Bonzini 2021-12-30 21:11 ` Sean Christopherson [this message] 2021-12-30 21:11 ` Sean Christopherson 2022-01-04 23:35 ` Michael Roth 2022-01-04 23:35 ` Michael Roth 2022-01-05 0:17 ` Sean Christopherson 2022-01-05 0:17 ` Sean Christopherson 2022-01-05 17:02 ` Michael Roth 2022-01-05 17:02 ` Michael Roth 2022-01-05 17:43 ` Sean Christopherson 2022-01-05 17:43 ` Sean Christopherson 2022-01-05 19:11 ` Michael Roth 2022-01-05 19:11 ` Michael Roth 2022-01-05 19:40 ` Sean Christopherson 2022-01-05 19:40 ` Sean Christopherson 2022-01-05 21:35 ` Michael Roth 2022-01-05 21:35 ` Michael Roth 2022-01-05 22:02 ` Sean Christopherson 2022-01-05 22:02 ` Sean Christopherson 2022-01-05 22:32 ` Michael Roth 2022-01-05 22:32 ` Michael Roth
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Yc4gcJdhxthBKUUd@google.com \ --to=seanjc@google.com \ --cc=alexandru.elisei@arm.com \ --cc=borntraeger@linux.ibm.com \ --cc=bp@alien8.de \ --cc=brijesh.singh@amd.com \ --cc=david@redhat.com \ --cc=dwmw@amazon.co.uk \ --cc=frankja@linux.ibm.com \ --cc=hpa@zytor.com \ --cc=imbrenda@linux.ibm.com \ --cc=james.morse@arm.com \ --cc=jmattson@google.com \ --cc=joro@8bytes.org \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-kselftest@vger.kernel.org \ --cc=marcorr@google.com \ --cc=maz@kernel.org \ --cc=michael.roth@amd.com \ --cc=mingo@redhat.com \ --cc=mizhang@google.com \ --cc=natet@google.com \ --cc=ricarkol@google.com \ --cc=shuah@kernel.org \ --cc=srutherford@google.com \ --cc=suzuki.poulose@arm.com \ --cc=tglx@linutronix.de \ --cc=thomas.lendacky@amd.com \ --cc=varad.gautam@suse.com \ --cc=vkuznets@redhat.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.