linux-kernel - Re: [PATCH v3 1/2] KVM: selftests: Sync data verify of dirty logging with guest sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Apr 2021 10:07:16 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Peter Xu <peterx@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     Andrew Jones <drjones@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH v3 1/2] KVM: selftests: Sync data verify of dirty logging
 with guest sync

On 18/04/21 14:43, Peter Xu wrote:
> ----8<-----
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 25230e799bc4..d3050d1c2cd0 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -377,7 +377,7 @@ static void dirty_ring_after_vcpu_run(struct kvm_vm *vm, int ret, int err)
>          /* A ucall-sync or ring-full event is allowed */
>          if (get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC) {
>                  /* We should allow this to continue */
> -               ;
> +               vcpu_handle_sync_stop();
>          } else if (run->exit_reason == KVM_EXIT_DIRTY_RING_FULL ||
>                     (ret == -1 && err == EINTR)) {
>                  /* Update the flag first before pause */
> ----8<-----
> 
> That's my intention when I introduce vcpu_handle_sync_stop(), but forgot to
> add...

And possibly even this (untested though):

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index ffa4e2791926..918954f01cef 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -383,6 +383,7 @@ static void dirty_ring_after_vcpu_run(struct kvm_vm *vm, int ret, int err)
  		/* Update the flag first before pause */
  		WRITE_ONCE(dirty_ring_vcpu_ring_full,
  			   run->exit_reason == KVM_EXIT_DIRTY_RING_FULL);
+		atomic_set(&vcpu_sync_stop_requested, false);
  		sem_post(&sem_vcpu_stop);
  		pr_info("vcpu stops because %s...\n",
  			dirty_ring_vcpu_ring_full ?
@@ -804,8 +805,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
  		 * the flush of the last page, and since we handle the last
  		 * page specially verification will succeed anyway.
  		 */
-		assert(host_log_mode == LOG_MODE_DIRTY_RING ||
-		       atomic_read(&vcpu_sync_stop_requested) == false);
+		assert(atomic_read(&vcpu_sync_stop_requested) == false);
  		vm_dirty_log_verify(mode, bmap);
  		sem_post(&sem_vcpu_cont);
  

You can submit all these as a separate patch.

Paolo