| Message ID | 20231212204647.2170650-17-sagis@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | TDX KVM selftests | expand |
On 12/13/2023 4:46 AM, Sagi Shahar wrote: > The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the > guest vCPU enters to the halted state. > > Signed-off-by: Erdem Aktas <erdemaktas@google.com> > Signed-off-by: Sagi Shahar <sagis@google.com> > Signed-off-by: Ackerley Tng <ackerleytng@google.com> > Signed-off-by: Ryan Afranji <afranji@google.com> > --- > .../selftests/kvm/include/x86_64/tdx/tdx.h | 2 + > .../selftests/kvm/lib/x86_64/tdx/tdx.c | 10 +++ > .../selftests/kvm/x86_64/tdx_vm_tests.c | 78 +++++++++++++++++++ > 3 files changed, 90 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > index 85ba6aab79a7..b18e39d20498 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > @@ -8,6 +8,7 @@ > #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000 > #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003 > > +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12 > #define TDG_VP_VMCALL_INSTRUCTION_IO 30 > #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31 > #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32 > @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12, > uint64_t *r13, uint64_t *r14); > uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value); > uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value); > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag); > > #endif // SELFTEST_TDX_TDX_H > diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > index 88ea6f2a6469..9485bafedc38 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value) > > return __tdx_hypercall(&args, 0); > } > + > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag) > +{ > + struct tdx_hypercall_args args = { > + .r11 = TDG_VP_VMCALL_INSTRUCTION_HLT, > + .r12 = interrupt_blocked_flag, > + }; > + > + return __tdx_hypercall(&args, 0); > +} > diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > index 5db3701cc6d9..5fae4c6e5f95 100644 > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void) > printf("\t ... PASSED\n"); > } > > +/* > + * Verifies HLT functionality. > + */ > +void guest_hlt(void) > +{ > + uint64_t ret; > + uint64_t interrupt_blocked_flag; > + > + interrupt_blocked_flag = 0; > + ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag); > + if (ret) > + tdx_test_fatal(ret); > + > + tdx_test_success(); > +} > + > +void _verify_guest_hlt(int signum); > + > +void wake_me(int interval) > +{ > + struct sigaction action; > + > + action.sa_handler = _verify_guest_hlt; > + sigemptyset(&action.sa_mask); > + action.sa_flags = 0; > + > + TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0, > + "Could not set the alarm handler!"); > + > + alarm(interval); > +} > + > +void _verify_guest_hlt(int signum) > +{ > + struct kvm_vm *vm; > + static struct kvm_vcpu *vcpu; > + > + /* > + * This function will also be called by SIGALRM handler to check the > + * vCPU MP State. If vm has been initialized, then we are in the signal > + * handler. Check the MP state and let the guest run again. > + */ > + if (vcpu != NULL) { What if the following case if there is a bug in KVM so that: In guest, execution of tdg_vp_vmcall_instruction_hlt() return 0, but the vcpu is not actually halted. Then guest will call tdx_test_success(). And the first call of _verify_guest_hlt() will call kvm_vm_free(vm) to free the vm, which also frees the vcpu, and 1 second later, in this path vcpu will be accessed after free. > + struct kvm_mp_state mp_state; > + > + vcpu_mp_state_get(vcpu, &mp_state); > + TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED); > + > + /* Let the guest to run and finish the test.*/ > + mp_state.mp_state = KVM_MP_STATE_RUNNABLE; > + vcpu_mp_state_set(vcpu, &mp_state); > + return; > + } > + > + vm = td_create(); > + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0); > + vcpu = td_vcpu_add(vm, 0, guest_hlt); > + td_finalize(vm); > + > + printf("Verifying HLT:\n"); > + > + printf("\t ... Running guest\n"); > + > + /* Wait 1 second for guest to execute HLT */ > + wake_me(1); > + td_vcpu_run(vcpu); > + > + TDX_TEST_ASSERT_SUCCESS(vcpu); > + > + kvm_vm_free(vm); > + printf("\t ... PASSED\n"); > +} > + > +void verify_guest_hlt(void) > +{ > + _verify_guest_hlt(0); > +} > > int main(int argc, char **argv) > { > @@ -740,6 +817,7 @@ int main(int argc, char **argv) > run_in_new_process(&verify_guest_reads); > run_in_new_process(&verify_guest_msr_writes); > run_in_new_process(&verify_guest_msr_reads); > + run_in_new_process(&verify_guest_hlt); > > return 0; > }
On Sat, Mar 02, 2024 at 03:31:07PM +0800, Binbin Wu wrote: > On 12/13/2023 4:46 AM, Sagi Shahar wrote: > > The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the > > guest vCPU enters to the halted state. > > > > Signed-off-by: Erdem Aktas <erdemaktas@google.com> > > Signed-off-by: Sagi Shahar <sagis@google.com> > > Signed-off-by: Ackerley Tng <ackerleytng@google.com> > > Signed-off-by: Ryan Afranji <afranji@google.com> > > --- > > .../selftests/kvm/include/x86_64/tdx/tdx.h | 2 + > > .../selftests/kvm/lib/x86_64/tdx/tdx.c | 10 +++ > > .../selftests/kvm/x86_64/tdx_vm_tests.c | 78 +++++++++++++++++++ > > 3 files changed, 90 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > > index 85ba6aab79a7..b18e39d20498 100644 > > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h > > @@ -8,6 +8,7 @@ > > #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000 > > #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003 > > +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12 > > #define TDG_VP_VMCALL_INSTRUCTION_IO 30 > > #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31 > > #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32 > > @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12, > > uint64_t *r13, uint64_t *r14); > > uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value); > > uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value); > > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag); > > #endif // SELFTEST_TDX_TDX_H > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > > index 88ea6f2a6469..9485bafedc38 100644 > > --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > > +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c > > @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value) > > return __tdx_hypercall(&args, 0); > > } > > + > > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag) > > +{ > > + struct tdx_hypercall_args args = { > > + .r11 = TDG_VP_VMCALL_INSTRUCTION_HLT, > > + .r12 = interrupt_blocked_flag, > > + }; > > + > > + return __tdx_hypercall(&args, 0); > > +} > > diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > index 5db3701cc6d9..5fae4c6e5f95 100644 > > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void) > > printf("\t ... PASSED\n"); > > } > > +/* > > + * Verifies HLT functionality. > > + */ > > +void guest_hlt(void) > > +{ > > + uint64_t ret; > > + uint64_t interrupt_blocked_flag; > > + > > + interrupt_blocked_flag = 0; > > + ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag); > > + if (ret) > > + tdx_test_fatal(ret); > > + > > + tdx_test_success(); > > +} > > + > > +void _verify_guest_hlt(int signum); > > + > > +void wake_me(int interval) > > +{ > > + struct sigaction action; > > + > > + action.sa_handler = _verify_guest_hlt; > > + sigemptyset(&action.sa_mask); > > + action.sa_flags = 0; > > + > > + TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0, > > + "Could not set the alarm handler!"); > > + > > + alarm(interval); > > +} > > + > > +void _verify_guest_hlt(int signum) > > +{ > > + struct kvm_vm *vm; > > + static struct kvm_vcpu *vcpu; > > + > > + /* > > + * This function will also be called by SIGALRM handler to check the > > + * vCPU MP State. If vm has been initialized, then we are in the signal > > + * handler. Check the MP state and let the guest run again. > > + */ > > + if (vcpu != NULL) { > > What if the following case if there is a bug in KVM so that: > > In guest, execution of tdg_vp_vmcall_instruction_hlt() return 0, but the > vcpu is not actually halted. Then guest will call tdx_test_success(). > > And the first call of _verify_guest_hlt() will call kvm_vm_free(vm) to free > the vm, which also frees the vcpu, and 1 second later, in this path vcpu > will > be accessed after free. > Right. Another possibility is that if buggy KVM returns success to guest without putting guest to halted state, the selftest will still print "PASSED" because the second _verify_guest_hlt() (after waiting for 1s) has no chance to get executed before the process exits. > > + struct kvm_mp_state mp_state; > > + > > + vcpu_mp_state_get(vcpu, &mp_state); > > + TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED); > > + > > + /* Let the guest to run and finish the test.*/ > > + mp_state.mp_state = KVM_MP_STATE_RUNNABLE; > > + vcpu_mp_state_set(vcpu, &mp_state); > > + return; > > + } > > + > > + vm = td_create(); > > + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0); > > + vcpu = td_vcpu_add(vm, 0, guest_hlt); > > + td_finalize(vm); > > + > > + printf("Verifying HLT:\n"); > > + > > + printf("\t ... Running guest\n"); > > + > > + /* Wait 1 second for guest to execute HLT */ > > + wake_me(1); > > + td_vcpu_run(vcpu); > > + > > + TDX_TEST_ASSERT_SUCCESS(vcpu); > > + > > + kvm_vm_free(vm); > > + printf("\t ... PASSED\n"); > > +} > > + > > +void verify_guest_hlt(void) > > +{ > > + _verify_guest_hlt(0); > > +} > > int main(int argc, char **argv) > > { > > @@ -740,6 +817,7 @@ int main(int argc, char **argv) > > run_in_new_process(&verify_guest_reads); > > run_in_new_process(&verify_guest_msr_writes); > > run_in_new_process(&verify_guest_msr_reads); > > + run_in_new_process(&verify_guest_hlt); > > return 0; > > } > >
diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h index 85ba6aab79a7..b18e39d20498 100644 --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h @@ -8,6 +8,7 @@ #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000 #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003 +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12 #define TDG_VP_VMCALL_INSTRUCTION_IO 30 #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31 #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32 @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12, uint64_t *r13, uint64_t *r14); uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value); uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value); +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag); #endif // SELFTEST_TDX_TDX_H diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c index 88ea6f2a6469..9485bafedc38 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value) return __tdx_hypercall(&args, 0); } + +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag) +{ + struct tdx_hypercall_args args = { + .r11 = TDG_VP_VMCALL_INSTRUCTION_HLT, + .r12 = interrupt_blocked_flag, + }; + + return __tdx_hypercall(&args, 0); +} diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c index 5db3701cc6d9..5fae4c6e5f95 100644 --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void) printf("\t ... PASSED\n"); } +/* + * Verifies HLT functionality. + */ +void guest_hlt(void) +{ + uint64_t ret; + uint64_t interrupt_blocked_flag; + + interrupt_blocked_flag = 0; + ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag); + if (ret) + tdx_test_fatal(ret); + + tdx_test_success(); +} + +void _verify_guest_hlt(int signum); + +void wake_me(int interval) +{ + struct sigaction action; + + action.sa_handler = _verify_guest_hlt; + sigemptyset(&action.sa_mask); + action.sa_flags = 0; + + TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0, + "Could not set the alarm handler!"); + + alarm(interval); +} + +void _verify_guest_hlt(int signum) +{ + struct kvm_vm *vm; + static struct kvm_vcpu *vcpu; + + /* + * This function will also be called by SIGALRM handler to check the + * vCPU MP State. If vm has been initialized, then we are in the signal + * handler. Check the MP state and let the guest run again. + */ + if (vcpu != NULL) { + struct kvm_mp_state mp_state; + + vcpu_mp_state_get(vcpu, &mp_state); + TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED); + + /* Let the guest to run and finish the test.*/ + mp_state.mp_state = KVM_MP_STATE_RUNNABLE; + vcpu_mp_state_set(vcpu, &mp_state); + return; + } + + vm = td_create(); + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0); + vcpu = td_vcpu_add(vm, 0, guest_hlt); + td_finalize(vm); + + printf("Verifying HLT:\n"); + + printf("\t ... Running guest\n"); + + /* Wait 1 second for guest to execute HLT */ + wake_me(1); + td_vcpu_run(vcpu); + + TDX_TEST_ASSERT_SUCCESS(vcpu); + + kvm_vm_free(vm); + printf("\t ... PASSED\n"); +} + +void verify_guest_hlt(void) +{ + _verify_guest_hlt(0); +} int main(int argc, char **argv) { @@ -740,6 +817,7 @@ int main(int argc, char **argv) run_in_new_process(&verify_guest_reads); run_in_new_process(&verify_guest_msr_writes); run_in_new_process(&verify_guest_msr_reads); + run_in_new_process(&verify_guest_hlt); return 0; }