From: Paolo Bonzini <pbonzini@redhat.com>
To: David Matlack <dmatlack@google.com>
Cc: kvm@vger.kernel.org, Ben Gardon <bgardon@google.com>,
Andrew Jones <drjones@redhat.com>
Subject: Re: [PATCH] KVM: selftests: Move vcpu_args_set into perf_test_util
Date: Fri, 6 Aug 2021 13:55:18 +0200 [thread overview]
Message-ID: <dc4bac54-9e3c-822c-2ba5-f4380486c0e3@redhat.com> (raw)
In-Reply-To: <20210805172821.2622793-1-dmatlack@google.com>
On 05/08/21 19:28, David Matlack wrote:
> perf_test_util is used to set up KVM selftests where vCPUs touch a
> region of memory. The guest code is implemented in perf_test_util.c (not
> the calling selftests). The guest code requires a 1 parameter, the
> vcpuid, which has to be set by calling vcpu_args_set(vm, vcpu_id, 1,
> vcpu_id).
>
> Today all of the selftests that use perf_test_util are making this call.
> Instead, perf_test_util should just do it. This will save some code but
> more importantly prevents mistakes since totally non-obvious that this
> needs to be called and failing to do so results in vCPUs not accessing
> the right regions of memory.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
Queued, thanks.
Paolo
> ---
> tools/testing/selftests/kvm/access_tracking_perf_test.c | 2 --
> tools/testing/selftests/kvm/demand_paging_test.c | 1 -
> tools/testing/selftests/kvm/dirty_log_perf_test.c | 1 -
> tools/testing/selftests/kvm/lib/perf_test_util.c | 2 ++
> tools/testing/selftests/kvm/memslot_modification_stress_test.c | 1 -
> 5 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> index e2baa187a21e..72714573ba4f 100644
> --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> @@ -222,8 +222,6 @@ static void *vcpu_thread_main(void *arg)
> int vcpu_id = vcpu_args->vcpu_id;
> int current_iteration = -1;
>
> - vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> -
> while (spin_wait_for_next_iteration(¤t_iteration)) {
> switch (READ_ONCE(iteration_work)) {
> case ITERATION_ACCESS_MEMORY:
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index b74704305835..950f2eb634e6 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -52,7 +52,6 @@ static void *vcpu_worker(void *data)
> struct timespec start;
> struct timespec ts_diff;
>
> - vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> run = vcpu_state(vm, vcpu_id);
>
> clock_gettime(CLOCK_MONOTONIC, &start);
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 80cbd3a748c0..ef45a133560f 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -44,7 +44,6 @@ static void *vcpu_worker(void *data)
> struct perf_test_vcpu_args *vcpu_args = (struct perf_test_vcpu_args *)data;
> int vcpu_id = vcpu_args->vcpu_id;
>
> - vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> run = vcpu_state(vm, vcpu_id);
>
> while (!READ_ONCE(host_quit)) {
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index b488f4aefea8..f6aa81af3e6f 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -140,6 +140,8 @@ void perf_test_setup_vcpus(struct kvm_vm *vm, int vcpus,
> vcpu_gpa = guest_test_phys_mem;
> }
>
> + vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> +
> pr_debug("Added VCPU %d with test mem gpa [%lx, %lx)\n",
> vcpu_id, vcpu_gpa, vcpu_gpa +
> (vcpu_args->pages * perf_test_args.guest_page_size));
> diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> index 98351ba0933c..b6f7cc298e4d 100644
> --- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> +++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
> @@ -45,7 +45,6 @@ static void *vcpu_worker(void *data)
> struct kvm_vm *vm = perf_test_args.vm;
> struct kvm_run *run;
>
> - vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> run = vcpu_state(vm, vcpu_id);
>
> /* Let the guest access its memory until a stop signal is received */
>
prev parent reply other threads:[~2021-08-06 11:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-05 17:28 [PATCH] KVM: selftests: Move vcpu_args_set into perf_test_util David Matlack
2021-08-06 11:55 ` Paolo Bonzini [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dc4bac54-9e3c-822c-2ba5-f4380486c0e3@redhat.com \
--to=pbonzini@redhat.com \
--cc=bgardon@google.com \
--cc=dmatlack@google.com \
--cc=drjones@redhat.com \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).