KVM: Dirty ring interface
Subsystems (6)
v7 cover KVM: Dirty ring interface
KVM branch:
https://github.com/xzpeter/linux/tree/kvm-dirty-ring
QEMU branch for testing:
https://github.com/xzpeter/qemu/tree/kvm-dirty-ring
v7
- rebase to kvm/queue
- make as_id an u16 [Sean]
- init_rmode_tss: rename idx to i, remove data init to zero [Sean]
- update doc for __x86_set_memory_region on ret value after rebase [Sean]
- fix sparse warnings for __x86_set_memory_region [syzbot]
- test cases [Drew]
- use print_skip()
- dump log modes without '\b'
- support "all" log mode
- exit(s/-1/1/)
- assert gfns mmap in vcpu_map_dirty_ring
- make vcpu_thread/current_vm static
For previous versions, please refer to:
V1: https://lore.kernel.org/kvm/[email protected]
V2: https://lore.kernel.org/kvm/[email protected]
V3: https://lore.kernel.org/kvm/[email protected]
V4: https://lore.kernel.org/kvm/[email protected]
V5: https://lore.kernel.org/kvm/[email protected]
V6: https://lore.kernel.org/kvm/[email protected]
Overview
============
This is a continued work from Lei Cao <[email protected]> and Paolo Bonzini on the KVM dirty ring interface.
The new dirty ring interface is another way to collect dirty pages for the virtual machines. It is different from the existing dirty logging interface in a few ways, majorly:
- Data format: The dirty data was in a ring format rather than a bitmap format, so dirty bits to sync for dirty logging does not depend on the size of guest memory any more, but speed of dirtying. Also, the dirty ring is per-vcpu, while the dirty bitmap is per-vm.
- Data copy: The sync of dirty pages does not need data copy any more, but instead the ring is shared between the userspace and kernel by page sharings (mmap() on vcpu fd)
- Interface: Instead of using the old KVM_GET_DIRTY_LOG, KVM_CLEAR_DIRTY_LOG interfaces, the new ring uses the new KVM_RESET_DIRTY_RINGS ioctl when we want to reset the collected dirty pages to protected mode again (works like KVM_CLEAR_DIRTY_LOG, but ring based). To collecting dirty bits, we only need to read the ring data, no ioctl is needed.
Ring Layout
===========
KVM dirty ring is per-vcpu. Each ring is an array of kvm_dirty_gfn defined as:
struct kvm_dirty_gfn {
__u32 flags;
__u32 slot; /* as_id | slot_id */
__u64 offset;
};Each GFN is a state machine itself. The state is embeded in the flags field, as defined in the uapi header:
/*
* KVM dirty GFN flags, defined as:
*
* |---------------+---------------+--------------| * | bit 1 (reset) | bit 0 (dirty) | Status | * |---------------+---------------+--------------| * | 0 | 0 | Invalid GFN | * | 0 | 1 | Dirty GFN | * | 1 | X | GFN to reset | * |---------------+---------------+--------------| * * Lifecycle of a dirty GFN goes like: * * dirtied collected reset * 00 -----------> 01 -------------> 1X -------+ * ^ | * | | * +------------------------------------------+
*
* The userspace program is only responsible for the 01->1X state
* conversion (to collect dirty bits). Also, it must not skip any
* dirty bits so that dirty bits are always collected in sequence. */
Testing
=======
This series provided both the implementation of the KVM dirty ring and the test case. Also I've implemented the QEMU counterpart that can run with the new KVM, link can be found at the top of the cover letter. However that's still a very initial version which is prone to change and future optimizations.
I did some measurement with the new method with 24G guest running some dirty workload, I don't see any speedup so far, even in some heavy dirty load it'll be slower (e.g., when 800MB/s random dirty rate, kvm dirty ring takes average of ~73s to complete migration while dirty logging only needs average of ~55s). However that's understandable because 24G guest means only 1M dirty bitmap, that's still a suitable case for dirty logging. Meanwhile heavier workload means worst case for dirty ring.
More tests are welcomed if there's bigger host/guest, especially on COLO-like workload.
Please review, thanks.
Peter Xu (14):
KVM: X86: Change parameter for fast_page_fault tracepoint
KVM: Cache as_id in kvm_memory_slot
KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
KVM: Pass in kvm pointer into mark_page_dirty_in_slot()
KVM: X86: Implement ring-based dirty memory tracking
KVM: Make dirty ring exclusive to dirty bitmap log
KVM: Don't allocate dirty bitmap if dirty ring is enabled
KVM: selftests: Always clear dirty bitmap after iteration
KVM: selftests: Sync uapi/linux/kvm.h to tools/
KVM: selftests: Use a single binary for dirty/clear log test
KVM: selftests: Introduce after_vcpu_run hook for dirty log test
KVM: selftests: Add dirty ring buffer test
KVM: selftests: Let dirty_log_test async for dirty ring test
KVM: selftests: Add "-c" parameter to dirty log test
Documentation/virt/kvm/api.rst | 123 +++++ arch/x86/include/asm/kvm_host.h | 6 +- arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/Makefile | 3 +- arch/x86/kvm/mmu/mmu.c | 10 +- arch/x86/kvm/mmutrace.h | 9 +- arch/x86/kvm/svm.c | 9 +- arch/x86/kvm/vmx/vmx.c | 89 +-- arch/x86/kvm/x86.c | 50 +- include/linux/kvm_dirty_ring.h | 103 ++++ include/linux/kvm_host.h | 19 + include/trace/events/kvm.h | 78 +++ include/uapi/linux/kvm.h | 53 ++ tools/include/uapi/linux/kvm.h | 99 +++- tools/testing/selftests/kvm/Makefile | 2 - .../selftests/kvm/clear_dirty_log_test.c | 6 - tools/testing/selftests/kvm/dirty_log_test.c | 505 ++++++++++++++++-- .../testing/selftests/kvm/include/kvm_util.h | 4 + tools/testing/selftests/kvm/lib/kvm_util.c | 68 +++ .../selftests/kvm/lib/kvm_util_internal.h | 4 + virt/kvm/dirty_ring.c | 195 +++++++ virt/kvm/kvm_main.c | 162 +++++- 22 files changed, 1460 insertions(+), 138 deletions(-) create mode 100644 include/linux/kvm_dirty_ring.h delete mode 100644 tools/testing/selftests/kvm/clear_dirty_log_test.c create mode 100644 virt/kvm/dirty_ring.c
Signature · 1 line
2.24.1
PATCH v7 1/14 KVM: X86: Change parameter for fast_page_fault tracepoint
It would be clearer to dump the return value to know easily on whether did we go through the fast path for handling current page fault. Remove the old two last parameters because after all the old/new sptes were dumped in the same line.
--- arch/x86/kvm/mmutrace.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
arch/x86/kvm/mmutrace.h +2 -7
@@ -244,9 +244,6 @@ TRACE_EVENT( __entry->access) ); -#define __spte_satisfied (__spte) \ - (__entry->retry && is_writable_pte(__entry->__spte)) - TRACE_EVENT( fast_page_fault, TP_PROTO(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u32 error_code,
@@ -274,12 +271,10 @@ TRACE_EVENT( ),TP_printk ("vcpu %d gva %llx error_code %s sptep %p old %#llx" -" new %llx spurious %d fixed %d" , __entry->vcpu_id, +" new %llx ret %d" , __entry->vcpu_id, __entry->cr2_or_gpa,__print_flags (__entry->error_code,"|" , kvm_mmu_trace_pferr_flags), __entry->sptep, - __entry->old_spte, __entry->new_spte, -__spte_satisfied (old_spte),__spte_satisfied (new_spte) - ) + __entry->old_spte, __entry->new_spte, __entry->retry) ); TRACE_EVENT(
Signature · 1 line
2.24.1
PATCH v7 2/14 KVM: Cache as_id in kvm_memory_slot
In reply to Peter XuCache the address space ID just like the slot ID. It will be used in order to fill in the dirty ring entries.
--- include/linux/kvm_host.h | 1 + virt/kvm/kvm_main.c | 1 + 2 files changed, 2 insertions(+)
include/linux/kvm_host.h +1 -0
@@ -346,6 +346,7 @@ struct kvm_memory_slot {unsigned long userspace_addr;u32 flags;short id; +u16 as_id; };static inline unsigned long kvm_dirty_bitmap_bytes (struct kvm_memory_slot *memslot)
virt/kvm/kvm_main.c +1 -0
@@ -1240,6 +1240,7 @@ int __kvm_set_memory_region(struct kvm *kvm,if (!mem->memory_size)return kvm_delete_memslot (kvm, mem, &old, as_id); + new.as_id = as_id; new.id = id; new.base_gfn = mem->guest_phys_addr >>PAGE_SHIFT ; new.npages = mem->memory_size >>PAGE_SHIFT ;
Signature · 1 line
2.24.1
PATCH v7 3/14 KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
In reply to Peter XuOriginally, we have three code paths that can dirty a page without vcpu context for X86:
- init_rmode_identity_map
- init_rmode_tss
- kvmgt_rw_gpa
init_rmode_identity_map and init_rmode_tss will be setup on destination VM no matter what (and the guest cannot even see them), so it does not make sense to track them at all.
To do this, allow __x86_set_memory_region() to return the userspace address that just allocated to the caller. Then in both of the functions we directly write to the userspace address instead of calling kvm_write_*() APIs.
Another trivial change is that we don't need to explicitly clear the identity page table root in init_rmode_identity_map() because no matter what we'll write to the whole page with 4M huge page entries.
--- arch/x86/include/asm/kvm_host.h | 3 +- arch/x86/kvm/svm.c | 9 ++-- arch/x86/kvm/vmx/vmx.c | 82 ++++++++++++++++----------------- arch/x86/kvm/x86.c | 41 ++++++++++++++--- 4 files changed, 83 insertions(+), 52 deletions(-)
arch/x86/include/asm/kvm_host.h +2 -1
@@ -1645,7 +1645,8 @@ void __kvm_request_immediate_exit(struct kvm_vcpu *vcpu);int kvm_is_in_guest (void ); -int __x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa,u32 size); +void __user *__x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa, +u32 size);bool kvm_vcpu_is_reset_bsp (struct kvm_vcpu *vcpu);bool kvm_vcpu_is_bsp (struct kvm_vcpu *vcpu);
arch/x86/kvm/svm.c +6 -3
@@ -1783,7 +1783,8 @@ static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu,*/ static int avic_update_access_page (struct kvm *kvm,bool activate) { -int ret =0 ; +void __user *ret; +int r =0 ;mutex_lock (&kvm->slots_lock);/*
@@ -1799,13 +1800,15 @@ static int avic_update_access_page(struct kvm *kvm, bool activate)APIC_ACCESS_PAGE_PRIVATE_MEMSLOT ,APIC_DEFAULT_PHYS_BASE , activate ? PAGE_SIZE : 0); -if (ret) +if (IS_ERR (ret)) { + r =PTR_ERR (ret); goto out; + } kvm->arch.apic_access_page_done = activate; out:mutex_unlock (&kvm->slots_lock); -return ret; +return r; }static int avic_init_backing_page (struct kvm_vcpu *vcpu)
arch/x86/kvm/vmx/vmx.c +41 -41
146 lines not on this page · raw message
arch/x86/kvm/x86.c +34 -7
76 lines not on this page · raw message
Signature · 1 line
2.24.1
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 18 lines · Peter Xu
arch/x86/kvm/x86.c +9
-1@@ -9786,7 +9786,34 @@ void kvm_arch_sync_events(struct kvm *kvm)kvm_free_pit (kvm); } -int __x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa,u32 size) +#define ERR_PTR_USR (e) ((void __user *)ERR_PTR(e)) + +/** +* __x86_set_memory_region: Setup KVM internal memory slot +* +* @kvm: the kvm pointer to the VM. +* @id: the slot ID to setup. +* @gpa: the GPA to install the slot (unused when @size == 0). +* @size: the size of the slot. Set to zero to uninstall a slot.
arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9807
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* +* This function helps to setup a KVM internal memory slot. Specify +* @size > 0 to install a new slot, while @size == 0 to uninstall a +* slot. The return code can be one of the following: +* +* HVA: on success (uninstall will return a bogus HVA) +* -errno: on error +* +* The caller should always use IS_ERR() to check the return value +* before use. NOTE: KVM internal memory slots are guaranteed and
"are guaranteed" to ...
arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9813
Quoted 6 lines
arch/x86/kvm/x86.c +6
-0+* won't change until the VM is destroyed. This is also true to the +* returned HVA when installing a new memory slot. The HVA can be +* invalidated by either an errornous userspace program or a VM under +* destruction, however as long as we use __copy_{to|from}_user() +* properly upon the HVAs and handle the failure paths always then +* we're safe.
Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the GPA->HVA is guaranteed to be valid, but the HVA->HPA is not.
/*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. the GPA->HVA translation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 44 lines · Sean Christopherson
Quoted 18 lines · Peter Xu
arch/x86/kvm/x86.c +9
-1@@ -9786,7 +9786,34 @@ void kvm_arch_sync_events(struct kvm *kvm)kvm_free_pit (kvm); } -int __x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa,u32 size) +#define ERR_PTR_USR (e) ((void __user *)ERR_PTR(e)) + +/** +* __x86_set_memory_region: Setup KVM internal memory slot +* +* @kvm: the kvm pointer to the VM. +* @id: the slot ID to setup. +* @gpa: the GPA to install the slot (unused when @size == 0). +* @size: the size of the slot. Set to zero to uninstall a slot. arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9807
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* +* This function helps to setup a KVM internal memory slot. Specify +* @size > 0 to install a new slot, while @size == 0 to uninstall a +* slot. The return code can be one of the following: +* +* HVA: on success (uninstall will return a bogus HVA) +* -errno: on error +* +* The caller should always use IS_ERR() to check the return value +* before use. NOTE: KVM internal memory slots are guaranteed and "are guaranteed" to ...
arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9813
Quoted 6 lines
arch/x86/kvm/x86.c +6
-0+* won't change until the VM is destroyed. This is also true to the +* returned HVA when installing a new memory slot. The HVA can be +* invalidated by either an errornous userspace program or a VM under +* destruction, however as long as we use __copy_{to|from}_user() +* properly upon the HVAs and handle the failure paths always then +* we're safe. Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the
GPA->HVAis guaranteed to be valid, but theHVA->HPAis not.
Quoted 6 lines
/*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. theGPA->HVAtranslation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */
Sure I can switch to this, though note that I still think the GPA->HVA is not guaranteed logically because the userspace can unmap any HVA it wants.. However I agree that shouldn't be important from kvm's perspective as long as we always emphasize on using legal HVA accessors.
Signature · 1 line
Peter Xu
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 52 lines · Peter Xu
Quoted 44 lines · Sean Christopherson
Quoted 18 lines · Peter Xu
arch/x86/kvm/x86.c +9
-1@@ -9786,7 +9786,34 @@ void kvm_arch_sync_events(struct kvm *kvm)kvm_free_pit (kvm); } -int __x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa,u32 size) +#define ERR_PTR_USR (e) ((void __user *)ERR_PTR(e)) + +/** +* __x86_set_memory_region: Setup KVM internal memory slot +* +* @kvm: the kvm pointer to the VM. +* @id: the slot ID to setup. +* @gpa: the GPA to install the slot (unused when @size == 0). +* @size: the size of the slot. Set to zero to uninstall a slot. arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9807
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* +* This function helps to setup a KVM internal memory slot. Specify +* @size > 0 to install a new slot, while @size == 0 to uninstall a +* slot. The return code can be one of the following: +* +* HVA: on success (uninstall will return a bogus HVA) +* -errno: on error +* +* The caller should always use IS_ERR() to check the return value +* before use. NOTE: KVM internal memory slots are guaranteed and "are guaranteed" to ...
arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9813
Quoted 6 lines
arch/x86/kvm/x86.c +6
-0+* won't change until the VM is destroyed. This is also true to the +* returned HVA when installing a new memory slot. The HVA can be +* invalidated by either an errornous userspace program or a VM under +* destruction, however as long as we use __copy_{to|from}_user() +* properly upon the HVAs and handle the failure paths always then +* we're safe. Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the
GPA->HVAis guaranteed to be valid, but theHVA->HPAis not.Quoted 6 lines
/*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. theGPA->HVAtranslation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */
Quoted 3 lines
Sure I can switch to this, though note that I still think the
GPA->HVAis not guaranteed logically because the userspace can unmap any HVA it wants..
You're conflating the GPA->HVA translation with the validity of the HVA, i.e. the HVA->HPA and/or HVA->VMA translation/association. GPA->HVA is guaranteed because userspace doesn't have access to the memslot which defines that transation.
Quoted 2 lines
However I agree that shouldn't be important from kvm's perspective as long as we always emphasize on using legal HVA accessors.
The fact that GPA->HVA can't change _is_ important, otherwise KVM would need to take steps to ensure that whatever can change GPA->HVA can't run concurrently with consuming the HVA.
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 53 lines · Sean Christopherson
Quoted 52 lines · Peter Xu
Quoted 50 lines · Sean Christopherson
Quoted 18 lines · Peter Xu
arch/x86/kvm/x86.c +9
-1@@ -9786,7 +9786,34 @@ void kvm_arch_sync_events(struct kvm *kvm)kvm_free_pit (kvm); } -int __x86_set_memory_region (struct kvm *kvm,int id,gpa_t gpa,u32 size) +#define ERR_PTR_USR (e) ((void __user *)ERR_PTR(e)) + +/** +* __x86_set_memory_region: Setup KVM internal memory slot +* +* @kvm: the kvm pointer to the VM. +* @id: the slot ID to setup. +* @gpa: the GPA to install the slot (unused when @size == 0). +* @size: the size of the slot. Set to zero to uninstall a slot. arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9807
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* +* This function helps to setup a KVM internal memory slot. Specify +* @size > 0 to install a new slot, while @size == 0 to uninstall a +* slot. The return code can be one of the following: +* +* HVA: on success (uninstall will return a bogus HVA) +* -errno: on error +* +* The caller should always use IS_ERR() to check the return value +* before use. NOTE: KVM internal memory slots are guaranteed and "are guaranteed" to ...
arch/x86/kvm/x86.c void kvm_arch_sync_events(struct kvm *kvm) · line 9813
Quoted 6 lines
arch/x86/kvm/x86.c +6
-0+* won't change until the VM is destroyed. This is also true to the +* returned HVA when installing a new memory slot. The HVA can be +* invalidated by either an errornous userspace program or a VM under +* destruction, however as long as we use __copy_{to|from}_user() +* properly upon the HVAs and handle the failure paths always then +* we're safe. Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the
GPA->HVAis guaranteed to be valid, but theHVA->HPAis not./*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. theGPA->HVAtranslation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */
Quoted 8 lines
Quoted 3 lines
Sure I can switch to this, though note that I still think the
GPA->HVAis not guaranteed logically because the userspace can unmap any HVA it wants..You're conflating the
GPA->HVAtranslation with the validity of the HVA, i.e. theHVA->HPAand/orHVA->VMAtranslation/association.GPA->HVAis guaranteed because userspace doesn't have access to the memslot which defines that transation.
Yes I completely agree if you mean the pure mapping of GPA->HVA.
I think it's a matter of how to define the "valid" when you say "guaranteed to remain valid", because I don't think the mapping is still valid from the most strict sense if e.g. the backing HVA does not exist any more for that GPA->HVA mapping, then the memslot won't be anything useful.
Quoted 7 lines
Quoted 3 lines
However I agree that shouldn't be important from kvm's perspective as long as we always emphasize on using legal HVA accessors.
The fact that
GPA->HVAcan't change _is_ important, otherwise KVM would need to take steps to ensure that whatever can changeGPA->HVAcan't run concurrently with consuming the HVA.
I wanted to mean "the userspace unmaps the HVA" is not important. The mapping is for sure important!
Thanks,
Signature · 1 line
Peter Xu
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 22 lines · Peter Xu
Quoted 12 lines · Sean Christopherson
Quoted 12 lines
Quoted 11 lines
Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the
GPA->HVAis guaranteed to be valid, but theHVA->HPAis not./*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. theGPA->HVAtranslation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */Quoted 8 lines
Quoted 3 lines
Sure I can switch to this, though note that I still think the
GPA->HVAis not guaranteed logically because the userspace can unmap any HVA it wants..You're conflating the
GPA->HVAtranslation with the validity of the HVA, i.e. theHVA->HPAand/orHVA->VMAtranslation/association.GPA->HVAis guaranteed because userspace doesn't have access to the memslot which defines that transation.
Quoted 7 lines
Yes I completely agree if you mean the pure mapping of
GPA->HVA.I think it's a matter of how to define the "valid" when you say "guaranteed to remain valid", because I don't think the mapping is still valid from the most strict sense if e.g. the backing HVA does not exist any more for that
GPA->HVAmapping, then the memslot won't be anything useful.
Yes. That's why my proposed comment is worded to state that the _memslot_ will remain valid. It deliberately avoids mentioning "valid HVA".
Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]
Quoted 25 lines · Sean Christopherson
Quoted 24 lines · Peter Xu
Quoted 12 lines · Sean Christopherson
Quoted 12 lines
Quoted 11 lines
Regarding the HVA, it's a bit confusing saying that it's guaranteed to be valid, and then contradicting that in the second clause. Maybe something like this to explain the
GPA->HVAis guaranteed to be valid, but theHVA->HPAis not./*
* before use. Note, KVM internal memory slots are guaranteed to remain valid
* and unchanged until the VM is destroyed, i.e. theGPA->HVAtranslation will
* not change. However, the HVA is a user address, i.e. its accessibility is
* not guaranteed, and must be accessed via __copy_{to,from}_user(). */Quoted 8 lines
Quoted 3 lines
Sure I can switch to this, though note that I still think the
GPA->HVAis not guaranteed logically because the userspace can unmap any HVA it wants..You're conflating the
GPA->HVAtranslation with the validity of the HVA, i.e. theHVA->HPAand/orHVA->VMAtranslation/association.GPA->HVAis guaranteed because userspace doesn't have access to the memslot which defines that transation.Yes I completely agree if you mean the pure mapping of
GPA->HVA.
Quoted 8 lines
Quoted 5 lines
I think it's a matter of how to define the "valid" when you say "guaranteed to remain valid", because I don't think the mapping is still valid from the most strict sense if e.g. the backing HVA does not exist any more for that
GPA->HVAmapping, then the memslot won't be anything useful.Yes. That's why my proposed comment is worded to state that the _memslot_ will remain valid. It deliberately avoids mentioning "valid HVA".
OK, I see the point. I did re-read the two versions again, I agree yours is better, which I'll replace with. Thanks!
Signature · 1 line
Peter Xu
PATCH v7 4/14 KVM: Pass in kvm pointer into mark_page_dirty_in_slot()
In reply to Peter XuThe context will be needed to implement the kvm dirty ring.
--- virt/kvm/kvm_main.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)
virt/kvm/kvm_main.c +19 -14
@@ -144,7 +144,9 @@ static void hardware_disable_all(void);static void kvm_io_bus_destroy (struct kvm_io_bus *bus); -static void mark_page_dirty_in_slot (struct kvm_memory_slot *memslot,gfn_t gfn); +static void mark_page_dirty_in_slot (struct kvm *kvm, +struct kvm_memory_slot *memslot, +gfn_t gfn); __visible bool kvm_rebooting;EXPORT_SYMBOL_GPL (kvm_rebooting);
@@ -2117,7 +2119,8 @@ int kvm_vcpu_map(struct kvm_vcpu *vcpu, gfn_t gfn, struct kvm_host_map *map) }EXPORT_SYMBOL_GPL (kvm_vcpu_map); -static void __kvm_unmap_gfn (struct kvm_memory_slot *memslot, +static void __kvm_unmap_gfn (struct kvm *kvm, +struct kvm_memory_slot *memslot,struct kvm_host_map *map,struct gfn_to_pfn_cache *cache,bool dirty,bool atomic)
@@ -2142,7 +2145,7 @@ static void __kvm_unmap_gfn(struct kvm_memory_slot *memslot,#endif if (dirty) -mark_page_dirty_in_slot (memslot, map->gfn); +mark_page_dirty_in_slot (kvm, memslot, map->gfn);if (cache) cache->dirty |= dirty;
@@ -2156,7 +2159,7 @@ static void __kvm_unmap_gfn(struct kvm_memory_slot *memslot,int kvm_unmap_gfn (struct kvm_vcpu *vcpu,struct kvm_host_map *map,struct gfn_to_pfn_cache *cache,bool dirty,bool atomic) { -__kvm_unmap_gfn (gfn_to_memslot (vcpu->kvm, map->gfn), map, +__kvm_unmap_gfn (vcpu->kvm,gfn_to_memslot (vcpu->kvm, map->gfn), map, cache, dirty, atomic);return 0 ; }
@@ -2164,8 +2167,8 @@ EXPORT_SYMBOL_GPL(kvm_unmap_gfn);void kvm_vcpu_unmap (struct kvm_vcpu *vcpu,struct kvm_host_map *map,bool dirty) { -__kvm_unmap_gfn (kvm_vcpu_gfn_to_memslot (vcpu, map->gfn), map,NULL , - dirty, false); +__kvm_unmap_gfn (vcpu->kvm,kvm_vcpu_gfn_to_memslot (vcpu, map->gfn), + map,NULL , dirty, false); }EXPORT_SYMBOL_GPL (kvm_vcpu_unmap);
@@ -2339,7 +2342,8 @@ int kvm_vcpu_read_guest_atomic(struct kvm_vcpu *vcpu, gpa_t gpa, }EXPORT_SYMBOL_GPL (kvm_vcpu_read_guest_atomic); -static int __kvm_write_guest_page (struct kvm_memory_slot *memslot,gfn_t gfn, +static int __kvm_write_guest_page (struct kvm *kvm, +struct kvm_memory_slot *memslot,gfn_t gfn,const void *data,int offset,int len) {int r;
@@ -2351,7 +2355,7 @@ static int __kvm_write_guest_page(struct kvm_memory_slot *memslot, gfn_t gfn, r =__copy_to_user ((void __user *)addr + offset, data, len);if (r)return -EFAULT ; -mark_page_dirty_in_slot (memslot, gfn); +mark_page_dirty_in_slot (kvm, memslot, gfn);return 0 ; }
@@ -2360,7 +2364,7 @@ int kvm_write_guest_page(struct kvm *kvm, gfn_t gfn, {struct kvm_memory_slot *slot =gfn_to_memslot (kvm, gfn); -return __kvm_write_guest_page (slot, gfn, data, offset, len); +return __kvm_write_guest_page (kvm, slot, gfn, data, offset, len); }EXPORT_SYMBOL_GPL (kvm_write_guest_page);
@@ -2369,7 +2373,7 @@ int kvm_vcpu_write_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn, {struct kvm_memory_slot *slot =kvm_vcpu_gfn_to_memslot (vcpu, gfn); -return __kvm_write_guest_page (slot, gfn, data, offset, len); +return __kvm_write_guest_page (vcpu->kvm, slot, gfn, data, offset, len); }EXPORT_SYMBOL_GPL (kvm_vcpu_write_guest_page);
@@ -2488,7 +2492,7 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc, r =__copy_to_user ((void __user *)ghc->hva + offset, data, len);if (r)return -EFAULT ; -mark_page_dirty_in_slot (ghc->memslot, gpa >>PAGE_SHIFT ); +mark_page_dirty_in_slot (kvm, ghc->memslot, gpa >>PAGE_SHIFT );return 0 ; }
@@ -2555,7 +2559,8 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len) }EXPORT_SYMBOL_GPL (kvm_clear_guest); -static void mark_page_dirty_in_slot (struct kvm_memory_slot *memslot, +static void mark_page_dirty_in_slot (struct kvm *kvm, +struct kvm_memory_slot *memslot,gfn_t gfn) {if (memslot && memslot->dirty_bitmap) {
@@ -2570,7 +2575,7 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)struct kvm_memory_slot *memslot; memslot =gfn_to_memslot (kvm, gfn); -mark_page_dirty_in_slot (memslot, gfn); +mark_page_dirty_in_slot (kvm, memslot, gfn); }EXPORT_SYMBOL_GPL (mark_page_dirty);
@@ -2579,7 +2584,7 @@ void kvm_vcpu_mark_page_dirty(struct kvm_vcpu *vcpu, gfn_t gfn)struct kvm_memory_slot *memslot; memslot =kvm_vcpu_gfn_to_memslot (vcpu, gfn); -mark_page_dirty_in_slot (memslot, gfn); +mark_page_dirty_in_slot (vcpu->kvm, memslot, gfn); }EXPORT_SYMBOL_GPL (kvm_vcpu_mark_page_dirty);
Signature · 1 line
2.24.1
PATCH v7 5/14 KVM: X86: Implement ring-based dirty memory tracking
In reply to Peter XuThis patch is heavily based on previous work from Lei Cao <[email protected]> and Paolo Bonzini <[email protected]>. [1]
KVM currently uses large bitmaps to track dirty memory. These bitmaps are copied to userspace when userspace queries KVM for its dirty page information. The use of bitmaps is mostly sufficient for live migration, as large parts of memory are be dirtied from one log-dirty pass to another. However, in a checkpointing system, the number of dirty pages is small and in fact it is often bounded---the VM is paused when it has dirtied a pre-defined number of pages. Traversing a large, sparsely populated bitmap to find set bits is time-consuming, as is copying the bitmap to user-space.
A similar issue will be there for live migration when the guest memory is huge while the page dirty procedure is trivial. In that case for each dirty sync we need to pull the whole dirty bitmap to userspace and analyse every bit even if it's mostly zeros.
The preferred data structure for above scenarios is a dense list of guest frame numbers (GFN). This patch series stores the dirty list in kernel memory that can be memory mapped into userspace to allow speedy harvesting.
This patch enables dirty ring for X86 only. However it should be easily extended to other archs as well.
[1] https://patchwork.kernel.org/patch/10471409/
--- Documentation/virt/kvm/api.rst | 116 +++++++++++++++++++ arch/x86/include/asm/kvm_host.h | 3 + arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/Makefile | 3 +- arch/x86/kvm/mmu/mmu.c | 6 + arch/x86/kvm/vmx/vmx.c | 7 ++ arch/x86/kvm/x86.c | 9 ++ include/linux/kvm_dirty_ring.h | 103 +++++++++++++++++ include/linux/kvm_host.h | 13 +++ include/trace/events/kvm.h | 78 +++++++++++++ include/uapi/linux/kvm.h | 53 +++++++++ virt/kvm/dirty_ring.c | 195 ++++++++++++++++++++++++++++++++ virt/kvm/kvm_main.c | 112 +++++++++++++++++- 13 files changed, 697 insertions(+), 2 deletions(-) create mode 100644 include/linux/kvm_dirty_ring.h create mode 100644 virt/kvm/dirty_ring.c
Documentation/virt/kvm/api.rst +116 -0
@@ -249,6 +249,7 @@ Based on their initialization different VMs may have different capabilities. It is thus encouraged to use the vm ioctl to query for capabilities (available with KVM_CAP_CHECK_EXTENSION_VM on the vm fd) + 4.5 KVM_GET_VCPU_MMAP_SIZE --------------------------
@@ -262,6 +263,18 @@ The KVM_RUN ioctl (cf.) communicates with userspace via a shared memory region. This ioctl returns the size of that region. See the KVM_RUN documentation for details. +Besides the size of the KVM_RUN communication region, other areas of +the VCPU file descriptor can be mmap-ed, including: + +- if KVM_CAP_COALESCED_MMIO is available, a page at + KVM_COALESCED_MMIO_PAGE_OFFSET * PAGE_SIZE; for historical reasons, + this page is included in the result of KVM_GET_VCPU_MMAP_SIZE. + KVM_CAP_COALESCED_MMIO is not documented yet. + +- if KVM_CAP_DIRTY_LOG_RING is available, a number of pages at + KVM_DIRTY_LOG_PAGE_OFFSET * PAGE_SIZE. For more information on + KVM_CAP_DIRTY_LOG_RING, see section 8.3. + 4.6 KVM_SET_MEMORY_REGION -------------------------
@@ -6086,3 +6099,106 @@ KVM can therefore start protected VMs. This capability governs the KVM_S390_PV_COMMAND ioctl and the KVM_MP_STATE_LOAD MP_STATE. KVM_SET_MP_STATE can fail for protected guests when the state change is invalid. + +8.24 KVM_CAP_DIRTY_LOG_RING + +Architectures: x86 +Parameters: args[0] - size of the dirty log ring + +KVM is capable of tracking dirty memory using ring buffers that are +mmaped into userspace; there is one dirty ring per vcpu. + +One dirty ring is defined as below internally: + +struct kvm_dirty_ring { + u32 dirty_index; + u32 reset_index; + u32 size; + u32 soft_limit; + struct kvm_dirty_gfn *dirty_gfns; + int index; +}; + +Dirty GFNs (Guest Frame Numbers) are stored in the dirty_gfns array. +For each of the dirty entry it's defined as: + +struct kvm_dirty_gfn { + __u32 flags; + __u32 slot; /* as_id | slot_id */ + __u64 offset; +}; + +Each GFN is a state machine itself. The state is embeded in the flags +field, as defined in the uapi header: + +/* + * KVM dirty GFN flags, defined as: + * + * |---------------+---------------+--------------| + * | bit 1 (reset) | bit 0 (dirty) | Status | + * |---------------+---------------+--------------| + * | 0 | 0 | Invalid GFN | + * | 0 | 1 | Dirty GFN | + * | 1 | X | GFN to reset | + * |---------------+---------------+--------------| + * + * Lifecycle of a dirty GFN goes like: + * + * dirtied collected reset + * 00 -----------> 01 -------------> 1X -------+ + * ^ | + * | | + * +------------------------------------------+ + * + * The userspace program is only responsible for the 01->1X state + * conversion (to collect dirty bits). Also, it must not skip any + * dirty bits so that dirty bits are always collected in sequence. + */ +#define KVM_DIRTY_GFN_F_DIRTY BIT(0) +#define KVM_DIRTY_GFN_F_RESET BIT(1) +#define KVM_DIRTY_GFN_F_MASK 0x3 + +Userspace calls KVM_ENABLE_CAP ioctl right after KVM_CREATE_VM ioctl +to enable this capability for the new guest and set the size of the +rings. It is only allowed before creating any vCPU, and the size of +the ring must be a power of two. The larger the ring buffer, the less +likely the ring is full and the VM is forced to exit to userspace. The +optimal size depends on the workload, but it is recommended that it be +at least 64 KiB (4096 entries). + +Just like for dirty page bitmaps, the buffer tracks writes to +all user memory regions for which the KVM_MEM_LOG_DIRTY_PAGES flag was +set in KVM_SET_USER_MEMORY_REGION. Once a memory region is registered +with the flag set, userspace can start harvesting dirty pages from the +ring buffer. + +To harvest the dirty pages, userspace accesses the mmaped ring buffer +to read the dirty GFNs starting from zero. If the flags has the DIRTY +bit set (at this stage the RESET bit must be cleared), then it means +this GFN is a dirty GFN. The userspace should collect this GFN and +mark the flags from state 01b to 1Xb (bit 0 will be ignored by KVM, +but bit 1 must be set to show that this GFN is collected and waiting +for a reset), and move on to the next GFN. The userspace should +continue to do this until when the flags of a GFN has the DIRTY bit +cleared, it means we've collected all the dirty GFNs we have for now. +It's not a must that the userspace collects the all dirty GFNs in +once. However it must collect the dirty GFNs in sequence, i.e., the +userspace program cannot skip one dirty GFN to collect the one next to +it. + +After processing one or more entries in the ring buffer, userspace +calls the VM ioctl KVM_RESET_DIRTY_RINGS to notify the kernel about +it, so that the kernel will reprotect those collected GFNs. +Therefore, the ioctl must be called *before* reading the content of +the dirty pages. + +The dirty ring interface has a major difference comparing to the +KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from +userspace it's still possible that the kernel has not yet flushed the +hardware dirty buffers into the kernel buffer (the flushing was +previously done by the KVM_GET_DIRTY_LOG ioctl). To achieve that, one +needs to kick the vcpu out for a hardware buffer flush (vmexit) to +make sure all the existing dirty gfns are flushed to the dirty rings. + +The dirty ring can gets full. When it happens, the KVM_RUN of the +vcpu will return with exit reason KVM_EXIT_DIRTY_LOG_FULL.
arch/x86/include/asm/kvm_host.h +3 -0
@@ -1201,6 +1201,7 @@ struct kvm_x86_ops {struct kvm_memory_slot *slot, gfn_t offset, unsigned long mask);int (*write_log_dirty)(struct kvm_vcpu *vcpu); +int (*cpu_dirty_log_size)(void );/* pmu operations of sub-arch */ const struct kvm_pmu_ops *pmu_ops;
@@ -1693,4 +1694,6 @@ static inline int kvm_cpu_get_apicid(int mps_cpu)#define GET_SMSTATE (type, buf, offset) \ (*(type *)((buf) + (offset) - 0x7e00)) +int kvm_cpu_dirty_log_size (void ); +#endif /* _ASM_X86_KVM_HOST_H */
arch/x86/include/uapi/asm/kvm.h +1 -0
@@ -12,6 +12,7 @@#define KVM_PIO_PAGE_OFFSET 1#define KVM_COALESCED_MMIO_PAGE_OFFSET 2 +#define KVM_DIRTY_LOG_PAGE_OFFSET 64#define DE_VECTOR 0#define DB_VECTOR 1
arch/x86/kvm/Makefile +2 -1
@@ -6,7 +6,8 @@ ccflags-y += -Werror KVM := ../../../virt/kvm kvm-y += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \ - $(KVM)/eventfd.o $(KVM)/irqchip.o $(KVM)/vfio.o + $(KVM)/eventfd.o $(KVM)/irqchip.o $(KVM)/vfio.o \ + $(KVM)/dirty_ring.o kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o kvm-y += x86.o emulate.o i8259.o irq.o lapic.o \
arch/x86/kvm/mmu/mmu.c +6 -0
@@ -1749,7 +1749,13 @@ int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu) {if (kvm_x86_ops->write_log_dirty)return kvm_x86_ops->write_log_dirty (vcpu); +return 0 ; +} +int kvm_cpu_dirty_log_size (void ) +{ +if (kvm_x86_ops->cpu_dirty_log_size) +return kvm_x86_ops->cpu_dirty_log_size ();return 0 ; }
arch/x86/kvm/vmx/vmx.c +7 -0
@@ -7752,6 +7752,7 @@ static __init int hardware_setup(void) kvm_x86_ops->slot_disable_log_dirty =NULL ; kvm_x86_ops->flush_log_dirty =NULL ; kvm_x86_ops->enable_log_dirty_pt_masked =NULL ; + kvm_x86_ops->cpu_dirty_log_size =NULL ; }if (!cpu_has_vmx_preemption_timer ())
@@ -7826,6 +7827,11 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit)return supported &BIT (bit); } +static int vmx_cpu_dirty_log_size (void ) +{ +return enable_pml ?PML_ENTITY_NUM :0 ; +} +static struct kvm_x86_ops vmx_x86_ops__ro_after_init = { .cpu_has_kvm_support = cpu_has_kvm_support, .disabled_by_bios = vmx_disabled_by_bios,
@@ -7936,6 +7942,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { .flush_log_dirty = vmx_flush_log_dirty, .enable_log_dirty_pt_masked = vmx_enable_log_dirty_pt_masked, .write_log_dirty = vmx_write_pml_buffer, + .cpu_dirty_log_size = vmx_cpu_dirty_log_size, .pre_block = vmx_pre_block, .post_block = vmx_post_block,
arch/x86/kvm/x86.c +9 -0
@@ -8171,6 +8171,15 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)bool req_immediate_exit = false; +/* Forbid vmenter if vcpu dirty ring is soft-full */ +if (unlikely (vcpu->kvm->dirty_ring_size && +kvm_dirty_ring_soft_full (&vcpu->dirty_ring))) { + vcpu->run->exit_reason =KVM_EXIT_DIRTY_RING_FULL ; +trace_kvm_dirty_ring_exit (vcpu); + r =0 ; + goto out; + } +if (kvm_request_pending (vcpu)) {if (kvm_check_request (KVM_REQ_GET_VMCS12_PAGES , vcpu)) {if (unlikely (!kvm_x86_ops->get_vmcs12_pages (vcpu))) {
include/linux/kvm_dirty_ring.h new file +103 -0
103 lines not on this page · raw message
include/linux/kvm_host.h +13 -0
@@ -34,6 +34,7 @@#include <linux/kvm_types.h> #include <asm/kvm_host.h> +#include <linux/kvm_dirty_ring.h> #ifndef KVM_MAX_VCPU_ID #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS
@@ -319,6 +320,7 @@ struct kvm_vcpu {bool ready;struct kvm_vcpu_arch arch;struct dentry *debugfs_dentry; +struct kvm_dirty_ring dirty_ring; };static inline int kvm_vcpu_exiting_guest_mode (struct kvm_vcpu *vcpu)
@@ -504,6 +506,7 @@ struct kvm {struct srcu_struct srcu;struct srcu_struct irq_srcu;pid_t userspace_pid; +u32 dirty_ring_size; };#define kvm_err (fmt, ...) \
@@ -1419,4 +1422,14 @@ int kvm_vm_create_worker_thread(struct kvm *kvm, kvm_vm_thread_fn_t thread_fn, uintptr_t data, const char *name, struct task_struct **thread_ptr); +/* +* This defines how many reserved entries we want to keep before we +* kick the vcpu to the userspace to avoid dirty ring full. This +* value can be tuned to higher if e.g. PML is enabled on the host. +*/ +#define KVM_DIRTY_RING_RSVD_ENTRIES 64 + +/* Max number of entries allowed for each kvm dirty ring */ +#define KVM_DIRTY_RING_MAX_ENTRIES 65536 +#endif
include/trace/events/kvm.h +78 -0
84 lines not on this page · raw message
include/uapi/linux/kvm.h +53 -0
75 lines not on this page · raw message
virt/kvm/dirty_ring.c new file +195 -0
195 lines not on this page · raw message
virt/kvm/kvm_main.c +111 -1
185 lines not on this page · raw message
Signature · 1 line
2.24.1
PATCH v7 6/14 KVM: Make dirty ring exclusive to dirty bitmap log
In reply to Peter XuThere's no good reason to use both the dirty bitmap logging and the new dirty ring buffer to track dirty bits. We should be able to even support both of them at the same time, but it could complicate things which could actually help little. Let's simply make it the rule before we enable dirty ring on any arch, that we don't allow these two interfaces to be used together.
The big world switch would be KVM_CAP_DIRTY_LOG_RING capability enablement. That's where we'll switch from the default dirty logging way to the dirty ring way. As long as kvm->dirty_ring_size is setup correctly, we'll once and for all switch to the dirty ring buffer mode for the current virtual machine.
--- Documentation/virt/kvm/api.rst | 7 +++++++ virt/kvm/kvm_main.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)
Documentation/virt/kvm/api.rst +7 -0
@@ -6202,3 +6202,10 @@ make sure all the existing dirty gfns are flushed to the dirty rings. The dirty ring can gets full. When it happens, the KVM_RUN of the vcpu will return with exit reason KVM_EXIT_DIRTY_LOG_FULL. + +NOTE: the KVM_CAP_DIRTY_LOG_RING capability and the new ioctl +KVM_RESET_DIRTY_RINGS are exclusive to the existing KVM_GET_DIRTY_LOG +interface. After enabling KVM_CAP_DIRTY_LOG_RING with an acceptable +dirty ring size, the virtual machine will switch to the dirty ring +tracking mode, and KVM_GET_DIRTY_LOG, KVM_CLEAR_DIRTY_LOG ioctls will +stop working.
virt/kvm/kvm_main.c +12 -0
@@ -1352,6 +1352,10 @@ int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log,unsigned long n;unsigned long any =0 ; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + *memslot =NULL ; *is_dirty =0 ;
@@ -1413,6 +1417,10 @@ static int kvm_get_dirty_log_protect(struct kvm *kvm, struct kvm_dirty_log *log)unsigned long *dirty_bitmap_buffer;bool flush; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + as_id = log->slot >>16 ; id = (u16 )log->slot;if (as_id >=KVM_ADDRESS_SPACE_NUM || id >=KVM_USER_MEM_SLOTS )
@@ -1521,6 +1529,10 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,unsigned long *dirty_bitmap_buffer;bool flush; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + as_id = log->slot >>16 ; id = (u16 )log->slot;if (as_id >=KVM_ADDRESS_SPACE_NUM || id >=KVM_USER_MEM_SLOTS )
Signature · 1 line
2.24.1
Re: [PATCH v7 06/14] KVM: Make dirty ring exclusive to dirty bitmap log
Quoted 24 lines · Peter Xu
There's no good reason to use both the dirty bitmap logging and the new dirty ring buffer to track dirty bits. We should be able to even support both of them at the same time, but it could complicate things which could actually help little. Let's simply make it the rule before we enable dirty ring on any arch, that we don't allow these two interfaces to be used together.
The big world switch would be
KVM_CAP_DIRTY_LOG_RINGcapability enablement. That's where we'll switch from the default dirty logging way to the dirty ring way. As long askvm->dirty_ring_sizeis setup correctly, we'll once and for all switch to the dirty ring buffer mode for the current virtual machine.Signed-off-by Peter Xu <[email protected]>--- Documentation/virt/kvm/api.rst | 7 +++++++ virt/kvm/kvm_main.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)Documentation/virt/kvm/api.rst +0
-0
Documentation/virt/kvm/api.rst make sure all the existing dirty gfns are flushed to the dirty rings. · line 6206
Quoted 5 lines
Documentation/virt/kvm/api.rst +2
-0@@ -6202,3 +6202,10 @@ make sure all the existing dirty gfns are flushed to the dirty rings. The dirty ring can gets full. When it happens, the KVM_RUN of the vcpu will return with exit reason KVM_EXIT_DIRTY_LOG_FULL. + +NOTE: the KVM_CAP_DIRTY_LOG_RING capability and the new ioctl
Leave off "new", it'll be stale a few months/years from now.
Documentation/virt/kvm/api.rst make sure all the existing dirty gfns are flushed to the dirty rings. · line 6207
Quoted 1 line
Documentation/virt/kvm/api.rst +1
-0+KVM_RESET_DIRTY_RINGS are exclusive to the existing KVM_GET_DIRTY_LOG
Did you mean "mutually exclusive with"? "exclusive to" would mean they can only be used by KVM_GET_DIRTY_LOG with doesn't match the next sentence.
Quoted 44 lines
Documentation/virt/kvm/api.rst +4
-0+interface. After enabling KVM_CAP_DIRTY_LOG_RING with an acceptable +dirty ring size, the virtual machine will switch to the dirty ring +tracking mode, and KVM_GET_DIRTY_LOG, KVM_CLEAR_DIRTY_LOG ioctls will +stop working.virt/kvm/kvm_main.c +12
-0@@ -1352,6 +1352,10 @@ int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log,unsigned long n;unsigned long any =0 ; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + *memslot =NULL ; *is_dirty =0 ;@@ -1413,6 +1417,10 @@ static int kvm_get_dirty_log_protect(struct kvm *kvm, struct kvm_dirty_log *log)unsigned long *dirty_bitmap_buffer;bool flush; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + as_id = log->slot >>16 ; id = (u16 )log->slot;if (as_id >=KVM_ADDRESS_SPACE_NUM || id >=KVM_USER_MEM_SLOTS )@@ -1521,6 +1529,10 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,unsigned long *dirty_bitmap_buffer;bool flush; +/* Dirty ring tracking is exclusive to dirty log tracking */ +if (kvm->dirty_ring_size) +return -EINVAL ; + as_id = log->slot >>16 ; id = (u16 )log->slot;if (as_id >=KVM_ADDRESS_SPACE_NUM || id >=KVM_USER_MEM_SLOTS )Signature · 1 line
2.24.1
Re: [PATCH v7 06/14] KVM: Make dirty ring exclusive to dirty bitmap log
Quoted 26 lines · Sean Christopherson
Quoted 25 lines · Peter Xu
There's no good reason to use both the dirty bitmap logging and the new dirty ring buffer to track dirty bits. We should be able to even support both of them at the same time, but it could complicate things which could actually help little. Let's simply make it the rule before we enable dirty ring on any arch, that we don't allow these two interfaces to be used together.
The big world switch would be
KVM_CAP_DIRTY_LOG_RINGcapability enablement. That's where we'll switch from the default dirty logging way to the dirty ring way. As long askvm->dirty_ring_sizeis setup correctly, we'll once and for all switch to the dirty ring buffer mode for the current virtual machine.Signed-off-by Peter Xu <[email protected]>--- Documentation/virt/kvm/api.rst | 7 +++++++ virt/kvm/kvm_main.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)Documentation/virt/kvm/api.rst +0
-0@@ -6202,3 +6202,10 @@ make sure all the existing dirty gfns are flushed to the dirty rings.
Quoted 6 lines
Documentation/virt/kvm/api.rst make sure all the existing dirty gfns are flushed to the dirty rings. · line 6206
Quoted 4 lines
Documentation/virt/kvm/api.rst +2
-0The dirty ring can gets full. When it happens, the KVM_RUN of the vcpu will return with exit reason KVM_EXIT_DIRTY_LOG_FULL. + +NOTE: the KVM_CAP_DIRTY_LOG_RING capability and the new ioctlLeave off "new", it'll be stale a few months/years from now.
Ok.
Quoted 6 lines
Documentation/virt/kvm/api.rst make sure all the existing dirty gfns are flushed to the dirty rings. · line 6207
Quoted 2 lines
Documentation/virt/kvm/api.rst +1
-0+KVM_RESET_DIRTY_RINGS are exclusive to the existing KVM_GET_DIRTY_LOGDid you mean "mutually exclusive with"? "exclusive to" would mean they can only be used by
KVM_GET_DIRTY_LOGwith doesn't match the next sentence.
I meant "mutual-exclusive". I'll fix it up.
Thanks,
Signature · 1 line
Peter Xu
PATCH v7 7/14 KVM: Don't allocate dirty bitmap if dirty ring is enabled
In reply to Peter XuBecause kvm dirty rings and kvm dirty log is used in an exclusive way, Let's avoid creating the dirty_bitmap when kvm dirty ring is enabled. At the meantime, since the dirty_bitmap will be conditionally created now, we can't use it as a sign of "whether this memory slot enabled dirty tracking". Change users like that to check against the kvm memory slot flags.
Note that there still can be chances where the kvm memory slot got its dirty_bitmap allocated, _if_ the memory slots are created before enabling of the dirty rings and at the same time with the dirty tracking capability enabled, they'll still with the dirty_bitmap. However it should not hurt much (e.g., the bitmaps will always be freed if they are there), and the real users normally won't trigger this because dirty bit tracking flag should in most cases only be applied to kvm slots only before migration starts, that should be far latter than kvm initializes (VM starts).
--- arch/x86/kvm/mmu/mmu.c | 4 ++-- include/linux/kvm_host.h | 5 +++++ virt/kvm/kvm_main.c | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu/mmu.c +2 -2
@@ -1276,8 +1276,8 @@ gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu, gfn_t gfn, slot =kvm_vcpu_gfn_to_memslot (vcpu, gfn);if (!slot || slot->flags &KVM_MEMSLOT_INVALID )return NULL ; -if (no_dirty_log && slot->dirty_bitmap) -return NULL ; +if (no_dirty_log &&kvm_slot_dirty_track_enabled (slot)) +return false;return slot; }
include/linux/kvm_host.h +5 -0
@@ -351,6 +351,11 @@ struct kvm_memory_slot {u16 as_id; }; +static inline bool kvm_slot_dirty_track_enabled (struct kvm_memory_slot *slot) +{ +return slot->flags &KVM_MEM_LOG_DIRTY_PAGES ; +} +static inline unsigned long kvm_dirty_bitmap_bytes (struct kvm_memory_slot *memslot) {return ALIGN (memslot->npages,BITS_PER_LONG ) /8 ;
virt/kvm/kvm_main.c +2 -2
@@ -1291,7 +1291,7 @@ int __kvm_set_memory_region(struct kvm *kvm,/* Allocate/free page dirty bitmap as needed */ if (!(new.flags &KVM_MEM_LOG_DIRTY_PAGES )) new.dirty_bitmap =NULL ; -else if (!new.dirty_bitmap) { +else if (!new.dirty_bitmap && !kvm->dirty_ring_size) { r =kvm_alloc_dirty_bitmap (&new);if (r)return r;
@@ -2578,7 +2578,7 @@ static void mark_page_dirty_in_slot(struct kvm *kvm, struct kvm_memory_slot *memslot, gfn_t gfn) { -if (memslot && memslot->dirty_bitmap) { +if (memslot &&kvm_slot_dirty_track_enabled (memslot)) {unsigned long rel_gfn = gfn - memslot->base_gfn;u32 slot = (memslot->as_id <<16 ) | memslot->id;
Signature · 1 line
2.24.1
PATCH v7 8/14 KVM: selftests: Always clear dirty bitmap after iteration
In reply to Peter XuWe don't clear the dirty bitmap before because KVM_GET_DIRTY_LOG will clear it for us before copying the dirty log onto it. However we'd still better to clear it explicitly instead of assuming the kernel will always do it for us.
More importantly, in the upcoming dirty ring tests we'll start to fetch dirty pages from a ring buffer, so no one is going to clear the dirty bitmap for us.
--- tools/testing/selftests/kvm/dirty_log_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/kvm/dirty_log_test.c +1 -1
@@ -195,7 +195,7 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) page); } -if (test_bit_le (page, bmap)) { +if (test_and_clear_bit_le (page, bmap)) { host_dirty_count++;/* * If the bit is set, the value written onto
Signature · 1 line
2.24.1
PATCH v7 9/14 KVM: selftests: Sync uapi/linux/kvm.h to tools/
In reply to Peter XuThis will be needed to extend the kvm selftest program.
--- tools/include/uapi/linux/kvm.h | 99 +++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 2 deletions(-)
tools/include/uapi/linux/kvm.h +97 -2
@@ -236,6 +236,7 @@ struct kvm_hyperv_exit {#define KVM_EXIT_IOAPIC_EOI 26#define KVM_EXIT_HYPERV 27#define KVM_EXIT_ARM_NISV 28 +#define KVM_EXIT_DIRTY_RING_FULL 29/* For KVM_EXIT_INTERNAL_ERROR */ /* Emulate instruction failed. */
@@ -474,12 +475,17 @@ struct kvm_s390_mem_op {__u32 size;/* amount of bytes */ __u32 op;/* type of operation */ __u64 buf;/* buffer in userspace */ -__u8 ar;/* the access register number */ -__u8 reserved[31 ];/* should be set to 0 */ +union { +__u8 ar;/* the access register number */ +__u32 sida_offset;/* offset into the sida */ +__u8 reserved[32 ];/* should be set to 0 */ + }; };/* types for kvm_s390_mem_op->op */ #define KVM_S390_MEMOP_LOGICAL_READ 0#define KVM_S390_MEMOP_LOGICAL_WRITE 1 +#define KVM_S390_MEMOP_SIDA_READ 2 +#define KVM_S390_MEMOP_SIDA_WRITE 3/* flags for kvm_s390_mem_op->flags */ #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)#define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
@@ -1010,6 +1016,8 @@ struct kvm_ppc_resize_hpt {#define KVM_CAP_ARM_NISV_TO_USER 177#define KVM_CAP_ARM_INJECT_EXT_DABT 178#define KVM_CAP_S390_VCPU_RESETS 179 +#define KVM_CAP_S390_PROTECTED 180 +#define KVM_CAP_DIRTY_LOG_RING 181#ifdef KVM_CAP_IRQ_ROUTING
@@ -1478,6 +1486,42 @@ struct kvm_enc_region {#define KVM_S390_NORMAL_RESET _IO(KVMIO, 0xc3)#define KVM_S390_CLEAR_RESET _IO(KVMIO, 0xc4) +struct kvm_s390_pv_sec_parm { +__u64 origin; +__u64 length; +}; + +struct kvm_s390_pv_unp { +__u64 addr; +__u64 size; +__u64 tweak; +}; + +enum pv_cmd_id { +KVM_PV_ENABLE , +KVM_PV_DISABLE , +KVM_PV_SET_SEC_PARMS , +KVM_PV_UNPACK , +KVM_PV_VERIFY , +KVM_PV_PREP_RESET , +KVM_PV_UNSHARE_ALL , +}; + +struct kvm_pv_cmd { +__u32 cmd;/* Command to be executed */ +__u16 rc;/* Ultravisor return code */ +__u16 rrc;/* Ultravisor return reason code */ +__u64 data;/* Data or address */ +__u32 flags;/* flags for future extensions. Must be 0 for now */ +__u32 reserved[3 ]; +}; + +/* Available with KVM_CAP_S390_PROTECTED */ +#define KVM_S390_PV_COMMAND _IOWR(KVMIO, 0xc5, struct kvm_pv_cmd) + +/* Available with KVM_CAP_DIRTY_LOG_RING */ +#define KVM_RESET_DIRTY_RINGS _IO(KVMIO, 0xc6) +/* Secure Encrypted Virtualization command */ enum sev_cmd_id {/* Guest initialization commands */
@@ -1628,4 +1672,55 @@ 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) + +/* +* Arch needs to define the macro after implementing the dirty ring +* feature. KVM_DIRTY_LOG_PAGE_OFFSET should be defined as the +* starting page offset of the dirty ring structures. +*/ +#ifndef KVM_DIRTY_LOG_PAGE_OFFSET +#define KVM_DIRTY_LOG_PAGE_OFFSET 0 +#endif + +/* +* KVM dirty GFN flags, defined as: +* +* |---------------+---------------+--------------| +* | bit 1 (reset) | bit 0 (dirty) | Status | +* |---------------+---------------+--------------| +* | 0 | 0 | Invalid GFN | +* | 0 | 1 | Dirty GFN | +* | 1 | X | GFN to reset | +* |---------------+---------------+--------------| +* +* Lifecycle of a dirty GFN goes like: +* +* dirtied collected reset +* 00 -----------> 01 -------------> 1X -------+ +* ^ | +* | | +* +------------------------------------------+ +* +* The userspace program is only responsible for the 01->1X state +* conversion (to collect dirty bits). Also, it must not skip any +* dirty bits so that dirty bits are always collected in sequence. +*/ +#define KVM_DIRTY_GFN_F_DIRTY BIT(0) +#define KVM_DIRTY_GFN_F_RESET BIT(1) +#define KVM_DIRTY_GFN_F_MASK 0x3 + +/* +* KVM dirty rings should be mapped at KVM_DIRTY_LOG_PAGE_OFFSET of +* per-vcpu mmaped regions as an array of struct kvm_dirty_gfn. The +* size of the gfn buffer is decided by the first argument when +* enabling KVM_CAP_DIRTY_LOG_RING. +*/ +struct kvm_dirty_gfn { +__u32 flags; +__u32 slot; +__u64 offset; +}; +#endif /* __LINUX_KVM_H */
Signature · 1 line
2.24.1
PATCH v7 10/14 KVM: selftests: Use a single binary for dirty/clear log test
In reply to Peter XuRemove the clear_dirty_log test, instead merge it into the existing dirty_log_test. It should be cleaner to use this single binary to do both tests, also it's a preparation for the upcoming dirty ring test.
The default behavior will run all the modes in sequence.
--- tools/testing/selftests/kvm/Makefile | 2 - .../selftests/kvm/clear_dirty_log_test.c | 6 - tools/testing/selftests/kvm/dirty_log_test.c | 187 +++++++++++++++--- 3 files changed, 156 insertions(+), 39 deletions(-) delete mode 100644 tools/testing/selftests/kvm/clear_dirty_log_test.c
tools/testing/selftests/kvm/Makefile +0 -2
@@ -28,13 +28,11 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_dirty_log_test TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test TEST_GEN_PROGS_x86_64 += x86_64/vmx_tsc_adjust_test TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test -TEST_GEN_PROGS_x86_64 += clear_dirty_log_test TEST_GEN_PROGS_x86_64 += demand_paging_test TEST_GEN_PROGS_x86_64 += dirty_log_test TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus TEST_GEN_PROGS_x86_64 += steal_time -TEST_GEN_PROGS_aarch64 += clear_dirty_log_test TEST_GEN_PROGS_aarch64 += demand_paging_test TEST_GEN_PROGS_aarch64 += dirty_log_test TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
tools/testing/selftests/kvm/clear_dirty_log_test.c deleted +0 -6
@@ -1,6 +0,0 @@ -#define USE_CLEAR_DIRTY_LOG -#define KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE (1 << 0) -#define KVM_DIRTY_LOG_INITIALLY_SET (1 << 1) -#define KVM_DIRTY_LOG_MANUAL_CAPS (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \ - KVM_DIRTY_LOG_INITIALLY_SET) -#include "dirty_log_test.c"
tools/testing/selftests/kvm/dirty_log_test.c +156 -31
259 lines not on this page · raw message
Signature · 1 line
2.24.1
Re: [PATCH v7 10/14] KVM: selftests: Use a single binary for dirty/clear log test
Quoted 6 lines · Peter Xu
Remove the
clear_dirty_logtest, instead merge it into the existingdirty_log_test. It should be cleaner to use this single binary to do both tests, also it's a preparation for the upcoming dirty ring test.The default behavior will run all the modes in sequence.
Quoted 8 lines
Signed-off-by Peter Xu <[email protected]>--- tools/testing/selftests/kvm/Makefile | 2 - .../selftests/kvm/clear_dirty_log_test.c | 6 - tools/testing/selftests/kvm/dirty_log_test.c | 187 +++++++++++++++--- 3 files changed, 156 insertions(+), 39 deletions(-) delete mode 100644 tools/testing/selftests/kvm/clear_dirty_log_test.c
PATCH v7 11/14 KVM: selftests: Introduce after_vcpu_run hook for dirty log test
In reply to Peter XuProvide a hook for the checks after vcpu_run() completes. Preparation for the dirty ring test because we'll need to take care of another exit reason.
Since at it, drop the pages_count because after all we have a better summary right now with statistics, and clean it up a bit.
--- tools/testing/selftests/kvm/dirty_log_test.c | 41 ++++++++++++++------ 1 file changed, 30 insertions(+), 11 deletions(-)
tools/testing/selftests/kvm/dirty_log_test.c +30 -11
@@ -178,6 +178,15 @@ static void clear_log_collect_dirty_pages(struct kvm_vm *vm, int slot,kvm_vm_clear_dirty_log (vm, slot, bitmap,0 , num_pages); } +static void default_after_vcpu_run (struct kvm_vm *vm) +{ +struct kvm_run *run =vcpu_state (vm,VCPU_ID ); + +TEST_ASSERT (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC , +"Invalid guest sync status: exit_reason=%s\n" , +exit_reason_str (run->exit_reason)); +} +struct log_mode {const char *name;/* Return true if this mode is supported, otherwise false */
@@ -187,16 +196,20 @@ struct log_mode {/* Hook to collect the dirty pages into the bitmap provided */ void (*collect_dirty_pages) (struct kvm_vm *vm,int slot,void *bitmap,uint32_t num_pages); +/* Hook to call when after each vcpu run */ +void (*after_vcpu_run)(struct kvm_vm *vm); } log_modes[LOG_MODE_NUM] = { { .name = "dirty-log", .collect_dirty_pages = dirty_log_collect_dirty_pages, + .after_vcpu_run = default_after_vcpu_run, }, { .name = "clear-log", .supported = clear_log_supported, .create_vm_done = clear_log_create_vm_done, .collect_dirty_pages = clear_log_collect_dirty_pages, + .after_vcpu_run = default_after_vcpu_run, }, };
@@ -247,6 +260,14 @@ static void log_mode_collect_dirty_pages(struct kvm_vm *vm, int slot, mode->collect_dirty_pages (vm, slot, bitmap, num_pages); } +static void log_mode_after_vcpu_run (struct kvm_vm *vm) +{ +struct log_mode *mode = &log_modes[host_log_mode]; + +if (mode->after_vcpu_run) + mode->after_vcpu_run (vm); +} +static void generate_random_array (uint64_t *guest_array,uint64_t size) {uint64_t i;
@@ -261,25 +282,23 @@ static void *vcpu_worker(void *data)struct kvm_vm *vm = data;uint64_t *guest_array;uint64_t pages_count =0 ; -struct kvm_run *run; +struct sigaction sigact; - run =vcpu_state (vm,VCPU_ID ); + current_vm = vm; + vcpu_fd =vcpu_get_fd (vm,VCPU_ID ); +memset (&sigact,0 ,sizeof (sigact)); + sigact.sa_handler = vcpu_sig_handler; +sigaction (SIG_IPI , &sigact,NULL ); guest_array =addr_gva2hva (vm, (vm_vaddr_t )random_array); -generate_random_array (guest_array,TEST_PAGES_PER_LOOP );while (!READ_ONCE (host_quit)) { +generate_random_array (guest_array,TEST_PAGES_PER_LOOP ); + pages_count +=TEST_PAGES_PER_LOOP ;/* Let the guest dirty the random pages */ ret =_vcpu_run (vm,VCPU_ID );TEST_ASSERT (ret ==0 ,"vcpu_run failed: %d\n" , ret); -if (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC ) { - pages_count +=TEST_PAGES_PER_LOOP ; -generate_random_array (guest_array,TEST_PAGES_PER_LOOP ); - }else { -TEST_FAIL ("Invalid guest sync status: " -"exit_reason=%s\n" , -exit_reason_str (run->exit_reason)); - } +log_mode_after_vcpu_run (vm); }pr_info ("Dirtied %" PRIu64" pages\n" , pages_count);
Signature · 1 line
2.24.1
Re: [PATCH v7 11/14] KVM: selftests: Introduce after_vcpu_run hook for dirty log test
Quoted 69 lines · Peter Xu
Provide a hook for the checks after
vcpu_run()completes. Preparation for the dirty ring test because we'll need to take care of another exit reason.Since at it, drop the
pages_countbecause after all we have a better summary right now with statistics, and clean it up a bit.Signed-off-by Peter Xu <[email protected]>--- tools/testing/selftests/kvm/dirty_log_test.c | 41 ++++++++++++++------ 1 file changed, 30 insertions(+), 11 deletions(-)tools/testing/selftests/kvm/dirty_log_test.c +21
-0@@ -178,6 +178,15 @@ static void clear_log_collect_dirty_pages(struct kvm_vm *vm, int slot,kvm_vm_clear_dirty_log (vm, slot, bitmap,0 , num_pages); } +static void default_after_vcpu_run (struct kvm_vm *vm) +{ +struct kvm_run *run =vcpu_state (vm,VCPU_ID ); + +TEST_ASSERT (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC , +"Invalid guest sync status: exit_reason=%s\n" , +exit_reason_str (run->exit_reason)); +} +struct log_mode {const char *name;/* Return true if this mode is supported, otherwise false */ @@ -187,16 +196,20 @@ struct log_mode {/* Hook to collect the dirty pages into the bitmap provided */ void (*collect_dirty_pages) (struct kvm_vm *vm,int slot,void *bitmap,uint32_t num_pages); +/* Hook to call when after each vcpu run */ +void (*after_vcpu_run)(struct kvm_vm *vm); } log_modes[LOG_MODE_NUM] = { { .name = "dirty-log", .collect_dirty_pages = dirty_log_collect_dirty_pages, + .after_vcpu_run = default_after_vcpu_run, }, { .name = "clear-log", .supported = clear_log_supported, .create_vm_done = clear_log_create_vm_done, .collect_dirty_pages = clear_log_collect_dirty_pages, + .after_vcpu_run = default_after_vcpu_run, }, };@@ -247,6 +260,14 @@ static void log_mode_collect_dirty_pages(struct kvm_vm *vm, int slot, mode->collect_dirty_pages (vm, slot, bitmap, num_pages); } +static void log_mode_after_vcpu_run (struct kvm_vm *vm) +{ +struct log_mode *mode = &log_modes[host_log_mode]; + +if (mode->after_vcpu_run) + mode->after_vcpu_run (vm); +} +static void generate_random_array (uint64_t *guest_array,uint64_t size) {uint64_t i;
tools/testing/selftests/kvm/dirty_log_test.c static void *vcpu_worker(void *data) · line 288
Quoted 9 lines
tools/testing/selftests/kvm/dirty_log_test.c +3
-2@@ -261,25 +282,23 @@ static void *vcpu_worker(void *data)struct kvm_vm *vm = data;uint64_t *guest_array;uint64_t pages_count =0 ; -struct kvm_run *run; +struct sigaction sigact; - run =vcpu_state (vm,VCPU_ID ); + current_vm = vm; + vcpu_fd =vcpu_get_fd (vm,VCPU_ID );
You don't add this call until 13/14, which means bisection is broken. Please test the series with 'git rebase -i -x make'.
Quoted 28 lines
tools/testing/selftests/kvm/dirty_log_test.c +6
-9+memset (&sigact,0 ,sizeof (sigact)); + sigact.sa_handler = vcpu_sig_handler; +sigaction (SIG_IPI , &sigact,NULL ); guest_array =addr_gva2hva (vm, (vm_vaddr_t )random_array); -generate_random_array (guest_array,TEST_PAGES_PER_LOOP );while (!READ_ONCE (host_quit)) { +generate_random_array (guest_array,TEST_PAGES_PER_LOOP ); + pages_count +=TEST_PAGES_PER_LOOP ;/* Let the guest dirty the random pages */ ret =_vcpu_run (vm,VCPU_ID );TEST_ASSERT (ret ==0 ,"vcpu_run failed: %d\n" , ret); -if (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC ) { - pages_count +=TEST_PAGES_PER_LOOP ; -generate_random_array (guest_array,TEST_PAGES_PER_LOOP ); - }else { -TEST_FAIL ("Invalid guest sync status: " -"exit_reason=%s\n" , -exit_reason_str (run->exit_reason)); - } +log_mode_after_vcpu_run (vm); }pr_info ("Dirtied %" PRIu64" pages\n" , pages_count);Signature · 1 line
2.24.1
Re: [PATCH v7 11/14] KVM: selftests: Introduce after_vcpu_run hook for dirty log test
Quoted 10 lines · Andrew Jones
Quoted 10 lines
static void
generate_random_array(uint64_t *guest_array, uint64_t size){tools/testing/selftests/kvm/dirty_log_test.c +1
-1uint64_t i;@@ -261,25 +282,23 @@ static void *vcpu_worker(void *data)struct kvm_vm *vm = data;uint64_t *guest_array;uint64_t pages_count =0 ; -struct kvm_run *run; +struct sigaction sigact;
Quoted 6 lines
tools/testing/selftests/kvm/dirty_log_test.c static void *vcpu_worker(void *data) · line 288
Quoted 3 lines
tools/testing/selftests/kvm/dirty_log_test.c +2
-1- run =vcpu_state (vm,VCPU_ID ); + current_vm = vm; + vcpu_fd =vcpu_get_fd (vm,VCPU_ID );You don't add this call until 13/14, which means bisection is broken. Please test the series with 'git rebase -i -x make'.
You're right... I do per-patch test but only for the initial version, then I test the fixups as standalone changes for latter versions. This must be a wrongly squashed fixup into the wrong patch. Sorry.
I'll wait for some more time for another repost. Thanks,
Signature · 1 line
Peter Xu
PATCH v7 12/14 KVM: selftests: Add dirty ring buffer test
In reply to Peter XuAdd the initial dirty ring buffer test.
The current test implements the userspace dirty ring collection, by only reaping the dirty ring when the ring is full.
So it's still running synchronously like this:
vcpu main thread
1. vcpu dirties pages
2. vcpu gets dirty ring full
(userspace exit)
3. main thread waits until full (so hardware buffers flushed)
4. main thread collects
5. main thread continues vcpu
6. vcpu continues, goes back to 1
We can't directly collects dirty bits during vcpu execution because otherwise we can't guarantee the hardware dirty bits were flushed when we collect and we're very strict on the dirty bits so otherwise we can fail the future verify procedure. A follow up patch will make this test to support async just like the existing dirty log test, by adding a vcpu kick mechanism.
--- tools/testing/selftests/kvm/dirty_log_test.c | 201 +++++++++++++++++- .../testing/selftests/kvm/include/kvm_util.h | 3 + tools/testing/selftests/kvm/lib/kvm_util.c | 59 +++++ .../selftests/kvm/lib/kvm_util_internal.h | 4 + 4 files changed, 265 insertions(+), 2 deletions(-)
tools/testing/selftests/kvm/dirty_log_test.c +199 -2
@@ -12,8 +12,10 @@#include <unistd.h> #include <time.h> #include <pthread.h> +#include <semaphore.h> #include <linux/bitmap.h> #include <linux/bitops.h> +#include <asm/barrier.h> #include "test_util.h" #include "kvm_util.h"
@@ -57,6 +59,8 @@# define test_and_clear_bit_le test_and_clear_bit#endif +#define TEST_DIRTY_RING_COUNT 1024 +/* * Guest/Host shared variables. Ensure addr_gva2hva() and/or * sync_global_to/from_guest() are used when accessing from
@@ -128,6 +132,10 @@ static uint64_t host_dirty_count;static uint64_t host_clear_count;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; +enum log_mode_t {/* Only use KVM_GET_DIRTY_LOG for logging */ LOG_MODE_DIRTY_LOG =0 ,
@@ -135,6 +143,9 @@ enum log_mode_t {/* Use both KVM_[GET|CLEAR]_DIRTY_LOG for logging */ LOG_MODE_CLEAR_LOG =1 , +/* Use dirty ring for logging */ +LOG_MODE_DIRTY_RING =2 , + LOG_MODE_NUM,/* Run all supported modes */
@@ -187,6 +198,120 @@ static void default_after_vcpu_run(struct kvm_vm *vm) exit_reason_str(run->exit_reason)); } +static bool dirty_ring_supported (void ) +{ +return kvm_check_cap (KVM_CAP_DIRTY_LOG_RING ); +} + +static void dirty_ring_create_vm_done (struct kvm_vm *vm) +{ +/* +* Switch to dirty ring mode after VM creation but before any +* of the vcpu creation. +*/ +vm_enable_dirty_ring (vm,TEST_DIRTY_RING_COUNT * +sizeof (struct kvm_dirty_gfn )); +} + +static inline bool dirty_gfn_is_dirtied (struct kvm_dirty_gfn *gfn) +{ +return gfn->flags ==KVM_DIRTY_GFN_F_DIRTY ; +} + +static inline void dirty_gfn_set_collected (struct kvm_dirty_gfn *gfn) +{ + gfn->flags =KVM_DIRTY_GFN_F_RESET ; +} + +static uint32_t dirty_ring_collect_one (struct kvm_dirty_gfn *dirty_gfns, +int slot,void *bitmap, +uint32_t num_pages,uint32_t *fetch_index) +{ +struct kvm_dirty_gfn *cur; +uint32_t count =0 ; + +while (true) { + cur = &dirty_gfns[*fetch_index %TEST_DIRTY_RING_COUNT ]; +if (!dirty_gfn_is_dirtied (cur)) +break ; +TEST_ASSERT (cur->slot == slot,"Slot number didn't match: " +"%u != %u" , cur->slot, slot); +TEST_ASSERT (cur->offset < num_pages,"Offset overflow: " +"0x%llx >= 0x%x" , cur->offset, num_pages); +pr_info ("fetch 0x%x page %llu\n" , *fetch_index, cur->offset); +set_bit (cur->offset, bitmap); +dirty_gfn_set_collected (cur); + (*fetch_index)++; + count++; + } + +return count; +} + +static void dirty_ring_collect_dirty_pages (struct kvm_vm *vm,int slot, +void *bitmap,uint32_t num_pages) +{ +/* We only have one vcpu */ +static uint32_t fetch_index =0 ; +uint32_t count =0 , cleared; + +/* +* Before fetching the dirty pages, we need a vmexit of the +* worker vcpu to make sure the hardware dirty buffers were +* flushed. This is not needed for dirty-log/clear-log tests +* because get dirty log will natually do so. +* +* For now we do it in the simple way - we simply wait until +* the vcpu uses up the soft dirty ring, then it'll always +* do a vmexit to make sure that PML buffers will be flushed. +* In real hypervisors, we probably need a vcpu kick or to +* stop the vcpus (before the final sync) to make sure we'll +* get all the existing dirty PFNs even cached in hardware. +*/ +sem_wait (&dirty_ring_vcpu_stop); + +/* Only have one vcpu */ + count =dirty_ring_collect_one (vcpu_map_dirty_ring (vm,VCPU_ID ), + slot, bitmap, num_pages, &fetch_index); + + cleared =kvm_vm_reset_dirty_ring (vm); + +/* Cleared pages should be the same as collected */ +TEST_ASSERT (cleared == count,"Reset dirty pages (%u) mismatch " +"with collected (%u)" , cleared, count); + +DEBUG ("Notifying vcpu to continue\n" ); +sem_post (&dirty_ring_vcpu_cont); + +pr_info ("Iteration %ld collected %u pages\n" , iteration, count); +} + +static void dirty_ring_after_vcpu_run (struct kvm_vm *vm) +{ +struct kvm_run *run =vcpu_state (vm,VCPU_ID ); + +/* A ucall-sync or ring-full event is allowed */ +if (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC ) { +/* We should allow this to continue */ + ; + }else if (run->exit_reason ==KVM_EXIT_DIRTY_RING_FULL ) { +sem_post (&dirty_ring_vcpu_stop); +pr_info ("vcpu stops because dirty ring full...\n" ); +sem_wait (&dirty_ring_vcpu_cont); +pr_info ("vcpu continues now.\n" ); + }else { +TEST_ASSERT (false,"Invalid guest sync status: " +"exit_reason=%s\n" , +exit_reason_str (run->exit_reason)); + } +} + +static void dirty_ring_before_vcpu_join (void ) +{ +/* Kick another round of vcpu just to make sure it will quit */ +sem_post (&dirty_ring_vcpu_cont); +} +struct log_mode {const char *name;/* Return true if this mode is supported, otherwise false */
@@ -198,6 +323,7 @@ struct log_mode { void *bitmap, uint32_t num_pages);/* Hook to call when after each vcpu run */ void (*after_vcpu_run)(struct kvm_vm *vm); +void (*before_vcpu_join) (void ); } log_modes[LOG_MODE_NUM] = { { .name = "dirty-log",
@@ -211,6 +337,14 @@ struct log_mode { .collect_dirty_pages = clear_log_collect_dirty_pages, .after_vcpu_run = default_after_vcpu_run, }, + { + .name = "dirty-ring", + .supported = dirty_ring_supported, + .create_vm_done = dirty_ring_create_vm_done, + .collect_dirty_pages = dirty_ring_collect_dirty_pages, + .before_vcpu_join = dirty_ring_before_vcpu_join, + .after_vcpu_run = dirty_ring_after_vcpu_run, + }, };/*
@@ -268,6 +402,14 @@ static void log_mode_after_vcpu_run(struct kvm_vm *vm) mode->after_vcpu_run (vm); } +static void log_mode_before_vcpu_join (void ) +{ +struct log_mode *mode = &log_modes[host_log_mode]; + +if (mode->before_vcpu_join) + mode->before_vcpu_join (); +} +static void generate_random_array (uint64_t *guest_array,uint64_t size) {uint64_t i;
@@ -325,14 +467,65 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) }if (test_and_clear_bit_le (page, bmap)) { +bool matched; + host_dirty_count++; +/* * If the bit is set, the value written onto * the corresponding page should be either the * previous iteration number or the current one. */ -TEST_ASSERT (*value_ptr == iteration || - *value_ptr == iteration -1 , + matched = (*value_ptr == iteration || + *value_ptr == iteration -1 ); + +if (host_log_mode ==LOG_MODE_DIRTY_RING && !matched) { +if (*value_ptr == iteration -2 ) { +/* +* Short answer: this case is special +* only for dirty ring test where the +* page is the last page before a kvm +* dirty ring full in iteration N-2. +* +* Long answer: Assuming ring size R, +* one possible condition is: +* +* main thr vcpu thr +* -------- -------- +* iter=1 +* write 1 to page 0~(R-1) +* full, vmexit +* collect 0~(R-1) +* kick vcpu +* write 1 to (R-1)~(2R-2) +* full, vmexit +* iter=2 +* collect (R-1)~(2R-2) +* kick vcpu +* write 1 to (2R-2) +* (NOTE!!! "1" cached in cpu reg) +* write 2 to (2R-1)~(3R-3) +* full, vmexit +* iter=3 +* collect (2R-2)~(3R-3) +* (here if we read value on page +* "2R-2" is 1, while iter=3!!!) +*/ + matched = true; + }else { +/* +* This is also special for dirty ring +* when this page is exactly the last +* page touched before vcpu ring full. +* If it happens, we should expect the +* value to change in the next round. +*/ +set_bit_le (page, host_bmap_track); +continue ; + } + } + +TEST_ASSERT (matched,"Set page %" PRIu64" value %" PRIu64" incorrect (iteration=%" PRIu64")" , page, *value_ptr, iteration);
@@ -495,6 +688,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,/* Tell the vcpu thread to quit */ host_quit = true; +log_mode_before_vcpu_join ();pthread_join (vcpu_thread,NULL ); pr_info("Total bits checked: dirty (%"PRIu64"), clear (%"PRIu64"), "
@@ -555,6 +749,9 @@ int main(int argc, char *argv[])unsigned int mode;int opt, i, j; +sem_init (&dirty_ring_vcpu_stop,0 ,0 ); +sem_init (&dirty_ring_vcpu_cont,0 ,0 ); +#ifdef __x86_64__guest_mode_init (VM_MODE_PXXV48_4K , true, true);#endif
tools/testing/selftests/kvm/include/kvm_util.h +3 -0
@@ -62,6 +62,7 @@ enum vm_mem_backing_src_type {int kvm_check_cap (long cap);int vm_enable_cap (struct kvm_vm *vm,struct kvm_enable_cap *cap); +void vm_enable_dirty_ring (struct kvm_vm *vm,uint32_t ring_size);struct kvm_vm *vm_create (enum vm_guest_mode mode,uint64_t phy_pages,int perm);struct kvm_vm *_vm_create (enum vm_guest_mode mode,uint64_t phy_pages,int perm);
@@ -71,6 +72,7 @@ void kvm_vm_release(struct kvm_vm *vmp);void kvm_vm_get_dirty_log (struct kvm_vm *vm,int slot,void *log);void kvm_vm_clear_dirty_log (struct kvm_vm *vm,int slot,void *log,uint64_t first_page,uint32_t num_pages); +uint32_t kvm_vm_reset_dirty_ring (struct kvm_vm *vm);int kvm_memcmp_hva_gva (void *hva,struct kvm_vm *vm,const vm_vaddr_t gva,size_t len);
@@ -192,6 +194,7 @@ void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,int vcpu_nested_state_set (struct kvm_vm *vm,uint32_t vcpuid,struct kvm_nested_state *state,bool ignore_error);#endif +void *vcpu_map_dirty_ring (struct kvm_vm *vm,uint32_t vcpuid);const char *exit_reason_str (unsigned int exit_reason);
tools/testing/selftests/kvm/lib/kvm_util.c +59 -0
89 lines not on this page · raw message
tools/testing/selftests/kvm/lib/kvm_util_internal.h +4 -0
@@ -28,6 +28,9 @@ struct vcpu {uint32_t id;int fd;struct kvm_run *state; +struct kvm_dirty_gfn *dirty_gfns; +uint32_t fetch_index; +uint32_t dirty_gfns_count; };struct kvm_vm {
@@ -50,6 +53,7 @@ struct kvm_vm {vm_paddr_t pgd;vm_vaddr_t gdt;vm_vaddr_t tss; +uint32_t dirty_ring_size; };struct vcpu *vcpu_find (struct kvm_vm *vm,uint32_t vcpuid);
Signature · 1 line
2.24.1
Re: [PATCH v7 12/14] KVM: selftests: Add dirty ring buffer test
Quoted 23 lines · Peter Xu
tools/testing/selftests/kvm/dirty_log_test.c +23
-0+static void dirty_ring_collect_dirty_pages (struct kvm_vm *vm,int slot, +void *bitmap,uint32_t num_pages) +{ +/* We only have one vcpu */ +static uint32_t fetch_index =0 ; +uint32_t count =0 , cleared; + +/* +* Before fetching the dirty pages, we need a vmexit of the +* worker vcpu to make sure the hardware dirty buffers were +* flushed. This is not needed for dirty-log/clear-log tests +* because get dirty log will natually do so. +* +* For now we do it in the simple way - we simply wait until +* the vcpu uses up the soft dirty ring, then it'll always +* do a vmexit to make sure that PML buffers will be flushed. +* In real hypervisors, we probably need a vcpu kick or to +* stop the vcpus (before the final sync) to make sure we'll +* get all the existing dirty PFNs even cached in hardware. +*/ +sem_wait (&dirty_ring_vcpu_stop); + +/* Only have one vcpu */
tools/testing/selftests/kvm/dirty_log_test.c static void default_after_vcpu_run(struct kvm_vm *vm) · line 283
Quoted 10 lines
tools/testing/selftests/kvm/dirty_log_test.c +10
-0+ count =dirty_ring_collect_one (vcpu_map_dirty_ring (vm,VCPU_ID ), + slot, bitmap, num_pages, &fetch_index); + + cleared =kvm_vm_reset_dirty_ring (vm); + +/* Cleared pages should be the same as collected */ +TEST_ASSERT (cleared == count,"Reset dirty pages (%u) mismatch " +"with collected (%u)" , cleared, count); + +DEBUG ("Notifying vcpu to continue\n" );
This line is removed later so hidden from my eyes too... but it'll also break the bisection. Fixed now.
Quoted 4 lines
tools/testing/selftests/kvm/dirty_log_test.c +4
-0+sem_post (&dirty_ring_vcpu_cont); + +pr_info ("Iteration %ld collected %u pages\n" , iteration, count); +}
Signature · 1 line
Peter Xu
PATCH v7 13/14 KVM: selftests: Let dirty_log_test async for dirty ring test
In reply to Peter XuPreviously 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.
--- tools/testing/selftests/kvm/dirty_log_test.c | 119 +++++++++++++----- .../testing/selftests/kvm/include/kvm_util.h | 1 + tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++ 3 files changed, 99 insertions(+), 30 deletions(-)
tools/testing/selftests/kvm/dirty_log_test.c +89 -30
@@ -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; +static pthread_t vcpu_thread; + +/* Only way to pass this to the signal handler */ +static struct kvm_vm *current_vm; + +static void vcpu_sig_handler (int sig) +{ +TEST_ASSERT (sig ==SIG_IPI ,"unknown signal: %d" , sig); +} + +static void vcpu_kick (void ) +{ +pthread_kill (vcpu_thread,SIG_IPI ); +} + +/* +* In our test we do signal tricks, let's use a better version of +* sem_wait to avoid signal interrupts +*/ +static void sem_wait_until (sem_t *sem) +{ +int ret; + +do + ret =sem_wait (sem); +while (ret ==-1 && errno ==EINTR ); +}static bool clear_log_supported (void ) {
@@ -189,10 +227,13 @@ static void clear_log_collect_dirty_pages(struct kvm_vm *vm, int slot,kvm_vm_clear_dirty_log (vm, slot, bitmap,0 , num_pages); } -static void default_after_vcpu_run (struct kvm_vm *vm) +static void default_after_vcpu_run (struct kvm_vm *vm,int ret,int err) {struct kvm_run *run =vcpu_state (vm,VCPU_ID ); +TEST_ASSERT (ret ==0 || (ret ==-1 && err ==EINTR ), +"vcpu run failed: errno=%d" , err); +TEST_ASSERT (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC ,"Invalid guest sync status: exit_reason=%s\n" ,exit_reason_str (run->exit_reason));
@@ -248,27 +289,37 @@ static uint32_t dirty_ring_collect_one(struct kvm_dirty_gfn *dirty_gfns,return count; } +static void dirty_ring_wait_vcpu (void ) +{ +/* This makes sure that hardware PML cache flushed */ +vcpu_kick (); +sem_wait_until (&dirty_ring_vcpu_stop); +} + +static void dirty_ring_continue_vcpu (void ) +{ +pr_info ("Notifying vcpu to continue\n" ); +sem_post (&dirty_ring_vcpu_cont); +} +static void dirty_ring_collect_dirty_pages (struct kvm_vm *vm,int slot,void *bitmap,uint32_t num_pages) {/* We only have one vcpu */ static uint32_t fetch_index =0 ;uint32_t count =0 , cleared; +bool continued_vcpu = false; -/* -* Before fetching the dirty pages, we need a vmexit of the -* worker vcpu to make sure the hardware dirty buffers were -* flushed. This is not needed for dirty-log/clear-log tests -* because get dirty log will natually do so. -* -* For now we do it in the simple way - we simply wait until -* the vcpu uses up the soft dirty ring, then it'll always -* do a vmexit to make sure that PML buffers will be flushed. -* In real hypervisors, we probably need a vcpu kick or to -* stop the vcpus (before the final sync) to make sure we'll -* get all the existing dirty PFNs even cached in hardware. -*/ -sem_wait (&dirty_ring_vcpu_stop); +dirty_ring_wait_vcpu (); + +if (!dirty_ring_vcpu_ring_full) { +/* +* This is not a ring-full event, it's safe to allow +* vcpu to continue +*/ +dirty_ring_continue_vcpu (); + continued_vcpu = true; + }/* Only have one vcpu */ count = dirty_ring_collect_one(vcpu_map_dirty_ring(vm, VCPU_ID),
@@ -280,13 +331,16 @@ static void dirty_ring_collect_dirty_pages(struct kvm_vm *vm, int slot,TEST_ASSERT (cleared == count,"Reset dirty pages (%u) mismatch " "with collected (%u)" , cleared, count); -DEBUG ("Notifying vcpu to continue\n" ); -sem_post (&dirty_ring_vcpu_cont); +if (!continued_vcpu) { +TEST_ASSERT (dirty_ring_vcpu_ring_full, +"Didn't continue vcpu even without ring full" ); +dirty_ring_continue_vcpu (); + }pr_info ("Iteration %ld collected %u pages\n" , iteration, count); } -static void dirty_ring_after_vcpu_run (struct kvm_vm *vm) +static void dirty_ring_after_vcpu_run (struct kvm_vm *vm,int ret,int err) {struct kvm_run *run =vcpu_state (vm,VCPU_ID );
@@ -294,10 +348,16 @@ static void dirty_ring_after_vcpu_run(struct kvm_vm *vm)if (get_ucall (vm,VCPU_ID ,NULL ) ==UCALL_SYNC ) {/* We should allow this to continue */ ; - }else if (run->exit_reason ==KVM_EXIT_DIRTY_RING_FULL ) { + }else if (run->exit_reason ==KVM_EXIT_DIRTY_RING_FULL || + (ret ==-1 && err ==EINTR )) { +/* Update the flag first before pause */ +WRITE_ONCE (dirty_ring_vcpu_ring_full, + run->exit_reason ==KVM_EXIT_DIRTY_RING_FULL );sem_post (&dirty_ring_vcpu_stop); -pr_info ("vcpu stops because dirty ring full...\n" ); -sem_wait (&dirty_ring_vcpu_cont); +pr_info ("vcpu stops because %s...\n" , + dirty_ring_vcpu_ring_full ? +"dirty ring is full" :"vcpu is kicked out" ); +sem_wait_until (&dirty_ring_vcpu_cont);pr_info ("vcpu continues now.\n" ); }else { TEST_ASSERT(false, "Invalid guest sync status: "
@@ -322,7 +382,7 @@ struct log_mode {void (*collect_dirty_pages) (struct kvm_vm *vm,int slot,void *bitmap,uint32_t num_pages);/* Hook to call when after each vcpu run */ -void (*after_vcpu_run)(struct kvm_vm *vm); +void (*after_vcpu_run)(struct kvm_vm *vm,int ret,int err);void (*before_vcpu_join) (void ); } log_modes[LOG_MODE_NUM] = { {
@@ -394,12 +454,12 @@ static void log_mode_collect_dirty_pages(struct kvm_vm *vm, int slot, mode->collect_dirty_pages (vm, slot, bitmap, num_pages); } -static void log_mode_after_vcpu_run (struct kvm_vm *vm) +static void log_mode_after_vcpu_run (struct kvm_vm *vm,int ret,int err) {struct log_mode *mode = &log_modes[host_log_mode];if (mode->after_vcpu_run) - mode->after_vcpu_run (vm); + mode->after_vcpu_run (vm, ret, err); }static void log_mode_before_vcpu_join (void )
@@ -420,7 +480,7 @@ static void generate_random_array(uint64_t *guest_array, uint64_t size)static void *vcpu_worker (void *data) { -int ret; +int ret, vcpu_fd;struct kvm_vm *vm = data;uint64_t *guest_array;uint64_t pages_count =0 ;
@@ -435,12 +495,12 @@ static void *vcpu_worker(void *data) guest_array =addr_gva2hva (vm, (vm_vaddr_t )random_array);while (!READ_ONCE (host_quit)) { +/* Clear any existing kick signals */ generate_random_array (guest_array,TEST_PAGES_PER_LOOP ); pages_count +=TEST_PAGES_PER_LOOP ;/* Let the guest dirty the random pages */ - ret =_vcpu_run (vm,VCPU_ID ); -TEST_ASSERT (ret ==0 ,"vcpu_run failed: %d\n" , ret); -log_mode_after_vcpu_run (vm); + ret =ioctl (vcpu_fd,KVM_RUN ,NULL ); +log_mode_after_vcpu_run (vm, ret, errno); }pr_info ("Dirtied %" PRIu64" pages\n" , pages_count);
@@ -590,7 +650,6 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,static void run_test (enum vm_guest_mode mode,unsigned long iterations,unsigned long interval,uint64_t phys_offset) { -pthread_t vcpu_thread;struct kvm_vm *vm;unsigned long *bmap;
tools/testing/selftests/kvm/include/kvm_util.h +1 -0
@@ -144,6 +144,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva);struct kvm_run *vcpu_state (struct kvm_vm *vm,uint32_t vcpuid);void vcpu_run (struct kvm_vm *vm,uint32_t vcpuid);int _vcpu_run (struct kvm_vm *vm,uint32_t vcpuid); +int vcpu_get_fd (struct kvm_vm *vm,uint32_t vcpuid);void vcpu_run_complete_io (struct kvm_vm *vm,uint32_t vcpuid);void vcpu_set_mp_state (struct kvm_vm *vm,uint32_t vcpuid,struct kvm_mp_state *mp_state);
tools/testing/selftests/kvm/lib/kvm_util.c +9 -0
@@ -1207,6 +1207,15 @@ int _vcpu_run(struct kvm_vm *vm, uint32_t vcpuid)return rc; } +int vcpu_get_fd (struct kvm_vm *vm,uint32_t vcpuid) +{ +struct vcpu *vcpu =vcpu_find (vm, vcpuid); + +TEST_ASSERT (vcpu !=NULL ,"vcpu not found, vcpuid: %u" , vcpuid); + +return vcpu->fd; +} +void vcpu_run_complete_io (struct kvm_vm *vm,uint32_t vcpuid) {struct vcpu *vcpu =vcpu_find (vm, vcpuid);
Signature · 1 line
2.24.1
PATCH v7 14/14 KVM: selftests: Add "-c" parameter to dirty log test
In reply to Peter XuIt's only used to override the existing dirty ring size/count. If with a bigger ring count, we test async of dirty ring. If with a smaller ring count, we test ring full code path. Async is default.
It has no use for non-dirty-ring tests.
--- tools/testing/selftests/kvm/dirty_log_test.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
tools/testing/selftests/kvm/dirty_log_test.c +10 -3
@@ -168,6 +168,7 @@ 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;static pthread_t vcpu_thread; +static uint32_t test_dirty_ring_count =TEST_DIRTY_RING_COUNT ;/* Only way to pass this to the signal handler */ static struct kvm_vm *current_vm;
@@ -250,7 +251,7 @@ static void dirty_ring_create_vm_done(struct kvm_vm *vm)* Switch to dirty ring mode after VM creation but before any * of the vcpu creation. */ -vm_enable_dirty_ring (vm,TEST_DIRTY_RING_COUNT * +vm_enable_dirty_ring (vm, test_dirty_ring_count *sizeof (struct kvm_dirty_gfn )); }
@@ -272,7 +273,7 @@ static uint32_t dirty_ring_collect_one(struct kvm_dirty_gfn *dirty_gfns,uint32_t count =0 ;while (true) { - cur = &dirty_gfns[*fetch_index %TEST_DIRTY_RING_COUNT ]; + cur = &dirty_gfns[*fetch_index % test_dirty_ring_count];if (!dirty_gfn_is_dirtied (cur))break ; TEST_ASSERT(cur->slot == slot, "Slot number didn't match: "
@@ -778,6 +779,9 @@ static void help(char *name)printf ("usage: %s [-h] [-i iterations] [-I interval] " "[-p offset] [-m mode]\n" , name);puts ("" ); +printf (" -c: specify dirty ring size, in number of entries\n" ); +printf (" (only useful for dirty-ring test; default: %" PRIu32")\n" , +TEST_DIRTY_RING_COUNT );printf (" -i: specify iteration counts (default: %" PRIu64")\n" ,TEST_HOST_LOOP_N ); printf(" -I: specify interval in ms (default: %"PRIu64" ms)\n",
@@ -833,8 +837,11 @@ int main(int argc, char *argv[])guest_mode_init (VM_MODE_P40V48_4K , true, true);#endif -while ((opt =getopt (argc, argv,"hi:I:p:m:M:" )) !=-1 ) { +while ((opt =getopt (argc, argv,"c:hi:I:p:m:M:" )) !=-1 ) {switch (opt) { +case 'c' : + test_dirty_ring_count =strtol (optarg,NULL ,10 ); +break ;case 'i' : iterations =strtol (optarg,NULL ,10 );break ;
Signature · 1 line
2.24.1
Re: [PATCH v7 14/14] KVM: selftests: Add "-c" parameter to dirty log test
Quoted 11 lines · Peter Xu
It's only used to override the existing dirty ring size/count. If with a bigger ring count, we test async of dirty ring. If with a smaller ring count, we test ring full code path. Async is default.
It has no use for non-dirty-ring tests.
Signed-off-by Peter Xu <[email protected]>--- tools/testing/selftests/kvm/dirty_log_test.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)