[6/6] KVM: selftests: Test Hyper-V extended hypercall exit to userspace
Commit Message
Hyper-V extended hypercalls by default exit to userspace. Verify
userspace gets the call, update the result and then guest verifies
result it received.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/x86_64/hyperv_extended_hcalls.c | 90 +++++++++++++++++++
3 files changed, 92 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
Comments
On Fri, Nov 04, 2022 at 09:57:04PM -0700, Vipin Sharma wrote:
> Hyper-V extended hypercalls by default exit to userspace. Verify
> userspace gets the call, update the result and then guest verifies
> result it received.
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 1 +
> .../kvm/x86_64/hyperv_extended_hcalls.c | 90 +++++++++++++++++++
> 3 files changed, 92 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 2f0d705db9db..ffe06dd1cc6e 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -24,6 +24,7 @@
> /x86_64/kvm_pv_test
> /x86_64/hyperv_clock
> /x86_64/hyperv_cpuid
> +/x86_64/hyperv_extended_hcalls
nit: Any reason not to name this hyperv_extended_hypercalls? It's not
too long and as a non-Hyper-V developer it's easier to read.
> /x86_64/hyperv_features
> /x86_64/hyperv_svm_test
> /x86_64/max_vcpuid_cap_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 0172eb6cb6ee..366345099363 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -85,6 +85,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/emulator_error_test
> TEST_GEN_PROGS_x86_64 += x86_64/fix_hypercall_test
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_clock
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
> +TEST_GEN_PROGS_x86_64 += x86_64/hyperv_extended_hcalls
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
> TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
> new file mode 100644
> index 000000000000..d378877235d4
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Test Hyper-V extended hypercalls
It would probably be worth adding a note in this comment that the
negative tests for extended hypercalls live in hyperv_features.c, that
way someone doesn't accidentally go down the rabbit hole of adding
negative tests here in the future.
> + *
> + * Copyright 2020 Google LLC
2022 :)
> + * Author: Vipin Sharma <vipinsh@google.com>
> + */
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "hyperv.h"
> +
> +/* Any value is fine */
> +#define EXT_CAPABILITIES 0xbull
> +
> +static void guest_code(vm_vaddr_t pgs_gpa, vm_vaddr_t output_pg_gva)
> +{
> + uint64_t res, vector;
> + uint64_t *output_gva;
> +
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> + wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
> +
> + output_gva = (uint64_t *)output_pg_gva;
> +
> + vector = hypercall(HV_EXT_CALL_QUERY_CAPABILITIES, pgs_gpa,
> + pgs_gpa + 4096, &res);
> +
> + GUEST_ASSERT_1(!vector, vector);
> + GUEST_ASSERT_2(res == HV_STATUS_SUCCESS, res, HV_STATUS_SUCCESS);
GUEST_ASSERT_EQ(res, HV_STATUS_SUCCESS);
> +
> + /* TLFS states output will be a uint64_t value */
> + GUEST_ASSERT_2(*output_gva == EXT_CAPABILITIES, *output_gva,
> + EXT_CAPABILITIES);
GUEST_ASSERT_EQ(*output_gva, EXT_CAPABILITIES);
> +
> + GUEST_DONE();
> +}
> +
> +static void guest_extended_hcall_test(void)
> +{
> + struct kvm_vcpu *vcpu;
> + struct kvm_run *run;
> + struct kvm_vm *vm;
> + struct ucall uc;
> + vm_vaddr_t hcall_page;
> + uint64_t *outval;
> +
> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> + run = vcpu->run;
> + vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_ENFORCE_CPUID, 1);
> + vcpu_set_hv_cpuid(vcpu);
Check if KVM offers HV_ENABLE_EXTENDED_HYPERCALLS in CPUID, and skip the
test if not.
> +
> + /* Hypercall input/output */
> + hcall_page = vm_vaddr_alloc_pages(vm, 2);
> + memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
s/getpagesize()/vm->page_size/
> + vcpu_args_set(vcpu, 2, addr_gva2gpa(vm, hcall_page), hcall_page + 4096);
s/4096/vm->page_size/
And to avoid hard-coding 4096 in guest_code(), you could pass in the GPA
of the ouput page as another argument.
> +
> + vcpu_run(vcpu);
> +
> + TEST_ASSERT((run->exit_reason == KVM_EXIT_HYPERV),
> + "unexpected exit reason: %u (%s)", run->exit_reason,
> + exit_reason_str(run->exit_reason));
> +
> + outval = addr_gpa2hva(vm, run->hyperv.u.hcall.params[1]);
> + *outval = EXT_CAPABILITIES;
> + run->hyperv.u.hcall.result = HV_STATUS_SUCCESS;
> +
> + vcpu_run(vcpu);
> +
> + TEST_ASSERT((run->exit_reason == KVM_EXIT_IO),
> + "unexpected exit reason: %u (%s)", run->exit_reason,
> + exit_reason_str(run->exit_reason));
Optional: Asserting a specific exit reason is a pretty common pattern in
the x86 selftests. It'd be nice to create a common macro for it. e.g.
ASSERT_EXIT_REASON(vcpu, KVM_EXIT_IO);
> +
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT_2(uc, "arg1 = %ld, arg2 = %ld");
> + break;
> + case UCALL_DONE:
> + break;
> + default:
> + TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
> + }
> +
> + kvm_vm_free(vm);
> +}
> +
> +int main(void)
> +{
> + guest_extended_hcall_test();
Why not just put all this in main()?
> +}
return 0?
> --
> 2.38.1.273.g43a17bfeac-goog
>
On Mon, Nov 7, 2022 at 11:01 AM David Matlack <dmatlack@google.com> wrote:
>
> On Fri, Nov 04, 2022 at 09:57:04PM -0700, Vipin Sharma wrote:
> > Hyper-V extended hypercalls by default exit to userspace. Verify
> > userspace gets the call, update the result and then guest verifies
> > result it received.
> >
> > Signed-off-by: Vipin Sharma <vipinsh@google.com>
> > ---
> > tools/testing/selftests/kvm/.gitignore | 1 +
> > tools/testing/selftests/kvm/Makefile | 1 +
> > .../kvm/x86_64/hyperv_extended_hcalls.c | 90 +++++++++++++++++++
> > 3 files changed, 92 insertions(+)
> > create mode 100644 tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
> >
> > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> > index 2f0d705db9db..ffe06dd1cc6e 100644
> > --- a/tools/testing/selftests/kvm/.gitignore
> > +++ b/tools/testing/selftests/kvm/.gitignore
> > @@ -24,6 +24,7 @@
> > /x86_64/kvm_pv_test
> > /x86_64/hyperv_clock
> > /x86_64/hyperv_cpuid
> > +/x86_64/hyperv_extended_hcalls
>
> nit: Any reason not to name this hyperv_extended_hypercalls? It's not
> too long and as a non-Hyper-V developer it's easier to read.
>
I was keeping it consistent with other names like
KVM_EXIT_HYPERV_HCALL, and struct hcall{} in struct kvm_hyperv_exit{}.
I am fine with renaming it to hyperv_extended_hypercalls.
> > /x86_64/hyperv_features
> > /x86_64/hyperv_svm_test
> > /x86_64/max_vcpuid_cap_test
> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index 0172eb6cb6ee..366345099363 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -85,6 +85,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/emulator_error_test
> > TEST_GEN_PROGS_x86_64 += x86_64/fix_hypercall_test
> > TEST_GEN_PROGS_x86_64 += x86_64/hyperv_clock
> > TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
> > +TEST_GEN_PROGS_x86_64 += x86_64/hyperv_extended_hcalls
> > TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
> > TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
> > TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
> > new file mode 100644
> > index 000000000000..d378877235d4
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hcalls.c
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Test Hyper-V extended hypercalls
>
> It would probably be worth adding a note in this comment that the
> negative tests for extended hypercalls live in hyperv_features.c, that
> way someone doesn't accidentally go down the rabbit hole of adding
> negative tests here in the future.
>
Sure.
> > + *
> > + * Copyright 2020 Google LLC
>
> 2022 :)
>
> > + * Author: Vipin Sharma <vipinsh@google.com>
> > + */
> > +
> > +#include "kvm_util.h"
> > +#include "processor.h"
> > +#include "hyperv.h"
> > +
> > +/* Any value is fine */
> > +#define EXT_CAPABILITIES 0xbull
> > +
> > +static void guest_code(vm_vaddr_t pgs_gpa, vm_vaddr_t output_pg_gva)
> > +{
> > + uint64_t res, vector;
> > + uint64_t *output_gva;
> > +
> > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > + wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
> > +
> > + output_gva = (uint64_t *)output_pg_gva;
> > +
> > + vector = hypercall(HV_EXT_CALL_QUERY_CAPABILITIES, pgs_gpa,
> > + pgs_gpa + 4096, &res);
> > +
> > + GUEST_ASSERT_1(!vector, vector);
> > + GUEST_ASSERT_2(res == HV_STATUS_SUCCESS, res, HV_STATUS_SUCCESS);
>
> GUEST_ASSERT_EQ(res, HV_STATUS_SUCCESS);
>
Copied things from hyperv_features test. This looks better, I will change it.
> > +
> > + /* TLFS states output will be a uint64_t value */
> > + GUEST_ASSERT_2(*output_gva == EXT_CAPABILITIES, *output_gva,
> > + EXT_CAPABILITIES);
>
> GUEST_ASSERT_EQ(*output_gva, EXT_CAPABILITIES);
>
> > +
> > + GUEST_DONE();
> > +}
> > +
> > +static void guest_extended_hcall_test(void)
> > +{
> > + struct kvm_vcpu *vcpu;
> > + struct kvm_run *run;
> > + struct kvm_vm *vm;
> > + struct ucall uc;
> > + vm_vaddr_t hcall_page;
> > + uint64_t *outval;
> > +
> > + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> > + run = vcpu->run;
> > + vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_ENFORCE_CPUID, 1);
> > + vcpu_set_hv_cpuid(vcpu);
>
> Check if KVM offers HV_ENABLE_EXTENDED_HYPERCALLS in CPUID, and skip the
> test if not.
>
Sure.
> > +
> > + /* Hypercall input/output */
> > + hcall_page = vm_vaddr_alloc_pages(vm, 2);
> > + memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
>
> s/getpagesize()/vm->page_size/
>
> > + vcpu_args_set(vcpu, 2, addr_gva2gpa(vm, hcall_page), hcall_page + 4096);
>
> s/4096/vm->page_size/
>
> And to avoid hard-coding 4096 in guest_code(), you could pass in the GPA
> of the ouput page as another argument.
>
Sounds good.
> > +
> > + vcpu_run(vcpu);
> > +
> > + TEST_ASSERT((run->exit_reason == KVM_EXIT_HYPERV),
> > + "unexpected exit reason: %u (%s)", run->exit_reason,
> > + exit_reason_str(run->exit_reason));
> > +
> > + outval = addr_gpa2hva(vm, run->hyperv.u.hcall.params[1]);
> > + *outval = EXT_CAPABILITIES;
> > + run->hyperv.u.hcall.result = HV_STATUS_SUCCESS;
> > +
> > + vcpu_run(vcpu);
> > +
> > + TEST_ASSERT((run->exit_reason == KVM_EXIT_IO),
> > + "unexpected exit reason: %u (%s)", run->exit_reason,
> > + exit_reason_str(run->exit_reason));
>
> Optional: Asserting a specific exit reason is a pretty common pattern in
> the x86 selftests. It'd be nice to create a common macro for it. e.g.
>
> ASSERT_EXIT_REASON(vcpu, KVM_EXIT_IO);
>
This is much better. I can add a patch which creates this API.
Should it be run or vcpu? Seems like everything needed is in struct kvm_run{}.
> > +
> > + switch (get_ucall(vcpu, &uc)) {
> > + case UCALL_ABORT:
> > + REPORT_GUEST_ASSERT_2(uc, "arg1 = %ld, arg2 = %ld");
> > + break;
> > + case UCALL_DONE:
> > + break;
> > + default:
> > + TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
> > + }
> > +
> > + kvm_vm_free(vm);
> > +}
> > +
> > +int main(void)
> > +{
> > + guest_extended_hcall_test();
>
> Why not just put all this in main()?
>
I will.
> > +}
>
> return 0?
>
> > --
> > 2.38.1.273.g43a17bfeac-goog
> >
On Mon, Nov 7, 2022 at 6:05 PM Vipin Sharma <vipinsh@google.com> wrote:
>
> On Mon, Nov 7, 2022 at 11:01 AM David Matlack <dmatlack@google.com> wrote:
> >
> > On Fri, Nov 04, 2022 at 09:57:04PM -0700, Vipin Sharma wrote:
> > > +
> > > + TEST_ASSERT((run->exit_reason == KVM_EXIT_IO),
> > > + "unexpected exit reason: %u (%s)", run->exit_reason,
> > > + exit_reason_str(run->exit_reason));
> >
> > Optional: Asserting a specific exit reason is a pretty common pattern in
> > the x86 selftests. It'd be nice to create a common macro for it. e.g.
> >
> > ASSERT_EXIT_REASON(vcpu, KVM_EXIT_IO);
> >
>
> This is much better. I can add a patch which creates this API.
>
> Should it be run or vcpu? Seems like everything needed is in struct kvm_run{}.
Either one but I suspect vcpu will produce a cleaner final result
since all tests pass around struct kvm_vcpu and only use kvm_run to
check the exit reason. i.e. I suspect you'll be able to delete several
struct kvm_run local variables as part of adding ASSERT_EXIT_REASON().
@@ -24,6 +24,7 @@
/x86_64/kvm_pv_test
/x86_64/hyperv_clock
/x86_64/hyperv_cpuid
+/x86_64/hyperv_extended_hcalls
/x86_64/hyperv_features
/x86_64/hyperv_svm_test
/x86_64/max_vcpuid_cap_test
@@ -85,6 +85,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/emulator_error_test
TEST_GEN_PROGS_x86_64 += x86_64/fix_hypercall_test
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_clock
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
+TEST_GEN_PROGS_x86_64 += x86_64/hyperv_extended_hcalls
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
new file mode 100644
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test Hyper-V extended hypercalls
+ *
+ * Copyright 2020 Google LLC
+ * Author: Vipin Sharma <vipinsh@google.com>
+ */
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "hyperv.h"
+
+/* Any value is fine */
+#define EXT_CAPABILITIES 0xbull
+
+static void guest_code(vm_vaddr_t pgs_gpa, vm_vaddr_t output_pg_gva)
+{
+ uint64_t res, vector;
+ uint64_t *output_gva;
+
+ wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
+ wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
+
+ output_gva = (uint64_t *)output_pg_gva;
+
+ vector = hypercall(HV_EXT_CALL_QUERY_CAPABILITIES, pgs_gpa,
+ pgs_gpa + 4096, &res);
+
+ GUEST_ASSERT_1(!vector, vector);
+ GUEST_ASSERT_2(res == HV_STATUS_SUCCESS, res, HV_STATUS_SUCCESS);
+
+ /* TLFS states output will be a uint64_t value */
+ GUEST_ASSERT_2(*output_gva == EXT_CAPABILITIES, *output_gva,
+ EXT_CAPABILITIES);
+
+ GUEST_DONE();
+}
+
+static void guest_extended_hcall_test(void)
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_run *run;
+ struct kvm_vm *vm;
+ struct ucall uc;
+ vm_vaddr_t hcall_page;
+ uint64_t *outval;
+
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ run = vcpu->run;
+ vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_ENFORCE_CPUID, 1);
+ vcpu_set_hv_cpuid(vcpu);
+
+ /* Hypercall input/output */
+ hcall_page = vm_vaddr_alloc_pages(vm, 2);
+ memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
+ vcpu_args_set(vcpu, 2, addr_gva2gpa(vm, hcall_page), hcall_page + 4096);
+
+ vcpu_run(vcpu);
+
+ TEST_ASSERT((run->exit_reason == KVM_EXIT_HYPERV),
+ "unexpected exit reason: %u (%s)", run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ outval = addr_gpa2hva(vm, run->hyperv.u.hcall.params[1]);
+ *outval = EXT_CAPABILITIES;
+ run->hyperv.u.hcall.result = HV_STATUS_SUCCESS;
+
+ vcpu_run(vcpu);
+
+ TEST_ASSERT((run->exit_reason == KVM_EXIT_IO),
+ "unexpected exit reason: %u (%s)", run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT_2(uc, "arg1 = %ld, arg2 = %ld");
+ break;
+ case UCALL_DONE:
+ break;
+ default:
+ TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
+ }
+
+ kvm_vm_free(vm);
+}
+
+int main(void)
+{
+ guest_extended_hcall_test();
+}