RE: [Xen-ia64-devel] xen_timer_interrupt: can someone explain the code ?

["Magenheimer, Dan (HP Labs Fort Collins)" <dan.magenheimer@xxxxxx>]

> I am currently enabling SMP guest using SMP host.
> I have made some progresses, but I have just hit a barrier: I 
> got a *lot* of
>   Oops: timer tick before it's due
> 
> The good news is the message always appear and very 
> frequently.  So it is time 
> to fix it.

Yes, much of this code is very old (predating even domU
support), very fragile, and is full of hacks.  We agreed
some time ago on the list that we would only do what was
necessary to keep it working until it (eventually) gets
rewritten.  I was surprised that it worked OK with SMP
host.  SMP guest support may finally force the rewrite.

> I have look on the xen_timer_interrupt function in xentime.c, 
> but I don't 
> really understand it.  The questions are before the statements:

Since this code was written long ago and has been hacked
on a lot, I'm not positive I can answer all of these.
Others can feel free to correct me if I'm wrong. :-)
 
> What is the purpose of this statement ?
>       if (current->domain == dom0) {
>               // FIXME: there's gotta be a better way of doing this...
>               // We have to ensure that domain0 is launched before we
>               // call vcpu_timer_expired on it
>               //domain0_ready = 1; // moved to xensetup.c
>               VCPU(current,pending_interruption) = 1;
>       }

The FIXME comment refers to the domain0_ready=1 line that is
now commented out.  So the entire comment could go away.
Turning on pending_interruption here at some point fixed a bug
where dom0 was in its idle loop and missed a timer interrupt
so never got out of its idle loop.  It's possible this can
go away now too.

> Here dom0 is awaken if its timer has expired, correct ?
> Why is dom0 special ? (and why only vcpu[0] ?)
>       if (domain0_ready && current->domain != dom0) {
>               if(vcpu_timer_expired(dom0->vcpu[0])) {
>                       vcpu_pend_timer(dom0->vcpu[0]);
>                       //vcpu_set_next_timer(dom0->vcpu[0]);
>                       vcpu_wake(dom0->vcpu[0]);
>               }
>       }

Yes.  The code checks to see if dom0 is runnable (has at least
a timer tick to process).  Depending on the scheduler, it
may get scheduled.

Note that, vcpu_wake says to put a domain on the run queue; it doesn't
get immediately awakened.

Why only vcpu[0]?  This code predates SMP guest on x86 so the
code long ago just had dom0 (e.g. vcpu_pend_timer(dom0)) because
a "virtual CPU" and a "domain" were the same data strucure.
When SMP guest was added to x86, this code was converted to
pass a pointer to a virtual CPU data structure instead of a
domain data structure.  There probably should be a // SMP FIXME
line here :-)

> This is the core job.
> The only question is why vcpu_wake ?
>       if (!is_idle_domain(current->domain))  {
>               if (vcpu_timer_expired(current)) {
>                       vcpu_pend_timer(current);
>                       // ensure another timer interrupt 
> happens even if domain doesn't
>                       vcpu_set_next_timer(current);
>                       vcpu_wake(current);
>               }
>       }
>       new_itm = local_cpu_data->itm_next;
> 
>       if (!VMX_DOMAIN(current) && !time_after(ia64_get_itc(), 
> new_itm))
>               return;

I think this just reasserts that the current domain is
runnable (has some work to do because it at least needs
to process the clock tick).  This action may not be
necessary anymore.  Try removing it and see what happens :-)

> Why current must be always awaken in VTI ?
>       if (VMX_DOMAIN(current))
>               vcpu_wake(current);

Don't know this one.

Thanks,
Dan

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel