-----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:

quoted

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 <redacted>
Signed-off-by: Scott Wood <redacted>
Signed-off-by: Bharat Bhushan <redacted>
[bharat.bhushan@freescale.com: reworked patch]

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

quoted

+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.

quoted

@@ -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().

Thanks
-Bharat

quoted

 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