| Message ID | 1415320848-13813-3-git-send-email-m.smarduch@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 06 Nov 2014 16:40:43 -0800 Mario Smarduch <m.smarduch@samsung.com> wrote: > kvm_get_dirty_log() provides generic handling of dirty bitmap, currently reused > by several architectures. Building on that we intrdoduce > kvm_get_dirty_log_protect() adding write protection to mark these pages dirty > for future write access, before next KVM_GET_DIRTY_LOG ioctl call from user > space. > > Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> > --- > include/linux/kvm_host.h | 9 +++++ > virt/kvm/kvm_main.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 104 insertions(+) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 887df87..f017760 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -981,6 +981,101 @@ out: > } > EXPORT_SYMBOL_GPL(kvm_get_dirty_log); > > +#if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) || \ > + defined(CONFIG_IA64) || defined(CONFIG_X86) || defined(CONFIG_ARM) || \ > + defined(CONFIG_ARM64) Does this deserve a config symbol that can be selected by architectures actually using kvm_get_dirty_log_protect()? I.e., #ifndef CONFIG_KVM_ARCH_DIRTY_LOG_PROTECT or so? > +/* > + * For architectures that don't use kvm_get_dirty_log_protect() for dirty page > + * logging, calling this function is illegal. Otherwise the function is defined > + * in arch subtree and this restriction is removed. > + */ What about /* * For architectures that don't use kvm_get_dirty_log_protect() for dirty page * logging, we must never end up calling this function. Architectures that do * use it define their own version of this function. */ instead? > +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm, > + struct kvm_memory_slot *slot, > + gfn_t gfn_offset, > + unsigned long mask) > +{ > + BUG(); > +} > +#endif > + (...)
On 07/11/2014 10:07, Cornelia Huck wrote: >> > +#if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) || \ >> > + defined(CONFIG_IA64) || defined(CONFIG_X86) || defined(CONFIG_ARM) || \ >> > + defined(CONFIG_ARM64) > Does this deserve a config symbol that can be selected by architectures > actually using kvm_get_dirty_log_protect()? I.e., > > #ifndef CONFIG_KVM_ARCH_DIRTY_LOG_PROTECT > > or so? Yes, either that or invert the #if to negative logic. Paolo
On 11/07/2014 01:07 AM, Cornelia Huck wrote: > On Thu, 06 Nov 2014 16:40:43 -0800 > Mario Smarduch <m.smarduch@samsung.com> wrote: > >> kvm_get_dirty_log() provides generic handling of dirty bitmap, currently reused >> by several architectures. Building on that we intrdoduce >> kvm_get_dirty_log_protect() adding write protection to mark these pages dirty >> for future write access, before next KVM_GET_DIRTY_LOG ioctl call from user >> space. >> >> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> >> --- >> include/linux/kvm_host.h | 9 +++++ >> virt/kvm/kvm_main.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 104 insertions(+) >> > >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 887df87..f017760 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -981,6 +981,101 @@ out: >> } >> EXPORT_SYMBOL_GPL(kvm_get_dirty_log); >> >> +#if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) || \ >> + defined(CONFIG_IA64) || defined(CONFIG_X86) || defined(CONFIG_ARM) || \ >> + defined(CONFIG_ARM64) > > Does this deserve a config symbol that can be selected by architectures > actually using kvm_get_dirty_log_protect()? I.e., > > #ifndef CONFIG_KVM_ARCH_DIRTY_LOG_PROTECT > > or so? Yes definitely, not sure why I went this way after using Kconfig defines before. > >> +/* >> + * For architectures that don't use kvm_get_dirty_log_protect() for dirty page >> + * logging, calling this function is illegal. Otherwise the function is defined >> + * in arch subtree and this restriction is removed. >> + */ > > What about > > /* > * For architectures that don't use kvm_get_dirty_log_protect() for dirty page > * logging, we must never end up calling this function. Architectures that do > * use it define their own version of this function. > */ > > instead Yeah that reads better, getting the illegal out of there. > >> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm, >> + struct kvm_memory_slot *slot, >> + gfn_t gfn_offset, >> + unsigned long mask) >> +{ >> + BUG(); >> +} >> +#endif >> + > (...) >
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index a4c33b3..c55dd75 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -590,6 +590,15 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log, int *is_dirty); + +int kvm_get_dirty_log_protect(struct kvm *kvm, + struct kvm_dirty_log *log, bool *is_dirty); + +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm, + struct kvm_memory_slot *slot, + gfn_t gfn_offset, + unsigned long mask); + int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 887df87..f017760 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -981,6 +981,101 @@ out: } EXPORT_SYMBOL_GPL(kvm_get_dirty_log); +#if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) || \ + defined(CONFIG_IA64) || defined(CONFIG_X86) || defined(CONFIG_ARM) || \ + defined(CONFIG_ARM64) +/* + * For architectures that don't use kvm_get_dirty_log_protect() for dirty page + * logging, calling this function is illegal. Otherwise the function is defined + * in arch subtree and this restriction is removed. + */ +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm, + struct kvm_memory_slot *slot, + gfn_t gfn_offset, + unsigned long mask) +{ + BUG(); +} +#endif + +/** + * kvm_get_dirty_log_protect - get a snapshot of dirty pages, and if any pages + * are dirty write protect them for next write. + * @kvm: pointer to kvm instance + * @log: slot id and address to which we copy the log + * @is_dirty: flag set if any page is dirty + * + * We need to keep it in mind that VCPU threads can write to the bitmap + * concurrently. So, to avoid losing track of dirty pages we keep the + * following order: + * + * 1. Take a snapshot of the bit and clear it if needed. + * 2. Write protect the corresponding page. + * 3. Copy the snapshot to the userspace. + * 4. Upon return caller flushes TLB's if needed. + * + * Between 2 and 4, the guest may write to the page using the remaining TLB + * entry. This is not a problem because the page is reported dirty using + * the snapshot taken before and step 4 ensures that writes done after + * exiting to userspace will be logged for the next call. + * + */ +int kvm_get_dirty_log_protect(struct kvm *kvm, + struct kvm_dirty_log *log, bool *is_dirty) +{ + struct kvm_memory_slot *memslot; + int r, i; + unsigned long n; + unsigned long *dirty_bitmap; + unsigned long *dirty_bitmap_buffer; + + r = -EINVAL; + if (log->slot >= KVM_USER_MEM_SLOTS) + goto out; + + memslot = id_to_memslot(kvm->memslots, log->slot); + + dirty_bitmap = memslot->dirty_bitmap; + r = -ENOENT; + if (!dirty_bitmap) + goto out; + + n = kvm_dirty_bitmap_bytes(memslot); + + dirty_bitmap_buffer = dirty_bitmap + n / sizeof(long); + memset(dirty_bitmap_buffer, 0, n); + + spin_lock(&kvm->mmu_lock); + *is_dirty = false; + for (i = 0; i < n / sizeof(long); i++) { + unsigned long mask; + gfn_t offset; + + if (!dirty_bitmap[i]) + continue; + + *is_dirty = true; + + mask = xchg(&dirty_bitmap[i], 0); + dirty_bitmap_buffer[i] = mask; + + offset = i * BITS_PER_LONG; + kvm_arch_mmu_write_protect_pt_masked(kvm, memslot, offset, + mask); + } + + spin_unlock(&kvm->mmu_lock); + + r = -EFAULT; + if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) + goto out; + + r = 0; +out: + return r; +} +EXPORT_SYMBOL_GPL(kvm_get_dirty_log_protect); + bool kvm_largepages_enabled(void) { return largepages_enabled;
kvm_get_dirty_log() provides generic handling of dirty bitmap, currently reused by several architectures. Building on that we intrdoduce kvm_get_dirty_log_protect() adding write protection to mark these pages dirty for future write access, before next KVM_GET_DIRTY_LOG ioctl call from user space. Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> --- include/linux/kvm_host.h | 9 +++++ virt/kvm/kvm_main.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+)