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

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/06/20 05:01, Zhoujian (jay) wrote:
> 
> 
>> -----Original Message-----
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> Sent: Wednesday, March 18, 2020 6:48 PM
>> To: Zhoujian (jay) <jianjay.zhou@huawei.com>; qemu-devel@nongnu.org;
>> kvm@vger.kernel.org
>> Cc: mst@redhat.com; cohuck@redhat.com; peterx@redhat.com; wangxin (U)
>> <wangxinxin.wang@huawei.com>; Huangweidong (C)
>> <weidong.huang@huawei.com>; Liujinsong (Paul) <liu.jinsong@huawei.com>
>> Subject: Re: [PATCH] kvm: support to get/set dirty log initial-all-set capability
>>
>> 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.
>>
> 
> Hi Paolo,
> 
> It seems that this patch isn't included in your last pull request...
> If there's something else to be done, please let me know.

Sorry, I thought mistakenly that it was a 5.8 feature (so it would have
to wait for the 5.8-rc1 release and header update).  It's still queued
though.

Paolo