linux-kernel - Re: [PATCH v6 13/14] KVM: selftests: Let dirty_log

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, 10 Mar 2020 09:27:04 +0100
From:   Andrew Jones <drjones@...hat.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Yan Zhao <yan.y.zhao@...el.com>,
        Jason Wang <jasowang@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        Christophe de Dinechin <dinechin@...hat.com>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>
Subject: Re: [PATCH v6 13/14] KVM: selftests: Let dirty_log_test async for
 dirty ring test

On Mon, Mar 09, 2020 at 06:25:34PM -0400, Peter Xu wrote:
> Previously the dirty ring test was working in synchronous way, because
> only with a vmexit (with that it was the ring full event) we'll know
> the hardware dirty bits will be flushed to the dirty ring.
> 
> With this patch we first introduced the vcpu kick mechanism by using
> SIGUSR1, meanwhile we can have a guarantee of vmexit and also the
> flushing of hardware dirty bits.  With all these, we can keep the vcpu
> dirty work asynchronous of the whole collection procedure now.  Still,
> we need to be very careful that we can only do it async if the vcpu is
> not reaching soft limit (no KVM_EXIT_DIRTY_RING_FULL).  Otherwise we
> must collect the dirty bits before continuing the vcpu.
> 
> Further increase the dirty ring size to current maximum to make sure
> we torture more on the no-ring-full case, which should be the major
> scenario when the hypervisors like QEMU would like to use this feature.
> 
> Signed-off-by: Peter Xu <peterx@...hat.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_test.c  | 126 +++++++++++++-----
>  .../testing/selftests/kvm/include/kvm_util.h  |   1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c    |   9 ++
>  3 files changed, 106 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 134637267af4..b07e52858e87 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -13,6 +13,9 @@
>  #include <time.h>
>  #include <pthread.h>
>  #include <semaphore.h>
> +#include <sys/types.h>
> +#include <signal.h>
> +#include <errno.h>
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <asm/barrier.h>
> @@ -59,7 +62,9 @@
>  # define test_and_clear_bit_le	test_and_clear_bit
>  #endif
>  
> -#define TEST_DIRTY_RING_COUNT		1024
> +#define TEST_DIRTY_RING_COUNT		65536
> +
> +#define SIG_IPI SIGUSR1
>  
>  /*
>   * Guest/Host shared variables. Ensure addr_gva2hva() and/or
> @@ -135,6 +140,12 @@ static uint64_t host_track_next_count;
>  /* Whether dirty ring reset is requested, or finished */
>  static sem_t dirty_ring_vcpu_stop;
>  static sem_t dirty_ring_vcpu_cont;
> +/*
> + * This is updated by the vcpu thread to tell the host whether it's a
> + * ring-full event.  It should only be read until a sem_wait() of
> + * dirty_ring_vcpu_stop and before vcpu continues to run.
> + */
> +static bool dirty_ring_vcpu_ring_full;
>  
>  enum log_mode_t {
>  	/* Only use KVM_GET_DIRTY_LOG for logging */
> @@ -156,6 +167,33 @@ enum log_mode_t {
>  static enum log_mode_t host_log_mode_option = LOG_MODE_ALL;
>  /* Logging mode for current run */
>  static enum log_mode_t host_log_mode;
> +pthread_t vcpu_thread;
> +
> +/* Only way to pass this to the signal handler */
> +struct kvm_vm *current_vm;

nit: above two new globals could be static