* [PATCH v3 01/10] KVM: selftests: Rename emulator_error_test to smaller_maxphyaddr_emulation_test
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:00 ` [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes David Matlack
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Rename emulator_error_test to smaller_maxphyaddr_emulation_test and
update the comment at the top of the file to document that this is
explicitly a test to validate that KVM emulates instructions in response
to an EPT violation when emulating a smaller MAXPHYADDR.
Signed-off-by: David Matlack <dmatlack@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/.gitignore | 2 +-
tools/testing/selftests/kvm/Makefile | 2 +-
...ulator_error_test.c => smaller_maxphyaddr_emulation_test.c} | 3 ++-
3 files changed, 4 insertions(+), 3 deletions(-)
rename tools/testing/selftests/kvm/x86_64/{emulator_error_test.c => smaller_maxphyaddr_emulation_test.c} (97%)
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 2f0d705db9db..053e5d34cd03 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -17,7 +17,6 @@
/x86_64/cr4_cpuid_sync_test
/x86_64/debug_regs
/x86_64/evmcs_test
-/x86_64/emulator_error_test
/x86_64/fix_hypercall_test
/x86_64/get_msr_index_features
/x86_64/kvm_clock_test
@@ -36,6 +35,7 @@
/x86_64/set_boot_cpu_id
/x86_64/set_sregs_test
/x86_64/sev_migrate_tests
+/x86_64/smaller_maxphyaddr_emulation_test
/x86_64/smm_test
/x86_64/state_test
/x86_64/svm_vmcall_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0172eb6cb6ee..ab133b731a2d 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -81,7 +81,6 @@ TEST_GEN_PROGS_x86_64 = x86_64/cpuid_test
TEST_GEN_PROGS_x86_64 += x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
-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
@@ -96,6 +95,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test
TEST_GEN_PROGS_x86_64 += x86_64/pmu_event_filter_test
TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id
TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
+TEST_GEN_PROGS_x86_64 += x86_64/smaller_maxphyaddr_emulation_test
TEST_GEN_PROGS_x86_64 += x86_64/smm_test
TEST_GEN_PROGS_x86_64 += x86_64/state_test
TEST_GEN_PROGS_x86_64 += x86_64/vmx_preemption_timer_test
diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
similarity index 97%
rename from tools/testing/selftests/kvm/x86_64/emulator_error_test.c
rename to tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
index 236e11755ba6..6ed996988a5a 100644
--- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
@@ -2,7 +2,8 @@
/*
* Copyright (C) 2020, Google LLC.
*
- * Tests for KVM_CAP_EXIT_ON_EMULATION_FAILURE capability.
+ * Test that KVM emulates instructions in response to EPT violations when
+ * allow_smaller_maxphyaddr is enabled and guest.MAXPHYADDR < host.MAXPHYADDR.
*/
#define _GNU_SOURCE /* for program_invocation_short_name */
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
2022-10-31 18:00 ` [PATCH v3 01/10] KVM: selftests: Rename emulator_error_test to smaller_maxphyaddr_emulation_test David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:19 ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 03/10] KVM: selftests: Delete dead ucall code David Matlack
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Hard-code the flds instruction and assert the exact instruction bytes
are present in run->emulation_failure. The test already requires the
instruction bytes to be present because that's the only way the test
will advance the RIP past the flds and get to GUEST_DONE().
Note that KVM does not necessarily return exactly 2 bytes in
run->emulation_failure since it may not know the exact instruction
length in all cases. So just assert that
run->emulation_failure.insn_size is at least 2.
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../smaller_maxphyaddr_emulation_test.c | 68 ++++++-------------
1 file changed, 20 insertions(+), 48 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
index 6ed996988a5a..d92cd4139f6d 100644
--- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
@@ -19,41 +19,20 @@
#define MEM_REGION_SLOT 10
#define MEM_REGION_SIZE PAGE_SIZE
+#define FLDS_MEM_EAX ".byte 0xd9, 0x00"
+
static void guest_code(void)
{
- __asm__ __volatile__("flds (%[addr])"
- :: [addr]"r"(MEM_REGION_GVA));
+ __asm__ __volatile__(FLDS_MEM_EAX :: "a"(MEM_REGION_GVA));
GUEST_DONE();
}
-/*
- * Accessors to get R/M, REG, and Mod bits described in the SDM vol 2,
- * figure 2-2 "Table Interpretation of ModR/M Byte (C8H)".
- */
-#define GET_RM(insn_byte) (insn_byte & 0x7)
-#define GET_REG(insn_byte) ((insn_byte & 0x38) >> 3)
-#define GET_MOD(insn_byte) ((insn_byte & 0xc) >> 6)
-
-/* Ensure we are dealing with a simple 2-byte flds instruction. */
-static bool is_flds(uint8_t *insn_bytes, uint8_t insn_size)
-{
- return insn_size >= 2 &&
- insn_bytes[0] == 0xd9 &&
- GET_REG(insn_bytes[1]) == 0x0 &&
- GET_MOD(insn_bytes[1]) == 0x0 &&
- /* Ensure there is no SIB byte. */
- GET_RM(insn_bytes[1]) != 0x4 &&
- /* Ensure there is no displacement byte. */
- GET_RM(insn_bytes[1]) != 0x5;
-}
-
static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
{
struct kvm_run *run = vcpu->run;
struct kvm_regs regs;
uint8_t *insn_bytes;
- uint8_t insn_size;
uint64_t flags;
TEST_ASSERT(run->exit_reason == KVM_EXIT_INTERNAL_ERROR,
@@ -65,30 +44,23 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
"Unexpected suberror: %u",
run->emulation_failure.suberror);
- if (run->emulation_failure.ndata >= 1) {
- flags = run->emulation_failure.flags;
- if ((flags & KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES) &&
- run->emulation_failure.ndata >= 3) {
- insn_size = run->emulation_failure.insn_size;
- insn_bytes = run->emulation_failure.insn_bytes;
-
- TEST_ASSERT(insn_size <= 15 && insn_size > 0,
- "Unexpected instruction size: %u",
- insn_size);
-
- TEST_ASSERT(is_flds(insn_bytes, insn_size),
- "Unexpected instruction. Expected 'flds' (0xd9 /0)");
-
- /*
- * If is_flds() succeeded then the instruction bytes
- * contained an flds instruction that is 2-bytes in
- * length (ie: no prefix, no SIB, no displacement).
- */
- vcpu_regs_get(vcpu, ®s);
- regs.rip += 2;
- vcpu_regs_set(vcpu, ®s);
- }
- }
+ flags = run->emulation_failure.flags;
+ TEST_ASSERT(run->emulation_failure.ndata >= 3 &&
+ flags & KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES,
+ "run->emulation_failure is missing instruction bytes");
+
+ TEST_ASSERT(run->emulation_failure.insn_size >= 2,
+ "Expected a 2-byte opcode for 'flds', got %d bytes",
+ run->emulation_failure.insn_size);
+
+ insn_bytes = run->emulation_failure.insn_bytes;
+ TEST_ASSERT(insn_bytes[0] == 0xd9 && insn_bytes[1] == 0,
+ "Expected 'flds [eax]', opcode '0xd9 0x00', got opcode 0x%02x 0x%02x\n",
+ insn_bytes[0], insn_bytes[1]);
+
+ vcpu_regs_get(vcpu, ®s);
+ regs.rip += 2;
+ vcpu_regs_set(vcpu, ®s);
}
static void do_guest_assert(struct ucall *uc)
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes
2022-10-31 18:00 ` [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes David Matlack
@ 2022-10-31 18:19 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:19 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> Hard-code the flds instruction and assert the exact instruction bytes
> are present in run->emulation_failure. The test already requires the
> instruction bytes to be present because that's the only way the test
> will advance the RIP past the flds and get to GUEST_DONE().
>
> Note that KVM does not necessarily return exactly 2 bytes in
> run->emulation_failure since it may not know the exact instruction
> length in all cases. So just assert that
> run->emulation_failure.insn_size is at least 2.
Heh, I suspected this was the case.
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 03/10] KVM: selftests: Delete dead ucall code
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
2022-10-31 18:00 ` [PATCH v3 01/10] KVM: selftests: Rename emulator_error_test to smaller_maxphyaddr_emulation_test David Matlack
2022-10-31 18:00 ` [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:19 ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header David Matlack
` (6 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Delete a bunch of code related to ucall handling from
smaller_maxphyaddr_emulation_test. The only thing
smaller_maxphyaddr_emulation_test needs to check is that the vCPU exits
with UCALL_DONE after the second vcpu_run().
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../smaller_maxphyaddr_emulation_test.c | 61 +------------------
1 file changed, 1 insertion(+), 60 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
index d92cd4139f6d..f9fdf365dff7 100644
--- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
@@ -63,64 +63,6 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
vcpu_regs_set(vcpu, ®s);
}
-static void do_guest_assert(struct ucall *uc)
-{
- REPORT_GUEST_ASSERT(*uc);
-}
-
-static void check_for_guest_assert(struct kvm_vcpu *vcpu)
-{
- struct ucall uc;
-
- if (vcpu->run->exit_reason == KVM_EXIT_IO &&
- get_ucall(vcpu, &uc) == UCALL_ABORT) {
- do_guest_assert(&uc);
- }
-}
-
-static void process_ucall_done(struct kvm_vcpu *vcpu)
-{
- struct kvm_run *run = vcpu->run;
- struct ucall uc;
-
- check_for_guest_assert(vcpu);
-
- TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
- "Unexpected exit reason: %u (%s)",
- run->exit_reason,
- exit_reason_str(run->exit_reason));
-
- TEST_ASSERT(get_ucall(vcpu, &uc) == UCALL_DONE,
- "Unexpected ucall command: %lu, expected UCALL_DONE (%d)",
- uc.cmd, UCALL_DONE);
-}
-
-static uint64_t process_ucall(struct kvm_vcpu *vcpu)
-{
- struct kvm_run *run = vcpu->run;
- struct ucall uc;
-
- 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_SYNC:
- break;
- case UCALL_ABORT:
- do_guest_assert(&uc);
- break;
- case UCALL_DONE:
- process_ucall_done(vcpu);
- break;
- default:
- TEST_ASSERT(false, "Unexpected ucall");
- }
-
- return uc.cmd;
-}
-
int main(int argc, char *argv[])
{
struct kvm_vcpu *vcpu;
@@ -157,8 +99,7 @@ int main(int argc, char *argv[])
vcpu_run(vcpu);
process_exit_on_emulation_error(vcpu);
vcpu_run(vcpu);
-
- TEST_ASSERT(process_ucall(vcpu) == UCALL_DONE, "Expected UCALL_DONE");
+ ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
kvm_vm_free(vm);
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 03/10] KVM: selftests: Delete dead ucall code
2022-10-31 18:00 ` [PATCH v3 03/10] KVM: selftests: Delete dead ucall code David Matlack
@ 2022-10-31 18:19 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:19 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> Delete a bunch of code related to ucall handling from
> smaller_maxphyaddr_emulation_test. The only thing
> smaller_maxphyaddr_emulation_test needs to check is that the vCPU exits
> with UCALL_DONE after the second vcpu_run().
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (2 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 03/10] KVM: selftests: Delete dead ucall code David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:28 ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 05/10] KVM: x86/mmu: Use BIT{,_ULL}() for PFERR masks David Matlack
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Move the flds instruction emulation failure handling code to a header
so it can be re-used in an upcoming test.
No functional change intended.
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../selftests/kvm/x86_64/flds_emulation.h | 59 +++++++++++++++++++
.../smaller_maxphyaddr_emulation_test.c | 45 ++------------
2 files changed, 64 insertions(+), 40 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86_64/flds_emulation.h
diff --git a/tools/testing/selftests/kvm/x86_64/flds_emulation.h b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
new file mode 100644
index 000000000000..be0b4e0dd722
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTEST_KVM_FLDS_EMULATION_H
+#define SELFTEST_KVM_FLDS_EMULATION_H
+
+#include "kvm_util.h"
+
+#define FLDS_MEM_EAX ".byte 0xd9, 0x00"
+
+/*
+ * flds is an instruction that the KVM instruction emulator is known not to
+ * support. This can be used in guest code along with a mechanism to force
+ * KVM to emulate the instruction (e.g. by providing an MMIO address) to
+ * exercise emulation failures.
+ */
+static inline void flds(uint64_t address)
+{
+ __asm__ __volatile__(FLDS_MEM_EAX :: "a"(address));
+}
+
+static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
+{
+ struct kvm_run *run = vcpu->run;
+ uint8_t *insn_bytes;
+ uint64_t flags;
+
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_INTERNAL_ERROR,
+ "Unexpected exit reason: %u (%s)",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ TEST_ASSERT(run->emulation_failure.suberror == KVM_INTERNAL_ERROR_EMULATION,
+ "Unexpected suberror: %u",
+ run->emulation_failure.suberror);
+
+ flags = run->emulation_failure.flags;
+ TEST_ASSERT(run->emulation_failure.ndata >= 3 &&
+ flags & KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES,
+ "run->emulation_failure is missing instruction bytes");
+
+ TEST_ASSERT(run->emulation_failure.insn_size >= 2,
+ "Expected a 2-byte opcode for 'flds', got %d bytes",
+ run->emulation_failure.insn_size);
+
+ insn_bytes = run->emulation_failure.insn_bytes;
+ TEST_ASSERT(insn_bytes[0] == 0xd9 && insn_bytes[1] == 0,
+ "Expected 'flds [eax]', opcode '0xd9 0x00', got opcode 0x%02x 0x%02x\n",
+ insn_bytes[0], insn_bytes[1]);
+}
+
+static inline void skip_flds_instruction(struct kvm_vcpu *vcpu)
+{
+ struct kvm_regs regs;
+
+ vcpu_regs_get(vcpu, ®s);
+ regs.rip += 2;
+ vcpu_regs_set(vcpu, ®s);
+}
+
+#endif /* !SELFTEST_KVM_FLDS_EMULATION_H */
diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
index f9fdf365dff7..f438a98e8bb7 100644
--- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
@@ -8,6 +8,8 @@
#define _GNU_SOURCE /* for program_invocation_short_name */
+#include "flds_emulation.h"
+
#include "test_util.h"
#include "kvm_util.h"
#include "vmx.h"
@@ -19,50 +21,12 @@
#define MEM_REGION_SLOT 10
#define MEM_REGION_SIZE PAGE_SIZE
-#define FLDS_MEM_EAX ".byte 0xd9, 0x00"
-
static void guest_code(void)
{
- __asm__ __volatile__(FLDS_MEM_EAX :: "a"(MEM_REGION_GVA));
-
+ flds(MEM_REGION_GVA);
GUEST_DONE();
}
-static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
-{
- struct kvm_run *run = vcpu->run;
- struct kvm_regs regs;
- uint8_t *insn_bytes;
- uint64_t flags;
-
- TEST_ASSERT(run->exit_reason == KVM_EXIT_INTERNAL_ERROR,
- "Unexpected exit reason: %u (%s)",
- run->exit_reason,
- exit_reason_str(run->exit_reason));
-
- TEST_ASSERT(run->emulation_failure.suberror == KVM_INTERNAL_ERROR_EMULATION,
- "Unexpected suberror: %u",
- run->emulation_failure.suberror);
-
- flags = run->emulation_failure.flags;
- TEST_ASSERT(run->emulation_failure.ndata >= 3 &&
- flags & KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES,
- "run->emulation_failure is missing instruction bytes");
-
- TEST_ASSERT(run->emulation_failure.insn_size >= 2,
- "Expected a 2-byte opcode for 'flds', got %d bytes",
- run->emulation_failure.insn_size);
-
- insn_bytes = run->emulation_failure.insn_bytes;
- TEST_ASSERT(insn_bytes[0] == 0xd9 && insn_bytes[1] == 0,
- "Expected 'flds [eax]', opcode '0xd9 0x00', got opcode 0x%02x 0x%02x\n",
- insn_bytes[0], insn_bytes[1]);
-
- vcpu_regs_get(vcpu, ®s);
- regs.rip += 2;
- vcpu_regs_set(vcpu, ®s);
-}
-
int main(int argc, char *argv[])
{
struct kvm_vcpu *vcpu;
@@ -97,7 +61,8 @@ int main(int argc, char *argv[])
vm_set_page_table_entry(vm, vcpu, MEM_REGION_GVA, pte | (1ull << 36));
vcpu_run(vcpu);
- process_exit_on_emulation_error(vcpu);
+ assert_exit_for_flds_emulation_failure(vcpu);
+ skip_flds_instruction(vcpu);
vcpu_run(vcpu);
ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header
2022-10-31 18:00 ` [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header David Matlack
@ 2022-10-31 18:28 ` Sean Christopherson
2022-11-02 18:17 ` David Matlack
0 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:28 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> Move the flds instruction emulation failure handling code to a header
> so it can be re-used in an upcoming test.
>
> No functional change intended.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
> .../selftests/kvm/x86_64/flds_emulation.h | 59 +++++++++++++++++++
> .../smaller_maxphyaddr_emulation_test.c | 45 ++------------
> 2 files changed, 64 insertions(+), 40 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/x86_64/flds_emulation.h
>
> diff --git a/tools/testing/selftests/kvm/x86_64/flds_emulation.h b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
> new file mode 100644
> index 000000000000..be0b4e0dd722
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTEST_KVM_FLDS_EMULATION_H
> +#define SELFTEST_KVM_FLDS_EMULATION_H
> +
> +#include "kvm_util.h"
> +
> +#define FLDS_MEM_EAX ".byte 0xd9, 0x00"
> +
> +/*
> + * flds is an instruction that the KVM instruction emulator is known not to
> + * support. This can be used in guest code along with a mechanism to force
> + * KVM to emulate the instruction (e.g. by providing an MMIO address) to
> + * exercise emulation failures.
> + */
> +static inline void flds(uint64_t address)
> +{
> + __asm__ __volatile__(FLDS_MEM_EAX :: "a"(address));
> +}
> +
> +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
I think it makes sense to keeping the bundling of the assert+skip. As written,
the last test doesn't need to skip, but that may not always hold true, e.g. if
the test adds more stages to verify KVM handles page splits correctly, and even
when a skip is required, it does no harm. I can't think of a scenario where a
test would want an FLDS emulation error but wouldn't want to skip the instruction,
e.g. injecting a fault from userspace is largely an orthogonal test.
Maybe this as a helper name? I don't think it's necessary to include "assert"
anywhere in the name, the idea being that "emulated" provides a hint that it's a
non-trivial helper.
static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)
or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
"flds" is an instruction mnemonic.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header
2022-10-31 18:28 ` Sean Christopherson
@ 2022-11-02 18:17 ` David Matlack
2022-11-02 19:03 ` Sean Christopherson
0 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-11-02 18:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 31, 2022, David Matlack wrote:
> > +
> > +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
>
> I think it makes sense to keeping the bundling of the assert+skip. As written,
> the last test doesn't need to skip, but that may not always hold true, e.g. if
> the test adds more stages to verify KVM handles page splits correctly, and even
> when a skip is required, it does no harm. I can't think of a scenario where a
> test would want an FLDS emulation error but wouldn't want to skip the instruction,
> e.g. injecting a fault from userspace is largely an orthogonal test.
>
> Maybe this as a helper name? I don't think it's necessary to include "assert"
> anywhere in the name, the idea being that "emulated" provides a hint that it's a
> non-trivial helper.
>
> static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)
>
> or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
> "flds" is an instruction mnemonic.
I kept them separate for readability, but otherwise I have no argument
against bundling. I find skip_emulated*() somewhat misleading since
flds is not actually emulated (successfully). I'm trending toward
something like handle_flds_emulation_failure_exit() for v4.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header
2022-11-02 18:17 ` David Matlack
@ 2022-11-02 19:03 ` Sean Christopherson
2022-11-02 22:02 ` David Matlack
0 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2022-11-02 19:03 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Wed, Nov 02, 2022, David Matlack wrote:
> On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Oct 31, 2022, David Matlack wrote:
> > > +
> > > +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
> >
> > I think it makes sense to keeping the bundling of the assert+skip. As written,
> > the last test doesn't need to skip, but that may not always hold true, e.g. if
> > the test adds more stages to verify KVM handles page splits correctly, and even
> > when a skip is required, it does no harm. I can't think of a scenario where a
> > test would want an FLDS emulation error but wouldn't want to skip the instruction,
> > e.g. injecting a fault from userspace is largely an orthogonal test.
> >
> > Maybe this as a helper name? I don't think it's necessary to include "assert"
> > anywhere in the name, the idea being that "emulated" provides a hint that it's a
> > non-trivial helper.
> >
> > static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)
> >
> > or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
> > "flds" is an instruction mnemonic.
>
> I kept them separate for readability,
Ha, and of course I found assert_exit_for_flds_emulation_failure() hard to read :-)
> but otherwise I have no argument against bundling. I find skip_emulated*()
> somewhat misleading since flds is not actually emulated (successfully). I'm
> trending toward something like handle_flds_emulation_failure_exit() for v4.
How about "skip_flds_on_emulation_failure_exit()"? "handle" is quite generic and
doesn't provide any hints as to what the function actually does under the hood.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header
2022-11-02 19:03 ` Sean Christopherson
@ 2022-11-02 22:02 ` David Matlack
0 siblings, 0 replies; 20+ messages in thread
From: David Matlack @ 2022-11-02 22:02 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Wed, Nov 2, 2022 at 12:03 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Nov 02, 2022, David Matlack wrote:
> > On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Mon, Oct 31, 2022, David Matlack wrote:
> > > > +
> > > > +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
> > >
> > > I think it makes sense to keeping the bundling of the assert+skip. As written,
> > > the last test doesn't need to skip, but that may not always hold true, e.g. if
> > > the test adds more stages to verify KVM handles page splits correctly, and even
> > > when a skip is required, it does no harm. I can't think of a scenario where a
> > > test would want an FLDS emulation error but wouldn't want to skip the instruction,
> > > e.g. injecting a fault from userspace is largely an orthogonal test.
> > >
> > > Maybe this as a helper name? I don't think it's necessary to include "assert"
> > > anywhere in the name, the idea being that "emulated" provides a hint that it's a
> > > non-trivial helper.
> > >
> > > static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)
> > >
> > > or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
> > > "flds" is an instruction mnemonic.
> >
> > I kept them separate for readability,
>
> Ha, and of course I found assert_exit_for_flds_emulation_failure() hard to read :-)
>
> > but otherwise I have no argument against bundling. I find skip_emulated*()
> > somewhat misleading since flds is not actually emulated (successfully). I'm
> > trending toward something like handle_flds_emulation_failure_exit() for v4.
>
> How about "skip_flds_on_emulation_failure_exit()"? "handle" is quite generic and
> doesn't provide any hints as to what the function actually does under the hood.
LGTM. Paolo can you apply the name change when queueing v4 (assuming
there are no other comments)? If not I'd be happy to send a v5.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 05/10] KVM: x86/mmu: Use BIT{,_ULL}() for PFERR masks
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (3 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:00 ` [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests David Matlack
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Use the preferred BIT() and BIT_ULL() to construct the PFERR masks
rather than open-coding the bit shifting.
No functional change intended.
Signed-off-by: David Matlack <dmatlack@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 415113dea951..716f165cfa10 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -256,16 +256,16 @@ enum x86_intercept_stage;
#define PFERR_GUEST_PAGE_BIT 33
#define PFERR_IMPLICIT_ACCESS_BIT 48
-#define PFERR_PRESENT_MASK (1U << PFERR_PRESENT_BIT)
-#define PFERR_WRITE_MASK (1U << PFERR_WRITE_BIT)
-#define PFERR_USER_MASK (1U << PFERR_USER_BIT)
-#define PFERR_RSVD_MASK (1U << PFERR_RSVD_BIT)
-#define PFERR_FETCH_MASK (1U << PFERR_FETCH_BIT)
-#define PFERR_PK_MASK (1U << PFERR_PK_BIT)
-#define PFERR_SGX_MASK (1U << PFERR_SGX_BIT)
-#define PFERR_GUEST_FINAL_MASK (1ULL << PFERR_GUEST_FINAL_BIT)
-#define PFERR_GUEST_PAGE_MASK (1ULL << PFERR_GUEST_PAGE_BIT)
-#define PFERR_IMPLICIT_ACCESS (1ULL << PFERR_IMPLICIT_ACCESS_BIT)
+#define PFERR_PRESENT_MASK BIT(PFERR_PRESENT_BIT)
+#define PFERR_WRITE_MASK BIT(PFERR_WRITE_BIT)
+#define PFERR_USER_MASK BIT(PFERR_USER_BIT)
+#define PFERR_RSVD_MASK BIT(PFERR_RSVD_BIT)
+#define PFERR_FETCH_MASK BIT(PFERR_FETCH_BIT)
+#define PFERR_PK_MASK BIT(PFERR_PK_BIT)
+#define PFERR_SGX_MASK BIT(PFERR_SGX_BIT)
+#define PFERR_GUEST_FINAL_MASK BIT_ULL(PFERR_GUEST_FINAL_BIT)
+#define PFERR_GUEST_PAGE_MASK BIT_ULL(PFERR_GUEST_PAGE_BIT)
+#define PFERR_IMPLICIT_ACCESS BIT_ULL(PFERR_IMPLICIT_ACCESS_BIT)
#define PFERR_NESTED_GUEST_PAGE (PFERR_GUEST_PAGE_MASK | \
PFERR_WRITE_MASK | \
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (4 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 05/10] KVM: x86/mmu: Use BIT{,_ULL}() for PFERR masks David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:28 ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 07/10] KVM: selftests: Avoid JMP in non-faulting path of KVM_ASM_SAFE() David Matlack
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Copy KVM's macros for page fault error masks into processor.h so they
can be used in selftests.
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../selftests/kvm/include/x86_64/processor.h | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e8ca0d8a6a7e..f7249cb27e0d 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -882,4 +882,27 @@ void virt_map_level(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
#define XSTATE_XTILE_DATA_MASK (1ULL << XSTATE_XTILE_DATA_BIT)
#define XFEATURE_XTILE_MASK (XSTATE_XTILE_CFG_MASK | \
XSTATE_XTILE_DATA_MASK)
+
+#define PFERR_PRESENT_BIT 0
+#define PFERR_WRITE_BIT 1
+#define PFERR_USER_BIT 2
+#define PFERR_RSVD_BIT 3
+#define PFERR_FETCH_BIT 4
+#define PFERR_PK_BIT 5
+#define PFERR_SGX_BIT 15
+#define PFERR_GUEST_FINAL_BIT 32
+#define PFERR_GUEST_PAGE_BIT 33
+#define PFERR_IMPLICIT_ACCESS_BIT 48
+
+#define PFERR_PRESENT_MASK BIT(PFERR_PRESENT_BIT)
+#define PFERR_WRITE_MASK BIT(PFERR_WRITE_BIT)
+#define PFERR_USER_MASK BIT(PFERR_USER_BIT)
+#define PFERR_RSVD_MASK BIT(PFERR_RSVD_BIT)
+#define PFERR_FETCH_MASK BIT(PFERR_FETCH_BIT)
+#define PFERR_PK_MASK BIT(PFERR_PK_BIT)
+#define PFERR_SGX_MASK BIT(PFERR_SGX_BIT)
+#define PFERR_GUEST_FINAL_MASK BIT_ULL(PFERR_GUEST_FINAL_BIT)
+#define PFERR_GUEST_PAGE_MASK BIT_ULL(PFERR_GUEST_PAGE_BIT)
+#define PFERR_IMPLICIT_ACCESS BIT_ULL(PFERR_IMPLICIT_ACCESS_BIT)
+
#endif /* SELFTEST_KVM_PROCESSOR_H */
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests
2022-10-31 18:00 ` [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests David Matlack
@ 2022-10-31 18:28 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:28 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> Copy KVM's macros for page fault error masks into processor.h so they
> can be used in selftests.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 07/10] KVM: selftests: Avoid JMP in non-faulting path of KVM_ASM_SAFE()
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (5 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:00 ` [PATCH v3 08/10] KVM: selftests: Provide error code as a KVM_ASM_SAFE() output David Matlack
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
From: Sean Christopherson <seanjc@google.com>
Clear R9 in the non-faulting path of KVM_ASM_SAFE() and fall through to
to a common load of "vector" to effectively load "vector" with '0' to
reduce the code footprint of the asm blob, to reduce the runtime overhead
of the non-faulting path (when "vector" is stored in a register), and so
that additional output constraints that are valid if and only if a fault
occur are loaded even in the non-faulting case.
A future patch will add a 64-bit output for the error code, and if its
output is not explicitly loaded with _something_, the user of the asm
blob can end up technically consuming uninitialized data. Using a
common path to load the output constraints will allow using an existing
scratch register, e.g. r10, to hold the error code in the faulting path,
while also guaranteeing the error code is initialized with deterministic
data in the non-faulting patch (r10 is loaded with the RIP of
to-be-executed instruction).
Consuming the error code when a fault doesn't occur would obviously be a
test bug, but there's no guarantee the compiler will detect uninitialized
consumption. And conversely, it's theoretically possible that the
compiler might throw a false positive on uninitialized data, e.g. if the
compiler can't determine that the non-faulting path won't touch the error
code.
Alternatively, the error code could be explicitly loaded in the
non-faulting path, but loading a 64-bit memory|register output operand
with an explicitl value requires a sign-extended "MOV imm32, r/m64",
which isn't exactly straightforward and has a largish code footprint.
And loading the error code with what is effectively garbage (from a
scratch register) avoids having to choose an arbitrary value for the
non-faulting case.
Opportunistically remove a rogue asterisk in the block comment.
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
tools/testing/selftests/kvm/include/x86_64/processor.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index f7249cb27e0d..9efe80d52389 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -764,7 +764,7 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
* for recursive faults when accessing memory in the handler. The downside to
* using registers is that it restricts what registers can be used by the actual
* instruction. But, selftests are 64-bit only, making register* pressure a
- * minor concern. Use r9-r11 as they are volatile, i.e. don't need* to be saved
+ * minor concern. Use r9-r11 as they are volatile, i.e. don't need to be saved
* by the callee, and except for r11 are not implicit parameters to any
* instructions. Ideally, fixup would use r8-r10 and thus avoid implicit
* parameters entirely, but Hyper-V's hypercall ABI uses r8 and testing Hyper-V
@@ -786,11 +786,9 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
"lea 1f(%%rip), %%r10\n\t" \
"lea 2f(%%rip), %%r11\n\t" \
"1: " insn "\n\t" \
- "movb $0, %[vector]\n\t" \
- "jmp 3f\n\t" \
+ "xor %%r9, %%r9\n\t" \
"2:\n\t" \
- "mov %%r9b, %[vector]\n\t" \
- "3:\n\t"
+ "mov %%r9b, %[vector]\n\t"
#define KVM_ASM_SAFE_OUTPUTS(v) [vector] "=qm"(v)
#define KVM_ASM_SAFE_CLOBBERS "r9", "r10", "r11"
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v3 08/10] KVM: selftests: Provide error code as a KVM_ASM_SAFE() output
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (6 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 07/10] KVM: selftests: Avoid JMP in non-faulting path of KVM_ASM_SAFE() David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:00 ` [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled David Matlack
2022-10-31 18:00 ` [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE David Matlack
9 siblings, 0 replies; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
From: Sean Christopherson <seanjc@google.com>
Provide the error code on a fault in KVM_ASM_SAFE(), e.g. to allow tests
to assert that #PF generates the correct error code without needing to
manually install a #PF handler. Use r10 as the scratch register for the
error code, as it's already clobbered by the asm blob (loaded with the
RIP of the to-be-executed instruction). Deliberately load the output
"error_code" even in the non-faulting path so that error_code is always
initialized with deterministic data (the aforementioned RIP), i.e to
ensure a selftest won't end up with uninitialized consumption regardless
of how KVM_ASM_SAFE() is used.
Don't clear r10 in the non-faulting case and instead load error code with
the RIP (see above). The error code is valid if and only if an exception
occurs, and '0' isn't necessarily a better "invalid" value, e.g. '0'
could result in false passes for a buggy test.
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../selftests/kvm/include/x86_64/processor.h | 39 +++++++++++++------
.../selftests/kvm/lib/x86_64/processor.c | 1 +
.../selftests/kvm/x86_64/hyperv_features.c | 3 +-
3 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 9efe80d52389..33b0f19e502c 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -780,6 +780,7 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
*
* REGISTER OUTPUTS:
* r9 = exception vector (non-zero)
+ * r10 = error code
*/
#define KVM_ASM_SAFE(insn) \
"mov $" __stringify(KVM_EXCEPTION_MAGIC) ", %%r9\n\t" \
@@ -788,29 +789,43 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
"1: " insn "\n\t" \
"xor %%r9, %%r9\n\t" \
"2:\n\t" \
- "mov %%r9b, %[vector]\n\t"
+ "mov %%r9b, %[vector]\n\t" \
+ "mov %%r10, %[error_code]\n\t"
-#define KVM_ASM_SAFE_OUTPUTS(v) [vector] "=qm"(v)
+#define KVM_ASM_SAFE_OUTPUTS(v, ec) [vector] "=qm"(v), [error_code] "=rm"(ec)
#define KVM_ASM_SAFE_CLOBBERS "r9", "r10", "r11"
-#define kvm_asm_safe(insn, inputs...) \
-({ \
- uint8_t vector; \
- \
- asm volatile(KVM_ASM_SAFE(insn) \
- : KVM_ASM_SAFE_OUTPUTS(vector) \
- : inputs \
- : KVM_ASM_SAFE_CLOBBERS); \
- vector; \
+#define kvm_asm_safe(insn, inputs...) \
+({ \
+ uint64_t ign_error_code; \
+ uint8_t vector; \
+ \
+ asm volatile(KVM_ASM_SAFE(insn) \
+ : KVM_ASM_SAFE_OUTPUTS(vector, ign_error_code) \
+ : inputs \
+ : KVM_ASM_SAFE_CLOBBERS); \
+ vector; \
+})
+
+#define kvm_asm_safe_ec(insn, error_code, inputs...) \
+({ \
+ uint8_t vector; \
+ \
+ asm volatile(KVM_ASM_SAFE(insn) \
+ : KVM_ASM_SAFE_OUTPUTS(vector, error_code) \
+ : inputs \
+ : KVM_ASM_SAFE_CLOBBERS); \
+ vector; \
})
static inline uint8_t rdmsr_safe(uint32_t msr, uint64_t *val)
{
+ uint64_t error_code;
uint8_t vector;
uint32_t a, d;
asm volatile(KVM_ASM_SAFE("rdmsr")
- : "=a"(a), "=d"(d), KVM_ASM_SAFE_OUTPUTS(vector)
+ : "=a"(a), "=d"(d), KVM_ASM_SAFE_OUTPUTS(vector, error_code)
: "c"(msr)
: KVM_ASM_SAFE_CLOBBERS);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 39c4409ef56a..fc6c724e0d24 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1116,6 +1116,7 @@ static bool kvm_fixup_exception(struct ex_regs *regs)
regs->rip = regs->r11;
regs->r9 = regs->vector;
+ regs->r10 = regs->error_code;
return true;
}
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index 05b32e550a80..2b6d455acf8a 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -18,6 +18,7 @@
static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
vm_vaddr_t output_address, uint64_t *hv_status)
{
+ uint64_t error_code;
uint8_t vector;
/* Note both the hypercall and the "asm safe" clobber r9-r11. */
@@ -25,7 +26,7 @@ static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
KVM_ASM_SAFE("vmcall")
: "=a" (*hv_status),
"+c" (control), "+d" (input_address),
- KVM_ASM_SAFE_OUTPUTS(vector)
+ KVM_ASM_SAFE_OUTPUTS(vector, error_code)
: [output_address] "r"(output_address),
"a" (-EFAULT)
: "cc", "memory", "r8", KVM_ASM_SAFE_CLOBBERS);
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (7 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 08/10] KVM: selftests: Provide error code as a KVM_ASM_SAFE() output David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:07 ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE David Matlack
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Change smaller_maxphyaddr_emulation_test to expect a #PF(RSVD), rather
than an emulation failure, when TDP is disabled. KVM only needs to
emulate instructions to emulate a smaller guest.MAXPHYADDR when TDP is
enabled.
Fixes: 39bbcc3a4e39 ("selftests: kvm: Allows userspace to handle emulation errors.")
Signed-off-by: David Matlack <dmatlack@google.com>
---
.../smaller_maxphyaddr_emulation_test.c | 47 ++++++++++++++++---
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
index f438a98e8bb7..5934ddc94aa0 100644
--- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
@@ -21,9 +21,22 @@
#define MEM_REGION_SLOT 10
#define MEM_REGION_SIZE PAGE_SIZE
-static void guest_code(void)
+static void guest_code(bool tdp_enabled)
{
- flds(MEM_REGION_GVA);
+ uint64_t error_code;
+ uint64_t vector;
+
+ vector = kvm_asm_safe_ec(FLDS_MEM_EAX, error_code, "a"(MEM_REGION_GVA));
+
+ /*
+ * When TDP is disabled, no instruction emulation is required so flds
+ * should generate #PF(RSVD).
+ */
+ if (!tdp_enabled) {
+ GUEST_ASSERT_EQ(vector, PF_VECTOR);
+ GUEST_ASSERT(error_code & PFERR_RSVD_MASK);
+ }
+
GUEST_DONE();
}
@@ -32,6 +45,7 @@ int main(int argc, char *argv[])
struct kvm_vcpu *vcpu;
struct kvm_vm *vm;
uint64_t gpa, pte;
+ struct ucall uc;
uint64_t *hva;
int rc;
@@ -41,6 +55,10 @@ int main(int argc, char *argv[])
TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR));
vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ vcpu_args_set(vcpu, 1, kvm_is_tdp_enabled());
+
+ vm_init_descriptor_tables(vm);
+ vcpu_init_descriptor_tables(vcpu);
vcpu_set_cpuid_maxphyaddr(vcpu, MAXPHYADDR);
@@ -61,10 +79,27 @@ int main(int argc, char *argv[])
vm_set_page_table_entry(vm, vcpu, MEM_REGION_GVA, pte | (1ull << 36));
vcpu_run(vcpu);
- assert_exit_for_flds_emulation_failure(vcpu);
- skip_flds_instruction(vcpu);
- vcpu_run(vcpu);
- ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
+
+ /*
+ * When TDP is enabled, KVM must emulate the flds instruction, which
+ * results in an emulation failure out to userspace since KVM does not
+ * know how to emulate flds.
+ */
+ if (kvm_is_tdp_enabled()) {
+ assert_exit_for_flds_emulation_failure(vcpu);
+ skip_flds_instruction(vcpu);
+ vcpu_run(vcpu);
+ }
+
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ break;
+ case UCALL_DONE:
+ break;
+ default:
+ TEST_FAIL("Unrecognized ucall: %lu\n", uc.cmd);
+ }
kvm_vm_free(vm);
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled
2022-10-31 18:00 ` [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled David Matlack
@ 2022-10-31 18:07 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:07 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> -static void guest_code(void)
> +static void guest_code(bool tdp_enabled)
> {
> - flds(MEM_REGION_GVA);
> + uint64_t error_code;
> + uint64_t vector;
> +
> + vector = kvm_asm_safe_ec(FLDS_MEM_EAX, error_code, "a"(MEM_REGION_GVA));
> +
> + /*
> + * When TDP is disabled, no instruction emulation is required so flds
> + * should generate #PF(RSVD).
> + */
> + if (!tdp_enabled) {
> + GUEST_ASSERT_EQ(vector, PF_VECTOR);
> + GUEST_ASSERT(error_code & PFERR_RSVD_MASK);
> + }
Probably worth adding
} else {
GUEST_ASSERT(!vector);
}
to verify no fault occurs in the emulation case? Or to avoid the inverted check,
if (tdp_enabled) {
GUEST_ASSERT(!vector);
} else {
GUEST_ASSERT_EQ(vector, PF_VECTOR);
GUEST_ASSERT(error_code & PFERR_RSVD_MASK);
}
It's mostly redundant with assert_exit_for_flds_emulation_failure(), but at worst
it'll help docuemnts the expected behavior in the TDP case.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
` (8 preceding siblings ...)
2022-10-31 18:00 ` [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled David Matlack
@ 2022-10-31 18:00 ` David Matlack
2022-10-31 18:37 ` Sean Christopherson
9 siblings, 1 reply; 20+ messages in thread
From: David Matlack @ 2022-10-31 18:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, David Matlack, Peter Xu, Jim Mattson,
Yang Zhong, Wei Wang, Colton Lewis, Vitaly Kuznetsov,
Vipin Sharma, Aaron Lewis, kvm
Add a selftest to exercise the KVM_CAP_EXIT_ON_EMULATION_FAILURE
capability.
This capability is also exercised through
smaller_maxphyaddr_emulation_test, but that test requires
allow_smaller_maxphyaddr=Y, which is off by default on Intel when ept=Y
and unconditionally disabled on AMD when npt=Y. This new test ensures we
exercise KVM_CAP_EXIT_ON_EMULATION_FAILURE independent of
allow_smaller_maxphyaddr.
Signed-off-by: David Matlack <dmatlack@google.com>
---
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../x86_64/exit_on_emulation_failure_test.c | 42 +++++++++++++++++++
3 files changed, 44 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/exit_on_emulation_failure_test.c
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 053e5d34cd03..bef984e4c39d 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -17,6 +17,7 @@
/x86_64/cr4_cpuid_sync_test
/x86_64/debug_regs
/x86_64/evmcs_test
+/x86_64/exit_on_emulation_failure_test
/x86_64/fix_hypercall_test
/x86_64/get_msr_index_features
/x86_64/kvm_clock_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index ab133b731a2d..11a6104e6547 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -81,6 +81,7 @@ TEST_GEN_PROGS_x86_64 = x86_64/cpuid_test
TEST_GEN_PROGS_x86_64 += x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
+TEST_GEN_PROGS_x86_64 += x86_64/exit_on_emulation_failure_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
diff --git a/tools/testing/selftests/kvm/x86_64/exit_on_emulation_failure_test.c b/tools/testing/selftests/kvm/x86_64/exit_on_emulation_failure_test.c
new file mode 100644
index 000000000000..8e98ad3259de
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/exit_on_emulation_failure_test.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022, Google LLC.
+ *
+ * Test for KVM_CAP_EXIT_ON_EMULATION_FAILURE.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_short_name */
+
+#include "flds_emulation.h"
+
+#include "test_util.h"
+
+#define MMIO_GPA 0x700000000
+#define MMIO_GVA MMIO_GPA
+
+static void guest_code(void)
+{
+ /* Execute flds with an MMIO address to force KVM to emulate it. */
+ flds(MMIO_GVA);
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+
+ /* Tell stdout not to buffer its content */
+ setbuf(stdout, NULL);
+
+ TEST_REQUIRE(kvm_has_cap(KVM_CAP_EXIT_ON_EMULATION_FAILURE));
+
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ vm_enable_cap(vm, KVM_CAP_EXIT_ON_EMULATION_FAILURE, 1);
+ virt_map(vm, MMIO_GVA, MMIO_GPA, 1);
+
+ vcpu_run(vcpu);
+ assert_exit_for_flds_emulation_failure(vcpu);
+
+ kvm_vm_free(vm);
+ return 0;
+}
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE
2022-10-31 18:00 ` [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE David Matlack
@ 2022-10-31 18:37 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-10-31 18:37 UTC (permalink / raw)
To: David Matlack
Cc: Paolo Bonzini, Peter Xu, Jim Mattson, Yang Zhong, Wei Wang,
Colton Lewis, Vitaly Kuznetsov, Vipin Sharma, Aaron Lewis, kvm
On Mon, Oct 31, 2022, David Matlack wrote:
> Add a selftest to exercise the KVM_CAP_EXIT_ON_EMULATION_FAILURE
> capability.
>
> This capability is also exercised through
> smaller_maxphyaddr_emulation_test, but that test requires
> allow_smaller_maxphyaddr=Y, which is off by default on Intel when ept=Y
> and unconditionally disabled on AMD when npt=Y. This new test ensures we
Uber nit, avoid pronouns, purely so that "no pronouns" can be an unconditional
guideline, not because "we" is at all ambiguous in this case.
This new test ensures KVM_CAP_EXIT_ON_EMULATION_FAILURE is exercised
independent of allow_smaller_maxphyaddr.
> exercise KVM_CAP_EXIT_ON_EMULATION_FAILURE independent of
> allow_smaller_maxphyaddr.
>
> +static void guest_code(void)
> +{
> + /* Execute flds with an MMIO address to force KVM to emulate it. */
> + flds(MMIO_GVA);
Add tests to verify KVM handles cases where the memory operand splits pages on
both sides? Mostly because I'm curious if KVM actually does the right thing :-)
It'll require creating an extra memslot, but I don't think that should be too
difficult?
^ permalink raw reply [flat|nested] 20+ messages in thread