[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y6T4PJfF3nGU25Ee@google.com>
Date: Fri, 23 Dec 2022 00:37:16 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Vishal Annapurve <vannapurve@...gle.com>
Cc: x86@...nel.org, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, pbonzini@...hat.com,
shuah@...nel.org, bgardon@...gle.com, oupton@...gle.com,
peterx@...hat.com, vkuznets@...hat.com, dmatlack@...gle.com,
pgonda@...gle.com, andrew.jones@...ux.dev
Subject: Re: [V3 PATCH 2/2] KVM: selftests: x86: Add native hypercall support
On Thu, Dec 22, 2022, Vishal Annapurve wrote:
> Add an API to execute hypercall as per the cpu type by checking the
> underlying CPU. KVM emulates vmcall/vmmcall instruction by modifying
> guest memory contents with hypercall instruction as per the cpu type.
>
> Confidential VMs need to execute hypercall instruction without it being
> emulated by KVM as KVM can not modify guest memory contents.
>
> Signed-off-by: Vishal Annapurve <vannapurve@...gle.com>
> ---
> .../selftests/kvm/include/x86_64/processor.h | 3 +++
> .../selftests/kvm/lib/x86_64/processor.c | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 4d5dd9a467e1..3617f83bb2e5 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -1039,6 +1039,9 @@ uint64_t *vm_get_page_table_entry(struct kvm_vm *vm, uint64_t vaddr);
> uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
> uint64_t a3);
>
> +uint64_t kvm_native_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
> + uint64_t a3);
> +
> void __vm_xsave_require_permission(int bit, const char *name);
>
> #define vm_xsave_require_permission(perm) \
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index 1e93bb3cb058..429e55f2609f 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -1202,6 +1202,24 @@ uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
> return r;
> }
>
> +uint64_t kvm_native_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
Just do this in kvm_hypercall(). David didn't say "don't do that", he said "don't
do that in a single patch". Except for fix_hypercall_test, selftests should always
use the native hypercall instruction and not rely on KVM's patching, e.g. patching
will go sideways if someone gets clever and makes guest code not-writable.
> + uint64_t a3)
Align parameters.
> +{
> + uint64_t r;
> +
> + if (is_amd_cpu()) {
Curly brace is unnecessary. Though I think I'd prefer to not duplicate the asm
blob (too many darn inputs). It's a little ugly, but I prefer it over duplicating
the entire blob. The approach could also likely be macrofied for other hypercall
usage (future problem).
asm volatile("test %[use_vmmcall], %[use_vmmcall]\n\t"
"jnz 1f\n\t"
"vmcall\n\t"
"jmp 2f\n\t"
"1: vmmcall\n\t"
"2:"
: "=a"(r)
: "a"(nr), "b"(a0), "c"(a1), "d"(a2), "S"(a3),
[use_vmmcall] "r" (is_amd_cpu()));
return r;
Powered by blists - more mailing lists