Re: [PATCH 2/3] Improve DEC handling

2009-12-21 Thread Hollis Blanchard
On Mon, Dec 21, 2009 at 6:22 AM, Alexander Graf ag...@suse.de wrote:
 We treated the DEC interrupt like an edge based one. This is not true for
 Book3s. The DEC keeps firing until mtdec is issued again and thus clears
 the interrupt line.

That's not quite right. The decrementer keeps firing until the top bit
is cleared, i.e. with mtdec. However, not *every* mtdec clears it.
(Also, I'm pretty sure this varies between Book 3S implementations,
e.g. 970 behaves differently than POWERn. I don't remember specific
values of n though, and I could be misremembering...)

So is this the failure mode?
- a decrementer interrupt is delivered
- guest does *not* issue mtdec to clear it (ppc64's lazy interrupt disabling?)
- guest expects a second decrementer interrupt, but KVM doesn't deliver one

In that case, it seems like the real fix would be something like this:

 void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)
 {
unsigned long dec_nsec;

pr_debug(mtDEC: %x\n, vcpu-arch.dec);
 #ifdef CONFIG_PPC64
/* POWER4+ triggers a dec interrupt if the value is  0 */
if (vcpu-arch.dec  0x8000) {
hrtimer_try_to_cancel(vcpu-arch.dec_timer);
kvmppc_core_queue_dec(vcpu);
+   /* keep queuing interrupts until guest clears high MSR bit */
+   hrtimer_start(vcpu-arch.dec_timer, ktime_set(0, 100),
+ HRTIMER_MODE_REL);
return;
}
 #endif

-Hollis
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] Improve DEC handling

2009-12-21 Thread Hollis Blanchard
On Mon, Dec 21, 2009 at 10:13 AM, Hollis Blanchard
hol...@penguinppc.org wrote:
  void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)
  {
        unsigned long dec_nsec;

        pr_debug(mtDEC: %x\n, vcpu-arch.dec);
  #ifdef CONFIG_PPC64
        /* POWER4+ triggers a dec interrupt if the value is  0 */
        if (vcpu-arch.dec  0x8000) {
                hrtimer_try_to_cancel(vcpu-arch.dec_timer);
                kvmppc_core_queue_dec(vcpu);
 +               /* keep queuing interrupts until guest clears high MSR bit */
 +               hrtimer_start(vcpu-arch.dec_timer, ktime_set(0, 100),
 +                             HRTIMER_MODE_REL);
                return;
        }
  #endif

Of course, removing the hardcoded 100-ns timer would be better, and
indeed we can do that. What we *really* want is to key off of MSR[EE]
changes (there's no point in queuing anything until then). So why not
move your AGGRESSIVE_DEC check into Book 3S's kvmppc_set_msr()?

-Hollis
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] Improve DEC handling

2009-12-21 Thread Alexander Graf
Hollis Blanchard wrote:
 On Mon, Dec 21, 2009 at 10:13 AM, Hollis Blanchard
 hol...@penguinppc.org wrote:
   
  void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)
  {
unsigned long dec_nsec;

pr_debug(mtDEC: %x\n, vcpu-arch.dec);
  #ifdef CONFIG_PPC64
/* POWER4+ triggers a dec interrupt if the value is  0 */
if (vcpu-arch.dec  0x8000) {
hrtimer_try_to_cancel(vcpu-arch.dec_timer);
kvmppc_core_queue_dec(vcpu);
 +   /* keep queuing interrupts until guest clears high MSR bit */
 +   hrtimer_start(vcpu-arch.dec_timer, ktime_set(0, 100),
 + HRTIMER_MODE_REL);
return;
}
  #endif
 

 Of course, removing the hardcoded 100-ns timer would be better, and
 indeed we can do that. What we *really* want is to key off of MSR[EE]
 changes (there's no point in queuing anything until then).

Yes. mtmsr with EE=1 triggers a #VMEXIT which then goes off into a VM
entry which on entering checks that the DEC interrupt is still active
and fires it.

 So why not
 move your AGGRESSIVE_DEC check into Book 3S's kvmppc_set_msr()?
   

The idea was to get rid of AGGRESSIVE_DEC :-).

Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html