Re: [RFC][PATCH 1/5] [PATCH 1/5] kvm: register in task_struct

From: Fengguang Wu
Date: Mon Sep 03 2018 - 20:46:29 EST


On Tue, Sep 04, 2018 at 08:28:18AM +0800, Fengguang Wu wrote:
Hi Christian and Nikita,

On Mon, Sep 03, 2018 at 06:03:49PM +0200, Christian Borntraeger wrote:


On 09/03/2018 04:10 PM, Nikita Leshenko wrote:
On September 2, 2018 5:21:15 AM, fengguang.wu@xxxxxxxxx wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8b47507faab5..0c483720de8d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3892,6 +3892,7 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
if (type == KVM_EVENT_CREATE_VM) {
add_uevent_var(env, "EVENT=create");
kvm->userspace_pid = task_pid_nr(current);
+ current->kvm = kvm;

Is it OK to store `kvm` on the task_struct? What if the thread that
originally created the VM exits? From the documentation it seems
like a VM is associated with an address space and not a specific
thread, so maybe it should be stored on mm_struct?

Yes, ioctls accessing the kvm can happen from all threads.

Good point, thank you for the tips! I'll move kvm pointer to mm_struct.

From Documentation/virtual/kvm/api.txt:
Only run VM ioctls from the same process (address space) that was used
to create the VM.

-Nikita

Here it goes:

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 99ce070e7dcb..27c5446f3deb 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -27,6 +27,7 @@ typedef int vm_fault_t;
struct address_space;
struct mem_cgroup;
struct hmm;
+struct kvm;

/*
* Each physical page in the system has a struct page associated with
@@ -489,10 +490,19 @@ struct mm_struct {
/* HMM needs to track a few things per mm */
struct hmm *hmm;
#endif
+#if IS_ENABLED(CONFIG_KVM)
+ struct kvm *kvm;
+#endif
} __randomize_layout;

extern struct mm_struct init_mm;

+#if IS_ENABLED(CONFIG_KVM)
+static inline struct kvm *mm_kvm(struct mm_struct *mm) { return mm->kvm; }
+#else
+static inline struct kvm *mm_kvm(struct mm_struct *mm) { return NULL; }
+#endif
+
static inline void mm_init_cpumask(struct mm_struct *mm)
{
#ifdef CONFIG_CPUMASK_OFFSTACK
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0c483720de8d..dca6156a7b35 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3892,7 +3892,7 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
if (type == KVM_EVENT_CREATE_VM) {
add_uevent_var(env, "EVENT=create");
kvm->userspace_pid = task_pid_nr(current);
- current->kvm = kvm;
+ current->mm->kvm = kvm;
} else if (type == KVM_EVENT_DESTROY_VM) {
add_uevent_var(env, "EVENT=destroy");
}