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

[Scott Wood <scottwood@freescale.com>]

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.

> But interrupt will not be delivered to guest and Final expiry can still cause reset etc !! Right?

Not sure what your point is -- yes, if the interrupt is masked it will
not be delivered to the guest, and the enforcement mechanism of the
watchdog is still active.

>>> And we want that if TSR.WRS update
>>> wins the race with clearing of watchdog interrupt condition from guest
>>> then anyways let QEMU exit with reason KVM_EXIT_WDT?
>>
>> What race?  If ENW and WIS are both set when the watchdog timer fires, it's a
>> final expiration.  It's irrelevant what happens to WIS after that point, before
>> enforcement kicks in.
> 
> What I was thinking of was if we can remove this check in vcpu_runnable() and use watchdog bit in pending_exception to qualify vcpu_runnable()

No, we want a new bit.

-Scott