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@xxxxxxxxxxxxx>
> Signed-off-by: Scott Wood <scottwood@xxxxxxxxxxxxx>
> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
Please put a note before your signoff stating that it's been modified
since the previous signoffs, such as:
[bharat.bhushan@xxxxxxxxxxxxx: 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
