Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation

[Scott Wood <scottwood@freescale.com>]

On 07/17/2012 12:10 PM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, July 17, 2012 10:31 PM
>> To: Bhushan Bharat-R65777
>> Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org;
>> kvm@vger.kernel.org; bharatb.yadav@gmail.com; Benjamin Herrenschmidt; Kumar Gala
>> Subject: Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/17/2012 11:56 AM, Bhushan Bharat-R65777 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Tuesday, July 17, 2012 10:08 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org;
>>>> kvm@vger.kernel.org; bharatb.yadav@gmail.com; Benjamin Herrenschmidt;
>>>> Kumar Gala
>>>> Subject: Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation
>>>>
>>>> On 07/17/2012 06:31 AM, Bhushan Bharat-R65777 wrote:
>>>>>>>>  int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)  {
>>>>>>>> -    return !(v->arch.shared->msr & MSR_WE) ||
>>>>>>>> -           !!(v->arch.pending_exceptions) ||
>>>>>>>> -           v->requests;
>>>>>>>> +    bool ret = !(v->arch.shared->msr & MSR_WE) ||
>>>>>>>> +           !!(v->arch.pending_exceptions) ||
>>>>>>>> +           v->requests;
>>>>>>>> +
>>>>>>>> +    ret = ret || kvmppc_get_tsr_wrc(v);
>>>>>>>
>>>>>>> Why do you need to declare the cpu as non-runnable when a watchdog
>>>>>>> event occured?
>>>>>>
>>>>>> It's the other way around -- it's always runnable when a watchdog
>>>>>> exit is pending.  It's like a pending exception.
>>>>>
>>>>> With the above check, Are we trying to handle the case where
>>>>> watchdog interrupt bit in pending_exception is cleared by guest
>>>>> after final expiry but before the qemu exit?
>>>>
>>>> No, we're just trying to test the actual condition we want to exit on.
>>>> The watchdog interrupt might be masked (either with WIE or CE).
>>>
>>> If the interrupt is masked then still the pending_exception will be set.
>>
>> Not if it's masked by WIE -- and even when masked by CE, it's a bug that we
>> currently consider the vcpu runnable.  We shouldn't depend on that bug.
> 
> Scott can you please describe what is bug?

If an interrupt is masked by EE, CE, ME, etc. it is still in
pending_exceptions, so runnable still returns true, and we can't go idle.

> What I remember is that if
> vcpu is not run-able then we halt vcpu and cannot cause qemu exit
> also.

I agree that we want to be considered runnable if we have a final
expiration with an action.  What I disagree with is using the same
pending_exceptions bit as is used for the ordinary watchdog interrupt.
They're not the same thing.

-Scott