* [PATCH v8 1/7] KVM: selftests: Add missing break between -e and -g option in dirty_log_perf_test
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 2/7] KVM: selftests: Put command line options in alphabetical order " Vipin Sharma
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
Passing -e option (Run VCPUs while dirty logging is being disabled) in
dirty_log_perf_test also unintentionally enables -g (Do not enable
KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2). Add break between two switch case
logic.
Fixes: cfe12e64b065 ("KVM: selftests: Add an option to run vCPUs while disabling dirty logging")
Signed-off-by: Vipin Sharma <vipinsh@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/dirty_log_perf_test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index f99e39a672d3..56e08da3a87f 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -411,6 +411,7 @@ int main(int argc, char *argv[])
case 'e':
/* 'e' is for evil. */
run_vcpus_while_disabling_dirty_logging = true;
+ break;
case 'g':
dirty_log_manual_caps = 0;
break;
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v8 2/7] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 1/7] KVM: selftests: Add missing break between -e and -g option in dirty_log_perf_test Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 3/7] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi() Vipin Sharma
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
There are 13 command line options and they are not in any order. Put
them in alphabetical order to make it easy to add new options.
No functional change intended.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
Reviewed-by: Wei Wang <wei.w.wang@intel.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/dirty_log_perf_test.c | 36 ++++++++++---------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 56e08da3a87f..5bb6954b2358 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -406,50 +406,52 @@ int main(int argc, char *argv[])
guest_modes_append_default();
- while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
+ while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
switch (opt) {
+ case 'b':
+ guest_percpu_mem_size = parse_size(optarg);
+ break;
case 'e':
/* 'e' is for evil. */
run_vcpus_while_disabling_dirty_logging = true;
break;
+ case 'f':
+ p.wr_fract = atoi(optarg);
+ TEST_ASSERT(p.wr_fract >= 1,
+ "Write fraction cannot be less than one");
+ break;
case 'g':
dirty_log_manual_caps = 0;
break;
+ case 'h':
+ help(argv[0]);
+ break;
case 'i':
p.iterations = atoi(optarg);
break;
- case 'p':
- p.phys_offset = strtoull(optarg, NULL, 0);
- break;
case 'm':
guest_modes_cmdline(optarg);
break;
case 'n':
perf_test_args.nested = true;
break;
- case 'b':
- guest_percpu_mem_size = parse_size(optarg);
+ case 'o':
+ p.partition_vcpu_memory_access = false;
break;
- case 'f':
- p.wr_fract = atoi(optarg);
- TEST_ASSERT(p.wr_fract >= 1,
- "Write fraction cannot be less than one");
+ case 'p':
+ p.phys_offset = strtoull(optarg, NULL, 0);
+ break;
+ case 's':
+ p.backing_src = parse_backing_src_type(optarg);
break;
case 'v':
nr_vcpus = atoi(optarg);
TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
- case 'o':
- p.partition_vcpu_memory_access = false;
- break;
- case 's':
- p.backing_src = parse_backing_src_type(optarg);
- break;
case 'x':
p.slots = atoi(optarg);
break;
- case 'h':
default:
help(argv[0]);
break;
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v8 3/7] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi()
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 1/7] KVM: selftests: Add missing break between -e and -g option in dirty_log_perf_test Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 2/7] KVM: selftests: Put command line options in alphabetical order " Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-03 0:18 ` Sean Christopherson
2022-11-02 23:27 ` [PATCH v8 4/7] KVM: selftests: Use SZ_1G from sizes.h in max_guest_memory_test.c Vipin Sharma
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
atoi() doesn't detect errors. There is no way to know that a 0 return
is correct conversion or due to an error.
Introduce atoi_paranoid() to detect errors and provide correct
conversion. Replace all atoi() calls with atoi_paranoid().
Signed-off-by: Vipin Sharma <vipinsh@google.com>
Suggested-by: David Matlack <dmatlack@google.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/aarch64/arch_timer.c | 8 ++++----
.../selftests/kvm/aarch64/debug-exceptions.c | 2 +-
.../testing/selftests/kvm/aarch64/vgic_irq.c | 6 +++---
.../selftests/kvm/access_tracking_perf_test.c | 2 +-
.../selftests/kvm/demand_paging_test.c | 2 +-
.../selftests/kvm/dirty_log_perf_test.c | 8 ++++----
.../testing/selftests/kvm/include/test_util.h | 2 ++
.../selftests/kvm/kvm_page_table_test.c | 2 +-
tools/testing/selftests/kvm/lib/test_util.c | 19 +++++++++++++++++++
.../selftests/kvm/max_guest_memory_test.c | 6 +++---
.../kvm/memslot_modification_stress_test.c | 6 +++---
.../testing/selftests/kvm/memslot_perf_test.c | 10 +++++-----
.../selftests/kvm/set_memory_region_test.c | 2 +-
.../selftests/kvm/x86_64/nx_huge_pages_test.c | 4 ++--
14 files changed, 50 insertions(+), 29 deletions(-)
diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index 574eb73f0e90..251e7ff04883 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -414,7 +414,7 @@ static bool parse_args(int argc, char *argv[])
while ((opt = getopt(argc, argv, "hn:i:p:m:")) != -1) {
switch (opt) {
case 'n':
- test_args.nr_vcpus = atoi(optarg);
+ test_args.nr_vcpus = atoi_paranoid(optarg);
if (test_args.nr_vcpus <= 0) {
pr_info("Positive value needed for -n\n");
goto err;
@@ -425,21 +425,21 @@ static bool parse_args(int argc, char *argv[])
}
break;
case 'i':
- test_args.nr_iter = atoi(optarg);
+ test_args.nr_iter = atoi_paranoid(optarg);
if (test_args.nr_iter <= 0) {
pr_info("Positive value needed for -i\n");
goto err;
}
break;
case 'p':
- test_args.timer_period_ms = atoi(optarg);
+ test_args.timer_period_ms = atoi_paranoid(optarg);
if (test_args.timer_period_ms <= 0) {
pr_info("Positive value needed for -p\n");
goto err;
}
break;
case 'm':
- test_args.migration_freq_ms = atoi(optarg);
+ test_args.migration_freq_ms = atoi_paranoid(optarg);
if (test_args.migration_freq_ms < 0) {
pr_info("0 or positive value needed for -m\n");
goto err;
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 947bd201435c..19fffdf19c9f 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -423,7 +423,7 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "i:")) != -1) {
switch (opt) {
case 'i':
- ss_iteration = atoi(optarg);
+ ss_iteration = atoi_paranoid(optarg);
break;
case 'h':
default:
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
index 17417220a083..ae90b718070a 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
@@ -824,16 +824,16 @@ int main(int argc, char **argv)
while ((opt = getopt(argc, argv, "hn:e:l:")) != -1) {
switch (opt) {
case 'n':
- nr_irqs = atoi(optarg);
+ nr_irqs = atoi_paranoid(optarg);
if (nr_irqs > 1024 || nr_irqs % 32)
help(argv[0]);
break;
case 'e':
- eoi_split = (bool)atoi(optarg);
+ eoi_split = (bool)atoi_paranoid(optarg);
default_args = false;
break;
case 'l':
- level_sensitive = (bool)atoi(optarg);
+ level_sensitive = (bool)atoi_paranoid(optarg);
default_args = false;
break;
case 'h':
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index 76c583a07ea2..c6bcc5301e2c 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -368,7 +368,7 @@ int main(int argc, char *argv[])
params.vcpu_memory_bytes = parse_size(optarg);
break;
case 'v':
- params.nr_vcpus = atoi(optarg);
+ params.nr_vcpus = atoi_paranoid(optarg);
break;
case 'o':
overlap_memory_access = true;
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 779ae54f89c4..82597fb04146 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -427,7 +427,7 @@ int main(int argc, char *argv[])
p.src_type = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi(optarg);
+ nr_vcpus = atoi_paranoid(optarg);
TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 5bb6954b2358..ecda802b78ff 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -416,7 +416,7 @@ int main(int argc, char *argv[])
run_vcpus_while_disabling_dirty_logging = true;
break;
case 'f':
- p.wr_fract = atoi(optarg);
+ p.wr_fract = atoi_paranoid(optarg);
TEST_ASSERT(p.wr_fract >= 1,
"Write fraction cannot be less than one");
break;
@@ -427,7 +427,7 @@ int main(int argc, char *argv[])
help(argv[0]);
break;
case 'i':
- p.iterations = atoi(optarg);
+ p.iterations = atoi_paranoid(optarg);
break;
case 'm':
guest_modes_cmdline(optarg);
@@ -445,12 +445,12 @@ int main(int argc, char *argv[])
p.backing_src = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi(optarg);
+ nr_vcpus = atoi_paranoid(optarg);
TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 'x':
- p.slots = atoi(optarg);
+ p.slots = atoi_paranoid(optarg);
break;
default:
help(argv[0]);
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index befc754ce9b3..feae42863759 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -152,4 +152,6 @@ static inline void *align_ptr_up(void *x, size_t size)
return (void *)align_up((unsigned long)x, size);
}
+int atoi_paranoid(const char *num_str);
+
#endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
index f42c6ac6d71d..ea7feb69bb88 100644
--- a/tools/testing/selftests/kvm/kvm_page_table_test.c
+++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
@@ -461,7 +461,7 @@ int main(int argc, char *argv[])
p.test_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi(optarg);
+ nr_vcpus = atoi_paranoid(optarg);
TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 6d23878bbfe1..c2d9c6827779 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -334,3 +334,22 @@ long get_run_delay(void)
return val[1];
}
+
+int atoi_paranoid(const char *num_str)
+{
+ char *end_ptr;
+ long num;
+
+ errno = 0;
+ num = strtol(num_str, &end_ptr, 0);
+ TEST_ASSERT(!errno, "strtol(\"%s\") failed", num_str);
+ TEST_ASSERT(num_str != end_ptr,
+ "strtol(\"%s\") didn't find a valid integer.", num_str);
+ TEST_ASSERT(*end_ptr == '\0',
+ "strtol(\"%s\") failed to parse trailing characters \"%s\".",
+ num_str, end_ptr);
+ TEST_ASSERT(num >= INT_MIN && num <= INT_MAX,
+ "%ld not in range of [%d, %d]", num, INT_MIN, INT_MAX);
+
+ return num;
+}
diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/max_guest_memory_test.c
index 9a6e4f3ad6b5..1595b73dc09a 100644
--- a/tools/testing/selftests/kvm/max_guest_memory_test.c
+++ b/tools/testing/selftests/kvm/max_guest_memory_test.c
@@ -193,15 +193,15 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "c:h:m:s:H")) != -1) {
switch (opt) {
case 'c':
- nr_vcpus = atoi(optarg);
+ nr_vcpus = atoi_paranoid(optarg);
TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
break;
case 'm':
- max_mem = atoi(optarg) * size_1gb;
+ max_mem = atoi_paranoid(optarg) * size_1gb;
TEST_ASSERT(max_mem > 0, "memory size must be >0");
break;
case 's':
- slot_size = atoi(optarg) * size_1gb;
+ slot_size = atoi_paranoid(optarg) * size_1gb;
TEST_ASSERT(slot_size > 0, "slot size must be >0");
break;
case 'H':
diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
index 6ee7e1dde404..d7ddc8a105a2 100644
--- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
+++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
@@ -158,7 +158,7 @@ int main(int argc, char *argv[])
guest_modes_cmdline(optarg);
break;
case 'd':
- p.memslot_modification_delay = strtoul(optarg, NULL, 0);
+ p.memslot_modification_delay = atoi_paranoid(optarg);
TEST_ASSERT(p.memslot_modification_delay >= 0,
"A negative delay is not supported.");
break;
@@ -166,7 +166,7 @@ int main(int argc, char *argv[])
guest_percpu_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi(optarg);
+ nr_vcpus = atoi_paranoid(optarg);
TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d",
max_vcpus);
@@ -175,7 +175,7 @@ int main(int argc, char *argv[])
p.partition_vcpu_memory_access = false;
break;
case 'i':
- p.nr_memslot_modifications = atoi(optarg);
+ p.nr_memslot_modifications = atoi_paranoid(optarg);
break;
case 'h':
default:
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
index 44995446d942..4bae9e3f5ca1 100644
--- a/tools/testing/selftests/kvm/memslot_perf_test.c
+++ b/tools/testing/selftests/kvm/memslot_perf_test.c
@@ -885,21 +885,21 @@ static bool parse_args(int argc, char *argv[],
map_unmap_verify = true;
break;
case 's':
- targs->nslots = atoi(optarg);
+ targs->nslots = atoi_paranoid(optarg);
if (targs->nslots <= 0 && targs->nslots != -1) {
pr_info("Slot count cap has to be positive or -1 for no cap\n");
return false;
}
break;
case 'f':
- targs->tfirst = atoi(optarg);
+ targs->tfirst = atoi_paranoid(optarg);
if (targs->tfirst < 0) {
pr_info("First test to run has to be non-negative\n");
return false;
}
break;
case 'e':
- targs->tlast = atoi(optarg);
+ targs->tlast = atoi_paranoid(optarg);
if (targs->tlast < 0 || targs->tlast >= NTESTS) {
pr_info("Last test to run has to be non-negative and less than %zu\n",
NTESTS);
@@ -907,14 +907,14 @@ static bool parse_args(int argc, char *argv[],
}
break;
case 'l':
- targs->seconds = atoi(optarg);
+ targs->seconds = atoi_paranoid(optarg);
if (targs->seconds < 0) {
pr_info("Test length in seconds has to be non-negative\n");
return false;
}
break;
case 'r':
- targs->runs = atoi(optarg);
+ targs->runs = atoi_paranoid(optarg);
if (targs->runs <= 0) {
pr_info("Runs per test has to be positive\n");
return false;
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 0d55f508d595..c366949c8362 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -407,7 +407,7 @@ int main(int argc, char *argv[])
#ifdef __x86_64__
if (argc > 1)
- loops = atoi(argv[1]);
+ loops = atoi_paranoid(argv[1]);
else
loops = 10;
diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
index 59ffe7fd354f..354b6902849c 100644
--- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
+++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
@@ -241,10 +241,10 @@ int main(int argc, char **argv)
while ((opt = getopt(argc, argv, "hp:t:r")) != -1) {
switch (opt) {
case 'p':
- reclaim_period_ms = atoi(optarg);
+ reclaim_period_ms = atoi_paranoid(optarg);
break;
case 't':
- token = atoi(optarg);
+ token = atoi_paranoid(optarg);
break;
case 'r':
reboot_permissions = true;
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 3/7] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi()
2022-11-02 23:27 ` [PATCH v8 3/7] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi() Vipin Sharma
@ 2022-11-03 0:18 ` Sean Christopherson
0 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2022-11-03 0:18 UTC (permalink / raw)
To: Vipin Sharma
Cc: pbonzini, dmatlack, andrew.jones, wei.w.wang, kvm, linux-kernel
On Wed, Nov 02, 2022, Vipin Sharma wrote:
> atoi() doesn't detect errors. There is no way to know that a 0 return
> is correct conversion or due to an error.
>
> Introduce atoi_paranoid() to detect errors and provide correct
> conversion. Replace all atoi() calls with atoi_paranoid().
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> Suggested-by: David Matlack <dmatlack@google.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 4/7] KVM: selftests: Use SZ_1G from sizes.h in max_guest_memory_test.c
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
` (2 preceding siblings ...)
2022-11-02 23:27 ` [PATCH v8 3/7] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi() Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-02 23:44 ` Sean Christopherson
2022-11-02 23:27 ` [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c Vipin Sharma
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
Replace size_1gb defined in max_guest_memory_test.c with the SZ_1G from
linux/sizes.h header file.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/max_guest_memory_test.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/max_guest_memory_test.c
index 1595b73dc09a..8056dc5831b5 100644
--- a/tools/testing/selftests/kvm/max_guest_memory_test.c
+++ b/tools/testing/selftests/kvm/max_guest_memory_test.c
@@ -11,6 +11,7 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/atomic.h>
+#include <linux/sizes.h>
#include "kvm_util.h"
#include "test_util.h"
@@ -162,8 +163,7 @@ int main(int argc, char *argv[])
* just below the 4gb boundary. This test could create memory at
* 1gb-3gb,but it's simpler to skip straight to 4gb.
*/
- const uint64_t size_1gb = (1 << 30);
- const uint64_t start_gpa = (4ull * size_1gb);
+ const uint64_t start_gpa = SZ_4G;
const int first_slot = 1;
struct timespec time_start, time_run1, time_reset, time_run2;
@@ -180,13 +180,13 @@ int main(int argc, char *argv[])
* are quite common for x86, requires changing only max_mem (KVM allows
* 32k memslots, 32k * 2gb == ~64tb of guest memory).
*/
- slot_size = 2 * size_1gb;
+ slot_size = SZ_2G;
max_slots = kvm_check_cap(KVM_CAP_NR_MEMSLOTS);
TEST_ASSERT(max_slots > first_slot, "KVM is broken");
/* All KVM MMUs should be able to survive a 128gb guest. */
- max_mem = 128 * size_1gb;
+ max_mem = 128ull * SZ_1G;
calc_default_nr_vcpus();
@@ -197,11 +197,11 @@ int main(int argc, char *argv[])
TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
break;
case 'm':
- max_mem = atoi_paranoid(optarg) * size_1gb;
+ max_mem = 1ull * atoi_paranoid(optarg) * SZ_1G;
TEST_ASSERT(max_mem > 0, "memory size must be >0");
break;
case 's':
- slot_size = atoi_paranoid(optarg) * size_1gb;
+ slot_size = 1ull * atoi_paranoid(optarg) * SZ_1G;
TEST_ASSERT(slot_size > 0, "slot size must be >0");
break;
case 'H':
@@ -245,7 +245,7 @@ int main(int argc, char *argv[])
#ifdef __x86_64__
/* Identity map memory in the guest using 1gb pages. */
- for (i = 0; i < slot_size; i += size_1gb)
+ for (i = 0; i < slot_size; i += SZ_1G)
__virt_pg_map(vm, gpa + i, gpa + i, PG_LEVEL_1G);
#else
for (i = 0; i < slot_size; i += vm->page_size)
@@ -260,7 +260,7 @@ int main(int argc, char *argv[])
vcpus = NULL;
pr_info("Running with %lugb of guest memory and %u vCPUs\n",
- (gpa - start_gpa) / size_1gb, nr_vcpus);
+ (gpa - start_gpa) / SZ_1G, nr_vcpus);
rendezvous_with_vcpus(&time_start, "spawning");
rendezvous_with_vcpus(&time_run1, "run 1");
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
` (3 preceding siblings ...)
2022-11-02 23:27 ` [PATCH v8 4/7] KVM: selftests: Use SZ_1G from sizes.h in max_guest_memory_test.c Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-02 23:45 ` Sean Christopherson
2022-11-02 23:27 ` [PATCH v8 6/7] KVM: selftests: Add atoi_positive() and atoi_non_negative() for input validation Vipin Sharma
2022-11-02 23:27 ` [PATCH v8 7/7] KVM: selftests: Allowing running dirty_log_perf_test on specific CPUs Vipin Sharma
6 siblings, 1 reply; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
Change test args memslot_modification_delay and nr_memslot_modifications
to delay and nr_iterations for simplicity.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../kvm/memslot_modification_stress_test.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
index d7ddc8a105a2..d6089ccaa484 100644
--- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
+++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
@@ -87,8 +87,8 @@ static void add_remove_memslot(struct kvm_vm *vm, useconds_t delay,
}
struct test_params {
- useconds_t memslot_modification_delay;
- uint64_t nr_memslot_modifications;
+ useconds_t delay;
+ uint64_t nr_iterations;
bool partition_vcpu_memory_access;
};
@@ -107,8 +107,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
pr_info("Started all vCPUs\n");
- add_remove_memslot(vm, p->memslot_modification_delay,
- p->nr_memslot_modifications);
+ add_remove_memslot(vm, p->delay,
+ p->nr_iterations);
run_vcpus = false;
@@ -144,8 +144,8 @@ int main(int argc, char *argv[])
int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
int opt;
struct test_params p = {
- .memslot_modification_delay = 0,
- .nr_memslot_modifications =
+ .delay = 0,
+ .nr_iterations =
DEFAULT_MEMSLOT_MODIFICATION_ITERATIONS,
.partition_vcpu_memory_access = true
};
@@ -158,8 +158,8 @@ int main(int argc, char *argv[])
guest_modes_cmdline(optarg);
break;
case 'd':
- p.memslot_modification_delay = atoi_paranoid(optarg);
- TEST_ASSERT(p.memslot_modification_delay >= 0,
+ p.delay = atoi_paranoid(optarg);
+ TEST_ASSERT(p.delay >= 0,
"A negative delay is not supported.");
break;
case 'b':
@@ -175,7 +175,7 @@ int main(int argc, char *argv[])
p.partition_vcpu_memory_access = false;
break;
case 'i':
- p.nr_memslot_modifications = atoi_paranoid(optarg);
+ p.nr_iterations = atoi_paranoid(optarg);
break;
case 'h':
default:
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c
2022-11-02 23:27 ` [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c Vipin Sharma
@ 2022-11-02 23:45 ` Sean Christopherson
2022-11-03 0:16 ` Sean Christopherson
0 siblings, 1 reply; 14+ messages in thread
From: Sean Christopherson @ 2022-11-02 23:45 UTC (permalink / raw)
To: Vipin Sharma
Cc: pbonzini, dmatlack, andrew.jones, wei.w.wang, kvm, linux-kernel
On Wed, Nov 02, 2022, Vipin Sharma wrote:
> Change test args memslot_modification_delay and nr_memslot_modifications
> to delay and nr_iterations for simplicity.
Suggested-by: Sean Christopherson <seanjc@google.com>
I don't care about the credit so much as I don't want you getting yelled at for
one of my random ideas :-)
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
> .../kvm/memslot_modification_stress_test.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> index d7ddc8a105a2..d6089ccaa484 100644
> --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> @@ -87,8 +87,8 @@ static void add_remove_memslot(struct kvm_vm *vm, useconds_t delay,
> }
>
> struct test_params {
> - useconds_t memslot_modification_delay;
> - uint64_t nr_memslot_modifications;
> + useconds_t delay;
> + uint64_t nr_iterations;
> bool partition_vcpu_memory_access;
> };
>
> @@ -107,8 +107,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>
> pr_info("Started all vCPUs\n");
>
> - add_remove_memslot(vm, p->memslot_modification_delay,
> - p->nr_memslot_modifications);
> + add_remove_memslot(vm, p->delay,
> + p->nr_iterations);
This wrap is no longer necessary (which was part of the motivation for the
rename).
> run_vcpus = false;
>
> @@ -144,8 +144,8 @@ int main(int argc, char *argv[])
> int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> int opt;
> struct test_params p = {
> - .memslot_modification_delay = 0,
> - .nr_memslot_modifications =
> + .delay = 0,
> + .nr_iterations =
> DEFAULT_MEMSLOT_MODIFICATION_ITERATIONS,
> .partition_vcpu_memory_access = true
> };
> @@ -158,8 +158,8 @@ int main(int argc, char *argv[])
> guest_modes_cmdline(optarg);
> break;
> case 'd':
> - p.memslot_modification_delay = atoi_paranoid(optarg);
> - TEST_ASSERT(p.memslot_modification_delay >= 0,
> + p.delay = atoi_paranoid(optarg);
> + TEST_ASSERT(p.delay >= 0,
> "A negative delay is not supported.");
> break;
> case 'b':
> @@ -175,7 +175,7 @@ int main(int argc, char *argv[])
> p.partition_vcpu_memory_access = false;
> break;
> case 'i':
> - p.nr_memslot_modifications = atoi_paranoid(optarg);
> + p.nr_iterations = atoi_paranoid(optarg);
> break;
> case 'h':
> default:
> --
> 2.38.1.273.g43a17bfeac-goog
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c
2022-11-02 23:45 ` Sean Christopherson
@ 2022-11-03 0:16 ` Sean Christopherson
2022-11-03 18:56 ` Vipin Sharma
0 siblings, 1 reply; 14+ messages in thread
From: Sean Christopherson @ 2022-11-03 0:16 UTC (permalink / raw)
To: Vipin Sharma
Cc: pbonzini, dmatlack, andrew.jones, wei.w.wang, kvm, linux-kernel
On Wed, Nov 02, 2022, Sean Christopherson wrote:
> On Wed, Nov 02, 2022, Vipin Sharma wrote:
> > Change test args memslot_modification_delay and nr_memslot_modifications
> > to delay and nr_iterations for simplicity.
>
> Suggested-by: Sean Christopherson <seanjc@google.com>
>
> I don't care about the credit so much as I don't want you getting yelled at for
> one of my random ideas :-)
>
> > Signed-off-by: Vipin Sharma <vipinsh@google.com>
> > ---
> > .../kvm/memslot_modification_stress_test.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > index d7ddc8a105a2..d6089ccaa484 100644
> > --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > @@ -87,8 +87,8 @@ static void add_remove_memslot(struct kvm_vm *vm, useconds_t delay,
> > }
> >
> > struct test_params {
> > - useconds_t memslot_modification_delay;
> > - uint64_t nr_memslot_modifications;
> > + useconds_t delay;
> > + uint64_t nr_iterations;
> > bool partition_vcpu_memory_access;
> > };
> >
> > @@ -107,8 +107,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> >
> > pr_info("Started all vCPUs\n");
> >
> > - add_remove_memslot(vm, p->memslot_modification_delay,
> > - p->nr_memslot_modifications);
> > + add_remove_memslot(vm, p->delay,
> > + p->nr_iterations);
>
> This wrap is no longer necessary (which was part of the motivation for the
> rename).
Almost forgot. Nit aside,
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c
2022-11-03 0:16 ` Sean Christopherson
@ 2022-11-03 18:56 ` Vipin Sharma
0 siblings, 0 replies; 14+ messages in thread
From: Vipin Sharma @ 2022-11-03 18:56 UTC (permalink / raw)
To: Sean Christopherson
Cc: pbonzini, dmatlack, andrew.jones, wei.w.wang, kvm, linux-kernel
On Wed, Nov 2, 2022 at 5:16 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Nov 02, 2022, Sean Christopherson wrote:
> > On Wed, Nov 02, 2022, Vipin Sharma wrote:
> > > Change test args memslot_modification_delay and nr_memslot_modifications
> > > to delay and nr_iterations for simplicity.
> >
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> >
> > I don't care about the credit so much as I don't want you getting yelled at for
> > one of my random ideas :-)
> >
> > > Signed-off-by: Vipin Sharma <vipinsh@google.com>
> > > ---
> > > .../kvm/memslot_modification_stress_test.c | 18 +++++++++---------
> > > 1 file changed, 9 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > > index d7ddc8a105a2..d6089ccaa484 100644
> > > --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > > +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> > > @@ -87,8 +87,8 @@ static void add_remove_memslot(struct kvm_vm *vm, useconds_t delay,
> > > }
> > >
> > > struct test_params {
> > > - useconds_t memslot_modification_delay;
> > > - uint64_t nr_memslot_modifications;
> > > + useconds_t delay;
> > > + uint64_t nr_iterations;
> > > bool partition_vcpu_memory_access;
> > > };
> > >
> > > @@ -107,8 +107,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> > >
> > > pr_info("Started all vCPUs\n");
> > >
> > > - add_remove_memslot(vm, p->memslot_modification_delay,
> > > - p->nr_memslot_modifications);
> > > + add_remove_memslot(vm, p->delay,
> > > + p->nr_iterations);
> >
> > This wrap is no longer necessary (which was part of the motivation for the
> > rename).
>
> Almost forgot. Nit aside,
>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
I will update in v9.
Thanks
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 6/7] KVM: selftests: Add atoi_positive() and atoi_non_negative() for input validation
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
` (4 preceding siblings ...)
2022-11-02 23:27 ` [PATCH v8 5/7] KVM: selftests: Shorten the test args in memslot_modification_stress_test.c Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
2022-11-03 0:17 ` Sean Christopherson
2022-11-02 23:27 ` [PATCH v8 7/7] KVM: selftests: Allowing running dirty_log_perf_test on specific CPUs Vipin Sharma
6 siblings, 1 reply; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
Many KVM selftests take command line arguments which are supposed to be
positive (>0) or non-negative (>=0). Some tests do these validation and
some missed adding the check.
Add atoi_positive() and atoi_non_negative() to validate inputs in
selftests before proceeding to use those values.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/aarch64/arch_timer.c | 25 ++++---------------
.../selftests/kvm/aarch64/debug-exceptions.c | 2 +-
.../testing/selftests/kvm/aarch64/vgic_irq.c | 2 +-
.../selftests/kvm/access_tracking_perf_test.c | 2 +-
.../selftests/kvm/demand_paging_test.c | 4 +--
.../selftests/kvm/dirty_log_perf_test.c | 12 ++++-----
.../testing/selftests/kvm/include/test_util.h | 16 ++++++++++++
.../selftests/kvm/kvm_page_table_test.c | 4 +--
.../selftests/kvm/max_guest_memory_test.c | 9 +++----
.../kvm/memslot_modification_stress_test.c | 10 +++-----
.../testing/selftests/kvm/memslot_perf_test.c | 22 ++++------------
.../selftests/kvm/set_memory_region_test.c | 2 +-
.../selftests/kvm/x86_64/nx_huge_pages_test.c | 3 +--
13 files changed, 47 insertions(+), 66 deletions(-)
diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index 251e7ff04883..9409617fce9c 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -414,36 +414,21 @@ static bool parse_args(int argc, char *argv[])
while ((opt = getopt(argc, argv, "hn:i:p:m:")) != -1) {
switch (opt) {
case 'n':
- test_args.nr_vcpus = atoi_paranoid(optarg);
- if (test_args.nr_vcpus <= 0) {
- pr_info("Positive value needed for -n\n");
- goto err;
- } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+ test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+ if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
pr_info("Max allowed vCPUs: %u\n",
KVM_MAX_VCPUS);
goto err;
}
break;
case 'i':
- test_args.nr_iter = atoi_paranoid(optarg);
- if (test_args.nr_iter <= 0) {
- pr_info("Positive value needed for -i\n");
- goto err;
- }
+ test_args.nr_iter = atoi_positive("Number of iterations", optarg);
break;
case 'p':
- test_args.timer_period_ms = atoi_paranoid(optarg);
- if (test_args.timer_period_ms <= 0) {
- pr_info("Positive value needed for -p\n");
- goto err;
- }
+ test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
break;
case 'm':
- test_args.migration_freq_ms = atoi_paranoid(optarg);
- if (test_args.migration_freq_ms < 0) {
- pr_info("0 or positive value needed for -m\n");
- goto err;
- }
+ test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
break;
case 'h':
default:
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 19fffdf19c9f..878c334607e1 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -423,7 +423,7 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "i:")) != -1) {
switch (opt) {
case 'i':
- ss_iteration = atoi_paranoid(optarg);
+ ss_iteration = atoi_positive("Number of iterations", optarg);
break;
case 'h':
default:
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
index ae90b718070a..4ead42a072b7 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
@@ -824,7 +824,7 @@ int main(int argc, char **argv)
while ((opt = getopt(argc, argv, "hn:e:l:")) != -1) {
switch (opt) {
case 'n':
- nr_irqs = atoi_paranoid(optarg);
+ nr_irqs = atoi_non_negative("Number of IRQs", optarg);
if (nr_irqs > 1024 || nr_irqs % 32)
help(argv[0]);
break;
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index c6bcc5301e2c..a81e7a7ae18f 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -368,7 +368,7 @@ int main(int argc, char *argv[])
params.vcpu_memory_bytes = parse_size(optarg);
break;
case 'v':
- params.nr_vcpus = atoi_paranoid(optarg);
+ params.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
break;
case 'o':
overlap_memory_access = true;
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 82597fb04146..0c98181fa248 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -427,8 +427,8 @@ int main(int argc, char *argv[])
p.src_type = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 'o':
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index ecda802b78ff..4d639683b8ef 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -416,9 +416,7 @@ int main(int argc, char *argv[])
run_vcpus_while_disabling_dirty_logging = true;
break;
case 'f':
- p.wr_fract = atoi_paranoid(optarg);
- TEST_ASSERT(p.wr_fract >= 1,
- "Write fraction cannot be less than one");
+ p.wr_fract = atoi_positive("Write fraction", optarg);
break;
case 'g':
dirty_log_manual_caps = 0;
@@ -427,7 +425,7 @@ int main(int argc, char *argv[])
help(argv[0]);
break;
case 'i':
- p.iterations = atoi_paranoid(optarg);
+ p.iterations = atoi_positive("Number of iterations", optarg);
break;
case 'm':
guest_modes_cmdline(optarg);
@@ -445,12 +443,12 @@ int main(int argc, char *argv[])
p.backing_src = parse_backing_src_type(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 'x':
- p.slots = atoi_paranoid(optarg);
+ p.slots = atoi_positive("Number of slots", optarg);
break;
default:
help(argv[0]);
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index feae42863759..3be98e81189a 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -154,4 +154,20 @@ static inline void *align_ptr_up(void *x, size_t size)
int atoi_paranoid(const char *num_str);
+static inline uint32_t atoi_positive(const char *name, const char *num_str)
+{
+ int num = atoi_paranoid(num_str);
+
+ TEST_ASSERT(num > 0, "%s must be greater than 0, got '%s'", name, num_str);
+ return num;
+}
+
+static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
+{
+ int num = atoi_paranoid(num_str);
+
+ TEST_ASSERT(num >= 0, "%s must be non-negative, got '%s'", name, num_str);
+ return num;
+}
+
#endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
index ea7feb69bb88..696b366be06b 100644
--- a/tools/testing/selftests/kvm/kvm_page_table_test.c
+++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
@@ -461,8 +461,8 @@ int main(int argc, char *argv[])
p.test_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d", max_vcpus);
break;
case 's':
diff --git a/tools/testing/selftests/kvm/max_guest_memory_test.c b/tools/testing/selftests/kvm/max_guest_memory_test.c
index 8056dc5831b5..feaf2be20ff2 100644
--- a/tools/testing/selftests/kvm/max_guest_memory_test.c
+++ b/tools/testing/selftests/kvm/max_guest_memory_test.c
@@ -193,16 +193,13 @@ int main(int argc, char *argv[])
while ((opt = getopt(argc, argv, "c:h:m:s:H")) != -1) {
switch (opt) {
case 'c':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0, "number of vcpus must be >0");
+ nr_vcpus = atoi_positive("Number of vCPUs", optarg);
break;
case 'm':
- max_mem = 1ull * atoi_paranoid(optarg) * SZ_1G;
- TEST_ASSERT(max_mem > 0, "memory size must be >0");
+ max_mem = 1ull * atoi_positive("Memory size", optarg) * SZ_1G;
break;
case 's':
- slot_size = 1ull * atoi_paranoid(optarg) * SZ_1G;
- TEST_ASSERT(slot_size > 0, "slot size must be >0");
+ slot_size = 1ull * atoi_positive("Slot size", optarg) * SZ_1G;
break;
case 'H':
hugepages = true;
diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
index d6089ccaa484..c3b7c94aec8f 100644
--- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
+++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
@@ -158,16 +158,14 @@ int main(int argc, char *argv[])
guest_modes_cmdline(optarg);
break;
case 'd':
- p.delay = atoi_paranoid(optarg);
- TEST_ASSERT(p.delay >= 0,
- "A negative delay is not supported.");
+ p.delay = atoi_non_negative("Delay", optarg);
break;
case 'b':
guest_percpu_mem_size = parse_size(optarg);
break;
case 'v':
- nr_vcpus = atoi_paranoid(optarg);
- TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+ nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+ TEST_ASSERT(nr_vcpus <= max_vcpus,
"Invalid number of vcpus, must be between 1 and %d",
max_vcpus);
break;
@@ -175,7 +173,7 @@ int main(int argc, char *argv[])
p.partition_vcpu_memory_access = false;
break;
case 'i':
- p.nr_iterations = atoi_paranoid(optarg);
+ p.nr_iterations = atoi_positive("Number of iterations", optarg);
break;
case 'h':
default:
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
index 4bae9e3f5ca1..330aaef1c02f 100644
--- a/tools/testing/selftests/kvm/memslot_perf_test.c
+++ b/tools/testing/selftests/kvm/memslot_perf_test.c
@@ -892,33 +892,21 @@ static bool parse_args(int argc, char *argv[],
}
break;
case 'f':
- targs->tfirst = atoi_paranoid(optarg);
- if (targs->tfirst < 0) {
- pr_info("First test to run has to be non-negative\n");
- return false;
- }
+ targs->tfirst = atoi_non_negative("First test", optarg);
break;
case 'e':
- targs->tlast = atoi_paranoid(optarg);
- if (targs->tlast < 0 || targs->tlast >= NTESTS) {
+ targs->tlast = atoi_non_negative("Last test", optarg);
+ if (targs->tlast >= NTESTS) {
pr_info("Last test to run has to be non-negative and less than %zu\n",
NTESTS);
return false;
}
break;
case 'l':
- targs->seconds = atoi_paranoid(optarg);
- if (targs->seconds < 0) {
- pr_info("Test length in seconds has to be non-negative\n");
- return false;
- }
+ targs->seconds = atoi_non_negative("Test length", optarg);
break;
case 'r':
- targs->runs = atoi_paranoid(optarg);
- if (targs->runs <= 0) {
- pr_info("Runs per test has to be positive\n");
- return false;
- }
+ targs->runs = atoi_positive("Runs per test", optarg);
break;
}
}
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index c366949c8362..85c16f09a50e 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -407,7 +407,7 @@ int main(int argc, char *argv[])
#ifdef __x86_64__
if (argc > 1)
- loops = atoi_paranoid(argv[1]);
+ loops = atoi_positive("Number of iterations", argv[1]);
else
loops = 10;
diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
index 354b6902849c..ea0978f22db8 100644
--- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
+++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
@@ -241,7 +241,7 @@ int main(int argc, char **argv)
while ((opt = getopt(argc, argv, "hp:t:r")) != -1) {
switch (opt) {
case 'p':
- reclaim_period_ms = atoi_paranoid(optarg);
+ reclaim_period_ms = atoi_non_negative("Reclaim period", optarg);
break;
case 't':
token = atoi_paranoid(optarg);
@@ -257,7 +257,6 @@ int main(int argc, char **argv)
}
TEST_REQUIRE(kvm_has_cap(KVM_CAP_VM_DISABLE_NX_HUGE_PAGES));
- TEST_REQUIRE(reclaim_period_ms > 0);
__TEST_REQUIRE(token == MAGIC_TOKEN,
"This test must be run with the magic token %d.\n"
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v8 7/7] KVM: selftests: Allowing running dirty_log_perf_test on specific CPUs
2022-11-02 23:27 [PATCH v8 0/7] dirty_log_perf_test vCPU pinning Vipin Sharma
` (5 preceding siblings ...)
2022-11-02 23:27 ` [PATCH v8 6/7] KVM: selftests: Add atoi_positive() and atoi_non_negative() for input validation Vipin Sharma
@ 2022-11-02 23:27 ` Vipin Sharma
6 siblings, 0 replies; 14+ messages in thread
From: Vipin Sharma @ 2022-11-02 23:27 UTC (permalink / raw)
To: seanjc, pbonzini, dmatlack
Cc: andrew.jones, wei.w.wang, kvm, linux-kernel, Vipin Sharma
Add a command line option, -c, to pin vCPUs to physical CPUs (pCPUs),
i.e. to force vCPUs to run on specific pCPUs.
Requirement to implement this feature came in discussion on the patch
"Make page tables for eager page splitting NUMA aware"
https://lore.kernel.org/lkml/YuhPT2drgqL+osLl@google.com/
This feature is useful as it provides a way to analyze performance based
on the vCPUs and dirty log worker locations, like on the different NUMA
nodes or on the same NUMA nodes.
To keep things simple, implementation is intentionally very limited,
either all of the vCPUs will be pinned followed by an optional main
thread or nothing will be pinned.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
Suggested-by: David Matlack <dmatlack@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/dirty_log_perf_test.c | 25 ++++++++-
.../selftests/kvm/include/kvm_util_base.h | 4 ++
.../selftests/kvm/include/perf_test_util.h | 4 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 54 +++++++++++++++++++
.../selftests/kvm/lib/perf_test_util.c | 8 ++-
5 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 4d639683b8ef..0612158329aa 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -353,7 +353,7 @@ static void help(char *name)
puts("");
printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
"[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]"
- "[-x memslots]\n", name);
+ "[-x memslots] [-c physical cpus to run test on]\n", name);
puts("");
printf(" -i: specify iteration counts (default: %"PRIu64")\n",
TEST_HOST_LOOP_N);
@@ -383,6 +383,17 @@ static void help(char *name)
backing_src_help("-s");
printf(" -x: Split the memory region into this number of memslots.\n"
" (default: 1)\n");
+ printf(" -c: Pin tasks to physical CPUs. Takes a list of comma separated\n"
+ " values (target pCPU), one for each vCPU, plus an optional\n"
+ " entry for the main application task (specified via entry\n"
+ " <nr_vcpus + 1>). If used, entries must be provided for all\n"
+ " vCPUs, i.e. pinning vCPUs is all or nothing.\n\n"
+ " E.g. to create 3 vCPUs, pin vCPU0=>pCPU22, vCPU1=>pCPU23,\n"
+ " vCPU2=>pCPU24, and pin the application task to pCPU50:\n\n"
+ " ./dirty_log_perf_test -v 3 -c 22,23,24,50\n\n"
+ " To leave the application task unpinned, drop the final entry:\n\n"
+ " ./dirty_log_perf_test -v 3 -c 22,23,24\n\n"
+ " (default: no pinning)\n");
puts("");
exit(0);
}
@@ -390,6 +401,7 @@ static void help(char *name)
int main(int argc, char *argv[])
{
int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
+ const char *pcpu_list = NULL;
struct test_params p = {
.iterations = TEST_HOST_LOOP_N,
.wr_fract = 1,
@@ -406,11 +418,14 @@ int main(int argc, char *argv[])
guest_modes_append_default();
- while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
+ while ((opt = getopt(argc, argv, "b:c:ef:ghi:m:nop:s:v:x:")) != -1) {
switch (opt) {
case 'b':
guest_percpu_mem_size = parse_size(optarg);
break;
+ case 'c':
+ pcpu_list = optarg;
+ break;
case 'e':
/* 'e' is for evil. */
run_vcpus_while_disabling_dirty_logging = true;
@@ -456,6 +471,12 @@ int main(int argc, char *argv[])
}
}
+ if (pcpu_list) {
+ kvm_parse_vcpu_pinning(pcpu_list, perf_test_args.vcpu_to_pcpu,
+ nr_vcpus);
+ perf_test_args.pin_vcpus = true;
+ }
+
TEST_ASSERT(p.iterations >= 2, "The test should have at least two iterations");
pr_info("Test iterations: %"PRIu64"\n", p.iterations);
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index e42a09cd24a0..3bf2333ef95d 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -688,6 +688,10 @@ static inline struct kvm_vm *vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
struct kvm_vcpu *vm_recreate_with_one_vcpu(struct kvm_vm *vm);
+void kvm_pin_this_task_to_pcpu(uint32_t pcpu);
+void kvm_parse_vcpu_pinning(const char *pcpus_string, uint32_t vcpu_to_pcpu[],
+ int nr_vcpus);
+
unsigned long vm_compute_max_gfn(struct kvm_vm *vm);
unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, size_t size);
unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages);
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index eaa88df0555a..849c875dd0ff 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -39,6 +39,10 @@ struct perf_test_args {
/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
bool nested;
+ /* True if all vCPUs are pinned to pCPUs */
+ bool pin_vcpus;
+ /* The vCPU=>pCPU pinning map. Only valid if pin_vcpus is true. */
+ uint32_t vcpu_to_pcpu[KVM_MAX_VCPUS];
struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
};
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index f1cb1627161f..3b7710fb3784 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -11,6 +11,7 @@
#include "processor.h"
#include <assert.h>
+#include <sched.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <sys/stat.h>
@@ -443,6 +444,59 @@ struct kvm_vcpu *vm_recreate_with_one_vcpu(struct kvm_vm *vm)
return vm_vcpu_recreate(vm, 0);
}
+void kvm_pin_this_task_to_pcpu(uint32_t pcpu)
+{
+ cpu_set_t mask;
+ int r;
+
+ CPU_ZERO(&mask);
+ CPU_SET(pcpu, &mask);
+ r = sched_setaffinity(0, sizeof(mask), &mask);
+ TEST_ASSERT(!r, "sched_setaffinity() failed for pCPU '%u'.\n", pcpu);
+}
+
+static uint32_t parse_pcpu(const char *cpu_str, const cpu_set_t *allowed_mask)
+{
+ uint32_t pcpu = atoi_non_negative("CPU number", cpu_str);
+
+ TEST_ASSERT(CPU_ISSET(pcpu, allowed_mask),
+ "Not allowed to run on pCPU '%d', check cgroups?\n", pcpu);
+ return pcpu;
+}
+
+void kvm_parse_vcpu_pinning(const char *pcpus_string, uint32_t vcpu_to_pcpu[],
+ int nr_vcpus)
+{
+ cpu_set_t allowed_mask;
+ char *cpu, *cpu_list;
+ char delim[2] = ",";
+ int i, r;
+
+ cpu_list = strdup(pcpus_string);
+ TEST_ASSERT(cpu_list, "strdup() allocation failed.\n");
+
+ r = sched_getaffinity(0, sizeof(allowed_mask), &allowed_mask);
+ TEST_ASSERT(!r, "sched_getaffinity() failed");
+
+ cpu = strtok(cpu_list, delim);
+
+ /* 1. Get all pcpus for vcpus. */
+ for (i = 0; i < nr_vcpus; i++) {
+ TEST_ASSERT(cpu, "pCPU not provided for vCPU '%d'\n", i);
+ vcpu_to_pcpu[i] = parse_pcpu(cpu, &allowed_mask);
+ cpu = strtok(NULL, delim);
+ }
+
+ /* 2. Check if the main worker needs to be pinned. */
+ if (cpu) {
+ kvm_pin_this_task_to_pcpu(parse_pcpu(cpu, &allowed_mask));
+ cpu = strtok(NULL, delim);
+ }
+
+ TEST_ASSERT(!cpu, "pCPU list contains trailing garbage characters '%s'", cpu);
+ free(cpu_list);
+}
+
/*
* Userspace Memory Region Find
*
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index 9618b37c66f7..3a1d0a44419b 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -2,6 +2,8 @@
/*
* Copyright (C) 2020, Google LLC.
*/
+#define _GNU_SOURCE
+
#include <inttypes.h>
#include "kvm_util.h"
@@ -243,6 +245,10 @@ void __weak perf_test_setup_nested(struct kvm_vm *vm, int nr_vcpus, struct kvm_v
static void *vcpu_thread_main(void *data)
{
struct vcpu_thread *vcpu = data;
+ int vcpu_idx = vcpu->vcpu_idx;
+
+ if (perf_test_args.pin_vcpus)
+ kvm_pin_this_task_to_pcpu(perf_test_args.vcpu_to_pcpu[vcpu_idx]);
WRITE_ONCE(vcpu->running, true);
@@ -255,7 +261,7 @@ static void *vcpu_thread_main(void *data)
while (!READ_ONCE(all_vcpu_threads_running))
;
- vcpu_thread_fn(&perf_test_args.vcpu_args[vcpu->vcpu_idx]);
+ vcpu_thread_fn(&perf_test_args.vcpu_args[vcpu_idx]);
return NULL;
}
--
2.38.1.273.g43a17bfeac-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread