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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, July 21, 2012 2:59 AM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; 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@xxxxxxxxxxxxx>
> > Signed-off-by: Scott Wood <scottwood@xxxxxxxxxxxxx>
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
> > [bharat.bhushan@xxxxxxxxxxxxx: 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()?

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

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

��.n��������+%������w��{.n�����o��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�

[Index of Archives]     [KVM Development]     [KVM ARM]     [KVM ia64]     [Linux Virtualization]     [Linux USB Devel]     [Linux Video]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux