* [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
@ 2012-07-18 9:51 ` Bharat Bhushan
0 siblings, 0 replies; 8+ messages in thread
From: Bharat Bhushan @ 2012-07-18 9:39 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf; +Cc: Bharat Bhushan
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>
---
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 | 27 ++++++-
include/linux/kvm.h | 2 +
include/linux/kvm_host.h | 1 +
8 files changed, 187 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 50ea12f..01047f4 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_enable;
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..9682506 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,112 @@ 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;
+
+ nr_jiffies = watchdog_next_timeout(vcpu);
+ spin_lock(&vcpu->arch.wdt_lock);
+ /*
+ * 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)) {
+ 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 +627,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.watchdog_enable) {
+ kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
+ ret = 0;
+ goto out;
+ }
+
kvm_guest_enter();
#ifdef CONFIG_PPC_FPU
@@ -978,6 +1096,12 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
}
}
+ if (kvm_check_request(KVM_REQ_WATCHDOG, vcpu) &&
+ vcpu->arch.watchdog_enable) {
+ run->exit_reason = KVM_EXIT_WATCHDOG;
+ r = RESUME_HOST | (r & RESUME_FLAG_NV);
+ }
+
return r;
}
@@ -1106,7 +1230,14 @@ 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 | TCR_WRC_MASK))
+ 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 | TCR_WRC_MASK))
+ 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..3c50b81 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -38,9 +38,11 @@
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;
+
+ return ret;
}
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
@@ -237,6 +239,7 @@ int kvm_dev_ioctl_check_extension(long ext)
case KVM_CAP_PPC_GET_PVINFO:
#if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC)
case KVM_CAP_SW_TLB:
+ case KVM_CAP_PPC_WDT:
#endif
r = 1;
break;
@@ -386,13 +389,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 +650,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_WDT:
+ r = 0;
+ vcpu->arch.watchdog_enable = 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..07dcab3 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_WDT 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
--
1.7.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
@ 2012-07-18 9:51 ` Bharat Bhushan
0 siblings, 0 replies; 8+ messages in thread
From: Bharat Bhushan @ 2012-07-18 9:51 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf; +Cc: Bharat Bhushan
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>
---
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 | 27 ++++++-
include/linux/kvm.h | 2 +
include/linux/kvm_host.h | 1 +
8 files changed, 187 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 50ea12f..01047f4 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_enable;
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..9682506 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,112 @@ 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;
+
+ nr_jiffies = watchdog_next_timeout(vcpu);
+ spin_lock(&vcpu->arch.wdt_lock);
+ /*
+ * 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)) {
+ 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 +627,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.watchdog_enable) {
+ kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
+ ret = 0;
+ goto out;
+ }
+
kvm_guest_enter();
#ifdef CONFIG_PPC_FPU
@@ -978,6 +1096,12 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
}
}
+ if (kvm_check_request(KVM_REQ_WATCHDOG, vcpu) &&
+ vcpu->arch.watchdog_enable) {
+ run->exit_reason = KVM_EXIT_WATCHDOG;
+ r = RESUME_HOST | (r & RESUME_FLAG_NV);
+ }
+
return r;
}
@@ -1106,7 +1230,14 @@ 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 | TCR_WRC_MASK))
+ 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 | TCR_WRC_MASK))
+ 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..3c50b81 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -38,9 +38,11 @@
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;
+
+ return ret;
}
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
@@ -237,6 +239,7 @@ int kvm_dev_ioctl_check_extension(long ext)
case KVM_CAP_PPC_GET_PVINFO:
#if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC)
case KVM_CAP_SW_TLB:
+ case KVM_CAP_PPC_WDT:
#endif
r = 1;
break;
@@ -386,13 +389,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 +650,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_WDT:
+ r = 0;
+ vcpu->arch.watchdog_enable = 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..07dcab3 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_WDT 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
--
1.7.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
2012-07-18 9:51 ` Bharat Bhushan
@ 2012-07-19 2:26 ` Scott Wood
-1 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2012-07-19 2:26 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: kvm-ppc, kvm, agraf, Bharat Bhushan
On 07/18/2012 04:39 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>
Please put a note before your signoff stating that it's been modified
since the previous signoffs, such as:
[bharat.bhushan@freescale.com: reworked patch]
> @@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
> u8 osi_needed;
> u8 osi_enabled;
> u8 papr_enabled;
> + u8 watchdog_enable;
s/enable;/enabled;/
> +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
> +{
> + unsigned long nr_jiffies;
> +
> + nr_jiffies = watchdog_next_timeout(vcpu);
> + spin_lock(&vcpu->arch.wdt_lock);
Do watchdog_next_timeout() from within the 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;
> + }
Unnecessary braces on the inner if/else.
> + } 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)) {
> + 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)
Blank line between functions
> static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
> @@ -516,6 +627,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.watchdog_enable) {
Check .watchdog_enabled when you make the request, not here.
> + kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
> + ret = 0;
> + goto out;
> + }
I think we should check that ENW/WIS are still set. Now that we don't
use TSR[WRS], this is the only way QEMU can preemptively clear a pending
final expiration after leaving debug halt. If QEMU doesn't do this, and
KVM comes back with a watchdog exit, QEMU won't know if it's a new
legitimate one, or if it expired during the debug halt. This would be
less ugly than QEMU doing a reset-TSR[WIS,ENW]-and-ignore on the first
watchdog exit after a debug halt, and letting the expiration happen
again if the guest is really stuck.
> 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 | TCR_WRC_MASK))
> + arm_next_watchdog(vcpu);
Do we still do anything with TSR[WRS] at all?
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 87f4dc8..3c50b81 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -38,9 +38,11 @@
>
> 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;
> +
> + return ret;
> }
There's no need to change this function anymore.
> +#define KVM_CAP_PPC_WDT 81
s/WDT/WATCHDOG/
or better, s/WDT/BOOKE_WATCHDOG/
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
@ 2012-07-19 2:26 ` Scott Wood
0 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2012-07-19 2:26 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: kvm-ppc, kvm, agraf, Bharat Bhushan
On 07/18/2012 04:39 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>
Please put a note before your signoff stating that it's been modified
since the previous signoffs, such as:
[bharat.bhushan@freescale.com: reworked patch]
> @@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
> u8 osi_needed;
> u8 osi_enabled;
> u8 papr_enabled;
> + u8 watchdog_enable;
s/enable;/enabled;/
> +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
> +{
> + unsigned long nr_jiffies;
> +
> + nr_jiffies = watchdog_next_timeout(vcpu);
> + spin_lock(&vcpu->arch.wdt_lock);
Do watchdog_next_timeout() from within the 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;
> + }
Unnecessary braces on the inner if/else.
> + } 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)) {
> + 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)
Blank line between functions
> static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
> @@ -516,6 +627,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.watchdog_enable) {
Check .watchdog_enabled when you make the request, not here.
> + kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
> + ret = 0;
> + goto out;
> + }
I think we should check that ENW/WIS are still set. Now that we don't
use TSR[WRS], this is the only way QEMU can preemptively clear a pending
final expiration after leaving debug halt. If QEMU doesn't do this, and
KVM comes back with a watchdog exit, QEMU won't know if it's a new
legitimate one, or if it expired during the debug halt. This would be
less ugly than QEMU doing a reset-TSR[WIS,ENW]-and-ignore on the first
watchdog exit after a debug halt, and letting the expiration happen
again if the guest is really stuck.
> 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 | TCR_WRC_MASK))
> + arm_next_watchdog(vcpu);
Do we still do anything with TSR[WRS] at all?
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 87f4dc8..3c50b81 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -38,9 +38,11 @@
>
> 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;
> +
> + return ret;
> }
There's no need to change this function anymore.
> +#define KVM_CAP_PPC_WDT 81
s/WDT/WATCHDOG/
or better, s/WDT/BOOKE_WATCHDOG/
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
2012-07-19 2:26 ` Scott Wood
@ 2012-07-19 5:35 ` Bhushan Bharat-R65777
-1 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-07-19 5:35 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: kvm-ppc, kvm, agraf
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, July 19, 2012 7:56 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 v4] KVM: PPC: booke: Add watchdog emulation
>
> On 07/18/2012 04:39 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>
>
> Please put a note before your signoff stating that it's been modified
> since the previous signoffs, such as:
>
> [bharat.bhushan@freescale.com: reworked patch]
>
> > @@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
> > u8 osi_needed;
> > u8 osi_enabled;
> > u8 papr_enabled;
> > + u8 watchdog_enable;
>
> s/enable;/enabled;/
>
> > +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
> > +{
> > + unsigned long nr_jiffies;
> > +
> > + nr_jiffies = watchdog_next_timeout(vcpu);
> > + spin_lock(&vcpu->arch.wdt_lock);
>
> Do watchdog_next_timeout() from within the lock.
Why you think so? Is the next timeout calculation needed to be protected?
>
> > +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;
> > + }
>
> Unnecessary braces on the inner if/else.
>
> > + } 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)) {
> > + 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)
>
> Blank line between functions
>
> > static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
> > @@ -516,6 +627,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.watchdog_enable) {
>
> Check .watchdog_enabled when you make the request, not here.
>
> > + kvm_run->exit_reason = KVM_EXIT_WATCHDOG;
> > + ret = 0;
> > + goto out;
> > + }
>
> I think we should check that ENW/WIS are still set. Now that we don't
> use TSR[WRS], this is the only way QEMU can preemptively clear a pending
> final expiration after leaving debug halt. If QEMU doesn't do this, and
> KVM comes back with a watchdog exit, QEMU won't know if it's a new
> legitimate one, or if it expired during the debug halt. This would be
> less ugly than QEMU doing a reset-TSR[WIS,ENW]-and-ignore on the first
> watchdog exit after a debug halt, and letting the expiration happen
> again if the guest is really stuck.
ahh, What about clearing KVM_REQ_WATCHDOG request when QEMU clears ENW|WIS (or whenever KVM or QEMU clears ENW|WIS)?
Thanks
-Bharat
>
> > 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 | TCR_WRC_MASK))
> > + arm_next_watchdog(vcpu);
>
> Do we still do anything with TSR[WRS] at all?
>
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 87f4dc8..3c50b81 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -38,9 +38,11 @@
> >
> > 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;
> > +
> > + return ret;
> > }
>
> There's no need to change this function anymore.
>
> > +#define KVM_CAP_PPC_WDT 81
>
> s/WDT/WATCHDOG/
>
> or better, s/WDT/BOOKE_WATCHDOG/
>
> -Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
@ 2012-07-19 5:35 ` Bhushan Bharat-R65777
0 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-07-19 5:35 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: kvm-ppc, kvm, agraf
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVGh1cnNkYXksIEp1bHkgMTksIDIwMTIgNzo1NiBBTQ0KPiBUbzogQmh1c2hh
biBCaGFyYXQtUjY1Nzc3DQo+IENjOiBrdm0tcHBjQHZnZXIua2VybmVsLm9yZzsga3ZtQHZnZXIu
a2VybmVsLm9yZzsgYWdyYWZAc3VzZS5kZTsgQmh1c2hhbiBCaGFyYXQtDQo+IFI2NTc3Nw0KPiBT
dWJqZWN0OiBSZTogW1BBVENIIDIvMiB2NF0gS1ZNOiBQUEM6IGJvb2tlOiBBZGQgd2F0Y2hkb2cg
ZW11bGF0aW9uDQo+IA0KPiBPbiAwNy8xOC8yMDEyIDA0OjM5IEFNLCBCaGFyYXQgQmh1c2hhbiB3
cm90ZToNCj4gPiBUaGlzIHBhdGNoIGFkZHMgdGhlIHdhdGNoZG9nIGVtdWxhdGlvbiBpbiBLVk0u
IFRoZSB3YXRjaGRvZw0KPiA+IGVtdWxhdGlvbiBpcyBlbmFibGVkIGJ5IEtWTV9FTkFCTEVfQ0FQ
KEtWTV9DQVBfUFBDX1dEVCkgaW9jdGwuDQo+ID4gVGhlIGtlcm5lbCB0aW1lciBhcmUgdXNlZCBm
b3Igd2F0Y2hkb2cgZW11bGF0aW9uIGFuZCBlbXVsYXRlcw0KPiA+IGgvdyB3YXRjaGRvZyBzdGF0
ZSBtYWNoaW5lLiBPbiB3YXRjaGRvZyB0aW1lciBleHBpcnksIGl0IGV4aXQgdG8gUUVNVQ0KPiA+
IGlmIFRDUi5XUkMgaXMgbm9uIFpFUk8uIFFFTVUgY2FuIHJlc2V0L3NodXRkb3duIGV0YyBkZXBl
bmRpbmcgdXBvbiBob3cNCj4gPiBpdCBpcyBjb25maWd1cmVkLg0KPiA+DQo+ID4gU2lnbmVkLW9m
Zi1ieTogTGl1IFl1IDx5dS5saXVAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBT
Y290dCBXb29kIDxzY290dHdvb2RAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBC
aGFyYXQgQmh1c2hhbiA8YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbT4NCj4gDQo+IFBsZWFz
ZSBwdXQgYSBub3RlIGJlZm9yZSB5b3VyIHNpZ25vZmYgc3RhdGluZyB0aGF0IGl0J3MgYmVlbiBt
b2RpZmllZA0KPiBzaW5jZSB0aGUgcHJldmlvdXMgc2lnbm9mZnMsIHN1Y2ggYXM6DQo+IA0KPiBb
YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbTogcmV3b3JrZWQgcGF0Y2hdDQo+IA0KPiA+IEBA
IC00ODIsNiArNDg0LDcgQEAgc3RydWN0IGt2bV92Y3B1X2FyY2ggew0KPiA+ICAJdTggb3NpX25l
ZWRlZDsNCj4gPiAgCXU4IG9zaV9lbmFibGVkOw0KPiA+ICAJdTggcGFwcl9lbmFibGVkOw0KPiA+
ICsJdTggd2F0Y2hkb2dfZW5hYmxlOw0KPiANCj4gcy9lbmFibGU7L2VuYWJsZWQ7Lw0KPiANCj4g
PiArc3RhdGljIHZvaWQgYXJtX25leHRfd2F0Y2hkb2coc3RydWN0IGt2bV92Y3B1ICp2Y3B1KQ0K
PiA+ICt7DQo+ID4gKwl1bnNpZ25lZCBsb25nIG5yX2ppZmZpZXM7DQo+ID4gKw0KPiA+ICsJbnJf
amlmZmllcyA9IHdhdGNoZG9nX25leHRfdGltZW91dCh2Y3B1KTsNCj4gPiArCXNwaW5fbG9jaygm
dmNwdS0+YXJjaC53ZHRfbG9jayk7DQo+IA0KPiBEbyB3YXRjaGRvZ19uZXh0X3RpbWVvdXQoKSBm
cm9tIHdpdGhpbiB0aGUgbG9jay4NCg0KV2h5IHlvdSB0aGluayBzbz8gSXMgdGhlIG5leHQgdGlt
ZW91dCBjYWxjdWxhdGlvbiBuZWVkZWQgdG8gYmUgcHJvdGVjdGVkPw0KDQo+IA0KPiA+ICt2b2lk
IGt2bXBwY193YXRjaGRvZ19mdW5jKHVuc2lnbmVkIGxvbmcgZGF0YSkNCj4gPiArew0KPiA+ICsJ
c3RydWN0IGt2bV92Y3B1ICp2Y3B1ID0gKHN0cnVjdCBrdm1fdmNwdSAqKWRhdGE7DQo+ID4gKwl1
MzIgdHNyLCBuZXdfdHNyOw0KPiA+ICsJaW50IGZpbmFsOw0KPiA+ICsNCj4gPiArCWRvIHsNCj4g
PiArCQluZXdfdHNyID0gdHNyID0gdmNwdS0+YXJjaC50c3I7DQo+ID4gKwkJZmluYWwgPSAwOw0K
PiA+ICsNCj4gPiArCQkvKiBUaW1lIG91dCBldmVudCAqLw0KPiA+ICsJCWlmICh0c3IgJiBUU1Jf
RU5XKSB7DQo+ID4gKwkJCWlmICh0c3IgJiBUU1JfV0lTKSB7DQo+ID4gKwkJCQlmaW5hbCA9IDE7
DQo+ID4gKwkJCX0gZWxzZSB7DQo+ID4gKwkJCQluZXdfdHNyID0gdHNyIHwgVFNSX1dJUzsNCj4g
PiArCQkJfQ0KPiANCj4gVW5uZWNlc3NhcnkgYnJhY2VzIG9uIHRoZSBpbm5lciBpZi9lbHNlLg0K
PiANCj4gPiArCQl9IGVsc2Ugew0KPiA+ICsJCQluZXdfdHNyID0gdHNyIHwgVFNSX0VOVzsNCj4g
PiArCQl9DQo+ID4gKwl9IHdoaWxlIChjbXB4Y2hnKCZ2Y3B1LT5hcmNoLnRzciwgdHNyLCBuZXdf
dHNyKSAhPSB0c3IpOw0KPiA+ICsNCj4gPiArCWlmIChuZXdfdHNyICYgVFNSX1dJUykgew0KPiA+
ICsJCXNtcF93bWIoKTsNCj4gPiArCQlrdm1fbWFrZV9yZXF1ZXN0KEtWTV9SRVFfUEVORElOR19U
SU1FUiwgdmNwdSk7DQo+ID4gKwkJa3ZtX3ZjcHVfa2ljayh2Y3B1KTsNCj4gPiArCX0NCj4gPiAr
DQo+ID4gKwkvKg0KPiA+ICsJICogSWYgdGhpcyBpcyBmaW5hbCB3YXRjaGRvZyBleHBpcnkgYW5k
IHNvbWUgYWN0aW9uIGlzIHJlcXVpcmVkDQo+ID4gKwkgKiB0aGVuIGV4aXQgdG8gdXNlcnNwYWNl
Lg0KPiA+ICsJICovDQo+ID4gKwlpZiAoZmluYWwgJiYgKHZjcHUtPmFyY2gudGNyICYgVENSX1dS
Q19NQVNLKSkgew0KPiA+ICsJCXNtcF93bWIoKTsNCj4gPiArCQlrdm1fbWFrZV9yZXF1ZXN0KEtW
TV9SRVFfV0FUQ0hET0csIHZjcHUpOw0KPiA+ICsJCWt2bV92Y3B1X2tpY2sodmNwdSk7DQo+ID4g
Kwl9DQo+ID4gKw0KPiA+ICsNCj4gPiArCS8qDQo+ID4gKwkgKiBTdG9wIHJ1bm5pbmcgdGhlIHdh
dGNoZG9nIHRpbWVyIGFmdGVyIGZpbmFsIGV4cGlyYXRpb24gdG8NCj4gPiArCSAqIHByZXZlbnQg
dGhlIGhvc3QgZnJvbSBiZWluZyBmbG9vZGVkIHdpdGggdGltZXJzIGlmIHRoZQ0KPiA+ICsJICog
Z3Vlc3Qgc2V0cyBhIHNob3J0IHBlcmlvZC4NCj4gPiArCSAqIFRpbWVycyB3aWxsIHJlc3VtZSB3
aGVuIFRTUi9UQ1IgaXMgdXBkYXRlZCBuZXh0IHRpbWUuDQo+ID4gKwkgKi8NCj4gPiArCWlmICgh
ZmluYWwpDQo+ID4gKwkJYXJtX25leHRfd2F0Y2hkb2codmNwdSk7DQo+ID4gK30NCj4gPiAgc3Rh
dGljIHZvaWQgdXBkYXRlX3RpbWVyX2ludHMoc3RydWN0IGt2bV92Y3B1ICp2Y3B1KQ0KPiANCj4g
QmxhbmsgbGluZSBiZXR3ZWVuIGZ1bmN0aW9ucw0KPiANCj4gPiAgc3RhdGljIHZvaWQga3ZtcHBj
X2NvcmVfY2hlY2tfZXhjZXB0aW9ucyhzdHJ1Y3Qga3ZtX3ZjcHUgKnZjcHUpDQo+ID4gQEAgLTUx
Niw2ICs2MjcsMTMgQEAgaW50IGt2bXBwY192Y3B1X3J1bihzdHJ1Y3Qga3ZtX3J1biAqa3ZtX3J1
biwgc3RydWN0DQo+IGt2bV92Y3B1ICp2Y3B1KQ0KPiA+ICAJCWdvdG8gb3V0Ow0KPiA+ICAJfQ0K
PiA+DQo+ID4gKwlpZiAoa3ZtX2NoZWNrX3JlcXVlc3QoS1ZNX1JFUV9XQVRDSERPRywgdmNwdSkg
JiYNCj4gPiArCSAgICB2Y3B1LT5hcmNoLndhdGNoZG9nX2VuYWJsZSkgew0KPiANCj4gQ2hlY2sg
LndhdGNoZG9nX2VuYWJsZWQgd2hlbiB5b3UgbWFrZSB0aGUgcmVxdWVzdCwgbm90IGhlcmUuDQo+
IA0KPiA+ICsJCWt2bV9ydW4tPmV4aXRfcmVhc29uID0gS1ZNX0VYSVRfV0FUQ0hET0c7DQo+ID4g
KwkJcmV0ID0gMDsNCj4gPiArCQlnb3RvIG91dDsNCj4gPiArCX0NCj4gDQo+IEkgdGhpbmsgd2Ug
c2hvdWxkIGNoZWNrIHRoYXQgRU5XL1dJUyBhcmUgc3RpbGwgc2V0LiAgTm93IHRoYXQgd2UgZG9u
J3QNCj4gdXNlIFRTUltXUlNdLCB0aGlzIGlzIHRoZSBvbmx5IHdheSBRRU1VIGNhbiBwcmVlbXB0
aXZlbHkgY2xlYXIgYSBwZW5kaW5nDQo+IGZpbmFsIGV4cGlyYXRpb24gYWZ0ZXIgbGVhdmluZyBk
ZWJ1ZyBoYWx0LiAgSWYgUUVNVSBkb2Vzbid0IGRvIHRoaXMsIGFuZA0KPiBLVk0gY29tZXMgYmFj
ayB3aXRoIGEgd2F0Y2hkb2cgZXhpdCwgUUVNVSB3b24ndCBrbm93IGlmIGl0J3MgYSBuZXcNCj4g
bGVnaXRpbWF0ZSBvbmUsIG9yIGlmIGl0IGV4cGlyZWQgZHVyaW5nIHRoZSBkZWJ1ZyBoYWx0LiAg
VGhpcyB3b3VsZCBiZQ0KPiBsZXNzIHVnbHkgdGhhbiBRRU1VIGRvaW5nIGEgcmVzZXQtVFNSW1dJ
UyxFTlddLWFuZC1pZ25vcmUgb24gdGhlIGZpcnN0DQo+IHdhdGNoZG9nIGV4aXQgYWZ0ZXIgYSBk
ZWJ1ZyBoYWx0LCBhbmQgbGV0dGluZyB0aGUgZXhwaXJhdGlvbiBoYXBwZW4NCj4gYWdhaW4gaWYg
dGhlIGd1ZXN0IGlzIHJlYWxseSBzdHVjay4NCg0KYWhoLCBXaGF0IGFib3V0IGNsZWFyaW5nIEtW
TV9SRVFfV0FUQ0hET0cgcmVxdWVzdCB3aGVuIFFFTVUgY2xlYXJzIEVOV3xXSVMgKG9yIHdoZW5l
dmVyIEtWTSBvciBRRU1VIGNsZWFycyBFTld8V0lTKT8NCg0KVGhhbmtzDQotQmhhcmF0DQoNCj4g
DQo+ID4gIAlpZiAoc3JlZ3MtPnUuZS51cGRhdGVfc3BlY2lhbCAmIEtWTV9TUkVHU19FX1VQREFU
RV9UU1IpIHsNCj4gPiArCQl1MzIgb2xkX3RzciA9IHZjcHUtPmFyY2gudHNyOw0KPiA+ICsNCj4g
PiAgCQl2Y3B1LT5hcmNoLnRzciA9IHNyZWdzLT51LmUudHNyOw0KPiA+ICsNCj4gPiArCQlpZiAo
KG9sZF90c3IgXiB2Y3B1LT5hcmNoLnRzcikgJg0KPiA+ICsJCSAgICAoVFNSX0VOVyB8IFRTUl9X
SVMgfCBUQ1JfV1JDX01BU0spKQ0KPiA+ICsJCQlhcm1fbmV4dF93YXRjaGRvZyh2Y3B1KTsNCj4g
DQo+IERvIHdlIHN0aWxsIGRvIGFueXRoaW5nIHdpdGggVFNSW1dSU10gYXQgYWxsPw0KPiANCj4g
PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9wb3dlcnBjLmMgYi9hcmNoL3Bvd2VycGMv
a3ZtL3Bvd2VycGMuYw0KPiA+IGluZGV4IDg3ZjRkYzguLjNjNTBiODEgMTAwNjQ0DQo+ID4gLS0t
IGEvYXJjaC9wb3dlcnBjL2t2bS9wb3dlcnBjLmMNCj4gPiArKysgYi9hcmNoL3Bvd2VycGMva3Zt
L3Bvd2VycGMuYw0KPiA+IEBAIC0zOCw5ICszOCwxMSBAQA0KPiA+DQo+ID4gIGludCBrdm1fYXJj
aF92Y3B1X3J1bm5hYmxlKHN0cnVjdCBrdm1fdmNwdSAqdikNCj4gPiAgew0KPiA+IC0JcmV0dXJu
ICEodi0+YXJjaC5zaGFyZWQtPm1zciAmIE1TUl9XRSkgfHwNCj4gPiAtCSAgICAgICAhISh2LT5h
cmNoLnBlbmRpbmdfZXhjZXB0aW9ucykgfHwNCj4gPiAtCSAgICAgICB2LT5yZXF1ZXN0czsNCj4g
PiArCWJvb2wgcmV0ID0gISh2LT5hcmNoLnNoYXJlZC0+bXNyICYgTVNSX1dFKSB8fA0KPiA+ICsJ
CSAgICEhKHYtPmFyY2gucGVuZGluZ19leGNlcHRpb25zKSB8fA0KPiA+ICsJCSAgIHYtPnJlcXVl
c3RzOw0KPiA+ICsNCj4gPiArCXJldHVybiByZXQ7DQo+ID4gIH0NCj4gDQo+IFRoZXJlJ3Mgbm8g
bmVlZCB0byBjaGFuZ2UgdGhpcyBmdW5jdGlvbiBhbnltb3JlLg0KPiANCj4gPiArI2RlZmluZSBL
Vk1fQ0FQX1BQQ19XRFQJODENCj4gDQo+IHMvV0RUL1dBVENIRE9HLw0KPiANCj4gb3IgYmV0dGVy
LCBzL1dEVC9CT09LRV9XQVRDSERPRy8NCj4gDQo+IC1TY290dA0KDQo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
2012-07-19 5:35 ` Bhushan Bharat-R65777
@ 2012-07-19 19:59 ` Scott Wood
-1 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2012-07-19 19:59 UTC (permalink / raw)
To: Bhushan Bharat-R65777; +Cc: Wood Scott-B07421, kvm-ppc, kvm, agraf
On 07/19/2012 12:35 AM, Bhushan Bharat-R65777 wrote:
>
>
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Thursday, July 19, 2012 7:56 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 v4] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/18/2012 04:39 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>
>>
>> Please put a note before your signoff stating that it's been modified
>> since the previous signoffs, such as:
>>
>> [bharat.bhushan@freescale.com: reworked patch]
>>
>>> @@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
>>> u8 osi_needed;
>>> u8 osi_enabled;
>>> u8 papr_enabled;
>>> + u8 watchdog_enable;
>>
>> s/enable;/enabled;/
>>
>>> +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
>>> +{
>>> + unsigned long nr_jiffies;
>>> +
>>> + nr_jiffies = watchdog_next_timeout(vcpu);
>>> + spin_lock(&vcpu->arch.wdt_lock);
>>
>> Do watchdog_next_timeout() from within the lock.
>
> Why you think so? Is the next timeout calculation needed to be protected?
The point is to make sure that the timeout is still valid:
CPU 0 CPU 1
arm_next_watchdog
watchdog_next_timeout
spin_lock
update TCR
arm_next_watchdog
watchdog_next_timeout
spin_lock
mod timer
spin_unlock
lock acquired
mod_timer
spin_unlock
In the above race, you'll end up with the watchdog armed based on the
old TCR.
>> I think we should check that ENW/WIS are still set. Now that we don't
>> use TSR[WRS], this is the only way QEMU can preemptively clear a pending
>> final expiration after leaving debug halt. If QEMU doesn't do this, and
>> KVM comes back with a watchdog exit, QEMU won't know if it's a new
>> legitimate one, or if it expired during the debug halt. This would be
>> less ugly than QEMU doing a reset-TSR[WIS,ENW]-and-ignore on the first
>> watchdog exit after a debug halt, and letting the expiration happen
>> again if the guest is really stuck.
>
> ahh, What about clearing KVM_REQ_WATCHDOG request when QEMU clears
> ENW|WIS (or whenever KVM or QEMU clears ENW|WIS)?
Whichever's easier.
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation
@ 2012-07-19 19:59 ` Scott Wood
0 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2012-07-19 19:59 UTC (permalink / raw)
To: Bhushan Bharat-R65777; +Cc: Wood Scott-B07421, kvm-ppc, kvm, agraf
On 07/19/2012 12:35 AM, Bhushan Bharat-R65777 wrote:
>
>
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Thursday, July 19, 2012 7:56 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 v4] KVM: PPC: booke: Add watchdog emulation
>>
>> On 07/18/2012 04:39 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>
>>
>> Please put a note before your signoff stating that it's been modified
>> since the previous signoffs, such as:
>>
>> [bharat.bhushan@freescale.com: reworked patch]
>>
>>> @@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
>>> u8 osi_needed;
>>> u8 osi_enabled;
>>> u8 papr_enabled;
>>> + u8 watchdog_enable;
>>
>> s/enable;/enabled;/
>>
>>> +static void arm_next_watchdog(struct kvm_vcpu *vcpu)
>>> +{
>>> + unsigned long nr_jiffies;
>>> +
>>> + nr_jiffies = watchdog_next_timeout(vcpu);
>>> + spin_lock(&vcpu->arch.wdt_lock);
>>
>> Do watchdog_next_timeout() from within the lock.
>
> Why you think so? Is the next timeout calculation needed to be protected?
The point is to make sure that the timeout is still valid:
CPU 0 CPU 1
arm_next_watchdog
watchdog_next_timeout
spin_lock
update TCR
arm_next_watchdog
watchdog_next_timeout
spin_lock
mod timer
spin_unlock
lock acquired
mod_timer
spin_unlock
In the above race, you'll end up with the watchdog armed based on the
old TCR.
>> I think we should check that ENW/WIS are still set. Now that we don't
>> use TSR[WRS], this is the only way QEMU can preemptively clear a pending
>> final expiration after leaving debug halt. If QEMU doesn't do this, and
>> KVM comes back with a watchdog exit, QEMU won't know if it's a new
>> legitimate one, or if it expired during the debug halt. This would be
>> less ugly than QEMU doing a reset-TSR[WIS,ENW]-and-ignore on the first
>> watchdog exit after a debug halt, and letting the expiration happen
>> again if the guest is really stuck.
>
> ahh, What about clearing KVM_REQ_WATCHDOG request when QEMU clears
> ENW|WIS (or whenever KVM or QEMU clears ENW|WIS)?
Whichever's easier.
-Scott
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-07-19 19:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-18 9:39 [PATCH 2/2 v4] KVM: PPC: booke: Add watchdog emulation Bharat Bhushan
2012-07-18 9:51 ` Bharat Bhushan
2012-07-19 2:26 ` Scott Wood
2012-07-19 2:26 ` Scott Wood
2012-07-19 5:35 ` Bhushan Bharat-R65777
2012-07-19 5:35 ` Bhushan Bharat-R65777
2012-07-19 19:59 ` Scott Wood
2012-07-19 19:59 ` Scott Wood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.