perf & kvm: Enhance perf to collect KVM guest os statistics from host side

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

+
diff -Nraup linux-2.6_tip0413/arch/x86/kvm/x86.c linux-2.6_tip0413_perfkvm/arch/x86/kvm/x86.c

Quoted 2 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Sheng, did you make any progress with the NMI injection issue?

Quoted 11 lines

+
diff -Nraup linux-2.6_tip0413/arch/x86/kvm/x86.c linux-2.6_tip0413_perfkvm/arch/x86/kvm/x86.c ---
linux-2.6_tip0413/arch/x86/kvm/x86.c 2010-04-14 11:11:04.341042024 +0800
+++ linux-2.6_tip0413_perfkvm/arch/x86/kvm/x86.c 2010-04-14
11:32:45.841278890 +0800 @@ -3765,6 +3765,35 @@ static void kvm_timer_init(void)
}
}

arch/x86/kvm/x86.c static void kvm_timer_init(void) · line 3772

Quoted 5 lines

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

+static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
+
+static int kvm_is_in_guest(void)
+{
+	return percpu_read(current_vcpu) != NULL;

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

arch/x86/kvm/x86.c static void kvm_timer_init(void) · line 3789

Quoted 7 lines

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

+static unsigned long kvm_get_guest_ip(void)
+{
+	unsigned long ip = 0;
+	if (percpu_read(current_vcpu))
+		ip = kvm_rip_read(percpu_read(current_vcpu));
+	return ip;
+}

This may be racy. kvm_rip_read() accesses a cache in memory; if we're in the process of updating the cache, then we may read a stale value. See below.

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 5 lines

  	trace_kvm_entry(vcpu->vcpu_id);

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

+
+	percpu_write(current_vcpu, vcpu);
  	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Quoted 10 lines · On Wednesday 14 April 2010 17:20:15 Avi Kivity wrote:

Quoted 3 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Quoted 2 lines

Sheng, did you make any progress with the NMI injection issue?

Yes, though some other works interrupt me lately...

The very first version has issue due to SELF_IPI mode can't be used to send NMI according to SDM. That's the reason why x2apic don't have way to do this.

But later I found another issue of fail to inspect inside the guest. I think it's due to NMI is asynchronous event, though it should be triggered very quickly, you can't guarantee that the handler would be triggered before the state(current_vcpu) is cleared with current code.

Maybe just extended the "guest state" region would be fine, if the latency is stable enough(though I think it maybe platform depended). I am working on this now.

Quoted 10 lines · Sheng Yang

Quoted 9 lines · On Wednesday 14 April 2010 17:20:15 Avi Kivity wrote:

Quoted 2 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Quoted 7 lines

Quoted 1 line

Sheng, did you make any progress with the NMI injection issue?

Yes, though some other works interrupt me lately...

The very first version has issue due to SELF_IPI mode can't be used to send NMI according to SDM. That's the reason why x2apic don't have way to do this.

Yes, I see that now. Looks like others have the same questions...

Quoted 8 lines

But later I found another issue of fail to inspect inside the guest. I think it's due to NMI is asynchronous event, though it should be triggered very quickly, you can't guarantee that the handler would be triggered before the state(current_vcpu) is cleared with current code.

Maybe just extended the "guest state" region would be fine, if the latency is stable enough(though I think it maybe platform depended). I am working on this now.

I wouldn't like to depend on model specific behaviour.

One option is to read all the information synchronously and store it in a per-cpu area with atomic instructions, then queue the NMI. Another option is to have another callback which tells us that the NMI is done, and have a busy loop wait until the NMI is delivered.

Quoted 9 lines

I wouldn't like to depend on model specific behaviour.

One option is to read all the information synchronously and store it in a per-cpu area with atomic instructions, then queue the NMI. Another option is to have another callback which tells us that the NMI is done, and have a busy loop wait until the NMI is delivered.

Callback seems too heavy, may affect the performance badly. Maybe a short queue would help, though this one is more complex.

But I am still curious if we extend the region, how much it would help. Would get a result soon...

Quoted 9 lines · Sheng Yang

Quoted 6 lines

I wouldn't like to depend on model specific behaviour.

One option is to read all the information synchronously and store it in a per-cpu area with atomic instructions, then queue the NMI. Another option is to have another callback which tells us that the NMI is done, and have a busy loop wait until the NMI is delivered.

Callback seems too heavy, may affect the performance badly. Maybe a short queue would help, though this one is more complex.

The patch we're replying to adds callbacks (to read rip, etc.), so it's no big deal. For the queue solution, a queue of size one would probably be sufficient even if not guaranteed by the spec. I don't see how the cpu can do another guest entry without delivering the NMI.

Quoted 2 lines

But I am still curious if we extend the region, how much it would help. Would get a result soon...

Yes, interesting to see what the latency is. If it's reasonably short (and I expect it will be so), we can do the busy wait solution.

If we have an NMI counter somewhere, we can simply wait until it changes.

Quoted 6 lines

Yes, interesting to see what the latency is. If it's reasonably short (and I expect it will be so), we can do the busy wait solution.

If we have an NMI counter somewhere, we can simply wait until it changes.

Good idea. Of course we have one(at least on x86). There is irq_stat.irq__nmi_count for per cpu. :)

Quoted 3 lines · Sheng Yang

Quoted 3 lines

Yes, interesting to see what the latency is. If it's reasonably short (and I expect it will be so), we can do the busy wait solution.

Quoted 5 lines

Quoted 2 lines

If we have an NMI counter somewhere, we can simply wait until it changes.

Good idea. Of course we have one(at least on x86). There is irq_stat.irq__nmi_count for per cpu. :)

Okay, but kvm doesn't want to know about it. How about a new arch function, invoke_nmi_sync(), that will trigger the NMI and wait for it?

Quoted 3 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Quoted 6 lines

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Once it gets into a state that it can be applied could you please create a separate, -git based branch for it, so that i can pull it for testing and integration with the tools/perf/ bits?

Assuming there are no serious conflicts with pending KVM work.

Quoted 4 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Sheng, did you make any progress with the NMI injection issue?

Quoted 8 lines

+
diff -Nraup linux-2.6_tip0413/arch/x86/kvm/x86.c linux-2.6_tip0413_perfkvm/arch/x86/kvm/x86.c

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

@@ -3765,6 +3765,35 @@ static void kvm_timer_init(void)
  	}
  }
 

arch/x86/kvm/x86.c static void kvm_timer_init(void)

Quoted 6 lines

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

+static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
+
+static int kvm_is_in_guest(void)
+{
+	return percpu_read(current_vcpu) != NULL;
 

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

arch/x86/kvm/x86.c static void kvm_timer_init(void)

Quoted 9 lines

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

+static unsigned long kvm_get_guest_ip(void)
+{
+	unsigned long ip = 0;
+	if (percpu_read(current_vcpu))
+		ip = kvm_rip_read(percpu_read(current_vcpu));
+	return ip;
+}
 

This may be racy. kvm_rip_read() accesses a cache in memory; if we're in the process of updating the cache, then we may read a stale value. See below.

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 8 lines

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

  	trace_kvm_entry(vcpu->vcpu_id);
+
+	percpu_write(current_vcpu, vcpu);
  	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Quoted 4 lines · Yanmin

Here is the new patch of V3 against tip/master of April 13th if anyone wants to try it.

Thanks for persisting despite the flames.

Can you please separate arch/x86/kvm part of the patch? That will make for easier reviewing, and will need to go through separate trees.

Sheng, did you make any progress with the NMI injection issue?

Quoted 8 lines

+
diff -Nraup linux-2.6_tip0413/arch/x86/kvm/x86.c linux-2.6_tip0413_perfkvm/arch/x86/kvm/x86.c

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

@@ -3765,6 +3765,35 @@ static void kvm_timer_init(void)
  	}
  }
 

arch/x86/kvm/x86.c static void kvm_timer_init(void)

Quoted 6 lines

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

+static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
+
+static int kvm_is_in_guest(void)
+{
+	return percpu_read(current_vcpu) != NULL;
 

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

arch/x86/kvm/x86.c static void kvm_timer_init(void)

Quoted 9 lines

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

+static unsigned long kvm_get_guest_ip(void)
+{
+	unsigned long ip = 0;
+	if (percpu_read(current_vcpu))
+		ip = kvm_rip_read(percpu_read(current_vcpu));
+	return ip;
+}
 

This may be racy. kvm_rip_read() accesses a cache in memory; if we're in the process of updating the cache, then we may read a stale value. See below.

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 8 lines

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

  	trace_kvm_entry(vcpu->vcpu_id);
+
+	percpu_write(current_vcpu, vcpu);
  	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Quoted 7 lines

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

Yes. But the frame pointer checking seems a little complicated.

Quoted 11 lines

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 7 lines

   	trace_kvm_entry(vcpu->vcpu_id);

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

+
+	percpu_write(current_vcpu, vcpu);
   	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Right. The kernel part has dependency on the self-IPI implementation. I will move above percpu_write(current_vcpu, vcpu) (or a new wrapper function) just around 'int $2'.

Sheng would find a solution on the self-IPI delivery. Let's separate my patch and self-IPI as 2 issues as we don't know when the self-IPI delivery would be resolved.

Quoted 9 lines · Yanmin

Quoted 7 lines

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

Yes. But the frame pointer checking seems a little complicated.

An even bigger disadvantage is that it won't work with Sheng's patch, self-NMIs are not synchronous.

Quoted 11 lines

Quoted 11 lines

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 7 lines

   	trace_kvm_entry(vcpu->vcpu_id);

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

+
+	percpu_write(current_vcpu, vcpu);
   	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Quoted 5 lines

Right. The kernel part has dependency on the self-IPI implementation. I will move above percpu_write(current_vcpu, vcpu) (or a new wrapper function) just around 'int $2'.

Or create a new function to inject the interrupt in x86.c. That will reduce duplication between svm.c and vmx.c.

Quoted 9 lines · Yanmin

Quoted 7 lines

An even more accurate way to determine this is to check whether the interrupt frame points back at the 'int $2' instruction. However we plan to switch to a self-IPI method to inject the NMI, and I'm not sure wether APIC NMIs are accepted on an instruction boundary or whether there's some latency involved.

Yes. But the frame pointer checking seems a little complicated.

An even bigger disadvantage is that it won't work with Sheng's patch, self-NMIs are not synchronous.

Quoted 11 lines

Quoted 11 lines

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4378

Quoted 7 lines

   	trace_kvm_entry(vcpu->vcpu_id);

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

+
+	percpu_write(current_vcpu, vcpu);
   	kvm_x86_ops->run(vcpu);
+	percpu_write(current_vcpu, NULL);

If you move this around the 'int $2' instructions you will close the race, as a stray NMI won't catch us updating the rip cache. But that depends on whether self-IPI is accepted on the next instruction or not.

Quoted 5 lines

Right. The kernel part has dependency on the self-IPI implementation. I will move above percpu_write(current_vcpu, vcpu) (or a new wrapper function) just around 'int $2'.

Or create a new function to inject the interrupt in x86.c. That will reduce duplication between svm.c and vmx.c.

I checked svm.c and it seems svm.c doesn't trigger a NMI to host if the NMI happens in guest os. In addition, svm_complete_interrupts is called after interrupt is enabled.

Quoted 4 lines · Yanmin

I checked svm.c and it seems svm.c doesn't trigger a NMI to host if the NMI happens in guest os. In addition, svm_complete_interrupts is called after interrupt is enabled.

Yes. The NMI is held pending by the hardware until the STGI instruction is executed.
And for nested svm the svm_complete_interrupts function needs to be executed after the nested exit handling. Therefore it is done late on svm.

Quoted 7 lines · oerg Roedel

Quoted 4 lines · Yanmin

I checked svm.c and it seems svm.c doesn't trigger a NMI to host if the NMI happens in guest os. In addition, svm_complete_interrupts is called after interrupt is enabled.

Quoted 6 lines

Yes. The NMI is held pending by the hardware until the STGI instruction is executed.
And for nested svm the svm_complete_interrupts function needs to be executed after the nested exit handling. Therefore it is done late on svm.

So, we'd need something like the following:

   if (exit == NMI)
       __get_cpu_var(nmi_vcpu) = vcpu;

   stgi();

   if (exit == NMI) {
       while (!nmi_handled())
           cpu_relax();
       __get_cpu_var(nmi_vcpu) = NULL;
   }

Quoted 14 lines

So, we'd need something like the following:

    if (exit == NMI)
        __get_cpu_var(nmi_vcpu) = vcpu;

    stgi();

    if (exit == NMI) {
        while (!nmi_handled())
            cpu_relax();
        __get_cpu_var(nmi_vcpu) = NULL;
    }

Hmm, looks a bit complicated to me. The NMI should happen shortly after the stgi instruction. Interrupts are still disabled so we stay on this cpu. Can't we just set and erase the cpu_var at vcpu_load/vcpu_put time?

Quoted 14 lines · oerg Roedel

Quoted 14 lines

So, we'd need something like the following:

    if (exit == NMI)
        __get_cpu_var(nmi_vcpu) = vcpu;

    stgi();

    if (exit == NMI) {
        while (!nmi_handled())
            cpu_relax();
        __get_cpu_var(nmi_vcpu) = NULL;
    }

Quoted 5 lines

Hmm, looks a bit complicated to me. The NMI should happen shortly after the stgi instruction. Interrupts are still disabled so we stay on this cpu. Can't we just set and erase the cpu_var at vcpu_load/vcpu_put time?

That means an NMI that happens outside guest code (for example, in the mmu, or during the exit itself) would be counted as if in guest code.

Quoted 4 lines

That means an NMI that happens outside guest code (for example, in the mmu, or during the exit itself) would be counted as if in guest code.

Hmm, true. The same is true for an NMI that happens between VMSAVE and STGI but that window is smaller. Anyway, I think we don't need the busy-wait loop. The NMI should be executed at a well defined point and we set the cpu_var back to NULL after that point.

Quoted 7 lines · Joerg Roedel

Quoted 2 lines

That means an NMI that happens outside guest code (for example, in the mmu, or during the exit itself) would be counted as if in guest code.

Hmm, true. The same is true for an NMI that happens between VMSAVE and STGI but that window is smaller. Anyway, I think we don't need the busy-wait loop. The NMI should be executed at a well defined point and we set the cpu_var back to NULL after that point.

The point is not well defined. Considering there are already at least two implementations svm, I don't want to rely on implementation details.

The following bullets detail when architectural state is and is not updated

- An NMI causes subsequent NMIs to be blocked, but only after the VM exit

We could tune the position of the loop so that zero iterations are executed on the implementations we know about.

Quoted 5 lines · On Thursday 15 April 2010 18:44:15 Avi Kivity wrote:

Quoted 3 lines · Joerg Roedel

Quoted 3 lines

That means an NMI that happens outside guest code (for example, in the mmu, or during the exit itself) would be counted as if in guest code.

Quoted 8 lines

Quoted 5 lines

Hmm, true. The same is true for an NMI that happens between VMSAVE and STGI but that window is smaller. Anyway, I think we don't need the busy-wait loop. The NMI should be executed at a well defined point and we set the cpu_var back to NULL after that point.

The point is not well defined. Considering there are already at least two implementations svm, I don't want to rely on implementation details.

After more investigating, I realized that I had interpreted the SDM wrong. Sorry.

There is *no* risk with the original method of calling "int $2".

According to the SDM 24.1:

Quoted 2 lines

The following bullets detail when architectural state is and is not updated

in response to VM exits:
[...]

Quoted 2 lines

- An NMI causes subsequent NMIs to be blocked, but only after the VM exit

completes.

So the truth is, after NMI directly caused VMExit, the following NMIs would be blocked, until encountered next "iret". So execute "int $2" is safe in vmx_complete_interrupts(), no risk in causing nested NMI. And it would unblock the following NMIs as well due to "iret" it executed.

So there is unnecessary to make change to avoid "potential nested NMI".

The following bullets detail when architectural state is and is not updated in response to VM exits:
• If an event causes a VM exit directly, it does not update architectural state as it
would have if it had it not caused the VM exit:
— A debug exception does not update DR6, DR7.GD, or IA32_DEBUGCTL.LBR.
(Information about the nature of the debug exception is saved
in the exit
qualification field.)
— A page fault does not update CR2. (The linear address causing
the page fault
is saved in the exit-qualification field.)
— An NMI causes subsequent NMIs to be blocked, but only after the
VM exit

completes.
— An external interrupt does not acknowledge the interrupt
controller and the
interrupt remains pending, unless the “acknowledge interrupt
on exit”
VM-exit control is 1. In such a case, the interrupt controller
is acknowledged
and the interrupt is no longer pending.

I think you were right the first time around.

Quoted 1 line · Avi Kivity

I think you were right the first time around.

Re-reading again (esp. the part about treatment of indirect NMI vmexits), I think this was wrong, and that the code is correct. I am now thoroughly confused.

Quoted 2 lines · On Monday 19 April 2010 16:25:17 Avi Kivity wrote:

On 04/17/2010 09:12 PM, Avi Kivity wrote:

Quoted 7 lines

Quoted 2 lines

I think you were right the first time around.

Re-reading again (esp. the part about treatment of indirect NMI vmexits), I think this was wrong, and that the code is correct. I am now thoroughly confused.

My fault...

To my understanding now, "If an event causes a VM exit directly, it does not update architectural state as it would have if it had it not caused the VM exit:", means: in NMI case, NMI would involve the NMI handler, and change the "architectural state" to NMI block. In VMX non-root mode, the behavior of calling NMI handler changed(determine by some VMCS fields), but not the affection to the "architectural state". So the NMI block state would remain the same.

This is based on the research I did with optimizing my machine for graphics.
I also wrote the following article:
http://www.paradoxuncreated.com/articles/Millennium/Millennium.html
It is a bit outdated now, but I will update it with current information. The value might iterate.

Peace Be With You,
Uwaysi Bin Kareem.

Kconfig.hz renamed from Kconfig.hzorig +4 -0

@@ -45,6 +45,18 @@
  	 1000 Hz is the preferred choice for desktop systems and other
  	 systems requiring fast interactive responses to events.
 
+	config HZ_3956
+		bool "3956 HZ"
+	help
+	 3956 Hz is nearly the highest timer interrupt rate supported in the  

kernel.

Kconfig.hz renamed from Kconfig.hzorig +4 -0

+	 profile of "where the human senses register the most information".
+	
+	
+
  endchoice
 
  config HZ