Re: [PATCH RFC v2 5/6] KVM: moving dirty bitmaps to user space

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20.04.2010, at 13:02, Takuya Yoshikawa wrote:

> We move dirty bitmaps to user space.
> 
> - Allocation and destruction: we use do_mmap() and do_munmap().
>   The new bitmap space is twice longer than the original one and we
>   use the additional space for double buffering: this makes it
>   possible to update the active bitmap while letting the user space
>   read the other one safely.
> 
> - Bitmap manipulations: we replace all functions which access dirty
>   bitmaps to *_user() functions. Note that some of these should be
>   optimized later.
> 
> - For ia64: moving the dirty bitmaps of memory slots does not effect
>   much to ia64 because it's using a different space to store bitmaps
>   which is directly updated: all we have to change are sync and get
>   of dirty log, so we don't need set_bit_user like function for ia64.
> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@xxxxxxxxxxxxx>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@xxxxxxxxxxxxx>
> ---
> arch/ia64/kvm/kvm-ia64.c  |   12 ++++-
> arch/powerpc/kvm/book3s.c |    2 +-
> arch/x86/kvm/x86.c        |   24 +++++-----
> include/linux/kvm_host.h  |    5 +-
> virt/kvm/kvm_main.c       |  101 ++++++++++++++++++++++++++++++++++++++++-----
> 5 files changed, 116 insertions(+), 28 deletions(-)
> 
> 

[...]

> @@ -778,13 +803,45 @@ int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
> }
> 
> int kvm_copy_dirty_bitmap(unsigned long __user *to,
> -			  const unsigned long *from,
> +			  const unsigned long __user *from,
> 			  unsigned long bytes)
> {
> -	if (copy_to_user(to, from, bytes))
> +#if defined(CONFIG_X86_64) || defined(CONFIG_PPC64) || defined(CONFIG_IA64)

Wouldn't it be better to have a define __HAS_COPY_IN_USER or so? I really dislike writing out all architectures that have this explicitly.

> +	if (copy_in_user(to, from, bytes)) {
> +		printk(KERN_WARNING "%s: copy_in_user failed.\n", __func__);
> 		return -EFAULT;
> +	}
> +	return 0;
> +#else

Worst case I'd at least like to see all the others written out here, so when someone adds a new arch he can at least check if it's supported or not.

> +	int num, bufbytes;
> +	unsigned long buf[32];
> 
> +	if (!access_ok(VERIFY_READ, from, bytes) ||
> +	    !access_ok(VERIFY_WRITE, to, bytes)) {
> +		goto out_fault;
> +	}
> +
> +	bufbytes = sizeof(buf);
> +	num = bufbytes / sizeof(buf[0]);

ARRAY_SIZE?

> +
> +	for (; bytes > bufbytes; bytes -= bufbytes, to += num, from += num) {
> +		if (__copy_from_user(buf, from, bufbytes))
> +			goto out_fault;
> +		if (__copy_to_user(to, buf, bufbytes))
> +			goto out_fault;
> +	}
> +	if (bytes > 0) {
> +		if (__copy_from_user(buf, from, bytes))
> +			goto out_fault;
> +		if (__copy_to_user(to, buf, bytes))
> +			goto out_fault;
> +	}
> 	return 0;
> +
> +out_fault:
> +	printk(KERN_WARNING "%s: copy to(from) user failed.\n", __func__);
> +	return -EFAULT;
> +#endif
> }
> 
> int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
> @@ -1194,13 +1251,35 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len)
> }
> EXPORT_SYMBOL_GPL(kvm_clear_guest);
> 
> +/*
> + * Please use generic *_user bitops once they become available.
> + * Be careful setting the bit won't be done atomically.
> + */
> static int __mark_page_dirty(unsigned long nr,
> -			     unsigned long *dirty_bitmap)
> +			     unsigned long __user *dirty_bitmap)
> {
> +	unsigned long user_addr;
> +	u8 val;
> +
> #ifdef __BIG_ENDIAN
> 	nr = nr ^ BITOP_LE_SWIZZLE;
> #endif
> -	__set_bit(nr, dirty_bitmap);
> +	user_addr = (unsigned long)dirty_bitmap + nr / 8;

BITS_PER_BYTE

> +	if (!access_ok(VERIFY_WRITE, user_addr, 1))
> +		goto out_fault;
> +
> +	if (__get_user(val, (u8 __user *)user_addr))
> +		goto out_fault;
> +
> +	val |= 1U << (nr % 8);

see above


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux