Re: [PATCH v9 05/14] KVM: X86: Implement ring-based dirty memory tracking
Hi Peter,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vhost/linux-next]
[also build test WARNING on linus/master v5.7-rc7]
[cannot apply to kvm/linux-next tip/auto-latest linux/master next-20200522]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Peter-Xu/KVM-Dirty-ring-interface/20200524-070926
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> arch/x86/kvm/../../../virt/kvm/dirty_ring.c:33:6: warning: no previous
>> prototype for 'kvm_dirty_ring_full' [-Wmissing-prototypes]
bool kvm_dirty_ring_full(struct kvm_dirty_ring *ring)
^~~
--
arch/x86/kvm/vmx/vmx.c: In function 'init_rmode_identity_map':
>> arch/x86/kvm/vmx/vmx.c:3472:12: warning: variable 'identity_map_pfn' set but
>> not used [-Wunused-but-set-variable]
kvm_pfn_t identity_map_pfn;
^~~~
vim +/kvm_dirty_ring_full +33 arch/x86/kvm/../../../virt/kvm/dirty_ring.c
32
> 33 bool kvm_dirty_ring_full(struct kvm_dirty_ring *ring)
34 {
35 return kvm_dirty_ring_used(ring) >= ring->size;
36 }
37
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v9 05/14] KVM: X86: Implement ring-based dirty memory tracking
On Mon, May 25, 2020 at 10:54:58PM +0800, kbuild test robot wrote:
> Hi Peter,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on vhost/linux-next]
> [also build test WARNING on linus/master v5.7-rc7]
> [cannot apply to kvm/linux-next tip/auto-latest linux/master next-20200522]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Peter-Xu/KVM-Dirty-ring-interface/20200524-070926
> base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> linux-next
> config: x86_64-allyesconfig (attached as .config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build):
> # save the attached .config to linux build tree
> make W=1 ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> >> arch/x86/kvm/../../../virt/kvm/dirty_ring.c:33:6: warning: no previous
> >> prototype for 'kvm_dirty_ring_full' [-Wmissing-prototypes]
> bool kvm_dirty_ring_full(struct kvm_dirty_ring *ring)
> ^~~
I'll add a "static" to quiesce this..
> --
> arch/x86/kvm/vmx/vmx.c: In function 'init_rmode_identity_map':
> >> arch/x86/kvm/vmx/vmx.c:3472:12: warning: variable 'identity_map_pfn' set
> >> but not used [-Wunused-but-set-variable]
> kvm_pfn_t identity_map_pfn;
> ^~~~
Hmm, this seems to be true but not related to this series afaict... but sure I
can add another patch to remove it.
>
> vim +/kvm_dirty_ring_full +33 arch/x86/kvm/../../../virt/kvm/dirty_ring.c
>
> 32
> > 33bool kvm_dirty_ring_full(struct kvm_dirty_ring *ring)
> 34{
> 35return kvm_dirty_ring_used(ring) >= ring->size;
> 36}
> 37
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
--
Peter Xu
Re: [PATCH v9 05/14] KVM: X86: Implement ring-based dirty memory tracking
On Mon, May 25, 2020 at 10:48:08AM +0800, kbuild test robot wrote:
> Hi Peter,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on vhost/linux-next]
> [also build test WARNING on linus/master v5.7-rc7]
> [cannot apply to kvm/linux-next tip/auto-latest linux/master next-20200522]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Peter-Xu/KVM-Dirty-ring-interface/20200524-070926
> base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> linux-next
> config: s390-randconfig-s002-20200524 (attached as .config)
> compiler: s390-linux-gcc (GCC) 9.3.0
> reproduce:
> # apt-get install sparse
> # sparse version: v0.6.1-240-gf0fe1cd9-dirty
> # save the attached .config to linux build tree
> make W=1 C=1 ARCH=s390 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> arch/s390/kvm/../../../virt/kvm/kvm_main.c: In function
> 'kvm_page_in_dirty_ring':
> >> arch/s390/kvm/../../../virt/kvm/kvm_main.c:2932:16: warning: comparison of
> >> unsigned expression >= 0 is always true [-Wtype-limits]
> 2932 | return (pgoff >= KVM_DIRTY_LOG_PAGE_OFFSET) &&
> |^~
Should be a false positive, since when KVM_DIRTY_LOG_PAGE_OFFSET==0 (true for
s390) then the code won't reach here at all due to the previous check [1].
I thought gcc should be able to directly remove the below code when it sees "if
(1) return false;", but it seems not...
I wanted to avoid using "#if" macros because I think it always makes the code
harder to read (especially when nested). Maybe it's still ok to use one more
time here.
Thanks,
>
> vim +2932 arch/s390/kvm/../../../virt/kvm/kvm_main.c
>
> 2926
> 2927static bool kvm_page_in_dirty_ring(struct kvm *kvm, unsigned
> long pgoff)
> 2928{
> 2929if (!KVM_DIRTY_LOG_PAGE_OFFSET)
> 2930return false;
[1]
> 2931
> > 2932return (pgoff >= KVM_DIRTY_LOG_PAGE_OFFSET) &&
> 2933(pgoff < KVM_DIRTY_LOG_PAGE_OFFSET +
> 2934 kvm->dirty_ring_size / PAGE_SIZE);
> 2935}
> 2936
--
Peter Xu
Re: [PATCH v9 05/14] KVM: X86: Implement ring-based dirty memory tracking
Hi Peter,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vhost/linux-next]
[also build test WARNING on linus/master v5.7-rc7]
[cannot apply to kvm/linux-next tip/auto-latest linux/master next-20200522]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Peter-Xu/KVM-Dirty-ring-interface/20200524-070926
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: s390-randconfig-s002-20200524 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-240-gf0fe1cd9-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=s390 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
arch/s390/kvm/../../../virt/kvm/kvm_main.c: In function
'kvm_page_in_dirty_ring':
>> arch/s390/kvm/../../../virt/kvm/kvm_main.c:2932:16: warning: comparison of
>> unsigned expression >= 0 is always true [-Wtype-limits]
2932 | return (pgoff >= KVM_DIRTY_LOG_PAGE_OFFSET) &&
|^~
vim +2932 arch/s390/kvm/../../../virt/kvm/kvm_main.c
2926
2927 static bool kvm_page_in_dirty_ring(struct kvm *kvm, unsigned long pgoff)
2928 {
2929 if (!KVM_DIRTY_LOG_PAGE_OFFSET)
2930 return false;
2931
> 2932 return (pgoff >= KVM_DIRTY_LOG_PAGE_OFFSET) &&
2933 (pgoff < KVM_DIRTY_LOG_PAGE_OFFSET +
2934 kvm->dirty_ring_size / PAGE_SIZE);
2935 }
2936
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH v9 05/14] KVM: X86: Implement ring-based dirty memory tracking
This patch is heavily based on previous work from Lei Cao
and Paolo Bonzini . [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/
Signed-off-by: Lei Cao
Signed-off-by: Paolo Bonzini
Signed-off-by: Peter Xu
---
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 | 8 ++
arch/x86/kvm/vmx/vmx.c | 8 +-
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 | 197
virt/kvm/kvm_main.c | 112 +-
13 files changed, 701 insertions(+), 3 deletions(-)
create mode 100644 include/linux/kvm_dirty_ring.h
create mode 100644 virt/kvm/dirty_ring.c
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index efbbe570aa9b..aa54a34077b7 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -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
-
@@ -6109,3 +6122,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 col
