On 04/20/2010 02:03 PM, Takuya Yoshikawa wrote:
We can now export the addree of the bitmap created by do_mmap()
to user space. For the sake of this, we introduce a new API:
KVM_SWITCH_DIRTY_LOG: application can use this to trigger the
switch of the bitmaps and get the address of the bitmap which
has been used until now. This reduces the copy of the dirty
bitmap from the kernel.
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index baa8fde..f3d1c2a 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -848,6 +848,29 @@ function properly, this is the place to put them.
__u8 pad[64];
};
+4.37 KVM_SWITCH_DIRTY_LOG (vm ioctl)
+
+Capability: basic
Capability: basic means that this is available on all kvm versions,
which isn't the case. This should say KVM_CAP_USER_DIRTY_BITMAP.
+Architectures: x86
All architecures...
+Type: vm ioctl
+Parameters: struct kvm_dirty_log (in/out)
+Returns: 0 on success, -1 on error
+
+/* for KVM_SWITCH_DIRTY_LOG */
+struct kvm_dirty_log {
+ __u32 slot;
+ __u32 padding;
Please put a flags field here (and verify it is zero in the ioctl
implementation). This allows us to extend it later.
+ union {
+ void __user *dirty_bitmap; /* one bit per page */
+ __u64 addr;
+ };
Just make dirty_bitmap a __u64. With the union there is the risk that
someone forgets to zero the structure so we get some random bits in the
pointer, and also issues with big endian and s390 compat.
Reserve some extra space here for future expansion.
Hm. I see that this is the existing kvm_dirty_log structure. So we
can't play with it, ignore my comments about it.
+};
+
+Given a memory slot, return the address of a bitmap containing any
+pages dirtied since the last call to this ioctl. Bit 0 is the first
+page in the memory slot. Ensure the entire structure is cleared to
+avoid padding issues.
Document that bitmaps but be aligned and padded to 64-bits to avoid
32-on-64 issues. Explain that dirty_bitmap will be the next returned
(do we actually need the return value? userspace can simply remember it
from the last call).
How is the dirty log set when we start logging? With kernel allocated
bitmaps this isn't a problem, but with user allocated bitmaps?
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ad55353..45b728c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2718,11 +2718,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
return 0;
}
-/*
- * Get (and clear) the dirty memory log for a memory slot.
- */
-int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
- struct kvm_dirty_log *log)
+static int kvm_x86_update_dirty_log(struct kvm *kvm,
+ struct kvm_dirty_log *log,
+ bool need_copy)
{
int r;
struct kvm_memory_slot *memslot;
@@ -2773,12 +2771,34 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
dirty_bitmap_old = dirty_bitmap;
}
- r = kvm_copy_dirty_bitmap(log->dirty_bitmap, dirty_bitmap_old, n);
+ if (need_copy) {
+ r = kvm_copy_dirty_bitmap(log->dirty_bitmap,
+ dirty_bitmap_old, n);
+ } else {
+ log->addr = (unsigned long)dirty_bitmap_old;
+ r = 0;
+ }
Instead of passing need_copy everywhere, you might check a flag in
log->. You'll need a flag to know whether to munmap() or not, no?
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 23ea022..9fa6f1e 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -12,7 +12,7 @@
#include<linux/ioctl.h>
#include<asm/kvm.h>
-#define KVM_API_VERSION 12
+#define KVM_API_VERSION 13
As Alex says, this breaks all known userspace.
/* *** Deprecated interfaces *** */
@@ -318,7 +318,7 @@ struct kvm_dirty_log {
__u32 padding1;
union {
void __user *dirty_bitmap; /* one bit per page */
- __u64 padding2;
+ __u64 addr;
};
};
Update the comment above to note it applies to KVM_SWITCH_DIRTY_LOG.
@@ -1699,6 +1714,21 @@ static long kvm_vm_ioctl(struct file *filp,
goto out;
break;
}
+ case KVM_SWITCH_DIRTY_LOG: {
+ struct kvm_dirty_log log;
+
+ r = -EFAULT;
+ if (copy_from_user(&log, argp, sizeof log))
+ goto out;
+ r = kvm_vm_ioctl_switch_dirty_log(kvm,&log);
+ if (r)
+ goto out;
+ r = -EFAULT;
+ if (copy_to_user(argp,&log, sizeof log))
+ goto out;
+ r = 0;
You return 0, contrary to the documentation...
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
