Re: [RFC] Paravirt timer for KVM

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

Glauber de Oliveira Costa wrote:
> Hi,
>
> Attached is a first draft to a paravirt implementation for a timer to
> KVM. It is inspired in anthony's last patch about it, but not that
> much based on it.
>
> I'm not using hypercalls to get the current time, but rather,
> registering an address that will get timer updates once in a while.
>
> Also, it includes a clockevent oneshot implementation (which is the
> very thing of this patch), that will allow us interest things like
> dynticks.
>
> It's still not yet working on SMP, and I'm currently not sure why (ok,
> ok, if you actually read the patch, it will become obvious the why: it
> only delivers interrupt for vector 0x20, but I'm further with it, this
> patch is just a snapshot)
>
> My next TODOs with it are:
> * Get SMP working
> * Try something for stolen time, as jeremy's last suggestion for anthony's patch
> * Measure the time it takes for a hypercall, and subtract this time
> for calculating the expiry time for the timer event.
> * Testing and fixing bugs: I'm sure they exist!
>
> Meanwhile, all your suggestions are welcome.
>
>   

<snip>

> +
> +void __init kvmclock_init(void)
> +{
> +
> +	unsigned long shared_page = (unsigned long)&hv_clock;
> +	/*
> +	 * If we can't use the paravirt clock, just go with
> +	 * the usual timekeeping
> +	 */
> +	if (!kvm_para_available() || no_kvmclock)
> +		return;
>   

You should also check kvm_para_has_feature() and define a feature flag 
for the clock.

> +	if (kvm_hypercall1(KVM_HCALL_SET_SHARED_PAGE, shared_page))
> +		return;
> +
> +	paravirt_ops.get_wallclock = kvm_get_wallclock;
> +	paravirt_ops.set_wallclock = kvm_set_wallclock;
> +	paravirt_ops.sched_clock = kvm_sched_clock;
> +	paravirt_ops.time_init = kvm_time_init;
> +	/*
> +	 * If we let the normal APIC initialization code run, they will
> +	 * override our event handler, relying that the APIC will deliver
> +	 * the interrupts in the LOCAL_TIMER_VECTOR. The easy solution is
> +	 * keep the PIT running until then
> +	 */
> +	paravirt_ops.setup_boot_clock = kvm_disable_pit;
> +}


> diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
> index 0f663fe..7baf798 100644
> --- a/drivers/kvm/irq.c
> +++ b/drivers/kvm/irq.c
> @@ -32,6 +32,8 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
>  {
>  	struct kvm_pic *s;
>  
> +	if (v->timer_vector != -1)
> +		return 1;
>  	if (kvm_apic_has_interrupt(v) == -1) {	/* LAPIC */
>  		if (kvm_apic_accept_pic_intr(v)) {
>  			s = pic_irqchip(v->kvm);	/* PIC */
> @@ -43,6 +45,12 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
>  }
>  EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt);
>  
> +static int kvm_get_pvclock_interrupt(struct kvm_vcpu *v)
> +{
> +	int ret = v->timer_vector;
> +	v->timer_vector = -1;
> +	return	ret;
> +}
>  /*
>   * Read pending interrupt vector and intack.
>   */
> @@ -51,7 +59,9 @@ int kvm_cpu_get_interrupt(struct kvm_vcpu *v)
>  	struct kvm_pic *s;
>  	int vector;
>  
> -	vector = kvm_get_apic_interrupt(v);	/* APIC */
> +	vector = kvm_get_pvclock_interrupt(v);
> +	if (vector == -1)
> +		vector = kvm_get_apic_interrupt(v);	/* APIC */
>   

It might be better to just rely on the in-kernel APIC to inject an 
interrupt for the clock (via kvm_pic_set_irq()).

Regards,

Anthony LIguori

>  	if (vector == -1) {
>  		if (kvm_apic_accept_pic_intr(v)) {
>  			s = pic_irqchip(v->kvm)

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