KVM/X86: vpmu migration, make perf_event associated with vcpu thread

After applying Paolo's patch, vpmu's data was migrated correctly.
https://patchwork.kernel.org/patch/2850813/

But when I wrote a test module to make IA32_PMC1 to count the event of unhalted cpu-cycles, after migration the value of IA32_PMC1 never grows up again. I found that after migration perf_event was created exactly, but when it was created, "current" is qemu's main thread which won't enter no-root mode, so the count of perf_event will never increase.

I have tried pid in the struct of kvm_vcpu to get the vcpu thread's task_struct, but after migration when create perf_event, pid is pointed to qemu's main thread but not vcpu thread because of the pid switching in vcpu_load. I don't understand this very well, I think vcpu is created in qemu_kvm_cpu_thread_fn, which is the vcpu thread, use the pid of current is enough, why switch is needed?

Maybe I was totally wrong, so I kept these code unchanged, add a extra "tid" to keep the vcpu thread's pid, and use this "tid" to get the task_struct of vcpu thread when create perf_event.

Thanks
Wang Hui

Signed-off-by Wang Hui <[email protected]>

---
 arch/x86/kvm/pmu.c       | 8 +++++++-
 arch/x86/kvm/x86.c       | 6 ++++++
 include/linux/kvm_host.h | 1 +
 virt/kvm/kvm_main.c      | 1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

arch/x86/kvm/pmu.c +7 -1

index 5c4f631..676227e 100644
@@ -173,6 +173,8 @@ static void reprogram_counter(struct kvm_pmc *pmc, u32 type,
 		.exclude_kernel = exclude_kernel,
 		.config = config,
 	};
+	struct task_struct *task = NULL;
+
 	if (in_tx)
 		attr.config |= HSW_IN_TX;
 	if (in_tx_cp)

@@ -180,7 +182,11 @@ static void reprogram_counter(struct kvm_pmc *pmc, u32 type,
 
 	attr.sample_period = (-pmc->counter) & pmc_bitmask(pmc);
 
-	event = perf_event_create_kernel_counter(&attr, -1, current,
+	if (pmc->vcpu)
+		task = pid_task(pmc->vcpu->tid, PIDTYPE_PID);
+	if (!task)
+		task = current;
+	event = perf_event_create_kernel_counter(&attr, -1, task,
 						 intr ? kvm_perf_overflow_intr :
 						 kvm_perf_overflow, pmc);
 	if (IS_ERR(event)) {

arch/x86/kvm/x86.c +6 -0

index 21ef1ba..f1f0e8e 100644
@@ -6704,11 +6704,17 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
 	int r;
+	struct pid *cpu_thread_pid;
 
 	vcpu->arch.mtrr_state.have_fixed = 1;
 	r = vcpu_load(vcpu);
 	if (r)
 		return r;
+
+	cpu_thread_pid = get_task_pid(current, PIDTYPE_PID);
+	rcu_assign_pointer(vcpu->tid, cpu_thread_pid);
+	synchronize_rcu();
+
 	kvm_vcpu_reset(vcpu);
 	kvm_mmu_setup(vcpu);
 	vcpu_put(vcpu);

include/linux/kvm_host.h +1 -0

index 9523d2a..ad7af9d 100644
@@ -235,6 +235,7 @@ struct kvm_vcpu {
 	int guest_fpu_loaded, guest_xcr0_loaded;
 	wait_queue_head_t wq;
 	struct pid *pid;
+	struct pid *tid;
 	int sigset_active;
 	sigset_t sigset;
 	struct kvm_vcpu_stat stat;

virt/kvm/kvm_main.c +1 -1

index a0aa84b..80bcce5 100644
@@ -242,6 +242,7 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
+	put_pid(vcpu->tid);
 	put_pid(vcpu->pid);
 	kvm_arch_vcpu_uninit(vcpu);
 	free_page((unsigned long)vcpu->run);
--

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

Quoted 9 lines · Wang Hui

After applying Paolo's patch, vpmu's data was migrated correctly.
https://patchwork.kernel.org/patch/2850813/

But when I wrote a test module to make IA32_PMC1 to count the event of unhalted cpu-cycles, after migration the value of IA32_PMC1 never grows up again. I found that after migration perf_event was created exactly, but when it was created, "current" is qemu's main thread which won't enter no-root mode, so the count of perf_event will never increase.

Quoted 6 lines

I have tried pid in the struct of kvm_vcpu to get the vcpu thread's task_struct, but after migration when create perf_event, pid is pointed to qemu's main thread but not vcpu thread because of the pid switching in vcpu_load. I don't understand this very well, I think vcpu is created in qemu_kvm_cpu_thread_fn, which is the vcpu thread, use the pid of current is enough, why switch is needed?

Because the fact that thread that creates vcpu is the one that will "run" the vcpu is QEMU implementation detail. Other userspace may create all vcpus in one thread and then handle each one to a dedicated thread. Your code will not work for such userspace. You need to use pid and reprogram all mmu counters when it changes.

But when I wrote a test module to make IA32_PMC1 to count the event of unhalted cpu-cycles, after migration the value of IA32_PMC1 never grows up again. I found that after migration perf_event was created exactly, but when it was created, "current" is qemu's main thread which won't enter no-root mode, so the count of perf_event will never increase.

Il 29/11/2013 18:59, Wang Hui ha scritto:

Quoted 5 lines

But when I wrote a test module to make IA32_PMC1 to count the event of unhalted cpu-cycles, after migration the value of IA32_PMC1 never grows up again. I found that after migration perf_event was created exactly, but when it was created, "current" is qemu's main thread which won't enter no-root mode, so the count of perf_event will never increase.

I think the fix is to apply the MSRs in QEMU's VCPU thread, through run_on_cpu. Interesting that I could not reproduce this when testing my patches.

Paolo