Re: [RFC] Paravirt timer for KVM

Glauber de Oliveira Costa
2007-10-15T16:58:24Z

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

Ok.

> > +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()).
>

After trying this option a little bit, it does not seemed worthwhile
to me. But it might well have been due to some misunderstanding from
my part, and I can take a look again. Basically, we can't do it in the
timer function because it is protected by a mutex. the hrtimer
function runs in interrupt context, so we'd have to flag and signal
anyway.

"Interrupts for the clock" needs special irq lines anyway. (Well, VMI
masks it and does a whole lot of work to use the apic, but I prefer
the xen way, in which irq lines are triggered directly), so if I
remember the code correctly, we'd have to put some code on for those
lines anyway.

I may be a little paranoid here, but as it does not seem we're going
to be able to use all code structure as is, at least this way we avoid
the apic code path in favour of a simpler one. But maybe it's really
not worth it

I'd see an direct advantage for that if we are going to use the local
apic for the delivery, but then it complicates the kernel side, as it
already has an apic clockevents registered, with a interrupt handler
on its own.

--
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."