kvm: Some more trivial fixes for clear dirty log
Subsystems (3)
v2 cover kvm: Some more trivial fixes for clear dirty log
v2:
- fix patch 2 to use DIV_ROUND_UP
- add one more patch to obsolete the old cap, and introduce new [Paolo]
Two issues I've noticed when I'm drafting the QEMU support of it. With these two patches applied the QEMU binary (with the clear dirty log supported [1]) can migrate correctly otherwise the migration can stall forever if with/after heavy memory workload.
Please have a look, thanks.
[1] https://github.com/xzpeter/qemu/tree/kvm-clear-dirty-log
Peter Xu (3):
KVM: Fix the bitmap range to copy during clear dirty
KVM: Fix loop of clear dirty with possible off-by-one
KVM: Introduce KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2
Documentation/virtual/kvm/api.txt | 13 ++++++++----- include/uapi/linux/kvm.h | 5 +++-- tools/testing/selftests/kvm/dirty_log_test.c | 4 ++-- virt/kvm/kvm_main.c | 10 +++++----- 4 files changed, 18 insertions(+), 14 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/3 KVM: Fix the bitmap range to copy during clear dirty
kvm_dirty_bitmap_bytes() will return the size of the dirty bitmap of the memslot rather than the size of bitmap passed over from the ioctl. Here for KVM_CLEAR_DIRTY_LOG we should only copy exactly the size of bitmap that covers kvm_clear_dirty_log.num_pages.
--- virt/kvm/kvm_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +1 -1
@@ -1251,7 +1251,7 @@ int kvm_clear_dirty_log_protect(struct kvm *kvm,if (!dirty_bitmap)return -ENOENT ; - n =kvm_dirty_bitmap_bytes (memslot); + n =ALIGN (log->num_pages,BITS_PER_LONG ) /8 ; if (log->first_page > memslot->npages || log->num_pages > memslot->npages - log->first_page ||
Signature · 1 line
2.17.1
PATCH v2 2/3 KVM: Fix loop of clear dirty with possible off-by-one
In reply to Peter XuJust imaging the case where num_pages < BITS_PER_LONG, then the loop will be skipped while it shouldn't.
--- virt/kvm/kvm_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +2 -2
@@ -1264,8 +1264,8 @@ int kvm_clear_dirty_log_protect(struct kvm *kvm,return -EFAULT ;spin_lock (&kvm->mmu_lock); -for (offset = log->first_page, - i = offset /BITS_PER_LONG , n = log->num_pages /BITS_PER_LONG ; n--; +for (offset = log->first_page, i = offset /BITS_PER_LONG , + n =DIV_ROUND_UP (log->num_pages,BITS_PER_LONG ); n--; i++, offset +=BITS_PER_LONG ) {unsigned long mask = *dirty_bitmap_buffer++;atomic_long_t *p = (atomic_long_t *) &dirty_bitmap[i];
Signature · 1 line
2.17.1
PATCH v2 3/3 KVM: Introduce KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2
In reply to Peter XuThe previous KVM_CAP_MANUAL_DIRTY_LOG_PROTECT has some problem which blocks the correct usage from userspace. Obsolete the old one and introduce a new capability bit for it.
--- Documentation/virtual/kvm/api.txt | 13 ++++++++----- include/uapi/linux/kvm.h | 5 +++-- tools/testing/selftests/kvm/dirty_log_test.c | 4 ++-- virt/kvm/kvm_main.c | 4 ++-- 4 files changed, 15 insertions(+), 11 deletions(-)
Documentation/virtual/kvm/api.txt +8 -5
@@ -330,7 +330,7 @@ They must be less than the value that KVM_CHECK_EXTENSION returns for the KVM_CAP_MULTI_ADDRESS_SPACE capability. The bits in the dirty bitmap are cleared before the ioctl returns, unless -KVM_CAP_MANUAL_DIRTY_LOG_PROTECT is enabled. For more information, +KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 is enabled. For more information, see the description of the capability. 4.9 KVM_SET_MEMORY_ALIAS
@@ -3791,7 +3791,7 @@ to I/O ports. 4.117 KVM_CLEAR_DIRTY_LOG (vm ioctl) -Capability: KVM_CAP_MANUAL_DIRTY_LOG_PROTECT +Capability: KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 Architectures: x86 Type: vm ioctl Parameters: struct kvm_dirty_log (in)
@@ -3824,10 +3824,13 @@ the address space for which you want to return the dirty bitmap. They must be less than the value that KVM_CHECK_EXTENSION returns for the KVM_CAP_MULTI_ADDRESS_SPACE capability. -This ioctl is mostly useful when KVM_CAP_MANUAL_DIRTY_LOG_PROTECT +This ioctl is mostly useful when KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 is enabled; for more information, see the description of the capability. However, it can always be used as long as KVM_CHECK_EXTENSION confirms -that KVM_CAP_MANUAL_DIRTY_LOG_PROTECT is present. +that KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 is present. + +The old name of KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 is +KVM_CAP_MANUAL_DIRTY_LOG_PROTECT but it was obsolete now. 4.118 KVM_GET_SUPPORTED_HV_CPUID
@@ -4780,7 +4783,7 @@ and injected exceptions. * For the new DR6 bits, note that bit 16 is set iff the #DB exception will clear DR6.RTM. -7.18 KVM_CAP_MANUAL_DIRTY_LOG_PROTECT +7.18 KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 Architectures: all Parameters: args[0] whether feature should be enabled or not
include/uapi/linux/kvm.h +3 -2
@@ -986,8 +986,9 @@ struct kvm_ppc_resize_hpt {#define KVM_CAP_HYPERV_ENLIGHTENED_VMCS 163#define KVM_CAP_EXCEPTION_PAYLOAD 164#define KVM_CAP_ARM_VM_IPA_SIZE 165 -#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 +#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166/* Obsolete */ #define KVM_CAP_HYPERV_CPUID 167 +#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 168#ifdef KVM_CAP_IRQ_ROUTING
@@ -1434,7 +1435,7 @@ struct kvm_enc_region {#define KVM_GET_NESTED_STATE _IOWR(KVMIO, 0xbe, struct kvm_nested_state)#define KVM_SET_NESTED_STATE _IOW(KVMIO, 0xbf, struct kvm_nested_state) -/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT */ +/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 */ #define KVM_CLEAR_DIRTY_LOG _IOWR(KVMIO, 0xc0, struct kvm_clear_dirty_log)/* Available with KVM_CAP_HYPERV_CPUID */
tools/testing/selftests/kvm/dirty_log_test.c +2 -2
@@ -311,7 +311,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,#ifdef USE_CLEAR_DIRTY_LOG struct kvm_enable_cap cap = {}; - cap.cap =KVM_CAP_MANUAL_DIRTY_LOG_PROTECT ; + cap.cap =KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 ; cap.args[0 ] =1 ;vm_enable_cap (vm, &cap);#endif
@@ -427,7 +427,7 @@ int main(int argc, char *argv[])int opt, i;#ifdef USE_CLEAR_DIRTY_LOG -if (!kvm_check_cap (KVM_CAP_MANUAL_DIRTY_LOG_PROTECT )) { +if (!kvm_check_cap (KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 )) {fprintf (stderr,"KVM_CLEAR_DIRTY_LOG not available, skipping tests\n" );exit (KSFT_SKIP ); }
virt/kvm/kvm_main.c +2 -2
@@ -3110,7 +3110,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)case KVM_CAP_CHECK_EXTENSION_VM :case KVM_CAP_ENABLE_CAP_VM :#ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT -case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT : +case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 :#endif return 1 ;#ifdef CONFIG_KVM_MMIO
@@ -3148,7 +3148,7 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm *kvm, {switch (cap->cap) {#ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT -case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT : +case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 :if (cap->flags || (cap->args[0 ] & ~1 ))return -EINVAL ; kvm->manual_dirty_log_protect = cap->args[0 ];
Signature · 1 line
2.17.1
Paolo Bonzini
maintainerRe: [PATCH v2 3/3] KVM: Introduce KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2
Quoted 6 lines · Peter Xu
The previous
KVM_CAP_MANUAL_DIRTY_LOG_PROTECThas some problem which blocks the correct usage from userspace. Obsolete the old one and introduce a new capability bit for it.Signed-off-by Peter Xu <[email protected]>---
I renamed it to KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2. Also, this paragraph
+The old name of KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 is +KVM_CAP_MANUAL_DIRTY_LOG_PROTECT but it was obsolete now.
should be under the description of the capability, and perhaps can be expanded like this:
+KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 was previously available under the name +KVM_CAP_MANUAL_DIRTY_LOG_PROTECT, but the implementation had bugs that make +it hard or impossible to use it correctly. The availability of +KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. +Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT.
No need to do anything on your part.
Paolo