Re: [PATCH 5/5] KVM: PPC: booke: Improve timer register emulation

[Scott Wood <scottwood@freescale.com>]

On 09/27/2011 11:21 AM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>> owner@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Tuesday, September 27, 2011 9:39 PM
>> To: Bhushan Bharat-R65777
>> Cc: Wood Scott-B07421; Liu Yu-B13201; kvm-ppc@vger.kernel.org
>> Subject: Re: [PATCH 5/5] KVM: PPC: booke: Improve timer register
>> emulation
>>
>>
>> On 27.09.2011, at 18:01, Bhushan Bharat-R65777 wrote:
>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>>>> owner@vger.kernel.org] On Behalf Of Alexander Graf
>>>> Sent: Tuesday, September 27, 2011 1:45 PM
>>>> To: Wood Scott-B07421
>>>> Cc: Liu Yu-B13201; Wood Scott-B07421; kvm-ppc@vger.kernel.org
>>>> Subject: Re: [PATCH 5/5] KVM: PPC: booke: Improve timer register
>>>> emulation
>>>>
>>>>
>>>> On 27.09.2011, at 02:44, Scott Wood wrote:
>>>>
>>>>> On 09/24/2011 02:27 AM, Alexander Graf wrote:
>>>>>> I think I'm getting your point. So what we want is:
>>>>>>
>>>>>> in timer handler:
>>>>>>
>>>>>> set_bit(TSR_DIS, vcpu->arch.tsr);
>>>>>> kvm_make_request(KVM_REQ_PPC_TSR_UPDATE, vcpu);
>>>>>> kvm_vcpu_kick(vcpu);
>>>
>>> Still there can be a case where first request not handled and another
>> event happens? Or we would like to pause till first request is handled by
>> vcpu?
>>
>> If the first DIS request is not handled and another event happens, the
>> interrupts get coalesced (like on real hardware). If there is another TSR
>> bit set still, int_pending should still be active and the guest exits the
>> next time it sets MSR_EE, making us inject the next interrupt.
>>
> 
> This may not work for watchdog, where every event causes a state change.

For watchdog, this would only be used for delivering the interrupt (or a
final expiration exit to qemu, which would be a separate request bit).
ENW/WIS would be handled from timer context, like DIS is here.

The watchdog timer could use a compare and swap to avoid this sort of race:

	vcpu thread		watchdog timer
	-----------		--------------
				*expires*
				set ENW

				*expires*
				see ENW set
	clear ENW+WIS
				set WIS

	sees WIS but not ENW

...if we care.  It's something that shouldn't be possible in hardware,
but not of much consequence (the guest is unlikely to clear ENW+WIS one
time but only ENW the next).  Not hard to avoid either, though.

Other races are possible, but (unless I'm missing one) they don't
produce results that couldn't have been produced by the vcpu thread
being a few cycles slower.

-Scott