| Message ID | 20190531141452.158909-1-aaronlewis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests: kvm: Check for a kernel warning | expand |
On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: > > When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, > test that a kernel warning does not occur informing us that > vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a > triple fault. > This test was made to detect a bug that was reported by Syzkaller > (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and > fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation > request to userspace"). > > Signed-off-by: Aaron Lewis <aaronlewis@google.com> > Reviewed-by: Jim Mattson <jmattson@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > --- > tools/testing/selftests/kvm/.gitignore | 1 + > tools/testing/selftests/kvm/Makefile | 1 + > .../testing/selftests/kvm/include/kvm_util.h | 2 + > .../selftests/kvm/include/x86_64/processor.h | 2 + > tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ > .../selftests/kvm/lib/x86_64/processor.c | 16 +++ > .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ > 7 files changed, 184 insertions(+) > create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > index df1bf9230a74..41266af0d3dc 100644 > --- a/tools/testing/selftests/kvm/.gitignore > +++ b/tools/testing/selftests/kvm/.gitignore > @@ -2,6 +2,7 @@ > /x86_64/evmcs_test > /x86_64/hyperv_cpuid > /x86_64/kvm_create_max_vcpus > +/x86_64/mmio_warning_test > /x86_64/platform_info_test > /x86_64/set_sregs_test > /x86_64/smm_test > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 79c524395ebe..670b938f1049 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test > TEST_GEN_PROGS_x86_64 += x86_64/smm_test > TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus > TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test > +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test > TEST_GEN_PROGS_x86_64 += dirty_log_test > TEST_GEN_PROGS_x86_64 += clear_dirty_log_test > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > index 8c6b9619797d..c5c427c86598 100644 > --- a/tools/testing/selftests/kvm/include/kvm_util.h > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, > void *guest_code); > void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); > > +bool vm_is_unrestricted_guest(struct kvm_vm *vm); > + > struct kvm_userspace_memory_region * > kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, > uint64_t end); > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > index 6063d5b2f356..af4d26de32d1 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) > return 0; > } > > +bool is_intel_cpu(void); > + > struct kvm_x86_state; > struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); > void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index e9113857f44e..b93b09ad9a11 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) > { > return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); > } > + > +/* > + * Is Unrestricted Guest > + * > + * Input Args: > + * vm - Virtual Machine > + * > + * Output Args: None > + * > + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. > + * > + * Check if the unrestricted guest flag is enabled. > + */ > +bool vm_is_unrestricted_guest(struct kvm_vm *vm) > +{ > + char val = 'N'; > + size_t count; > + FILE *f; > + > + if (vm == NULL) { > + /* Ensure that the KVM vendor-specific module is loaded. */ > + f = fopen(KVM_DEV_PATH, "r"); > + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", > + errno); > + fclose(f); > + } > + > + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); > + if (f) { > + count = fread(&val, sizeof(char), 1, f); > + TEST_ASSERT(count == 1, "Unable to read from param file."); > + fclose(f); > + } > + > + return val == 'Y'; > +} > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > index dc7fae9fa424..bcc0e70e1856 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s > r); > } > } > + > +bool is_intel_cpu(void) > +{ > + int eax, ebx, ecx, edx; > + const uint32_t *chunk; > + const int leaf = 0; > + > + __asm__ __volatile__( > + "cpuid" > + : /* output */ "=a"(eax), "=b"(ebx), > + "=c"(ecx), "=d"(edx) > + : /* input */ "0"(leaf), "2"(0)); > + > + chunk = (const uint32_t *)("GenuineIntel"); > + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); > +} > diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > new file mode 100644 > index 000000000000..00bb97d76000 > --- /dev/null > +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > @@ -0,0 +1,126 @@ > +/* > + * mmio_warning_test > + * > + * Copyright (C) 2019, Google LLC. > + * > + * This work is licensed under the terms of the GNU GPL, version 2. > + * > + * Test that we don't get a kernel warning when we call KVM_RUN after a > + * triple fault occurs. To get the triple fault to occur we call KVM_RUN > + * on a VCPU that hasn't been properly setup. > + * > + */ > + > +#define _GNU_SOURCE > +#include <fcntl.h> > +#include <kvm_util.h> > +#include <linux/kvm.h> > +#include <processor.h> > +#include <pthread.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/ioctl.h> > +#include <sys/mman.h> > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <sys/wait.h> > +#include <test_util.h> > +#include <unistd.h> > + > +#define NTHREAD 4 > +#define NPROCESS 5 > + > +struct thread_context { > + int kvmcpu; > + struct kvm_run *run; > +}; > + > +void *thr(void *arg) > +{ > + struct thread_context *tc = (struct thread_context *)arg; > + int res; > + int kvmcpu = tc->kvmcpu; > + struct kvm_run *run = tc->run; > + > + res = ioctl(kvmcpu, KVM_RUN, 0); > + printf("ret1=%d exit_reason=%d suberror=%d\n", > + res, run->exit_reason, run->internal.suberror); > + > + return 0; > +} > + > +void test(void) > +{ > + int i, kvm, kvmvm, kvmcpu; > + pthread_t th[NTHREAD]; > + struct kvm_run *run; > + struct thread_context tc; > + > + kvm = open("/dev/kvm", O_RDWR); > + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); > + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); > + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); > + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); > + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); > + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, > + kvmcpu, 0); > + tc.kvmcpu = kvmcpu; > + tc.run = run; > + srand(getpid()); > + for (i = 0; i < NTHREAD; i++) { > + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); > + usleep(rand() % 10000); > + } > + for (i = 0; i < NTHREAD; i++) > + pthread_join(th[i], NULL); > +} > + > +int get_warnings_count(void) > +{ > + int warnings; > + FILE *f; > + > + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); > + fscanf(f, "%d", &warnings); > + fclose(f); > + > + return warnings; > +} > + > +int main(void) > +{ > + int warnings_before, warnings_after; > + > + if (!is_intel_cpu()) { > + printf("Must be run on an Intel CPU, skipping test\n"); > + exit(KSFT_SKIP); > + } > + > + if (vm_is_unrestricted_guest(NULL)) { > + printf("Unrestricted guest must be disabled, skipping test\n"); > + exit(KSFT_SKIP); > + } > + > + warnings_before = get_warnings_count(); > + > + for (int i = 0; i < NPROCESS; ++i) { > + int status; > + int pid = fork(); > + > + if (pid < 0) > + exit(1); > + if (pid == 0) { > + test(); > + exit(0); > + } > + while (waitpid(pid, &status, __WALL) != pid) > + ; > + } > + > + warnings_after = get_warnings_count(); > + TEST_ASSERT(warnings_before == warnings_after, > + "Warnings found in kernel. Run 'dmesg' to inspect them."); > + > + return 0; > +} > -- > 2.22.0.rc1.311.g5d7573a151-goog > ping
On 06/18/2019 07:13 AM, Aaron Lewis wrote: > On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: >> When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, >> test that a kernel warning does not occur informing us that >> vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a >> triple fault. >> This test was made to detect a bug that was reported by Syzkaller >> (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and >> fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation >> request to userspace"). >> >> Signed-off-by: Aaron Lewis <aaronlewis@google.com> >> Reviewed-by: Jim Mattson <jmattson@google.com> >> Reviewed-by: Peter Shier <pshier@google.com> >> --- >> tools/testing/selftests/kvm/.gitignore | 1 + >> tools/testing/selftests/kvm/Makefile | 1 + >> .../testing/selftests/kvm/include/kvm_util.h | 2 + >> .../selftests/kvm/include/x86_64/processor.h | 2 + >> tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ >> .../selftests/kvm/lib/x86_64/processor.c | 16 +++ >> .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ >> 7 files changed, 184 insertions(+) >> create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >> >> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore >> index df1bf9230a74..41266af0d3dc 100644 >> --- a/tools/testing/selftests/kvm/.gitignore >> +++ b/tools/testing/selftests/kvm/.gitignore >> @@ -2,6 +2,7 @@ >> /x86_64/evmcs_test >> /x86_64/hyperv_cpuid >> /x86_64/kvm_create_max_vcpus >> +/x86_64/mmio_warning_test >> /x86_64/platform_info_test >> /x86_64/set_sregs_test >> /x86_64/smm_test >> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile >> index 79c524395ebe..670b938f1049 100644 >> --- a/tools/testing/selftests/kvm/Makefile >> +++ b/tools/testing/selftests/kvm/Makefile >> @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test >> TEST_GEN_PROGS_x86_64 += x86_64/smm_test >> TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus >> TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test >> +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test >> TEST_GEN_PROGS_x86_64 += dirty_log_test >> TEST_GEN_PROGS_x86_64 += clear_dirty_log_test >> >> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >> index 8c6b9619797d..c5c427c86598 100644 >> --- a/tools/testing/selftests/kvm/include/kvm_util.h >> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >> @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, >> void *guest_code); >> void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); >> >> +bool vm_is_unrestricted_guest(struct kvm_vm *vm); >> + >> struct kvm_userspace_memory_region * >> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, >> uint64_t end); >> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h >> index 6063d5b2f356..af4d26de32d1 100644 >> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h >> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h >> @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) >> return 0; >> } >> >> +bool is_intel_cpu(void); >> + >> struct kvm_x86_state; >> struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); >> void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, >> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >> index e9113857f44e..b93b09ad9a11 100644 >> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >> @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) >> { >> return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); >> } >> + >> +/* >> + * Is Unrestricted Guest >> + * >> + * Input Args: >> + * vm - Virtual Machine >> + * >> + * Output Args: None >> + * >> + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. >> + * >> + * Check if the unrestricted guest flag is enabled. >> + */ >> +bool vm_is_unrestricted_guest(struct kvm_vm *vm) >> +{ >> + char val = 'N'; >> + size_t count; >> + FILE *f; >> + >> + if (vm == NULL) { >> + /* Ensure that the KVM vendor-specific module is loaded. */ >> + f = fopen(KVM_DEV_PATH, "r"); >> + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", >> + errno); >> + fclose(f); >> + } >> + >> + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); >> + if (f) { >> + count = fread(&val, sizeof(char), 1, f); >> + TEST_ASSERT(count == 1, "Unable to read from param file."); >> + fclose(f); >> + } >> + >> + return val == 'Y'; >> +} >> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c >> index dc7fae9fa424..bcc0e70e1856 100644 >> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c >> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c >> @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s >> r); >> } >> } >> + >> +bool is_intel_cpu(void) >> +{ >> + int eax, ebx, ecx, edx; >> + const uint32_t *chunk; >> + const int leaf = 0; >> + >> + __asm__ __volatile__( >> + "cpuid" >> + : /* output */ "=a"(eax), "=b"(ebx), >> + "=c"(ecx), "=d"(edx) >> + : /* input */ "0"(leaf), "2"(0)); >> + >> + chunk = (const uint32_t *)("GenuineIntel"); >> + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); >> +} >> diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >> new file mode 100644 >> index 000000000000..00bb97d76000 >> --- /dev/null >> +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >> @@ -0,0 +1,126 @@ >> +/* >> + * mmio_warning_test >> + * >> + * Copyright (C) 2019, Google LLC. >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. >> + * >> + * Test that we don't get a kernel warning when we call KVM_RUN after a >> + * triple fault occurs. To get the triple fault to occur we call KVM_RUN >> + * on a VCPU that hasn't been properly setup. >> + * >> + */ >> + >> +#define _GNU_SOURCE >> +#include <fcntl.h> >> +#include <kvm_util.h> >> +#include <linux/kvm.h> >> +#include <processor.h> >> +#include <pthread.h> >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <string.h> >> +#include <sys/ioctl.h> >> +#include <sys/mman.h> >> +#include <sys/stat.h> >> +#include <sys/types.h> >> +#include <sys/wait.h> >> +#include <test_util.h> >> +#include <unistd.h> >> + >> +#define NTHREAD 4 >> +#define NPROCESS 5 >> + >> +struct thread_context { >> + int kvmcpu; >> + struct kvm_run *run; >> +}; >> + >> +void *thr(void *arg) >> +{ >> + struct thread_context *tc = (struct thread_context *)arg; >> + int res; >> + int kvmcpu = tc->kvmcpu; >> + struct kvm_run *run = tc->run; >> + >> + res = ioctl(kvmcpu, KVM_RUN, 0); >> + printf("ret1=%d exit_reason=%d suberror=%d\n", >> + res, run->exit_reason, run->internal.suberror); >> + >> + return 0; >> +} >> + >> +void test(void) >> +{ >> + int i, kvm, kvmvm, kvmcpu; >> + pthread_t th[NTHREAD]; >> + struct kvm_run *run; >> + struct thread_context tc; >> + >> + kvm = open("/dev/kvm", O_RDWR); >> + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); >> + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); >> + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); >> + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); >> + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); >> + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, >> + kvmcpu, 0); >> + tc.kvmcpu = kvmcpu; >> + tc.run = run; >> + srand(getpid()); >> + for (i = 0; i < NTHREAD; i++) { >> + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); >> + usleep(rand() % 10000); >> + } >> + for (i = 0; i < NTHREAD; i++) >> + pthread_join(th[i], NULL); >> +} >> + >> +int get_warnings_count(void) >> +{ >> + int warnings; >> + FILE *f; >> + >> + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); >> + fscanf(f, "%d", &warnings); >> + fclose(f); >> + >> + return warnings; >> +} >> + >> +int main(void) >> +{ >> + int warnings_before, warnings_after; >> + >> + if (!is_intel_cpu()) { >> + printf("Must be run on an Intel CPU, skipping test\n"); >> + exit(KSFT_SKIP); >> + } >> + >> + if (vm_is_unrestricted_guest(NULL)) { >> + printf("Unrestricted guest must be disabled, skipping test\n"); >> + exit(KSFT_SKIP); >> + } >> + >> + warnings_before = get_warnings_count(); >> + >> + for (int i = 0; i < NPROCESS; ++i) { >> + int status; >> + int pid = fork(); >> + >> + if (pid < 0) >> + exit(1); >> + if (pid == 0) { >> + test(); >> + exit(0); >> + } >> + while (waitpid(pid, &status, __WALL) != pid) >> + ; >> + } >> + >> + warnings_after = get_warnings_count(); Since you are grep'ing for the word "WARNING", is there a possibility that the test can detect a false positive based on Warnings generated due to some other cause while it ran ? >> + TEST_ASSERT(warnings_before == warnings_after, >> + "Warnings found in kernel. Run 'dmesg' to inspect them."); >> + >> + return 0; >> +} >> -- >> 2.22.0.rc1.311.g5d7573a151-goog >> > ping
On Tue, Jun 18, 2019 at 12:38 PM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > > > > On 06/18/2019 07:13 AM, Aaron Lewis wrote: > > On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: > >> When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, > >> test that a kernel warning does not occur informing us that > >> vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a > >> triple fault. > >> This test was made to detect a bug that was reported by Syzkaller > >> (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and > >> fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation > >> request to userspace"). > >> > >> Signed-off-by: Aaron Lewis <aaronlewis@google.com> > >> Reviewed-by: Jim Mattson <jmattson@google.com> > >> Reviewed-by: Peter Shier <pshier@google.com> > >> --- > >> tools/testing/selftests/kvm/.gitignore | 1 + > >> tools/testing/selftests/kvm/Makefile | 1 + > >> .../testing/selftests/kvm/include/kvm_util.h | 2 + > >> .../selftests/kvm/include/x86_64/processor.h | 2 + > >> tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ > >> .../selftests/kvm/lib/x86_64/processor.c | 16 +++ > >> .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ > >> 7 files changed, 184 insertions(+) > >> create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >> > >> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > >> index df1bf9230a74..41266af0d3dc 100644 > >> --- a/tools/testing/selftests/kvm/.gitignore > >> +++ b/tools/testing/selftests/kvm/.gitignore > >> @@ -2,6 +2,7 @@ > >> /x86_64/evmcs_test > >> /x86_64/hyperv_cpuid > >> /x86_64/kvm_create_max_vcpus > >> +/x86_64/mmio_warning_test > >> /x86_64/platform_info_test > >> /x86_64/set_sregs_test > >> /x86_64/smm_test > >> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > >> index 79c524395ebe..670b938f1049 100644 > >> --- a/tools/testing/selftests/kvm/Makefile > >> +++ b/tools/testing/selftests/kvm/Makefile > >> @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test > >> TEST_GEN_PROGS_x86_64 += x86_64/smm_test > >> TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus > >> TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test > >> +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test > >> TEST_GEN_PROGS_x86_64 += dirty_log_test > >> TEST_GEN_PROGS_x86_64 += clear_dirty_log_test > >> > >> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > >> index 8c6b9619797d..c5c427c86598 100644 > >> --- a/tools/testing/selftests/kvm/include/kvm_util.h > >> +++ b/tools/testing/selftests/kvm/include/kvm_util.h > >> @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, > >> void *guest_code); > >> void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); > >> > >> +bool vm_is_unrestricted_guest(struct kvm_vm *vm); > >> + > >> struct kvm_userspace_memory_region * > >> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, > >> uint64_t end); > >> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > >> index 6063d5b2f356..af4d26de32d1 100644 > >> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > >> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > >> @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) > >> return 0; > >> } > >> > >> +bool is_intel_cpu(void); > >> + > >> struct kvm_x86_state; > >> struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); > >> void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, > >> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > >> index e9113857f44e..b93b09ad9a11 100644 > >> --- a/tools/testing/selftests/kvm/lib/kvm_util.c > >> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > >> @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) > >> { > >> return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); > >> } > >> + > >> +/* > >> + * Is Unrestricted Guest > >> + * > >> + * Input Args: > >> + * vm - Virtual Machine > >> + * > >> + * Output Args: None > >> + * > >> + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. > >> + * > >> + * Check if the unrestricted guest flag is enabled. > >> + */ > >> +bool vm_is_unrestricted_guest(struct kvm_vm *vm) > >> +{ > >> + char val = 'N'; > >> + size_t count; > >> + FILE *f; > >> + > >> + if (vm == NULL) { > >> + /* Ensure that the KVM vendor-specific module is loaded. */ > >> + f = fopen(KVM_DEV_PATH, "r"); > >> + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", > >> + errno); > >> + fclose(f); > >> + } > >> + > >> + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); > >> + if (f) { > >> + count = fread(&val, sizeof(char), 1, f); > >> + TEST_ASSERT(count == 1, "Unable to read from param file."); > >> + fclose(f); > >> + } > >> + > >> + return val == 'Y'; > >> +} > >> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > >> index dc7fae9fa424..bcc0e70e1856 100644 > >> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > >> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > >> @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s > >> r); > >> } > >> } > >> + > >> +bool is_intel_cpu(void) > >> +{ > >> + int eax, ebx, ecx, edx; > >> + const uint32_t *chunk; > >> + const int leaf = 0; > >> + > >> + __asm__ __volatile__( > >> + "cpuid" > >> + : /* output */ "=a"(eax), "=b"(ebx), > >> + "=c"(ecx), "=d"(edx) > >> + : /* input */ "0"(leaf), "2"(0)); > >> + > >> + chunk = (const uint32_t *)("GenuineIntel"); > >> + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); > >> +} > >> diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >> new file mode 100644 > >> index 000000000000..00bb97d76000 > >> --- /dev/null > >> +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >> @@ -0,0 +1,126 @@ > >> +/* > >> + * mmio_warning_test > >> + * > >> + * Copyright (C) 2019, Google LLC. > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2. > >> + * > >> + * Test that we don't get a kernel warning when we call KVM_RUN after a > >> + * triple fault occurs. To get the triple fault to occur we call KVM_RUN > >> + * on a VCPU that hasn't been properly setup. > >> + * > >> + */ > >> + > >> +#define _GNU_SOURCE > >> +#include <fcntl.h> > >> +#include <kvm_util.h> > >> +#include <linux/kvm.h> > >> +#include <processor.h> > >> +#include <pthread.h> > >> +#include <stdio.h> > >> +#include <stdlib.h> > >> +#include <string.h> > >> +#include <sys/ioctl.h> > >> +#include <sys/mman.h> > >> +#include <sys/stat.h> > >> +#include <sys/types.h> > >> +#include <sys/wait.h> > >> +#include <test_util.h> > >> +#include <unistd.h> > >> + > >> +#define NTHREAD 4 > >> +#define NPROCESS 5 > >> + > >> +struct thread_context { > >> + int kvmcpu; > >> + struct kvm_run *run; > >> +}; > >> + > >> +void *thr(void *arg) > >> +{ > >> + struct thread_context *tc = (struct thread_context *)arg; > >> + int res; > >> + int kvmcpu = tc->kvmcpu; > >> + struct kvm_run *run = tc->run; > >> + > >> + res = ioctl(kvmcpu, KVM_RUN, 0); > >> + printf("ret1=%d exit_reason=%d suberror=%d\n", > >> + res, run->exit_reason, run->internal.suberror); > >> + > >> + return 0; > >> +} > >> + > >> +void test(void) > >> +{ > >> + int i, kvm, kvmvm, kvmcpu; > >> + pthread_t th[NTHREAD]; > >> + struct kvm_run *run; > >> + struct thread_context tc; > >> + > >> + kvm = open("/dev/kvm", O_RDWR); > >> + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); > >> + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); > >> + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); > >> + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); > >> + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); > >> + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, > >> + kvmcpu, 0); > >> + tc.kvmcpu = kvmcpu; > >> + tc.run = run; > >> + srand(getpid()); > >> + for (i = 0; i < NTHREAD; i++) { > >> + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); > >> + usleep(rand() % 10000); > >> + } > >> + for (i = 0; i < NTHREAD; i++) > >> + pthread_join(th[i], NULL); > >> +} > >> + > >> +int get_warnings_count(void) > >> +{ > >> + int warnings; > >> + FILE *f; > >> + > >> + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); > >> + fscanf(f, "%d", &warnings); > >> + fclose(f); > >> + > >> + return warnings; > >> +} > >> + > >> +int main(void) > >> +{ > >> + int warnings_before, warnings_after; > >> + > >> + if (!is_intel_cpu()) { > >> + printf("Must be run on an Intel CPU, skipping test\n"); > >> + exit(KSFT_SKIP); > >> + } > >> + > >> + if (vm_is_unrestricted_guest(NULL)) { > >> + printf("Unrestricted guest must be disabled, skipping test\n"); > >> + exit(KSFT_SKIP); > >> + } > >> + > >> + warnings_before = get_warnings_count(); > >> + > >> + for (int i = 0; i < NPROCESS; ++i) { > >> + int status; > >> + int pid = fork(); > >> + > >> + if (pid < 0) > >> + exit(1); > >> + if (pid == 0) { > >> + test(); > >> + exit(0); > >> + } > >> + while (waitpid(pid, &status, __WALL) != pid) > >> + ; > >> + } > >> + > >> + warnings_after = get_warnings_count(); > > Since you are grep'ing for the word "WARNING", is there a possibility > that the test can detect a false positive based on Warnings generated > due to some other cause while it ran ? > Yes, this is a possibility, however, it is still a warning and should still be dealt with. We could special case the grep message to be more specific to the case we are dealing with here, but I'd prefer to keep it this way to alert on any warning. That way other warnings, should they occur, are brought to our attention. > >> + TEST_ASSERT(warnings_before == warnings_after, > >> + "Warnings found in kernel. Run 'dmesg' to inspect them."); > >> + > >> + return 0; > >> +} > >> -- > >> 2.22.0.rc1.311.g5d7573a151-goog > >> > > ping >
On 6/20/19 7:12 AM, Aaron Lewis wrote: > On Tue, Jun 18, 2019 at 12:38 PM Krish Sadhukhan > <krish.sadhukhan@oracle.com> wrote: >> >> >> On 06/18/2019 07:13 AM, Aaron Lewis wrote: >>> On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: >>>> When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, >>>> test that a kernel warning does not occur informing us that >>>> vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a >>>> triple fault. >>>> This test was made to detect a bug that was reported by Syzkaller >>>> (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and >>>> fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation >>>> request to userspace"). >>>> >>>> Signed-off-by: Aaron Lewis <aaronlewis@google.com> >>>> Reviewed-by: Jim Mattson <jmattson@google.com> >>>> Reviewed-by: Peter Shier <pshier@google.com> >>>> --- >>>> tools/testing/selftests/kvm/.gitignore | 1 + >>>> tools/testing/selftests/kvm/Makefile | 1 + >>>> .../testing/selftests/kvm/include/kvm_util.h | 2 + >>>> .../selftests/kvm/include/x86_64/processor.h | 2 + >>>> tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ >>>> .../selftests/kvm/lib/x86_64/processor.c | 16 +++ >>>> .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ >>>> 7 files changed, 184 insertions(+) >>>> create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>> >>>> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore >>>> index df1bf9230a74..41266af0d3dc 100644 >>>> --- a/tools/testing/selftests/kvm/.gitignore >>>> +++ b/tools/testing/selftests/kvm/.gitignore >>>> @@ -2,6 +2,7 @@ >>>> /x86_64/evmcs_test >>>> /x86_64/hyperv_cpuid >>>> /x86_64/kvm_create_max_vcpus >>>> +/x86_64/mmio_warning_test >>>> /x86_64/platform_info_test >>>> /x86_64/set_sregs_test >>>> /x86_64/smm_test >>>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile >>>> index 79c524395ebe..670b938f1049 100644 >>>> --- a/tools/testing/selftests/kvm/Makefile >>>> +++ b/tools/testing/selftests/kvm/Makefile >>>> @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test >>>> TEST_GEN_PROGS_x86_64 += x86_64/smm_test >>>> TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus >>>> TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test >>>> +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test >>>> TEST_GEN_PROGS_x86_64 += dirty_log_test >>>> TEST_GEN_PROGS_x86_64 += clear_dirty_log_test >>>> >>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >>>> index 8c6b9619797d..c5c427c86598 100644 >>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h >>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >>>> @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, >>>> void *guest_code); >>>> void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); >>>> >>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm); >>>> + >>>> struct kvm_userspace_memory_region * >>>> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, >>>> uint64_t end); >>>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h >>>> index 6063d5b2f356..af4d26de32d1 100644 >>>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h >>>> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h >>>> @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) >>>> return 0; >>>> } >>>> >>>> +bool is_intel_cpu(void); >>>> + >>>> struct kvm_x86_state; >>>> struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); >>>> void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, >>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >>>> index e9113857f44e..b93b09ad9a11 100644 >>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>>> @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) >>>> { >>>> return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); >>>> } >>>> + >>>> +/* >>>> + * Is Unrestricted Guest >>>> + * >>>> + * Input Args: >>>> + * vm - Virtual Machine >>>> + * >>>> + * Output Args: None >>>> + * >>>> + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. >>>> + * >>>> + * Check if the unrestricted guest flag is enabled. >>>> + */ >>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm) >>>> +{ >>>> + char val = 'N'; >>>> + size_t count; >>>> + FILE *f; >>>> + >>>> + if (vm == NULL) { >>>> + /* Ensure that the KVM vendor-specific module is loaded. */ >>>> + f = fopen(KVM_DEV_PATH, "r"); >>>> + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", >>>> + errno); >>>> + fclose(f); >>>> + } >>>> + >>>> + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); >>>> + if (f) { >>>> + count = fread(&val, sizeof(char), 1, f); >>>> + TEST_ASSERT(count == 1, "Unable to read from param file."); >>>> + fclose(f); >>>> + } >>>> + >>>> + return val == 'Y'; >>>> +} >>>> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>> index dc7fae9fa424..bcc0e70e1856 100644 >>>> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>> @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s >>>> r); >>>> } >>>> } >>>> + >>>> +bool is_intel_cpu(void) >>>> +{ >>>> + int eax, ebx, ecx, edx; >>>> + const uint32_t *chunk; >>>> + const int leaf = 0; >>>> + >>>> + __asm__ __volatile__( >>>> + "cpuid" >>>> + : /* output */ "=a"(eax), "=b"(ebx), >>>> + "=c"(ecx), "=d"(edx) >>>> + : /* input */ "0"(leaf), "2"(0)); >>>> + >>>> + chunk = (const uint32_t *)("GenuineIntel"); >>>> + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); >>>> +} >>>> diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>> new file mode 100644 >>>> index 000000000000..00bb97d76000 >>>> --- /dev/null >>>> +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>> @@ -0,0 +1,126 @@ >>>> +/* >>>> + * mmio_warning_test >>>> + * >>>> + * Copyright (C) 2019, Google LLC. >>>> + * >>>> + * This work is licensed under the terms of the GNU GPL, version 2. >>>> + * >>>> + * Test that we don't get a kernel warning when we call KVM_RUN after a >>>> + * triple fault occurs. To get the triple fault to occur we call KVM_RUN >>>> + * on a VCPU that hasn't been properly setup. >>>> + * >>>> + */ >>>> + >>>> +#define _GNU_SOURCE >>>> +#include <fcntl.h> >>>> +#include <kvm_util.h> >>>> +#include <linux/kvm.h> >>>> +#include <processor.h> >>>> +#include <pthread.h> >>>> +#include <stdio.h> >>>> +#include <stdlib.h> >>>> +#include <string.h> >>>> +#include <sys/ioctl.h> >>>> +#include <sys/mman.h> >>>> +#include <sys/stat.h> >>>> +#include <sys/types.h> >>>> +#include <sys/wait.h> >>>> +#include <test_util.h> >>>> +#include <unistd.h> >>>> + >>>> +#define NTHREAD 4 >>>> +#define NPROCESS 5 >>>> + >>>> +struct thread_context { >>>> + int kvmcpu; >>>> + struct kvm_run *run; >>>> +}; >>>> + >>>> +void *thr(void *arg) >>>> +{ >>>> + struct thread_context *tc = (struct thread_context *)arg; >>>> + int res; >>>> + int kvmcpu = tc->kvmcpu; >>>> + struct kvm_run *run = tc->run; >>>> + >>>> + res = ioctl(kvmcpu, KVM_RUN, 0); >>>> + printf("ret1=%d exit_reason=%d suberror=%d\n", >>>> + res, run->exit_reason, run->internal.suberror); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +void test(void) >>>> +{ >>>> + int i, kvm, kvmvm, kvmcpu; >>>> + pthread_t th[NTHREAD]; >>>> + struct kvm_run *run; >>>> + struct thread_context tc; >>>> + >>>> + kvm = open("/dev/kvm", O_RDWR); >>>> + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); >>>> + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); >>>> + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); >>>> + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); >>>> + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); >>>> + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, >>>> + kvmcpu, 0); >>>> + tc.kvmcpu = kvmcpu; >>>> + tc.run = run; >>>> + srand(getpid()); >>>> + for (i = 0; i < NTHREAD; i++) { >>>> + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); >>>> + usleep(rand() % 10000); >>>> + } >>>> + for (i = 0; i < NTHREAD; i++) >>>> + pthread_join(th[i], NULL); >>>> +} >>>> + >>>> +int get_warnings_count(void) >>>> +{ >>>> + int warnings; >>>> + FILE *f; >>>> + >>>> + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); >>>> + fscanf(f, "%d", &warnings); >>>> + fclose(f); >>>> + >>>> + return warnings; >>>> +} >>>> + >>>> +int main(void) >>>> +{ >>>> + int warnings_before, warnings_after; >>>> + >>>> + if (!is_intel_cpu()) { >>>> + printf("Must be run on an Intel CPU, skipping test\n"); >>>> + exit(KSFT_SKIP); >>>> + } >>>> + >>>> + if (vm_is_unrestricted_guest(NULL)) { >>>> + printf("Unrestricted guest must be disabled, skipping test\n"); >>>> + exit(KSFT_SKIP); >>>> + } >>>> + >>>> + warnings_before = get_warnings_count(); >>>> + >>>> + for (int i = 0; i < NPROCESS; ++i) { >>>> + int status; >>>> + int pid = fork(); >>>> + >>>> + if (pid < 0) >>>> + exit(1); >>>> + if (pid == 0) { >>>> + test(); >>>> + exit(0); >>>> + } >>>> + while (waitpid(pid, &status, __WALL) != pid) >>>> + ; >>>> + } >>>> + >>>> + warnings_after = get_warnings_count(); >> Since you are grep'ing for the word "WARNING", is there a possibility >> that the test can detect a false positive based on Warnings generated >> due to some other cause while it ran ? >> > Yes, this is a possibility, however, it is still a warning and should > still be dealt with. We could special case the grep message to be > more specific to the case we are dealing with here, but I'd prefer to > keep it this way to alert on any warning. That way other warnings, > should they occur, are brought to our attention. OK. In that case, does it make sense to provide some additional info in the ASSERT message below ? dmesg can contain Warnings that may have occurred either before or after the run of this test and so we can at least link the false positives to this test. > >>>> + TEST_ASSERT(warnings_before == warnings_after, >>>> + "Warnings found in kernel. Run 'dmesg' to inspect them."); >>>> + >>>> + return 0; >>>> +} >>>> -- >>>> 2.22.0.rc1.311.g5d7573a151-goog >>>> >>> ping
On Thu, Jun 20, 2019 at 11:17 AM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > > > On 6/20/19 7:12 AM, Aaron Lewis wrote: > > On Tue, Jun 18, 2019 at 12:38 PM Krish Sadhukhan > > <krish.sadhukhan@oracle.com> wrote: > >> > >> > >> On 06/18/2019 07:13 AM, Aaron Lewis wrote: > >>> On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: > >>>> When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, > >>>> test that a kernel warning does not occur informing us that > >>>> vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a > >>>> triple fault. > >>>> This test was made to detect a bug that was reported by Syzkaller > >>>> (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and > >>>> fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation > >>>> request to userspace"). > >>>> > >>>> Signed-off-by: Aaron Lewis <aaronlewis@google.com> > >>>> Reviewed-by: Jim Mattson <jmattson@google.com> > >>>> Reviewed-by: Peter Shier <pshier@google.com> > >>>> --- > >>>> tools/testing/selftests/kvm/.gitignore | 1 + > >>>> tools/testing/selftests/kvm/Makefile | 1 + > >>>> .../testing/selftests/kvm/include/kvm_util.h | 2 + > >>>> .../selftests/kvm/include/x86_64/processor.h | 2 + > >>>> tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ > >>>> .../selftests/kvm/lib/x86_64/processor.c | 16 +++ > >>>> .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ > >>>> 7 files changed, 184 insertions(+) > >>>> create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >>>> > >>>> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore > >>>> index df1bf9230a74..41266af0d3dc 100644 > >>>> --- a/tools/testing/selftests/kvm/.gitignore > >>>> +++ b/tools/testing/selftests/kvm/.gitignore > >>>> @@ -2,6 +2,7 @@ > >>>> /x86_64/evmcs_test > >>>> /x86_64/hyperv_cpuid > >>>> /x86_64/kvm_create_max_vcpus > >>>> +/x86_64/mmio_warning_test > >>>> /x86_64/platform_info_test > >>>> /x86_64/set_sregs_test > >>>> /x86_64/smm_test > >>>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > >>>> index 79c524395ebe..670b938f1049 100644 > >>>> --- a/tools/testing/selftests/kvm/Makefile > >>>> +++ b/tools/testing/selftests/kvm/Makefile > >>>> @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test > >>>> TEST_GEN_PROGS_x86_64 += x86_64/smm_test > >>>> TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus > >>>> TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test > >>>> +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test > >>>> TEST_GEN_PROGS_x86_64 += dirty_log_test > >>>> TEST_GEN_PROGS_x86_64 += clear_dirty_log_test > >>>> > >>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > >>>> index 8c6b9619797d..c5c427c86598 100644 > >>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h > >>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h > >>>> @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, > >>>> void *guest_code); > >>>> void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); > >>>> > >>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm); > >>>> + > >>>> struct kvm_userspace_memory_region * > >>>> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, > >>>> uint64_t end); > >>>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > >>>> index 6063d5b2f356..af4d26de32d1 100644 > >>>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > >>>> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > >>>> @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) > >>>> return 0; > >>>> } > >>>> > >>>> +bool is_intel_cpu(void); > >>>> + > >>>> struct kvm_x86_state; > >>>> struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); > >>>> void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, > >>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > >>>> index e9113857f44e..b93b09ad9a11 100644 > >>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c > >>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > >>>> @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) > >>>> { > >>>> return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); > >>>> } > >>>> + > >>>> +/* > >>>> + * Is Unrestricted Guest > >>>> + * > >>>> + * Input Args: > >>>> + * vm - Virtual Machine > >>>> + * > >>>> + * Output Args: None > >>>> + * > >>>> + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. > >>>> + * > >>>> + * Check if the unrestricted guest flag is enabled. > >>>> + */ > >>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm) > >>>> +{ > >>>> + char val = 'N'; > >>>> + size_t count; > >>>> + FILE *f; > >>>> + > >>>> + if (vm == NULL) { > >>>> + /* Ensure that the KVM vendor-specific module is loaded. */ > >>>> + f = fopen(KVM_DEV_PATH, "r"); > >>>> + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", > >>>> + errno); > >>>> + fclose(f); > >>>> + } > >>>> + > >>>> + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); > >>>> + if (f) { > >>>> + count = fread(&val, sizeof(char), 1, f); > >>>> + TEST_ASSERT(count == 1, "Unable to read from param file."); > >>>> + fclose(f); > >>>> + } > >>>> + > >>>> + return val == 'Y'; > >>>> +} > >>>> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > >>>> index dc7fae9fa424..bcc0e70e1856 100644 > >>>> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > >>>> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > >>>> @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s > >>>> r); > >>>> } > >>>> } > >>>> + > >>>> +bool is_intel_cpu(void) > >>>> +{ > >>>> + int eax, ebx, ecx, edx; > >>>> + const uint32_t *chunk; > >>>> + const int leaf = 0; > >>>> + > >>>> + __asm__ __volatile__( > >>>> + "cpuid" > >>>> + : /* output */ "=a"(eax), "=b"(ebx), > >>>> + "=c"(ecx), "=d"(edx) > >>>> + : /* input */ "0"(leaf), "2"(0)); > >>>> + > >>>> + chunk = (const uint32_t *)("GenuineIntel"); > >>>> + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); > >>>> +} > >>>> diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >>>> new file mode 100644 > >>>> index 000000000000..00bb97d76000 > >>>> --- /dev/null > >>>> +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c > >>>> @@ -0,0 +1,126 @@ > >>>> +/* > >>>> + * mmio_warning_test > >>>> + * > >>>> + * Copyright (C) 2019, Google LLC. > >>>> + * > >>>> + * This work is licensed under the terms of the GNU GPL, version 2. > >>>> + * > >>>> + * Test that we don't get a kernel warning when we call KVM_RUN after a > >>>> + * triple fault occurs. To get the triple fault to occur we call KVM_RUN > >>>> + * on a VCPU that hasn't been properly setup. > >>>> + * > >>>> + */ > >>>> + > >>>> +#define _GNU_SOURCE > >>>> +#include <fcntl.h> > >>>> +#include <kvm_util.h> > >>>> +#include <linux/kvm.h> > >>>> +#include <processor.h> > >>>> +#include <pthread.h> > >>>> +#include <stdio.h> > >>>> +#include <stdlib.h> > >>>> +#include <string.h> > >>>> +#include <sys/ioctl.h> > >>>> +#include <sys/mman.h> > >>>> +#include <sys/stat.h> > >>>> +#include <sys/types.h> > >>>> +#include <sys/wait.h> > >>>> +#include <test_util.h> > >>>> +#include <unistd.h> > >>>> + > >>>> +#define NTHREAD 4 > >>>> +#define NPROCESS 5 > >>>> + > >>>> +struct thread_context { > >>>> + int kvmcpu; > >>>> + struct kvm_run *run; > >>>> +}; > >>>> + > >>>> +void *thr(void *arg) > >>>> +{ > >>>> + struct thread_context *tc = (struct thread_context *)arg; > >>>> + int res; > >>>> + int kvmcpu = tc->kvmcpu; > >>>> + struct kvm_run *run = tc->run; > >>>> + > >>>> + res = ioctl(kvmcpu, KVM_RUN, 0); > >>>> + printf("ret1=%d exit_reason=%d suberror=%d\n", > >>>> + res, run->exit_reason, run->internal.suberror); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +void test(void) > >>>> +{ > >>>> + int i, kvm, kvmvm, kvmcpu; > >>>> + pthread_t th[NTHREAD]; > >>>> + struct kvm_run *run; > >>>> + struct thread_context tc; > >>>> + > >>>> + kvm = open("/dev/kvm", O_RDWR); > >>>> + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); > >>>> + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); > >>>> + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); > >>>> + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); > >>>> + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); > >>>> + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, > >>>> + kvmcpu, 0); > >>>> + tc.kvmcpu = kvmcpu; > >>>> + tc.run = run; > >>>> + srand(getpid()); > >>>> + for (i = 0; i < NTHREAD; i++) { > >>>> + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); > >>>> + usleep(rand() % 10000); > >>>> + } > >>>> + for (i = 0; i < NTHREAD; i++) > >>>> + pthread_join(th[i], NULL); > >>>> +} > >>>> + > >>>> +int get_warnings_count(void) > >>>> +{ > >>>> + int warnings; > >>>> + FILE *f; > >>>> + > >>>> + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); > >>>> + fscanf(f, "%d", &warnings); > >>>> + fclose(f); > >>>> + > >>>> + return warnings; > >>>> +} > >>>> + > >>>> +int main(void) > >>>> +{ > >>>> + int warnings_before, warnings_after; > >>>> + > >>>> + if (!is_intel_cpu()) { > >>>> + printf("Must be run on an Intel CPU, skipping test\n"); > >>>> + exit(KSFT_SKIP); > >>>> + } > >>>> + > >>>> + if (vm_is_unrestricted_guest(NULL)) { > >>>> + printf("Unrestricted guest must be disabled, skipping test\n"); > >>>> + exit(KSFT_SKIP); > >>>> + } > >>>> + > >>>> + warnings_before = get_warnings_count(); > >>>> + > >>>> + for (int i = 0; i < NPROCESS; ++i) { > >>>> + int status; > >>>> + int pid = fork(); > >>>> + > >>>> + if (pid < 0) > >>>> + exit(1); > >>>> + if (pid == 0) { > >>>> + test(); > >>>> + exit(0); > >>>> + } > >>>> + while (waitpid(pid, &status, __WALL) != pid) > >>>> + ; > >>>> + } > >>>> + > >>>> + warnings_after = get_warnings_count(); > >> Since you are grep'ing for the word "WARNING", is there a possibility > >> that the test can detect a false positive based on Warnings generated > >> due to some other cause while it ran ? > >> > > Yes, this is a possibility, however, it is still a warning and should > > still be dealt with. We could special case the grep message to be > > more specific to the case we are dealing with here, but I'd prefer to > > keep it this way to alert on any warning. That way other warnings, > > should they occur, are brought to our attention. > > > OK. In that case, does it make sense to provide some additional info in > the ASSERT message below ? dmesg can contain Warnings that may have > occurred either before or after the run of this test and so we can at > least link the false positives to this test. > I can go either way on this. While there can be false positives in dmesg, if this asset fires there will be warnings worth looking at and addressing as it only fires if it detects warnings occurred during the test. That's the reason I made the asset message general and only say to look at warnings. We could add a more long winded message to the end of this informing the user that because the test doesn't clear dmesg there could be warnings not related to the test. However, there will be warnings caused by running this test as well. > > > > >>>> + TEST_ASSERT(warnings_before == warnings_after, > >>>> + "Warnings found in kernel. Run 'dmesg' to inspect them."); > >>>> + > >>>> + return 0; > >>>> +} > >>>> -- > >>>> 2.22.0.rc1.311.g5d7573a151-goog > >>>> > >>> ping
On 6/20/19 12:47 PM, Aaron Lewis wrote: > On Thu, Jun 20, 2019 at 11:17 AM Krish Sadhukhan > <krish.sadhukhan@oracle.com> wrote: >> >> On 6/20/19 7:12 AM, Aaron Lewis wrote: >>> On Tue, Jun 18, 2019 at 12:38 PM Krish Sadhukhan >>> <krish.sadhukhan@oracle.com> wrote: >>>> >>>> On 06/18/2019 07:13 AM, Aaron Lewis wrote: >>>>> On Fri, May 31, 2019 at 7:14 AM Aaron Lewis <aaronlewis@google.com> wrote: >>>>>> When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, >>>>>> test that a kernel warning does not occur informing us that >>>>>> vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a >>>>>> triple fault. >>>>>> This test was made to detect a bug that was reported by Syzkaller >>>>>> (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and >>>>>> fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation >>>>>> request to userspace"). >>>>>> >>>>>> Signed-off-by: Aaron Lewis <aaronlewis@google.com> >>>>>> Reviewed-by: Jim Mattson <jmattson@google.com> >>>>>> Reviewed-by: Peter Shier <pshier@google.com> >>>>>> --- >>>>>> tools/testing/selftests/kvm/.gitignore | 1 + >>>>>> tools/testing/selftests/kvm/Makefile | 1 + >>>>>> .../testing/selftests/kvm/include/kvm_util.h | 2 + >>>>>> .../selftests/kvm/include/x86_64/processor.h | 2 + >>>>>> tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ >>>>>> .../selftests/kvm/lib/x86_64/processor.c | 16 +++ >>>>>> .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ >>>>>> 7 files changed, 184 insertions(+) >>>>>> create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>>>> >>>>>> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore >>>>>> index df1bf9230a74..41266af0d3dc 100644 >>>>>> --- a/tools/testing/selftests/kvm/.gitignore >>>>>> +++ b/tools/testing/selftests/kvm/.gitignore >>>>>> @@ -2,6 +2,7 @@ >>>>>> /x86_64/evmcs_test >>>>>> /x86_64/hyperv_cpuid >>>>>> /x86_64/kvm_create_max_vcpus >>>>>> +/x86_64/mmio_warning_test >>>>>> /x86_64/platform_info_test >>>>>> /x86_64/set_sregs_test >>>>>> /x86_64/smm_test >>>>>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile >>>>>> index 79c524395ebe..670b938f1049 100644 >>>>>> --- a/tools/testing/selftests/kvm/Makefile >>>>>> +++ b/tools/testing/selftests/kvm/Makefile >>>>>> @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test >>>>>> TEST_GEN_PROGS_x86_64 += x86_64/smm_test >>>>>> TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus >>>>>> TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test >>>>>> +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test >>>>>> TEST_GEN_PROGS_x86_64 += dirty_log_test >>>>>> TEST_GEN_PROGS_x86_64 += clear_dirty_log_test >>>>>> >>>>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >>>>>> index 8c6b9619797d..c5c427c86598 100644 >>>>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h >>>>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >>>>>> @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, >>>>>> void *guest_code); >>>>>> void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); >>>>>> >>>>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm); >>>>>> + >>>>>> struct kvm_userspace_memory_region * >>>>>> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, >>>>>> uint64_t end); >>>>>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h >>>>>> index 6063d5b2f356..af4d26de32d1 100644 >>>>>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h >>>>>> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h >>>>>> @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +bool is_intel_cpu(void); >>>>>> + >>>>>> struct kvm_x86_state; >>>>>> struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); >>>>>> void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, >>>>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >>>>>> index e9113857f44e..b93b09ad9a11 100644 >>>>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>>>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>>>>> @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) >>>>>> { >>>>>> return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); >>>>>> } >>>>>> + >>>>>> +/* >>>>>> + * Is Unrestricted Guest >>>>>> + * >>>>>> + * Input Args: >>>>>> + * vm - Virtual Machine >>>>>> + * >>>>>> + * Output Args: None >>>>>> + * >>>>>> + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. >>>>>> + * >>>>>> + * Check if the unrestricted guest flag is enabled. >>>>>> + */ >>>>>> +bool vm_is_unrestricted_guest(struct kvm_vm *vm) >>>>>> +{ >>>>>> + char val = 'N'; >>>>>> + size_t count; >>>>>> + FILE *f; >>>>>> + >>>>>> + if (vm == NULL) { >>>>>> + /* Ensure that the KVM vendor-specific module is loaded. */ >>>>>> + f = fopen(KVM_DEV_PATH, "r"); >>>>>> + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", >>>>>> + errno); >>>>>> + fclose(f); >>>>>> + } >>>>>> + >>>>>> + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); >>>>>> + if (f) { >>>>>> + count = fread(&val, sizeof(char), 1, f); >>>>>> + TEST_ASSERT(count == 1, "Unable to read from param file."); >>>>>> + fclose(f); >>>>>> + } >>>>>> + >>>>>> + return val == 'Y'; >>>>>> +} >>>>>> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>>>> index dc7fae9fa424..bcc0e70e1856 100644 >>>>>> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>>>> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c >>>>>> @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s >>>>>> r); >>>>>> } >>>>>> } >>>>>> + >>>>>> +bool is_intel_cpu(void) >>>>>> +{ >>>>>> + int eax, ebx, ecx, edx; >>>>>> + const uint32_t *chunk; >>>>>> + const int leaf = 0; >>>>>> + >>>>>> + __asm__ __volatile__( >>>>>> + "cpuid" >>>>>> + : /* output */ "=a"(eax), "=b"(ebx), >>>>>> + "=c"(ecx), "=d"(edx) >>>>>> + : /* input */ "0"(leaf), "2"(0)); >>>>>> + >>>>>> + chunk = (const uint32_t *)("GenuineIntel"); >>>>>> + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); >>>>>> +} >>>>>> diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>>>> new file mode 100644 >>>>>> index 000000000000..00bb97d76000 >>>>>> --- /dev/null >>>>>> +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c >>>>>> @@ -0,0 +1,126 @@ >>>>>> +/* >>>>>> + * mmio_warning_test >>>>>> + * >>>>>> + * Copyright (C) 2019, Google LLC. >>>>>> + * >>>>>> + * This work is licensed under the terms of the GNU GPL, version 2. >>>>>> + * >>>>>> + * Test that we don't get a kernel warning when we call KVM_RUN after a >>>>>> + * triple fault occurs. To get the triple fault to occur we call KVM_RUN >>>>>> + * on a VCPU that hasn't been properly setup. >>>>>> + * >>>>>> + */ >>>>>> + >>>>>> +#define _GNU_SOURCE >>>>>> +#include <fcntl.h> >>>>>> +#include <kvm_util.h> >>>>>> +#include <linux/kvm.h> >>>>>> +#include <processor.h> >>>>>> +#include <pthread.h> >>>>>> +#include <stdio.h> >>>>>> +#include <stdlib.h> >>>>>> +#include <string.h> >>>>>> +#include <sys/ioctl.h> >>>>>> +#include <sys/mman.h> >>>>>> +#include <sys/stat.h> >>>>>> +#include <sys/types.h> >>>>>> +#include <sys/wait.h> >>>>>> +#include <test_util.h> >>>>>> +#include <unistd.h> >>>>>> + >>>>>> +#define NTHREAD 4 >>>>>> +#define NPROCESS 5 >>>>>> + >>>>>> +struct thread_context { >>>>>> + int kvmcpu; >>>>>> + struct kvm_run *run; >>>>>> +}; >>>>>> + >>>>>> +void *thr(void *arg) >>>>>> +{ >>>>>> + struct thread_context *tc = (struct thread_context *)arg; >>>>>> + int res; >>>>>> + int kvmcpu = tc->kvmcpu; >>>>>> + struct kvm_run *run = tc->run; >>>>>> + >>>>>> + res = ioctl(kvmcpu, KVM_RUN, 0); >>>>>> + printf("ret1=%d exit_reason=%d suberror=%d\n", >>>>>> + res, run->exit_reason, run->internal.suberror); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +void test(void) >>>>>> +{ >>>>>> + int i, kvm, kvmvm, kvmcpu; >>>>>> + pthread_t th[NTHREAD]; >>>>>> + struct kvm_run *run; >>>>>> + struct thread_context tc; >>>>>> + >>>>>> + kvm = open("/dev/kvm", O_RDWR); >>>>>> + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); >>>>>> + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); >>>>>> + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); >>>>>> + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); >>>>>> + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); >>>>>> + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, >>>>>> + kvmcpu, 0); >>>>>> + tc.kvmcpu = kvmcpu; >>>>>> + tc.run = run; >>>>>> + srand(getpid()); >>>>>> + for (i = 0; i < NTHREAD; i++) { >>>>>> + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); >>>>>> + usleep(rand() % 10000); >>>>>> + } >>>>>> + for (i = 0; i < NTHREAD; i++) >>>>>> + pthread_join(th[i], NULL); >>>>>> +} >>>>>> + >>>>>> +int get_warnings_count(void) >>>>>> +{ >>>>>> + int warnings; >>>>>> + FILE *f; >>>>>> + >>>>>> + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); >>>>>> + fscanf(f, "%d", &warnings); >>>>>> + fclose(f); >>>>>> + >>>>>> + return warnings; >>>>>> +} >>>>>> + >>>>>> +int main(void) >>>>>> +{ >>>>>> + int warnings_before, warnings_after; >>>>>> + >>>>>> + if (!is_intel_cpu()) { >>>>>> + printf("Must be run on an Intel CPU, skipping test\n"); >>>>>> + exit(KSFT_SKIP); >>>>>> + } >>>>>> + >>>>>> + if (vm_is_unrestricted_guest(NULL)) { >>>>>> + printf("Unrestricted guest must be disabled, skipping test\n"); >>>>>> + exit(KSFT_SKIP); >>>>>> + } >>>>>> + >>>>>> + warnings_before = get_warnings_count(); >>>>>> + >>>>>> + for (int i = 0; i < NPROCESS; ++i) { >>>>>> + int status; >>>>>> + int pid = fork(); >>>>>> + >>>>>> + if (pid < 0) >>>>>> + exit(1); >>>>>> + if (pid == 0) { >>>>>> + test(); >>>>>> + exit(0); >>>>>> + } >>>>>> + while (waitpid(pid, &status, __WALL) != pid) >>>>>> + ; >>>>>> + } >>>>>> + >>>>>> + warnings_after = get_warnings_count(); >>>> Since you are grep'ing for the word "WARNING", is there a possibility >>>> that the test can detect a false positive based on Warnings generated >>>> due to some other cause while it ran ? >>>> >>> Yes, this is a possibility, however, it is still a warning and should >>> still be dealt with. We could special case the grep message to be >>> more specific to the case we are dealing with here, but I'd prefer to >>> keep it this way to alert on any warning. That way other warnings, >>> should they occur, are brought to our attention. >> >> OK. In that case, does it make sense to provide some additional info in >> the ASSERT message below ? dmesg can contain Warnings that may have >> occurred either before or after the run of this test and so we can at >> least link the false positives to this test. >> > I can go either way on this. While there can be false positives in > dmesg, if this asset fires there will be warnings worth looking at and > addressing as it only fires if it detects warnings occurred during the > test. That's the reason I made the asset message general and only say > to look at warnings. We could add a more long winded message to the > end of this informing the user that because the test doesn't clear > dmesg there could be warnings not related to the test. However, there > will be warnings caused by running this test as well. Yes, a little more info will certainly help in debugging. > >>>>>> + TEST_ASSERT(warnings_before == warnings_after, >>>>>> + "Warnings found in kernel. Run 'dmesg' to inspect them."); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> -- >>>>>> 2.22.0.rc1.311.g5d7573a151-goog >>>>>> >>>>> ping
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore index df1bf9230a74..41266af0d3dc 100644 --- a/tools/testing/selftests/kvm/.gitignore +++ b/tools/testing/selftests/kvm/.gitignore @@ -2,6 +2,7 @@ /x86_64/evmcs_test /x86_64/hyperv_cpuid /x86_64/kvm_create_max_vcpus +/x86_64/mmio_warning_test /x86_64/platform_info_test /x86_64/set_sregs_test /x86_64/smm_test diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 79c524395ebe..670b938f1049 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -22,6 +22,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test TEST_GEN_PROGS_x86_64 += x86_64/smm_test TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test +TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test TEST_GEN_PROGS_x86_64 += dirty_log_test TEST_GEN_PROGS_x86_64 += clear_dirty_log_test diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 8c6b9619797d..c5c427c86598 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -137,6 +137,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size, void *guest_code); void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); +bool vm_is_unrestricted_guest(struct kvm_vm *vm); + struct kvm_userspace_memory_region * kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start, uint64_t end); diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 6063d5b2f356..af4d26de32d1 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -303,6 +303,8 @@ static inline unsigned long get_xmm(int n) return 0; } +bool is_intel_cpu(void); + struct kvm_x86_state; struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid); void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index e9113857f44e..b93b09ad9a11 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1584,3 +1584,39 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva) { return addr_gpa2hva(vm, addr_gva2gpa(vm, gva)); } + +/* + * Is Unrestricted Guest + * + * Input Args: + * vm - Virtual Machine + * + * Output Args: None + * + * Return: True if the unrestricted guest is set to 'Y', otherwise return false. + * + * Check if the unrestricted guest flag is enabled. + */ +bool vm_is_unrestricted_guest(struct kvm_vm *vm) +{ + char val = 'N'; + size_t count; + FILE *f; + + if (vm == NULL) { + /* Ensure that the KVM vendor-specific module is loaded. */ + f = fopen(KVM_DEV_PATH, "r"); + TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", + errno); + fclose(f); + } + + f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); + if (f) { + count = fread(&val, sizeof(char), 1, f); + TEST_ASSERT(count == 1, "Unable to read from param file."); + fclose(f); + } + + return val == 'Y'; +} diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index dc7fae9fa424..bcc0e70e1856 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -1139,3 +1139,19 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s r); } } + +bool is_intel_cpu(void) +{ + int eax, ebx, ecx, edx; + const uint32_t *chunk; + const int leaf = 0; + + __asm__ __volatile__( + "cpuid" + : /* output */ "=a"(eax), "=b"(ebx), + "=c"(ecx), "=d"(edx) + : /* input */ "0"(leaf), "2"(0)); + + chunk = (const uint32_t *)("GenuineIntel"); + return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); +} diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c new file mode 100644 index 000000000000..00bb97d76000 --- /dev/null +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c @@ -0,0 +1,126 @@ +/* + * mmio_warning_test + * + * Copyright (C) 2019, Google LLC. + * + * This work is licensed under the terms of the GNU GPL, version 2. + * + * Test that we don't get a kernel warning when we call KVM_RUN after a + * triple fault occurs. To get the triple fault to occur we call KVM_RUN + * on a VCPU that hasn't been properly setup. + * + */ + +#define _GNU_SOURCE +#include <fcntl.h> +#include <kvm_util.h> +#include <linux/kvm.h> +#include <processor.h> +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/ioctl.h> +#include <sys/mman.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <test_util.h> +#include <unistd.h> + +#define NTHREAD 4 +#define NPROCESS 5 + +struct thread_context { + int kvmcpu; + struct kvm_run *run; +}; + +void *thr(void *arg) +{ + struct thread_context *tc = (struct thread_context *)arg; + int res; + int kvmcpu = tc->kvmcpu; + struct kvm_run *run = tc->run; + + res = ioctl(kvmcpu, KVM_RUN, 0); + printf("ret1=%d exit_reason=%d suberror=%d\n", + res, run->exit_reason, run->internal.suberror); + + return 0; +} + +void test(void) +{ + int i, kvm, kvmvm, kvmcpu; + pthread_t th[NTHREAD]; + struct kvm_run *run; + struct thread_context tc; + + kvm = open("/dev/kvm", O_RDWR); + TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); + kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); + TEST_ASSERT(kvmvm != -1, "KVM_CREATE_VM failed"); + kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); + TEST_ASSERT(kvmcpu != -1, "KVM_CREATE_VCPU failed"); + run = (struct kvm_run *)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, + kvmcpu, 0); + tc.kvmcpu = kvmcpu; + tc.run = run; + srand(getpid()); + for (i = 0; i < NTHREAD; i++) { + pthread_create(&th[i], NULL, thr, (void *)(uintptr_t)&tc); + usleep(rand() % 10000); + } + for (i = 0; i < NTHREAD; i++) + pthread_join(th[i], NULL); +} + +int get_warnings_count(void) +{ + int warnings; + FILE *f; + + f = popen("dmesg | grep \"WARNING:\" | wc -l", "r"); + fscanf(f, "%d", &warnings); + fclose(f); + + return warnings; +} + +int main(void) +{ + int warnings_before, warnings_after; + + if (!is_intel_cpu()) { + printf("Must be run on an Intel CPU, skipping test\n"); + exit(KSFT_SKIP); + } + + if (vm_is_unrestricted_guest(NULL)) { + printf("Unrestricted guest must be disabled, skipping test\n"); + exit(KSFT_SKIP); + } + + warnings_before = get_warnings_count(); + + for (int i = 0; i < NPROCESS; ++i) { + int status; + int pid = fork(); + + if (pid < 0) + exit(1); + if (pid == 0) { + test(); + exit(0); + } + while (waitpid(pid, &status, __WALL) != pid) + ; + } + + warnings_after = get_warnings_count(); + TEST_ASSERT(warnings_before == warnings_after, + "Warnings found in kernel. Run 'dmesg' to inspect them."); + + return 0; +}
When running with /sys/module/kvm_intel/parameters/unrestricted_guest=N, test that a kernel warning does not occur informing us that vcpu->mmio_needed=1. This can happen when KVM_RUN is called after a triple fault. This test was made to detect a bug that was reported by Syzkaller (https://groups.google.com/forum/#!topic/syzkaller/lHfau8E3SOE) and fixed with commit bbeac2830f4de ("KVM: X86: Fix residual mmio emulation request to userspace"). Signed-off-by: Aaron Lewis <aaronlewis@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Reviewed-by: Peter Shier <pshier@google.com> --- tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 1 + .../testing/selftests/kvm/include/kvm_util.h | 2 + .../selftests/kvm/include/x86_64/processor.h | 2 + tools/testing/selftests/kvm/lib/kvm_util.c | 36 +++++ .../selftests/kvm/lib/x86_64/processor.c | 16 +++ .../selftests/kvm/x86_64/mmio_warning_test.c | 126 ++++++++++++++++++ 7 files changed, 184 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c