* [PATCH] tests: kvm: Check for a kernel warning
@ 2019-05-31 14:14 Aaron Lewis
2019-06-18 14:13 ` Aaron Lewis
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Lewis @ 2019-05-31 14:14 UTC (permalink / raw)
To: jmattson, pshier, marcorr, kvm; +Cc: Aaron Lewis
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-05-31 14:14 [PATCH] tests: kvm: Check for a kernel warning Aaron Lewis
@ 2019-06-18 14:13 ` Aaron Lewis
2019-06-18 19:38 ` Krish Sadhukhan
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Lewis @ 2019-06-18 14:13 UTC (permalink / raw)
To: Jim Mattson, Peter Shier, Marc Orr, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-06-18 14:13 ` Aaron Lewis
@ 2019-06-18 19:38 ` Krish Sadhukhan
2019-06-20 14:12 ` Aaron Lewis
0 siblings, 1 reply; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-18 19:38 UTC (permalink / raw)
To: Aaron Lewis, Jim Mattson, Peter Shier, Marc Orr, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-06-18 19:38 ` Krish Sadhukhan
@ 2019-06-20 14:12 ` Aaron Lewis
2019-06-20 18:15 ` Krish Sadhukhan
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Lewis @ 2019-06-20 14:12 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: Jim Mattson, Peter Shier, Marc Orr, kvm
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-06-20 14:12 ` Aaron Lewis
@ 2019-06-20 18:15 ` Krish Sadhukhan
2019-06-20 19:47 ` Aaron Lewis
0 siblings, 1 reply; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-20 18:15 UTC (permalink / raw)
To: Aaron Lewis; +Cc: Jim Mattson, Peter Shier, Marc Orr, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-06-20 18:15 ` Krish Sadhukhan
@ 2019-06-20 19:47 ` Aaron Lewis
2019-06-21 17:13 ` Krish Sadhukhan
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Lewis @ 2019-06-20 19:47 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: Jim Mattson, Peter Shier, Marc Orr, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: kvm: Check for a kernel warning
2019-06-20 19:47 ` Aaron Lewis
@ 2019-06-21 17:13 ` Krish Sadhukhan
0 siblings, 0 replies; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-21 17:13 UTC (permalink / raw)
To: Aaron Lewis; +Cc: Jim Mattson, Peter Shier, Marc Orr, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-21 17:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31 14:14 [PATCH] tests: kvm: Check for a kernel warning Aaron Lewis
2019-06-18 14:13 ` Aaron Lewis
2019-06-18 19:38 ` Krish Sadhukhan
2019-06-20 14:12 ` Aaron Lewis
2019-06-20 18:15 ` Krish Sadhukhan
2019-06-20 19:47 ` Aaron Lewis
2019-06-21 17:13 ` Krish Sadhukhan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).