On 2017-12-22 10:51 AM, alazar@xxxxxxxxxxxxxxx wrote:
On Fri, 22 Dec 2017 10:12:35 -0500, Patrick Colp <patrick.colp@xxxxxxxxxx> wrote:
On 2017-12-22 09:11 AM, Adalbert Laz����r wrote:
We've made changes in all the places pointed by you, but read below.
Thanks again,
Adalbert
On Fri, 22 Dec 2017 02:34:45 -0500, Patrick Colp <patrick.colp@xxxxxxxxxx> wrote:
On 2017-12-18 02:06 PM, Adalber Lazăr wrote:
From: Adalbert Lazar <alazar@xxxxxxxxxxxxxxx>
This subsystem is split into three source files:
- kvmi_msg.c - ABI and socket related functions
- kvmi_mem.c - handle map/unmap requests from the introspector
- kvmi.c - all the other
The new data used by this subsystem is attached to the 'kvm' and
'kvm_vcpu' structures as opaque pointers (to 'kvmi' and 'kvmi_vcpu'
structures).
Besides the KVMI system, this patch exports the
kvm_vcpu_ioctl_x86_get_xsave() and the mm_find_pmd() functions,
adds a new vCPU request (KVM_REQ_INTROSPECTION) and a new VM ioctl
(KVM_INTROSPECTION) used to pass the connection file handle from QEMU.
Signed-off-by: Mihai Donțu <mdontu@xxxxxxxxxxxxxxx>
Signed-off-by: Adalbert Lazăr <alazar@xxxxxxxxxxxxxxx>
Signed-off-by: Nicușor Cîțu <ncitu@xxxxxxxxxxxxxxx>
Signed-off-by: Mircea Cîrjaliu <mcirjaliu@xxxxxxxxxxxxxxx>
Signed-off-by: Marian Rotariu <mrotariu@xxxxxxxxxxxxxxx>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/Makefile | 1 +
arch/x86/kvm/x86.c | 4 +-
include/linux/kvm_host.h | 4 +
include/linux/kvmi.h | 32 +
include/linux/mm.h | 3 +
include/trace/events/kvmi.h | 174 +++++
include/uapi/linux/kvm.h | 8 +
mm/internal.h | 5 -
virt/kvm/kvmi.c | 1410 +++++++++++++++++++++++++++++++++++++++
virt/kvm/kvmi_int.h | 121 ++++
virt/kvm/kvmi_mem.c | 730 ++++++++++++++++++++
virt/kvm/kvmi_msg.c | 1134 +++++++++++++++++++++++++++++++
13 files changed, 3620 insertions(+), 7 deletions(-)
create mode 100644 include/linux/kvmi.h
create mode 100644 include/trace/events/kvmi.h
create mode 100644 virt/kvm/kvmi.c
create mode 100644 virt/kvm/kvmi_int.h
create mode 100644 virt/kvm/kvmi_mem.c
create mode 100644 virt/kvm/kvmi_msg.c
+bool kvmi_hook(struct kvm *kvm, struct kvm_introspection *qemu)
+{
+ kvm_info("Hooking vm with fd: %d\n", qemu->fd);
+
+ kvm_page_track_register_notifier(kvm, &kptn_node);
+
+ return (alloc_kvmi(kvm) && setup_socket(kvm, qemu));
Is this safe? It could return false if the alloc fails (in which case
the caller has to do nothing) or if setting up the socket fails (in
which case the caller needs to free the allocated kvmi).
If the socket fails for any reason (eg. the introspection tool is
stopped == socket closed) 'the plan' is to signal QEMU to reconnect
(and call kvmi_hook() again) or else let the introspected VM continue (and
try to reconnect asynchronously).
I see that kvm_page_track_register_notifier() should not be called more
than once.
Maybe we should rename this to kvmi_rehook() or kvmi_reconnect().
I assume that a kvmi_rehook() function would then not call
kvm_page_track_register_notifier() or at least have some check to make
sure it only calls it once?
One approach would be to have separate kvmi_hook() and kvmi_rehook()
functions. Another possibility is to have kvmi_hook() take an extra
argument that's a boolean to specify if it's the first attempt at
hooking or not.
alloc_kvmi() didn't worked with a second kvmi_hook() call.
For the moment I've changed the code to:
kvmi_hook
return (alloc_kvmi && setup_socket)
alloc_kvmi
return (IKVM(kvm) || __alloc_kvmi)
__alloc_kvmi
kzalloc
kvm_page_track_register_notifier
At least it works as 'advertised' :)
Yes, that seems a lot more clear :)
