Re: [PATCH] kvm: support to get/set dirty log initial-all-set capability

From: Paolo Bonzini <pbonzini@redhat.com>
To: Jay Zhou <jianjay.zhou@huawei.com>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org
Cc: mst@redhat.com, cohuck@redhat.com, peterx@redhat.com,
	wangxinxin.wang@huawei.com, weidong.huang@huawei.com,
	liu.jinsong@huawei.com
Subject: Re: [PATCH] kvm: support to get/set dirty log initial-all-set capability
Date: Wed, 18 Mar 2020 11:47:45 +0100	[thread overview]
Message-ID: <18e7b781-8a52-d78a-a653-898445a5ee53@redhat.com> (raw)
In-Reply-To: <20200304025554.2159-1-jianjay.zhou@huawei.com>

On 04/03/20 03:55, Jay Zhou wrote:
> Since the new capability KVM_DIRTY_LOG_INITIALLY_SET of
> KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 has been introduced in the
> kernel, tweak the userspace side to detect and enable this
> capability.
> 
> Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
> ---
>  accel/kvm/kvm-all.c       | 21 ++++++++++++++-------
>  linux-headers/linux/kvm.h |  3 +++
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 439a4efe52..45ab25be63 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -100,7 +100,7 @@ struct KVMState
>      bool kernel_irqchip_required;
>      OnOffAuto kernel_irqchip_split;
>      bool sync_mmu;
> -    bool manual_dirty_log_protect;
> +    uint64_t manual_dirty_log_protect;
>      /* The man page (and posix) say ioctl numbers are signed int, but
>       * they're not.  Linux, glibc and *BSD all treat ioctl numbers as
>       * unsigned, and treating them as signed here can break things */
> @@ -1882,6 +1882,7 @@ static int kvm_init(MachineState *ms)
>      int ret;
>      int type = 0;
>      const char *kvm_type;
> +    uint64_t dirty_log_manual_caps;
>  
>      s = KVM_STATE(ms->accelerator);
>  
> @@ -2007,14 +2008,20 @@ static int kvm_init(MachineState *ms)
>      s->coalesced_pio = s->coalesced_mmio &&
>                         kvm_check_extension(s, KVM_CAP_COALESCED_PIO);
>  
> -    s->manual_dirty_log_protect =
> +    dirty_log_manual_caps =
>          kvm_check_extension(s, KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
> -    if (s->manual_dirty_log_protect) {
> -        ret = kvm_vm_enable_cap(s, KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2, 0, 1);
> +    dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
> +                              KVM_DIRTY_LOG_INITIALLY_SET);
> +    s->manual_dirty_log_protect = dirty_log_manual_caps;
> +    if (dirty_log_manual_caps) {
> +        ret = kvm_vm_enable_cap(s, KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2, 0,
> +                                   dirty_log_manual_caps);
>          if (ret) {
> -            warn_report("Trying to enable KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 "
> -                        "but failed.  Falling back to the legacy mode. ");
> -            s->manual_dirty_log_protect = false;
> +            warn_report("Trying to enable capability %"PRIu64" of "
> +                        "KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 but failed. "
> +                        "Falling back to the legacy mode. ",
> +                        dirty_log_manual_caps);
> +            s->manual_dirty_log_protect = 0;
>          }
>      }
>  
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 265099100e..3cb71c2b19 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1628,4 +1628,7 @@ struct kvm_hyperv_eventfd {
>  #define KVM_HYPERV_CONN_ID_MASK		0x00ffffff
>  #define KVM_HYPERV_EVENTFD_DEASSIGN	(1 << 0)
>  
> +#define KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE    (1 << 0)
> +#define KVM_DIRTY_LOG_INITIALLY_SET            (1 << 1)
> +
>  #endif /* __LINUX_KVM_H */
> 

Queued, thanks.

Paolo