Re: [RFC][PATCH RFC 10/12] KVM: move dirty bitmaps to user space
mark_page_dirty is called with the mmu_lock spinlock held in set_spte. Must find a way to move it outside of the spinlock section. Oh, it's a serious problem. I have to consider it. Avi, Marcelo, Sorry but I have to say that mmu_lock spin_lock problem was completely out of my mind. Although I looked through the code, it seems not easy to move the set_bit_user to outside of spinlock section without breaking the semantics of its protection. So this may take some time to solve. But personally, I want to do something for x86's vmallc() every time problem even though moving dirty bitmaps to user space cannot be achieved soon. In that sense, do you mind if we do double buffering without moving dirty bitmaps to user space? I know that the resource for vmalloc() is precious for x86 but even now, at the timing of get_dirty_log, we use the same amount of memory as double buffering. Thanks, Takuya -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH RFC 10/12] KVM: move dirty bitmaps to user space
r = 0;
@@ -1195,11 +1232,16 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
gfn = unalias_gfn(kvm, gfn);
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot memslot-dirty_bitmap) {
- unsigned long rel_gfn = gfn - memslot-base_gfn;
+ int nr = generic_le_bit_offset(gfn - memslot-base_gfn);
- generic___set_le_bit(rel_gfn, memslot-dirty_bitmap);
+ if (kvm_set_bit_user(nr, memslot-dirty_bitmap))
+ goto out_fault;
mark_page_dirty is called with the mmu_lock spinlock held in set_spte.
Must find a way to move it outside of the spinlock section.
Oh, it's a serious problem. I have to consider it.
Thanks,
Takuya
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH RFC 10/12] KVM: move dirty bitmaps to user space
On Tue, May 04, 2010 at 10:07:02PM +0900, 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. For x86, we can also remove the vmalloc()
in kvm_vm_ioctl_get_dirty_log().
- Bitmap manipulations: we replace all functions which access dirty
bitmaps with *_user() functions.
- For ia64: moving the dirty bitmaps of memory slots does not affect
ia64 much because it's using a different place to store dirty logs
rather than the dirty bitmaps of memory slots: all we have to change
are sync and get of dirty log, so we don't need set_bit_user like
functions for ia64.
Signed-off-by: Takuya Yoshikawa yoshikawa.tak...@oss.ntt.co.jp
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
CC: Avi Kivity a...@redhat.com
CC: Alexander Graf ag...@suse.de
---
arch/ia64/kvm/kvm-ia64.c | 15 +-
arch/powerpc/kvm/book3s.c |5 +++-
arch/x86/kvm/x86.c| 25 --
include/linux/kvm_host.h |3 +-
virt/kvm/kvm_main.c | 62 +---
5 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 17fd65c..03503e6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1823,11 +1823,19 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
n = kvm_dirty_bitmap_bytes(memslot);
base = memslot-base_gfn / BITS_PER_LONG;
+ r = -EFAULT;
+ if (!access_ok(VERIFY_WRITE, memslot-dirty_bitmap, n))
+ goto out;
+
for (i = 0; i n/sizeof(long); ++i) {
if (dirty_bitmap[base + i])
memslot-is_dirty = true;
- memslot-dirty_bitmap[i] = dirty_bitmap[base + i];
+ if (__put_user(dirty_bitmap[base + i],
+memslot-dirty_bitmap[i])) {
+ r = -EFAULT;
+ goto out;
+ }
dirty_bitmap[base + i] = 0;
}
r = 0;
@@ -1858,7 +1866,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (memslot-is_dirty) {
kvm_flush_remote_tlbs(kvm);
n = kvm_dirty_bitmap_bytes(memslot);
- memset(memslot-dirty_bitmap, 0, n);
+ if (clear_user(memslot-dirty_bitmap, n)) {
+ r = -EFAULT;
+ goto out;
+ }
memslot-is_dirty = false;
}
r = 0;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 4b074f1..2a31d2f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1210,7 +1210,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
n = kvm_dirty_bitmap_bytes(memslot);
- memset(memslot-dirty_bitmap, 0, n);
+ if (clear_user(memslot-dirty_bitmap, n)) {
+ r = -EFAULT;
+ goto out;
+ }
memslot-is_dirty = false;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 023c7f8..32a3d94 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2760,40 +2760,37 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
/* If nothing is dirty, don't bother messing with page tables. */
if (memslot-is_dirty) {
struct kvm_memslots *slots, *old_slots;
- unsigned long *dirty_bitmap;
+ unsigned long __user *dirty_bitmap;
+ unsigned long __user *dirty_bitmap_old;
spin_lock(kvm-mmu_lock);
kvm_mmu_slot_remove_write_access(kvm, log-slot);
spin_unlock(kvm-mmu_lock);
- r = -ENOMEM;
- dirty_bitmap = vmalloc(n);
- if (!dirty_bitmap)
+ dirty_bitmap = memslot-dirty_bitmap;
+ dirty_bitmap_old = memslot-dirty_bitmap_old;
+ r = -EFAULT;
+ if (clear_user(dirty_bitmap_old, n))
goto out;
- memset(dirty_bitmap, 0, n);
r = -ENOMEM;
slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
- if (!slots) {
- vfree(dirty_bitmap);
+ if (!slots)
goto out;
- }
+
memcpy(slots, kvm-memslots, sizeof(struct kvm_memslots));
- slots-memslots[log-slot].dirty_bitmap = dirty_bitmap;
+ slots-memslots[log-slot].dirty_bitmap = dirty_bitmap_old;
+ slots-memslots[log-slot].dirty_bitmap_old = dirty_bitmap;
[RFC][PATCH RFC 10/12] KVM: move dirty bitmaps to user space
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. For x86, we can also remove the vmalloc()
in kvm_vm_ioctl_get_dirty_log().
- Bitmap manipulations: we replace all functions which access dirty
bitmaps with *_user() functions.
- For ia64: moving the dirty bitmaps of memory slots does not affect
ia64 much because it's using a different place to store dirty logs
rather than the dirty bitmaps of memory slots: all we have to change
are sync and get of dirty log, so we don't need set_bit_user like
functions for ia64.
Signed-off-by: Takuya Yoshikawa yoshikawa.tak...@oss.ntt.co.jp
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
CC: Avi Kivity a...@redhat.com
CC: Alexander Graf ag...@suse.de
---
arch/ia64/kvm/kvm-ia64.c | 15 +-
arch/powerpc/kvm/book3s.c |5 +++-
arch/x86/kvm/x86.c| 25 --
include/linux/kvm_host.h |3 +-
virt/kvm/kvm_main.c | 62 +---
5 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 17fd65c..03503e6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1823,11 +1823,19 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
n = kvm_dirty_bitmap_bytes(memslot);
base = memslot-base_gfn / BITS_PER_LONG;
+ r = -EFAULT;
+ if (!access_ok(VERIFY_WRITE, memslot-dirty_bitmap, n))
+ goto out;
+
for (i = 0; i n/sizeof(long); ++i) {
if (dirty_bitmap[base + i])
memslot-is_dirty = true;
- memslot-dirty_bitmap[i] = dirty_bitmap[base + i];
+ if (__put_user(dirty_bitmap[base + i],
+ memslot-dirty_bitmap[i])) {
+ r = -EFAULT;
+ goto out;
+ }
dirty_bitmap[base + i] = 0;
}
r = 0;
@@ -1858,7 +1866,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (memslot-is_dirty) {
kvm_flush_remote_tlbs(kvm);
n = kvm_dirty_bitmap_bytes(memslot);
- memset(memslot-dirty_bitmap, 0, n);
+ if (clear_user(memslot-dirty_bitmap, n)) {
+ r = -EFAULT;
+ goto out;
+ }
memslot-is_dirty = false;
}
r = 0;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 4b074f1..2a31d2f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1210,7 +1210,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
n = kvm_dirty_bitmap_bytes(memslot);
- memset(memslot-dirty_bitmap, 0, n);
+ if (clear_user(memslot-dirty_bitmap, n)) {
+ r = -EFAULT;
+ goto out;
+ }
memslot-is_dirty = false;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 023c7f8..32a3d94 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2760,40 +2760,37 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
/* If nothing is dirty, don't bother messing with page tables. */
if (memslot-is_dirty) {
struct kvm_memslots *slots, *old_slots;
- unsigned long *dirty_bitmap;
+ unsigned long __user *dirty_bitmap;
+ unsigned long __user *dirty_bitmap_old;
spin_lock(kvm-mmu_lock);
kvm_mmu_slot_remove_write_access(kvm, log-slot);
spin_unlock(kvm-mmu_lock);
- r = -ENOMEM;
- dirty_bitmap = vmalloc(n);
- if (!dirty_bitmap)
+ dirty_bitmap = memslot-dirty_bitmap;
+ dirty_bitmap_old = memslot-dirty_bitmap_old;
+ r = -EFAULT;
+ if (clear_user(dirty_bitmap_old, n))
goto out;
- memset(dirty_bitmap, 0, n);
r = -ENOMEM;
slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
- if (!slots) {
- vfree(dirty_bitmap);
+ if (!slots)
goto out;
- }
+
memcpy(slots, kvm-memslots, sizeof(struct kvm_memslots));
- slots-memslots[log-slot].dirty_bitmap = dirty_bitmap;
+ slots-memslots[log-slot].dirty_bitmap = dirty_bitmap_old;
+ slots-memslots[log-slot].dirty_bitmap_old = dirty_bitmap;
slots-memslots[log-slot].is_dirty = false;
