Re: [PATCH 2/3] KVM: fix apic timer save/migration when inactive

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

He, Qing wrote:
>     
>     When local apic timer is inactive or is expired in oneshot mode, it
>     should not be restarted in save/restore or hrtimer migration. This
>     patch fixes this.
>
> diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
> index 5f97e25..68d454c 100644
> --- a/drivers/kvm/irq.h
> +++ b/drivers/kvm/irq.h
> @@ -110,6 +110,7 @@ struct kvm_lapic {
>  	struct kvm_io_device dev;
>  	struct {
>  		atomic_t pending;
> +		atomic_t active;
>   

This is atomic, but you never use read-modify-write instructions (read 
and write are atomic on a simple int).

> +
> +	if (atomic_read(&apic->timer.active))
>   

What if the timer fires here?

> +		apic_set_reg(apic, APIC_TMCCT, tmict);
> +	else
> +		apic_set_reg(apic, APIC_TMCCT, 0);
> +}
> +
>  void kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
> @@ -1036,11 +1062,14 @@ void kvm_migrate_apic_timer(struct kvm_vcpu
> *vcpu)
>  	struct kvm_lapic *apic = vcpu->apic;
>  	struct hrtimer *timer;
>  
> -	if (apic) {
> -		timer = &apic->timer.dev;
> -		hrtimer_cancel(timer);
> -		hrtimer_start(timer, timer->expires, HRTIMER_MODE_ABS);
> -	}
> +	if (!apic)
> +		return;
> +
> +	timer = &apic->timer.dev;
> +	hrtimer_cancel(timer);
> +	if (atomic_read(&apic->timer.active))
>   

Or here?

> +		hrtimer_start(timer, timer->expires,
> +			      HRTIMER_MODE_ABS);
>  }
>  EXPORT_SYMBOL_GPL(kvm_migrate_apic_timer);
>  
>   


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/