diff mbox

[2/2,v5] KVM: PPC: booke: Add watchdog emulation

Message ID 1342760428-10858-1-git-send-email-Bharat.Bhushan@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bharat Bhushan July 20, 2012, 5 a.m. UTC
This patch adds the watchdog emulation in KVM. The watchdog
emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.
The kernel timer are used for watchdog emulation and emulates
h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
it is configured.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
Signed-off-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
[bharat.bhushan@freescale.com: reworked patch]

---
v5:
 - Checking that TSR_ENW/TSR_WIS are still set on KVM_EXIT_WATCHDOG.
 - Moved watchdog_next_timeout() in lock.

v4:
 - in v3 i forgot to add Scott Wood and Liu Yu signoff

v3:
 - Using KVM_REQ_WATCHDOG for userspace exit.
 - TSR changes left for vcpu thread.
 - Other review comments on v2

 arch/powerpc/include/asm/kvm_host.h  |    3 +
 arch/powerpc/include/asm/kvm_ppc.h   |    3 +
 arch/powerpc/include/asm/reg_booke.h |    7 ++
 arch/powerpc/kvm/booke.c             |  140 ++++++++++++++++++++++++++++++++++
 arch/powerpc/kvm/booke_emulate.c     |    8 ++
 arch/powerpc/kvm/powerpc.c           |   19 +++++-
 include/linux/kvm.h                  |    2 +
 include/linux/kvm_host.h             |    1 +
 8 files changed, 182 insertions(+), 1 deletions(-)

Comments

Scott Wood July 20, 2012, 9:29 p.m. UTC | #1
On 07/20/2012 12:00 AM, Bharat Bhushan wrote:
> This patch adds the watchdog emulation in KVM. The watchdog
> emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.
> The kernel timer are used for watchdog emulation and emulates
> h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
> if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
> it is configured.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> [bharat.bhushan@freescale.com: reworked patch]

Typically the [] note goes immediately before your signoff (but after
the others).

> +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
> +{
> +	unsigned long nr_jiffies;
> +
> +	spin_lock(&vcpu->arch.wdt_lock);
> +	nr_jiffies = watchdog_next_timeout(vcpu);
> +	/*
> +	 * If the number of jiffies of watchdog timer >= NEXT_TIMER_MAX_DELTA
> +	 * then do not run the watchdog timer as this can break timer APIs.
> +	 */
> +	if (nr_jiffies < NEXT_TIMER_MAX_DELTA)
> +		mod_timer(&vcpu->arch.wdt_timer, jiffies + nr_jiffies);
> +	else
> +		del_timer(&vcpu->arch.wdt_timer);
> +	spin_unlock(&vcpu->arch.wdt_lock);
> +}

This needs to be an irqsave lock.

> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  #ifdef CONFIG_KVM_EXIT_TIMING
>  	mutex_init(&vcpu->arch.exit_timing_lock);
>  #endif
> -
> +#ifdef CONFIG_BOOKE
> +	spin_lock_init(&vcpu->arch.wdt_lock);
> +	/* setup watchdog timer once */
> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
> +		    (unsigned long)vcpu);
> +#endif
>  	return 0;
>  }

Can you do this in kvmppc_booke_init()?

>  
>  void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
>  {
>  	kvmppc_mmu_destroy(vcpu);
> +#ifdef CONFIG_BOOKE
> +	spin_lock(&vcpu->arch.wdt_lock);
> +	del_timer(&vcpu->arch.wdt_timer);
> +	spin_unlock(&vcpu->arch.wdt_lock);
> +#endif
>  }

Don't acquire the lock here, but use del_timer_sync().

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan July 21, 2012, 8:37 a.m. UTC | #2
> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Saturday, July 21, 2012 2:59 AM

> To: Bhushan Bharat-R65777

> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat-

> R65777

> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation

> 

> On 07/20/2012 12:00 AM, Bharat Bhushan wrote:

> > This patch adds the watchdog emulation in KVM. The watchdog emulation

> > is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.

> > The kernel timer are used for watchdog emulation and emulates h/w

> > watchdog state machine. On watchdog timer expiry, it exit to QEMU if

> > TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how it

> > is configured.

> >

> > Signed-off-by: Liu Yu <yu.liu@freescale.com>

> > Signed-off-by: Scott Wood <scottwood@freescale.com>

> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>

> > [bharat.bhushan@freescale.com: reworked patch]

> 

> Typically the [] note goes immediately before your signoff (but after the

> others).


ok

> 

> > +static void arm_next_watchdog(struct kvm_vcpu *vcpu) {

> > +	unsigned long nr_jiffies;

> > +

> > +	spin_lock(&vcpu->arch.wdt_lock);

> > +	nr_jiffies = watchdog_next_timeout(vcpu);

> > +	/*

> > +	 * If the number of jiffies of watchdog timer >= NEXT_TIMER_MAX_DELTA

> > +	 * then do not run the watchdog timer as this can break timer APIs.

> > +	 */

> > +	if (nr_jiffies < NEXT_TIMER_MAX_DELTA)

> > +		mod_timer(&vcpu->arch.wdt_timer, jiffies + nr_jiffies);

> > +	else

> > +		del_timer(&vcpu->arch.wdt_timer);

> > +	spin_unlock(&vcpu->arch.wdt_lock);

> > +}

> 

> This needs to be an irqsave lock.


Ok, I want to understood why irqsave lock should be used here?
irqsave_lock is used when the critical section within lock can be referenced from interrupt context also. What part of critical section above can get affected from local irq? Is it jiffies here?


> 

> > @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)

> > #ifdef CONFIG_KVM_EXIT_TIMING

> >  	mutex_init(&vcpu->arch.exit_timing_lock);

> >  #endif

> > -

> > +#ifdef CONFIG_BOOKE

> > +	spin_lock_init(&vcpu->arch.wdt_lock);

> > +	/* setup watchdog timer once */

> > +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,

> > +		    (unsigned long)vcpu);

> > +#endif

> >  	return 0;

> >  }

> 

> Can you do this in kvmppc_booke_init()?


Ok, so the *_uninit part of code also needed to be moved in respective function.

> 

> >

> >  void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)  {

> >  	kvmppc_mmu_destroy(vcpu);

> > +#ifdef CONFIG_BOOKE

> > +	spin_lock(&vcpu->arch.wdt_lock);

> > +	del_timer(&vcpu->arch.wdt_timer);

> > +	spin_unlock(&vcpu->arch.wdt_lock);

> > +#endif

> >  }

> 

> Don't acquire the lock here, but use del_timer_sync().


Ok.

Thanks
-Bharat

> 

> -Scott
Bharat Bhushan July 23, 2012, 4:10 a.m. UTC | #3
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogU2F0dXJkYXksIEp1bHkgMjEsIDIwMTIgMjo1OSBBTQ0KPiBUbzogQmh1c2hh
biBCaGFyYXQtUjY1Nzc3DQo+IENjOiBrdm0tcHBjQHZnZXIua2VybmVsLm9yZzsga3ZtQHZnZXIu
a2VybmVsLm9yZzsgYWdyYWZAc3VzZS5kZTsgQmh1c2hhbiBCaGFyYXQtDQo+IFI2NTc3Nw0KPiBT
dWJqZWN0OiBSZTogW1BBVENIIDIvMiB2NV0gS1ZNOiBQUEM6IGJvb2tlOiBBZGQgd2F0Y2hkb2cg
ZW11bGF0aW9uDQo+IA0KPiBPbiAwNy8yMC8yMDEyIDEyOjAwIEFNLCBCaGFyYXQgQmh1c2hhbiB3
cm90ZToNCj4gPiBUaGlzIHBhdGNoIGFkZHMgdGhlIHdhdGNoZG9nIGVtdWxhdGlvbiBpbiBLVk0u
IFRoZSB3YXRjaGRvZyBlbXVsYXRpb24NCj4gPiBpcyBlbmFibGVkIGJ5IEtWTV9FTkFCTEVfQ0FQ
KEtWTV9DQVBfUFBDX1dEVCkgaW9jdGwuDQo+ID4gVGhlIGtlcm5lbCB0aW1lciBhcmUgdXNlZCBm
b3Igd2F0Y2hkb2cgZW11bGF0aW9uIGFuZCBlbXVsYXRlcyBoL3cNCj4gPiB3YXRjaGRvZyBzdGF0
ZSBtYWNoaW5lLiBPbiB3YXRjaGRvZyB0aW1lciBleHBpcnksIGl0IGV4aXQgdG8gUUVNVSBpZg0K
PiA+IFRDUi5XUkMgaXMgbm9uIFpFUk8uIFFFTVUgY2FuIHJlc2V0L3NodXRkb3duIGV0YyBkZXBl
bmRpbmcgdXBvbiBob3cgaXQNCj4gPiBpcyBjb25maWd1cmVkLg0KPiA+DQo+ID4gU2lnbmVkLW9m
Zi1ieTogTGl1IFl1IDx5dS5saXVAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBT
Y290dCBXb29kIDxzY290dHdvb2RAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBC
aGFyYXQgQmh1c2hhbiA8YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbT4NCj4gPiBbYmhhcmF0
LmJodXNoYW5AZnJlZXNjYWxlLmNvbTogcmV3b3JrZWQgcGF0Y2hdDQo+IA0KPiBUeXBpY2FsbHkg
dGhlIFtdIG5vdGUgZ29lcyBpbW1lZGlhdGVseSBiZWZvcmUgeW91ciBzaWdub2ZmIChidXQgYWZ0
ZXIgdGhlDQo+IG90aGVycykuDQo+IA0KPiA+ICtzdGF0aWMgdm9pZCBhcm1fbmV4dF93YXRjaGRv
ZyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpIHsNCj4gPiArCXVuc2lnbmVkIGxvbmcgbnJfamlmZmll
czsNCj4gPiArDQo+ID4gKwlzcGluX2xvY2soJnZjcHUtPmFyY2gud2R0X2xvY2spOw0KPiA+ICsJ
bnJfamlmZmllcyA9IHdhdGNoZG9nX25leHRfdGltZW91dCh2Y3B1KTsNCj4gPiArCS8qDQo+ID4g
KwkgKiBJZiB0aGUgbnVtYmVyIG9mIGppZmZpZXMgb2Ygd2F0Y2hkb2cgdGltZXIgPj0gTkVYVF9U
SU1FUl9NQVhfREVMVEENCj4gPiArCSAqIHRoZW4gZG8gbm90IHJ1biB0aGUgd2F0Y2hkb2cgdGlt
ZXIgYXMgdGhpcyBjYW4gYnJlYWsgdGltZXIgQVBJcy4NCj4gPiArCSAqLw0KPiA+ICsJaWYgKG5y
X2ppZmZpZXMgPCBORVhUX1RJTUVSX01BWF9ERUxUQSkNCj4gPiArCQltb2RfdGltZXIoJnZjcHUt
PmFyY2gud2R0X3RpbWVyLCBqaWZmaWVzICsgbnJfamlmZmllcyk7DQo+ID4gKwllbHNlDQo+ID4g
KwkJZGVsX3RpbWVyKCZ2Y3B1LT5hcmNoLndkdF90aW1lcik7DQo+ID4gKwlzcGluX3VubG9jaygm
dmNwdS0+YXJjaC53ZHRfbG9jayk7DQo+ID4gK30NCj4gDQo+IFRoaXMgbmVlZHMgdG8gYmUgYW4g
aXJxc2F2ZSBsb2NrLg0KPiANCj4gPiBAQCAtMzg2LDEzICszODcsMjMgQEAgaW50IGt2bV9hcmNo
X3ZjcHVfaW5pdChzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpDQo+ID4gI2lmZGVmIENPTkZJR19LVk1f
RVhJVF9USU1JTkcNCj4gPiAgCW11dGV4X2luaXQoJnZjcHUtPmFyY2guZXhpdF90aW1pbmdfbG9j
ayk7DQo+ID4gICNlbmRpZg0KPiA+IC0NCj4gPiArI2lmZGVmIENPTkZJR19CT09LRQ0KPiA+ICsJ
c3Bpbl9sb2NrX2luaXQoJnZjcHUtPmFyY2gud2R0X2xvY2spOw0KPiA+ICsJLyogc2V0dXAgd2F0
Y2hkb2cgdGltZXIgb25jZSAqLw0KPiA+ICsJc2V0dXBfdGltZXIoJnZjcHUtPmFyY2gud2R0X3Rp
bWVyLCBrdm1wcGNfd2F0Y2hkb2dfZnVuYywNCj4gPiArCQkgICAgKHVuc2lnbmVkIGxvbmcpdmNw
dSk7DQo+ID4gKyNlbmRpZg0KPiA+ICAJcmV0dXJuIDA7DQo+ID4gIH0NCj4gDQo+IENhbiB5b3Ug
ZG8gdGhpcyBpbiBrdm1wcGNfYm9va2VfaW5pdCgpPw0KDQpJIGRvIG5vdCB0aGluayB3ZSBjYW4g
ZG8gdGhpcyBpbiBrdm1wcGNfYm9va2VfaW5pdCgpLiBXYXRjaGRvZyBoYXZlIGFzc29jaWF0aW9u
IHdpdGggdmNwdSwgd2hpbGUgdGhlcmUgaXMgbm8gdmNwdSBhdCBrdm1wcGNfYm9va2VfaW5pdCgp
Lg0KDQpUaGFua3MNCi1CaGFyYXQNCg0KPiANCj4gPg0KPiA+ICB2b2lkIGt2bV9hcmNoX3ZjcHVf
dW5pbml0KHN0cnVjdCBrdm1fdmNwdSAqdmNwdSkgIHsNCj4gPiAgCWt2bXBwY19tbXVfZGVzdHJv
eSh2Y3B1KTsNCj4gPiArI2lmZGVmIENPTkZJR19CT09LRQ0KPiA+ICsJc3Bpbl9sb2NrKCZ2Y3B1
LT5hcmNoLndkdF9sb2NrKTsNCj4gPiArCWRlbF90aW1lcigmdmNwdS0+YXJjaC53ZHRfdGltZXIp
Ow0KPiA+ICsJc3Bpbl91bmxvY2soJnZjcHUtPmFyY2gud2R0X2xvY2spOw0KPiA+ICsjZW5kaWYN
Cj4gPiAgfQ0KPiANCj4gRG9uJ3QgYWNxdWlyZSB0aGUgbG9jayBoZXJlLCBidXQgdXNlIGRlbF90
aW1lcl9zeW5jKCkuDQo+IA0KPiAtU2NvdHQNCg0K

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 23, 2012, 3:32 p.m. UTC | #4
On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:
>>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>>> #ifdef CONFIG_KVM_EXIT_TIMING
>>>  	mutex_init(&vcpu->arch.exit_timing_lock);
>>>  #endif
>>> -
>>> +#ifdef CONFIG_BOOKE
>>> +	spin_lock_init(&vcpu->arch.wdt_lock);
>>> +	/* setup watchdog timer once */
>>> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
>>> +		    (unsigned long)vcpu);
>>> +#endif
>>>  	return 0;
>>>  }
>>
>> Can you do this in kvmppc_booke_init()?
> 
> I do not think we can do this in kvmppc_booke_init(). Watchdog have
> association with vcpu, while there is no vcpu at
> kvmppc_booke_init().

Sorry, I meant kvm_arch_vcpu_setup() in booke.c.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 23, 2012, 3:35 p.m. UTC | #5
On 07/21/2012 03:37 AM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Saturday, July 21, 2012 2:59 AM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat-
>> R65777
>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/20/2012 12:00 AM, Bharat Bhushan wrote:
>>> +static void arm_next_watchdog(struct kvm_vcpu *vcpu) {
>>> +	unsigned long nr_jiffies;
>>> +
>>> +	spin_lock(&vcpu->arch.wdt_lock);
>>> +	nr_jiffies = watchdog_next_timeout(vcpu);
>>> +	/*
>>> +	 * If the number of jiffies of watchdog timer >= NEXT_TIMER_MAX_DELTA
>>> +	 * then do not run the watchdog timer as this can break timer APIs.
>>> +	 */
>>> +	if (nr_jiffies < NEXT_TIMER_MAX_DELTA)
>>> +		mod_timer(&vcpu->arch.wdt_timer, jiffies + nr_jiffies);
>>> +	else
>>> +		del_timer(&vcpu->arch.wdt_timer);
>>> +	spin_unlock(&vcpu->arch.wdt_lock);
>>> +}
>>
>> This needs to be an irqsave lock.
> 
> Ok, I want to understood why irqsave lock should be used here?
>
> irqsave_lock is used when the critical section within lock can be
> referenced from interrupt context also. What part of critical section
> above can get affected from local irq? Is it jiffies here?

This can be called from the watchdog timer.  Timers run in interrupt
context.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan July 23, 2012, 3:43 p.m. UTC | #6
> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Monday, July 23, 2012 9:02 PM

> To: Bhushan Bharat-R65777

> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;

> agraf@suse.de

> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation

> 

> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:

> >>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)

> >>> #ifdef CONFIG_KVM_EXIT_TIMING

> >>>  	mutex_init(&vcpu->arch.exit_timing_lock);

> >>>  #endif

> >>> -

> >>> +#ifdef CONFIG_BOOKE

> >>> +	spin_lock_init(&vcpu->arch.wdt_lock);

> >>> +	/* setup watchdog timer once */

> >>> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,

> >>> +		    (unsigned long)vcpu);

> >>> +#endif

> >>>  	return 0;

> >>>  }

> >>

> >> Can you do this in kvmppc_booke_init()?

> >

> > I do not think we can do this in kvmppc_booke_init(). Watchdog have

> > association with vcpu, while there is no vcpu at kvmppc_booke_init().

> 

> Sorry, I meant kvm_arch_vcpu_setup() in booke.c.


Any specific reason to move this in above mentioned function? Want to avoid #ifdef config_booke ?


Thanks
-Bharat
Scott Wood July 23, 2012, 4 p.m. UTC | #7
On 07/23/2012 10:43 AM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Monday, July 23, 2012 9:02 PM
>> To: Bhushan Bharat-R65777
>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>> agraf@suse.de
>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:
>>>>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>>>>> #ifdef CONFIG_KVM_EXIT_TIMING
>>>>>  	mutex_init(&vcpu->arch.exit_timing_lock);
>>>>>  #endif
>>>>> -
>>>>> +#ifdef CONFIG_BOOKE
>>>>> +	spin_lock_init(&vcpu->arch.wdt_lock);
>>>>> +	/* setup watchdog timer once */
>>>>> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
>>>>> +		    (unsigned long)vcpu);
>>>>> +#endif
>>>>>  	return 0;
>>>>>  }
>>>>
>>>> Can you do this in kvmppc_booke_init()?
>>>
>>> I do not think we can do this in kvmppc_booke_init(). Watchdog have
>>> association with vcpu, while there is no vcpu at kvmppc_booke_init().
>>
>> Sorry, I meant kvm_arch_vcpu_setup() in booke.c.
> 
> Any specific reason to move this in above mentioned function? Want to avoid #ifdef config_booke ?

Yes, to avoid the ifdef.  We already have a (poorly named) place for
booke-specific vcpu init.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan July 23, 2012, 4:04 p.m. UTC | #8
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogTW9uZGF5LCBKdWx5IDIzLCAyMDEyIDk6MzEgUE0NCj4gVG86IEJodXNoYW4g
QmhhcmF0LVI2NTc3Nw0KPiBDYzogV29vZCBTY290dC1CMDc0MjE7IGt2bS1wcGNAdmdlci5rZXJu
ZWwub3JnOyBrdm1Admdlci5rZXJuZWwub3JnOw0KPiBhZ3JhZkBzdXNlLmRlDQo+IFN1YmplY3Q6
IFJlOiBbUEFUQ0ggMi8yIHY1XSBLVk06IFBQQzogYm9va2U6IEFkZCB3YXRjaGRvZyBlbXVsYXRp
b24NCj4gDQo+IE9uIDA3LzIzLzIwMTIgMTA6NDMgQU0sIEJodXNoYW4gQmhhcmF0LVI2NTc3NyB3
cm90ZToNCj4gPg0KPiA+DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4+IEZy
b206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4+IFNlbnQ6IE1vbmRheSwgSnVseSAyMywgMjAxMiA5
OjAyIFBNDQo+ID4+IFRvOiBCaHVzaGFuIEJoYXJhdC1SNjU3NzcNCj4gPj4gQ2M6IFdvb2QgU2Nv
dHQtQjA3NDIxOyBrdm0tcHBjQHZnZXIua2VybmVsLm9yZzsga3ZtQHZnZXIua2VybmVsLm9yZzsN
Cj4gPj4gYWdyYWZAc3VzZS5kZQ0KPiA+PiBTdWJqZWN0OiBSZTogW1BBVENIIDIvMiB2NV0gS1ZN
OiBQUEM6IGJvb2tlOiBBZGQgd2F0Y2hkb2cgZW11bGF0aW9uDQo+ID4+DQo+ID4+IE9uIDA3LzIy
LzIwMTIgMTE6MTAgUE0sIEJodXNoYW4gQmhhcmF0LVI2NTc3NyB3cm90ZToNCj4gPj4+Pj4gQEAg
LTM4NiwxMyArMzg3LDIzIEBAIGludCBrdm1fYXJjaF92Y3B1X2luaXQoc3RydWN0IGt2bV92Y3B1
DQo+ID4+Pj4+ICp2Y3B1KSAjaWZkZWYgQ09ORklHX0tWTV9FWElUX1RJTUlORw0KPiA+Pj4+PiAg
CW11dGV4X2luaXQoJnZjcHUtPmFyY2guZXhpdF90aW1pbmdfbG9jayk7DQo+ID4+Pj4+ICAjZW5k
aWYNCj4gPj4+Pj4gLQ0KPiA+Pj4+PiArI2lmZGVmIENPTkZJR19CT09LRQ0KPiA+Pj4+PiArCXNw
aW5fbG9ja19pbml0KCZ2Y3B1LT5hcmNoLndkdF9sb2NrKTsNCj4gPj4+Pj4gKwkvKiBzZXR1cCB3
YXRjaGRvZyB0aW1lciBvbmNlICovDQo+ID4+Pj4+ICsJc2V0dXBfdGltZXIoJnZjcHUtPmFyY2gu
d2R0X3RpbWVyLCBrdm1wcGNfd2F0Y2hkb2dfZnVuYywNCj4gPj4+Pj4gKwkJICAgICh1bnNpZ25l
ZCBsb25nKXZjcHUpOw0KPiA+Pj4+PiArI2VuZGlmDQo+ID4+Pj4+ICAJcmV0dXJuIDA7DQo+ID4+
Pj4+ICB9DQo+ID4+Pj4NCj4gPj4+PiBDYW4geW91IGRvIHRoaXMgaW4ga3ZtcHBjX2Jvb2tlX2lu
aXQoKT8NCj4gPj4+DQo+ID4+PiBJIGRvIG5vdCB0aGluayB3ZSBjYW4gZG8gdGhpcyBpbiBrdm1w
cGNfYm9va2VfaW5pdCgpLiBXYXRjaGRvZyBoYXZlDQo+ID4+PiBhc3NvY2lhdGlvbiB3aXRoIHZj
cHUsIHdoaWxlIHRoZXJlIGlzIG5vIHZjcHUgYXQga3ZtcHBjX2Jvb2tlX2luaXQoKS4NCj4gPj4N
Cj4gPj4gU29ycnksIEkgbWVhbnQga3ZtX2FyY2hfdmNwdV9zZXR1cCgpIGluIGJvb2tlLmMuDQo+
ID4NCj4gPiBBbnkgc3BlY2lmaWMgcmVhc29uIHRvIG1vdmUgdGhpcyBpbiBhYm92ZSBtZW50aW9u
ZWQgZnVuY3Rpb24/IFdhbnQgdG8gYXZvaWQNCj4gI2lmZGVmIGNvbmZpZ19ib29rZSA/DQo+IA0K
PiBZZXMsIHRvIGF2b2lkIHRoZSBpZmRlZi4gIFdlIGFscmVhZHkgaGF2ZSBhIChwb29ybHkgbmFt
ZWQpIHBsYWNlIGZvciBib29rZS0NCj4gc3BlY2lmaWMgdmNwdSBpbml0Lg0KDQpXaGVyZSB3ZSB3
YW50IHRvIGRlbGV0ZSB3YXRjaGRvZyB0aW1lcj8gDQoNCkt2bV9hcmNoX3ZjcHVfc2V0dXAoKSBp
cyBpbiBmbG93IG9mIENSRUFURV9WQ1BVIGlvY3RsLCBJIGRvIG5vdCBzZWUgREVTVFJPWV9WQ1BV
IHR5cGUgb2YgY29kZT8NCg0KU28gaXMgaXQgb2sgdG8gbGV0IGRlbF90aW1lcl9zeW5jKCkgYXMg
aXMgd2l0aCAjaWZkZWYgY29uZmlnX2Jvb2tlID8NCg0KVGhhbmtzDQotQmhhcmF0DQoNCg0K

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 23, 2012, 4:30 p.m. UTC | #9
On 07/23/2012 11:04 AM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Monday, July 23, 2012 9:31 PM
>> To: Bhushan Bharat-R65777
>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>> agraf@suse.de
>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/23/2012 10:43 AM, Bhushan Bharat-R65777 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Monday, July 23, 2012 9:02 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>>>> agraf@suse.de
>>>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>>>
>>>> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:
>>>>>>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu
>>>>>>> *vcpu) #ifdef CONFIG_KVM_EXIT_TIMING
>>>>>>>  	mutex_init(&vcpu->arch.exit_timing_lock);
>>>>>>>  #endif
>>>>>>> -
>>>>>>> +#ifdef CONFIG_BOOKE
>>>>>>> +	spin_lock_init(&vcpu->arch.wdt_lock);
>>>>>>> +	/* setup watchdog timer once */
>>>>>>> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
>>>>>>> +		    (unsigned long)vcpu);
>>>>>>> +#endif
>>>>>>>  	return 0;
>>>>>>>  }
>>>>>>
>>>>>> Can you do this in kvmppc_booke_init()?
>>>>>
>>>>> I do not think we can do this in kvmppc_booke_init(). Watchdog have
>>>>> association with vcpu, while there is no vcpu at kvmppc_booke_init().
>>>>
>>>> Sorry, I meant kvm_arch_vcpu_setup() in booke.c.
>>>
>>> Any specific reason to move this in above mentioned function? Want to avoid
>> #ifdef config_booke ?
>>
>> Yes, to avoid the ifdef.  We already have a (poorly named) place for booke-
>> specific vcpu init.
> 
> Where we want to delete watchdog timer? 
> 
> Kvm_arch_vcpu_setup() is in flow of CREATE_VCPU ioctl, I do not see DESTROY_VCPU type of code?
> 
> So is it ok to let del_timer_sync() as is with #ifdef config_booke ?

You could add such a function.  I suggest correcting the naming issue at
the same time -- have kvm_subarch_vcpu_init() and kvm_subarch_vcpu_free().

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan July 24, 2012, 7:45 a.m. UTC | #10
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogTW9uZGF5LCBKdWx5IDIzLCAyMDEyIDEwOjAwIFBNDQo+IFRvOiBCaHVzaGFu
IEJoYXJhdC1SNjU3NzcNCj4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBrdm0tcHBjQHZnZXIua2Vy
bmVsLm9yZzsga3ZtQHZnZXIua2VybmVsLm9yZzsNCj4gYWdyYWZAc3VzZS5kZQ0KPiBTdWJqZWN0
OiBSZTogW1BBVENIIDIvMiB2NV0gS1ZNOiBQUEM6IGJvb2tlOiBBZGQgd2F0Y2hkb2cgZW11bGF0
aW9uDQo+IA0KPiBPbiAwNy8yMy8yMDEyIDExOjA0IEFNLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcg
d3JvdGU6DQo+ID4NCj4gPg0KPiA+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+PiBG
cm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+PiBTZW50OiBNb25kYXksIEp1bHkgMjMsIDIwMTIg
OTozMSBQTQ0KPiA+PiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4+IENjOiBXb29kIFNj
b3R0LUIwNzQyMTsga3ZtLXBwY0B2Z2VyLmtlcm5lbC5vcmc7IGt2bUB2Z2VyLmtlcm5lbC5vcmc7
DQo+ID4+IGFncmFmQHN1c2UuZGUNCj4gPj4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzIgdjVdIEtW
TTogUFBDOiBib29rZTogQWRkIHdhdGNoZG9nIGVtdWxhdGlvbg0KPiA+Pg0KPiA+PiBPbiAwNy8y
My8yMDEyIDEwOjQzIEFNLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4+Pg0KPiA+
Pj4NCj4gPj4+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+Pj4+IEZyb206IFdvb2Qg
U2NvdHQtQjA3NDIxDQo+ID4+Pj4gU2VudDogTW9uZGF5LCBKdWx5IDIzLCAyMDEyIDk6MDIgUE0N
Cj4gPj4+PiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4+Pj4gQ2M6IFdvb2QgU2NvdHQt
QjA3NDIxOyBrdm0tcHBjQHZnZXIua2VybmVsLm9yZzsNCj4gPj4+PiBrdm1Admdlci5rZXJuZWwu
b3JnOyBhZ3JhZkBzdXNlLmRlDQo+ID4+Pj4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzIgdjVdIEtW
TTogUFBDOiBib29rZTogQWRkIHdhdGNoZG9nIGVtdWxhdGlvbg0KPiA+Pj4+DQo+ID4+Pj4gT24g
MDcvMjIvMjAxMiAxMToxMCBQTSwgQmh1c2hhbiBCaGFyYXQtUjY1Nzc3IHdyb3RlOg0KPiA+Pj4+
Pj4+IEBAIC0zODYsMTMgKzM4NywyMyBAQCBpbnQga3ZtX2FyY2hfdmNwdV9pbml0KHN0cnVjdCBr
dm1fdmNwdQ0KPiA+Pj4+Pj4+ICp2Y3B1KSAjaWZkZWYgQ09ORklHX0tWTV9FWElUX1RJTUlORw0K
PiA+Pj4+Pj4+ICAJbXV0ZXhfaW5pdCgmdmNwdS0+YXJjaC5leGl0X3RpbWluZ19sb2NrKTsNCj4g
Pj4+Pj4+PiAgI2VuZGlmDQo+ID4+Pj4+Pj4gLQ0KPiA+Pj4+Pj4+ICsjaWZkZWYgQ09ORklHX0JP
T0tFDQo+ID4+Pj4+Pj4gKwlzcGluX2xvY2tfaW5pdCgmdmNwdS0+YXJjaC53ZHRfbG9jayk7DQo+
ID4+Pj4+Pj4gKwkvKiBzZXR1cCB3YXRjaGRvZyB0aW1lciBvbmNlICovDQo+ID4+Pj4+Pj4gKwlz
ZXR1cF90aW1lcigmdmNwdS0+YXJjaC53ZHRfdGltZXIsIGt2bXBwY193YXRjaGRvZ19mdW5jLA0K
PiA+Pj4+Pj4+ICsJCSAgICAodW5zaWduZWQgbG9uZyl2Y3B1KTsNCj4gPj4+Pj4+PiArI2VuZGlm
DQo+ID4+Pj4+Pj4gIAlyZXR1cm4gMDsNCj4gPj4+Pj4+PiAgfQ0KPiA+Pj4+Pj4NCj4gPj4+Pj4+
IENhbiB5b3UgZG8gdGhpcyBpbiBrdm1wcGNfYm9va2VfaW5pdCgpPw0KPiA+Pj4+Pg0KPiA+Pj4+
PiBJIGRvIG5vdCB0aGluayB3ZSBjYW4gZG8gdGhpcyBpbiBrdm1wcGNfYm9va2VfaW5pdCgpLiBX
YXRjaGRvZw0KPiA+Pj4+PiBoYXZlIGFzc29jaWF0aW9uIHdpdGggdmNwdSwgd2hpbGUgdGhlcmUg
aXMgbm8gdmNwdSBhdCBrdm1wcGNfYm9va2VfaW5pdCgpLg0KPiA+Pj4+DQo+ID4+Pj4gU29ycnks
IEkgbWVhbnQga3ZtX2FyY2hfdmNwdV9zZXR1cCgpIGluIGJvb2tlLmMuDQo+ID4+Pg0KPiA+Pj4g
QW55IHNwZWNpZmljIHJlYXNvbiB0byBtb3ZlIHRoaXMgaW4gYWJvdmUgbWVudGlvbmVkIGZ1bmN0
aW9uPyBXYW50DQo+ID4+PiB0byBhdm9pZA0KPiA+PiAjaWZkZWYgY29uZmlnX2Jvb2tlID8NCj4g
Pj4NCj4gPj4gWWVzLCB0byBhdm9pZCB0aGUgaWZkZWYuICBXZSBhbHJlYWR5IGhhdmUgYSAocG9v
cmx5IG5hbWVkKSBwbGFjZSBmb3INCj4gPj4gYm9va2UtIHNwZWNpZmljIHZjcHUgaW5pdC4NCj4g
Pg0KPiA+IFdoZXJlIHdlIHdhbnQgdG8gZGVsZXRlIHdhdGNoZG9nIHRpbWVyPw0KPiA+DQo+ID4g
S3ZtX2FyY2hfdmNwdV9zZXR1cCgpIGlzIGluIGZsb3cgb2YgQ1JFQVRFX1ZDUFUgaW9jdGwsIEkg
ZG8gbm90IHNlZQ0KPiBERVNUUk9ZX1ZDUFUgdHlwZSBvZiBjb2RlPw0KPiA+DQo+ID4gU28gaXMg
aXQgb2sgdG8gbGV0IGRlbF90aW1lcl9zeW5jKCkgYXMgaXMgd2l0aCAjaWZkZWYgY29uZmlnX2Jv
b2tlID8NCj4gDQo+IFlvdSBjb3VsZCBhZGQgc3VjaCBhIGZ1bmN0aW9uLiAgSSBzdWdnZXN0IGNv
cnJlY3RpbmcgdGhlIG5hbWluZyBpc3N1ZSBhdCB0aGUNCj4gc2FtZSB0aW1lIC0tIGhhdmUga3Zt
X3N1YmFyY2hfdmNwdV9pbml0KCkgYW5kIGt2bV9zdWJhcmNoX3ZjcHVfZnJlZSgpLg0KDQpJIGFt
IHNvcnJ5IFNjb3R0IGJ1dCBJIGRpZCBub3QgZ2V0IHdoaWNoIGZ1bmN0aW9ucyB5b3Ugd2FudCB0
byByZW5hbWUuIFdoYXQgSSB1bmRlcnN0b29kIGZyb20gY3VycmVudCBjb2RlIGZvciBwb3dlcnBj
IGlzOg0KDQprdm1fdm1faW9jdGxfY3JlYXRlX3ZjcHUoKQ0KIHwNCiB8LT4ga3ZtX2FyY2hfdmNw
dV9jcmVhdGUoKQ0KIHwtPiBrdm1fYXJjaF92Y3B1X3NldHVwKCkgLSBUaGlzIGlzIHdoZXJlIHlv
dSB3YW50IG1lIHRvIGNhbGwgd2F0Y2hkb2cgdGltZXIgc2V0dXAuIFdlIGNhbm5vdCAocmVxdWly
ZSBtb3JlIGVmZm9ydHMpIHJlbmFtZSB0aGlzIGFzIHRoaXMgaXMgY2FsbGVkIGZyb20gdmlydC9r
dm0va3ZtX21haW4uYy4gU2F5IGlmIHdlIGFkZCwgbm93IGxldCB1cyBmaWd1cmUgb3V0IHdoZXJl
IHRvIHVuaW5pdCB3aGF0IGlzIGRvbmUgaW4gdGhpcyBmdW5jdGlvbjoNCg0KDQoxKQ0Ka3ZtX2Fy
Y2hfdmNwdV9kZXN0cm95KCkNCiB8DQogfC0+IGt2bV9hcmNoX3ZjcHVfZnJlZSgpIC0+IHRoaXMg
dW5kbyB3aGF0IGlzIGRvbmUgYnkgMSkga3ZtX2FyY2hfdmNwdV9pbml0KCkgMikga3ZtX2FyY2hf
dmNwdV9jcmVhdGUoKS4NCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBUaGlzIGFsc28g
ZG9lcyBub3QgdW5kbyB3aGF0IGlzIGRvbmUgaW4ga3ZtX2FyY2hfdmNwdV9zZXR1cCgpDQoNCiAg
QWxzbyBrdm1fYXJjaF92Y3B1X2Rlc3Ryb3koKSBpcyBjYWxsZWQgYXMgZXJyb3IgZmFsbGJhY2sg
b2Yga3ZtX3ZtX2lvY3RsX2NyZWF0ZV92Y3B1KCksIHNvIHRoaXMgaXMgbm90IG5vcm1hbCBjbGVh
dXAgZnVuY3Rpb24gY2FsbC4NCg0KDQoyKQ0KVGhlIG90aGVyIGZ1bmN0aW9uIHRvIHVuZG8gaXMg
a3ZtX2FyY2hfZGVzdHJveV92bSgpICh3aGljaCBpcyBjYWxsZWQgZnJvbSBrdm1fdm1fcmVsZWFz
ZSgpLCBrdm1fdmNwdV9yZWxlYXNlKCkgYW5kIGVycm9yIGZhbGxiYWNrIGluIGt2bV9kZXZfaW9j
dGxfY3JlYXRlX3ZtKCkva3ZtX3ZtX2lvY3RsX2NyZWF0ZV92Y3B1KCkuIFNvIHRoaXMgZnVuY3Rp
b24gaXMgbm9ybWFsIGNsZWFudXAgZnVuY3Rpb24gYW5kIHdoYXQgaXQgZG9lcyBpczoNCg0Ka3Zt
X2FyY2hfZGVzdHJveV92bSgpDQogfA0KIHwtPiBrdm1fZm9yX2VhY2hfdmNwdSgpDQogfCAgfA0K
IHwgIHwtPmt2bV9hcmNoX3ZjcHVfZnJlZSgpIC0+IHRoaXMgdW5kbyB3aGF0IGlzIGRvbmUgYnkg
MSkga3ZtX2FyY2hfdmNwdV9pbml0KCkgMikga3ZtX2FyY2hfdmNwdV9jcmVhdGUoKS4NCiB8ICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgVGhpcyBhbHNvIGRvZXMgbm90IHVuZG8gd2hhdCBp
cyBkb25lIGluIGt2bV9hcmNoX3ZjcHVfc2V0dXAoKQ0KIHwNCiB8LT4ga3ZtcHBjX2NvcmVfZGVz
dHJveV92bSgpIC0+IHRoaXMgdW5kbyB3aGF0IHdhcyBkb25lIGJ5IGt2bV9hcmNoX2luaXRfdm0o
KSBpbiBmbG93IG9mIGt2bV9jcmVhdGVfdm0oKQ0KDQpTbyB3aGF0IEkgdW5kZXJzdG9vZCBmcm9t
IHRoaXMgaXM6DQoNCjEpIE1vdmUgdGhlIGt2bV92Y3B1X2FyY2hfc2V0dXAoKSBpbnRvIHBvd2Vy
cGMuYw0KDQoyKSBkZWZpbmUga3ZtX3N1YmFyY2hfdmNwdV9pbml0KCkgaW4gKGJvb2tlL2Jvb2sz
cykgdG8gZG8gc3ViYXJjaCBzcGVjaWZpYyB3b3JrIGFuZCBjYWxsIHRoaXMgZnJvbSBrdm1fdmNw
dV9hcmNoX3NldHVwKCkNCg0KMykgZGVmaW5lIGt2bV9zdWJhcmNoX3ZjcHVfZnJlZSgpIHRvIGJl
IGNhbGxlZCBmcm9tIGt2bV9hcmNoX3ZjcHVfZnJlZSgpLiBUaGlzIHdpbGwgdW5kbyB3aGF0IGlz
IGRvbmUgYnkga3ZtX3N1YmFyY2hfdmNwdV9pbml0KCkuDQoNCjQpIEFsc28ga3ZtX2FyY2hfdmNw
dV9mcmVlKCkgc2hvdWxkIGRvIHdoYXQgaXMgZG9uZSBieSBrdm1fdmNwdV9zcmNoX3NldHVwKCkg
aW4gcG93ZXJwYy5jDQoNClRoYW5rcw0KLUJoYXJhdA0KDQo+IA0KPiAtU2NvdHQNCg0K

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 24, 2012, 10:32 p.m. UTC | #11
On 07/24/2012 02:45 AM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Monday, July 23, 2012 10:00 PM
>> To: Bhushan Bharat-R65777
>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>> agraf@suse.de
>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/23/2012 11:04 AM, Bhushan Bharat-R65777 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Monday, July 23, 2012 9:31 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>>>> agraf@suse.de
>>>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>>>
>>>> On 07/23/2012 10:43 AM, Bhushan Bharat-R65777 wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Wood Scott-B07421
>>>>>> Sent: Monday, July 23, 2012 9:02 PM
>>>>>> To: Bhushan Bharat-R65777
>>>>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org;
>>>>>> kvm@vger.kernel.org; agraf@suse.de
>>>>>> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>>>>>>
>>>>>> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:
>>>>>>>>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu
>>>>>>>>> *vcpu) #ifdef CONFIG_KVM_EXIT_TIMING
>>>>>>>>>  	mutex_init(&vcpu->arch.exit_timing_lock);
>>>>>>>>>  #endif
>>>>>>>>> -
>>>>>>>>> +#ifdef CONFIG_BOOKE
>>>>>>>>> +	spin_lock_init(&vcpu->arch.wdt_lock);
>>>>>>>>> +	/* setup watchdog timer once */
>>>>>>>>> +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
>>>>>>>>> +		    (unsigned long)vcpu);
>>>>>>>>> +#endif
>>>>>>>>>  	return 0;
>>>>>>>>>  }
>>>>>>>>
>>>>>>>> Can you do this in kvmppc_booke_init()?
>>>>>>>
>>>>>>> I do not think we can do this in kvmppc_booke_init(). Watchdog
>>>>>>> have association with vcpu, while there is no vcpu at kvmppc_booke_init().
>>>>>>
>>>>>> Sorry, I meant kvm_arch_vcpu_setup() in booke.c.
>>>>>
>>>>> Any specific reason to move this in above mentioned function? Want
>>>>> to avoid
>>>> #ifdef config_booke ?
>>>>
>>>> Yes, to avoid the ifdef.  We already have a (poorly named) place for
>>>> booke- specific vcpu init.
>>>
>>> Where we want to delete watchdog timer?
>>>
>>> Kvm_arch_vcpu_setup() is in flow of CREATE_VCPU ioctl, I do not see
>> DESTROY_VCPU type of code?
>>>
>>> So is it ok to let del_timer_sync() as is with #ifdef config_booke ?
>>
>> You could add such a function.  I suggest correcting the naming issue at the
>> same time -- have kvm_subarch_vcpu_init() and kvm_subarch_vcpu_free().
> 
> I am sorry Scott but I did not get which functions you want to
> rename. What I understood from current code for powerpc is:
> 
> kvm_vm_ioctl_create_vcpu()
>  |
>  |-> kvm_arch_vcpu_create()
> |-> kvm_arch_vcpu_setup() - This is where you want me to call
> watchdog timer setup. We cannot (require more efforts) rename this as
> this is called from virt/kvm/kvm_main.c. Say if we add, now let us
> figure out where to uninit what is done in this function:

Sigh, I didn't realize this came from generic code.  What, from generic
code's perspective, is the difference between kvm_arch_vcpu_init() and
kvm_arch_vcpu_setup() supposed to be?  They're synonyms!  And as always
in Linux, there's no documentation (that I can find) on what's expected
of functions that have multiple implementations.

Inferring from what the ppc code does, apparently kvm_arch_vcpu_create()
is supposed to call kvm_vcpu_init(), which then calls back into arch
code with kvm_arch_vcpu_init().  Then, after the preempt notifier is
initialized (but not actually activated), generic code goes back to arch
code with kvm_arch_vcpu_setup().  So basically the "setup" version comes
later.  And PPC KVM arbitrarily decides to use one of these for PPC-wide
init, and the other for subarch init.  This could really use some
untangling (or documentation if there really is a method to the madness).

In any case, we can still put this init code in kvmppc_arch_vcpu_setup()
(i.e. the subarch init), and add a kvmppc_subarch_vcpu_uninit() that we
call from kvm_arch_vcpu_uninit().  Or just move kvm_arch_vcpu_uninit()
into subarch code -- the only thing it does is call kvmppc_mmu_destroy,
which is a subarch hook anyway.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti July 25, 2012, 8:37 p.m. UTC | #12
On Fri, Jul 20, 2012 at 04:29:08PM -0500, Scott Wood wrote:
> On 07/20/2012 12:00 AM, Bharat Bhushan wrote:
> > This patch adds the watchdog emulation in KVM. The watchdog
> > emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.
> > The kernel timer are used for watchdog emulation and emulates
> > h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
> > if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
> > it is configured.

Why can't you do this in QEMU, given the kernel exits to userspace on
timer configuration?

The QEMU timer handler used to emulate the watchdog then would have to
only read the register value from the vCPU.

> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > [bharat.bhushan@freescale.com: reworked patch]
> 
> Typically the [] note goes immediately before your signoff (but after
> the others).
> 
> > +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
> > +{
> > +	unsigned long nr_jiffies;
> > +
> > +	spin_lock(&vcpu->arch.wdt_lock);
> > +	nr_jiffies = watchdog_next_timeout(vcpu);
> > +	/*
> > +	 * If the number of jiffies of watchdog timer >= NEXT_TIMER_MAX_DELTA
> > +	 * then do not run the watchdog timer as this can break timer APIs.
> > +	 */
> > +	if (nr_jiffies < NEXT_TIMER_MAX_DELTA)
> > +		mod_timer(&vcpu->arch.wdt_timer, jiffies + nr_jiffies);
> > +	else
> > +		del_timer(&vcpu->arch.wdt_timer);
> > +	spin_unlock(&vcpu->arch.wdt_lock);
> > +}
> 
> This needs to be an irqsave lock.
> 
> > @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> >  #ifdef CONFIG_KVM_EXIT_TIMING
> >  	mutex_init(&vcpu->arch.exit_timing_lock);
> >  #endif
> > -
> > +#ifdef CONFIG_BOOKE
> > +	spin_lock_init(&vcpu->arch.wdt_lock);
> > +	/* setup watchdog timer once */
> > +	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
> > +		    (unsigned long)vcpu);
> > +#endif
> >  	return 0;
> >  }
> 
> Can you do this in kvmppc_booke_init()?
> 
> >  
> >  void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
> >  {
> >  	kvmppc_mmu_destroy(vcpu);
> > +#ifdef CONFIG_BOOKE
> > +	spin_lock(&vcpu->arch.wdt_lock);
> > +	del_timer(&vcpu->arch.wdt_timer);
> > +	spin_unlock(&vcpu->arch.wdt_lock);
> > +#endif
> >  }
> 
> Don't acquire the lock here, but use del_timer_sync().
> 
> -Scott
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 25, 2012, 11:55 p.m. UTC | #13
On 07/25/2012 03:37 PM, Marcelo Tosatti wrote:
> On Fri, Jul 20, 2012 at 04:29:08PM -0500, Scott Wood wrote:
>> On 07/20/2012 12:00 AM, Bharat Bhushan wrote:
>>> This patch adds the watchdog emulation in KVM. The watchdog
>>> emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.
>>> The kernel timer are used for watchdog emulation and emulates
>>> h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
>>> if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
>>> it is configured.
> 
> Why can't you do this in QEMU, given the kernel exits to userspace on
> timer configuration?
>
> The QEMU timer handler used to emulate the watchdog then would have to
> only read the register value from the vCPU.

Do what specifically in QEMU?  The whole watchdog mechanism?

It's only on final (third) expiry, when a reset is needed, that it exits
to QEMU.  The first expiry sets a bit in TSR that software can clear to
clear the watchdog.  If software doesn't clear it, the second expiry
sets a different bit in TSR that triggers a critical interrupt to the
guest.  If software doesn't clear either of those bits by the time it
expires again, it's reset time (if a reset action is enabled).

The TSR/TCR registers are shared with other timers that are implemented
in the kernel, and it would be more complicated to implement just the
watchdog in QEMU.  Plus, we currently have no interface for exiting to
QEMU for emulation of an SPR.

Currently QEMU on PPC leaves all timekeeping stuff to the kernel, and
I'd like to keep it that way.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti July 26, 2012, 12:46 a.m. UTC | #14
On Wed, Jul 25, 2012 at 06:55:16PM -0500, Scott Wood wrote:
> On 07/25/2012 03:37 PM, Marcelo Tosatti wrote:
> > On Fri, Jul 20, 2012 at 04:29:08PM -0500, Scott Wood wrote:
> >> On 07/20/2012 12:00 AM, Bharat Bhushan wrote:
> >>> This patch adds the watchdog emulation in KVM. The watchdog
> >>> emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_WDT) ioctl.
> >>> The kernel timer are used for watchdog emulation and emulates
> >>> h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
> >>> if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
> >>> it is configured.
> > 
> > Why can't you do this in QEMU, given the kernel exits to userspace on
> > timer configuration?
> >
> > The QEMU timer handler used to emulate the watchdog then would have to
> > only read the register value from the vCPU.
> 
> Do what specifically in QEMU?  The whole watchdog mechanism?
>
> It's only on final (third) expiry, when a reset is needed, that it exits
> to QEMU.  The first expiry sets a bit in TSR that software can clear to
> clear the watchdog.  If software doesn't clear it, the second expiry
> sets a different bit in TSR that triggers a critical interrupt to the
> guest.  If software doesn't clear either of those bits by the time it
> expires again, it's reset time (if a reset action is enabled).
> 
> The TSR/TCR registers are shared with other timers that are implemented
> in the kernel, and it would be more complicated to implement just the
> watchdog in QEMU.  Plus, we currently have no interface for exiting to
> QEMU for emulation of an SPR.
> 
> Currently QEMU on PPC leaves all timekeeping stuff to the kernel, and
> I'd like to keep it that way.
> 
> -Scott
> 

OK.

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

Patch

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 50ea12f..43cac48 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -467,6 +467,8 @@  struct kvm_vcpu_arch {
 	ulong fault_esr;
 	ulong queued_dear;
 	ulong queued_esr;
+	spinlock_t wdt_lock;
+	struct timer_list wdt_timer;
 	u32 tlbcfg[4];
 	u32 mmucfg;
 	u32 epr;
@@ -482,6 +484,7 @@  struct kvm_vcpu_arch {
 	u8 osi_needed;
 	u8 osi_enabled;
 	u8 papr_enabled;
+	u8 watchdog_enabled;
 	u8 sane;
 	u8 cpu_type;
 	u8 hcall_needed;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 0124937..e5cf4b9 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -67,6 +67,7 @@  extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu);
 extern void kvmppc_emulate_dec(struct kvm_vcpu *vcpu);
 extern u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb);
 extern void kvmppc_decrementer_func(unsigned long data);
+extern void kvmppc_watchdog_func(unsigned long data);
 extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu);
 
 /* Core-specific hooks */
@@ -104,6 +105,8 @@  extern void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
                                        struct kvm_interrupt *irq);
 extern void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
                                          struct kvm_interrupt *irq);
+extern void kvmppc_core_queue_watchdog(struct kvm_vcpu *vcpu);
+extern void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu);
 
 extern int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                                   unsigned int op, int *advance);
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 2d916c4..e07e6af 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -539,6 +539,13 @@ 
 #define TCR_FIE		0x00800000	/* FIT Interrupt Enable */
 #define TCR_ARE		0x00400000	/* Auto Reload Enable */
 
+#ifdef CONFIG_E500
+#define TCR_GET_WP(tcr)  ((((tcr) & 0xC0000000) >> 30) | \
+			      (((tcr) & 0x1E0000) >> 15))
+#else
+#define TCR_GET_WP(tcr)  (((tcr) & 0xC0000000) >> 30)
+#endif
+
 /* Bit definitions for the TSR. */
 #define TSR_ENW		0x80000000	/* Enable Next Watchdog */
 #define TSR_WIS		0x40000000	/* WDT Interrupt Status */
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index d25a097..71b4ce9 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -206,6 +206,16 @@  void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
 	clear_bit(BOOKE_IRQPRIO_EXTERNAL_LEVEL, &vcpu->arch.pending_exceptions);
 }
 
+void kvmppc_core_queue_watchdog(struct kvm_vcpu *vcpu)
+{
+	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_WATCHDOG);
+}
+
+void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
+{
+	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
+}
+
 static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0, u32 srr1)
 {
 #ifdef CONFIG_KVM_BOOKE_HV
@@ -325,6 +335,7 @@  static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 		msr_mask = MSR_CE | MSR_ME | MSR_DE;
 		int_class = INT_CLASS_NONCRIT;
 		break;
+	case BOOKE_IRQPRIO_WATCHDOG:
 	case BOOKE_IRQPRIO_CRITICAL:
 	case BOOKE_IRQPRIO_DBELL_CRIT:
 		allowed = vcpu->arch.shared->msr & MSR_CE;
@@ -404,12 +415,113 @@  static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 	return allowed;
 }
 
+/*
+ * Return the number of jiffies until the next timeout.  If the timeout is
+ * longer than the NEXT_TIMER_MAX_DELTA, then return NEXT_TIMER_MAX_DELTA
+ * because the larger value can break the timer APIs.
+ */
+static unsigned long watchdog_next_timeout(struct kvm_vcpu *vcpu)
+{
+	u64 tb, wdt_tb, wdt_ticks = 0;
+	u64 nr_jiffies = 0;
+	u32 period = TCR_GET_WP(vcpu->arch.tcr);
+
+	wdt_tb = 1ULL << (63 - period);
+	tb = get_tb();
+	/*
+	 * The watchdog timeout will hapeen when TB bit corresponding
+	 * to watchdog will toggle from 0 to 1.
+	 */
+	if (tb & wdt_tb)
+		wdt_ticks = wdt_tb;
+
+	wdt_ticks += wdt_tb - (tb & (wdt_tb - 1));
+
+	/* Convert timebase ticks to jiffies */
+	nr_jiffies = wdt_ticks;
+
+	if (do_div(nr_jiffies, tb_ticks_per_jiffy))
+		nr_jiffies++;
+
+	return min_t(unsigned long long, nr_jiffies, NEXT_TIMER_MAX_DELTA);
+}
+
+static void arm_next_watchdog(struct kvm_vcpu *vcpu)
+{
+	unsigned long nr_jiffies;
+
+	spin_lock(&vcpu->arch.wdt_lock);
+	nr_jiffies = watchdog_next_timeout(vcpu);
+	/*
+	 * If the number of jiffies of watchdog timer >= NEXT_TIMER_MAX_DELTA
+	 * then do not run the watchdog timer as this can break timer APIs.
+	 */
+	if (nr_jiffies < NEXT_TIMER_MAX_DELTA)
+		mod_timer(&vcpu->arch.wdt_timer, jiffies + nr_jiffies);
+	else
+		del_timer(&vcpu->arch.wdt_timer);
+	spin_unlock(&vcpu->arch.wdt_lock);
+}
+
+void kvmppc_watchdog_func(unsigned long data)
+{
+	struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
+	u32 tsr, new_tsr;
+	int final;
+
+	do {
+		new_tsr = tsr = vcpu->arch.tsr;
+		final = 0;
+
+		/* Time out event */
+		if (tsr & TSR_ENW) {
+			if (tsr & TSR_WIS)
+				final = 1;
+			else
+				new_tsr = tsr | TSR_WIS;
+		} else {
+			new_tsr = tsr | TSR_ENW;
+		}
+	} while (cmpxchg(&vcpu->arch.tsr, tsr, new_tsr) != tsr);
+
+	if (new_tsr & TSR_WIS) {
+		smp_wmb();
+		kvm_make_request(KVM_REQ_PENDING_TIMER, vcpu);
+		kvm_vcpu_kick(vcpu);
+	}
+
+	/*
+	 * If this is final watchdog expiry and some action is required
+	 * then exit to userspace.
+	 */
+	if (final && (vcpu->arch.tcr & TCR_WRC_MASK) &&
+	    vcpu->arch.watchdog_enabled) { 
+		smp_wmb();
+		kvm_make_request(KVM_REQ_WATCHDOG, vcpu);
+		kvm_vcpu_kick(vcpu);
+	}
+
+	/*
+	 * Stop running the watchdog timer after final expiration to
+	 * prevent the host from being flooded with timers if the
+	 * guest sets a short period.
+	 * Timers will resume when TSR/TCR is updated next time.
+	 */
+	if (!final)
+		arm_next_watchdog(vcpu);
+}
+
 static void update_timer_ints(struct kvm_vcpu *vcpu)
 {
 	if ((vcpu->arch.tcr & TCR_DIE) && (vcpu->arch.tsr & TSR_DIS))
 		kvmppc_core_queue_dec(vcpu);
 	else
 		kvmppc_core_dequeue_dec(vcpu);
+
+	if ((vcpu->arch.tcr & TCR_WIE) && (vcpu->arch.tsr & TSR_WIS))
+		kvmppc_core_queue_watchdog(vcpu);
+	else
+		kvmppc_core_dequeue_watchdog(vcpu);
 }
 
 static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
@@ -516,6 +628,13 @@  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 		goto out;
 	}
 
+	if (kvm_check_request(KVM_REQ_WATCHDOG, vcpu) &&
+	    (vcpu->arch.tsr & (TSR_ENW | TSR_WIS))) {
+		kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
+		ret = 0;
+		goto out;
+	}
+
 	kvm_guest_enter();
 
 #ifdef CONFIG_PPC_FPU
@@ -978,6 +1097,12 @@  int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		}
 	}
 
+	if (kvm_check_request(KVM_REQ_WATCHDOG, vcpu) &&
+	    (vcpu->arch.tsr & (TSR_ENW | TSR_WIS))) {
+		run->exit_reason = KVM_EXIT_WATCHDOG;
+		r = RESUME_HOST | (r & RESUME_FLAG_NV);
+	}
+
 	return r;
 }
 
@@ -1106,7 +1231,13 @@  static int set_sregs_base(struct kvm_vcpu *vcpu,
 	}
 
 	if (sregs->u.e.update_special & KVM_SREGS_E_UPDATE_TSR) {
+		u32 old_tsr = vcpu->arch.tsr;
+
 		vcpu->arch.tsr = sregs->u.e.tsr;
+
+		if ((old_tsr ^ vcpu->arch.tsr) & (TSR_ENW | TSR_WIS))
+			arm_next_watchdog(vcpu);
+
 		update_timer_ints(vcpu);
 	}
 
@@ -1267,6 +1398,7 @@  void kvmppc_core_commit_memory_region(struct kvm *kvm,
 void kvmppc_set_tcr(struct kvm_vcpu *vcpu, u32 new_tcr)
 {
 	vcpu->arch.tcr = new_tcr;
+	arm_next_watchdog(vcpu);
 	update_timer_ints(vcpu);
 }
 
@@ -1281,6 +1413,14 @@  void kvmppc_set_tsr_bits(struct kvm_vcpu *vcpu, u32 tsr_bits)
 void kvmppc_clr_tsr_bits(struct kvm_vcpu *vcpu, u32 tsr_bits)
 {
 	clear_bits(tsr_bits, &vcpu->arch.tsr);
+
+	/*
+	 * We may have stopped the watchdog due to
+	 * being stuck on final expiration.
+	 */
+	if (tsr_bits & (TSR_ENW | TSR_WIS))
+		arm_next_watchdog(vcpu);
+
 	update_timer_ints(vcpu);
 }
 
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 12834bb..5a66ade 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -145,6 +145,14 @@  int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 		kvmppc_clr_tsr_bits(vcpu, spr_val);
 		break;
 	case SPRN_TCR:
+		/*
+		 * WRC is a 2-bit field that is supposed to preserve its
+		 * value once written to non-zero.
+		 */
+		if (vcpu->arch.tcr & TCR_WRC_MASK) {
+			spr_val &= ~TCR_WRC_MASK;
+			spr_val |= vcpu->arch.tcr & TCR_WRC_MASK;
+		}
 		kvmppc_set_tcr(vcpu, spr_val);
 		break;
 
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 87f4dc8..63457cb 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -220,6 +220,7 @@  int kvm_dev_ioctl_check_extension(long ext)
 	switch (ext) {
 #ifdef CONFIG_BOOKE
 	case KVM_CAP_PPC_BOOKE_SREGS:
+	case KVM_CAP_PPC_BOOKE_WATCHDOG:
 #else
 	case KVM_CAP_PPC_SEGSTATE:
 	case KVM_CAP_PPC_HIOR:
@@ -386,13 +387,23 @@  int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_KVM_EXIT_TIMING
 	mutex_init(&vcpu->arch.exit_timing_lock);
 #endif
-
+#ifdef CONFIG_BOOKE
+	spin_lock_init(&vcpu->arch.wdt_lock);
+	/* setup watchdog timer once */
+	setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
+		    (unsigned long)vcpu);
+#endif
 	return 0;
 }
 
 void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
 	kvmppc_mmu_destroy(vcpu);
+#ifdef CONFIG_BOOKE
+	spin_lock(&vcpu->arch.wdt_lock);
+	del_timer(&vcpu->arch.wdt_timer);
+	spin_unlock(&vcpu->arch.wdt_lock);
+#endif
 }
 
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
@@ -637,6 +648,12 @@  static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		r = 0;
 		vcpu->arch.papr_enabled = true;
 		break;
+#ifdef CONFIG_BOOKE
+	case KVM_CAP_PPC_BOOKE_WATCHDOG:
+		r = 0;
+		vcpu->arch.watchdog_enabled = true;
+		break;
+#endif
 #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC)
 	case KVM_CAP_SW_TLB: {
 		struct kvm_config_tlb cfg;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 2ce09aa..9ff5aa5 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -163,6 +163,7 @@  struct kvm_pit_config {
 #define KVM_EXIT_OSI              18
 #define KVM_EXIT_PAPR_HCALL	  19
 #define KVM_EXIT_S390_UCONTROL	  20
+#define KVM_EXIT_WATCHDOG         21
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 #define KVM_INTERNAL_ERROR_EMULATION 1
@@ -618,6 +619,7 @@  struct kvm_ppc_smmu_info {
 #define KVM_CAP_PPC_GET_SMMU_INFO 78
 #define KVM_CAP_S390_COW 79
 #define KVM_CAP_PPC_ALLOC_HTAB 80
+#define KVM_CAP_PPC_BOOKE_WATCHDOG 81
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 27ac8a4..41d32be 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -69,6 +69,7 @@ 
 #define KVM_REQ_IMMEDIATE_EXIT    15
 #define KVM_REQ_PMU               16
 #define KVM_REQ_PMI               17
+#define KVM_REQ_WATCHDOG          18
 
 #define KVM_USERSPACE_IRQ_SOURCE_ID	0