Re: [Qemu-devel] [PATCH] [RFC] Fix time drift of rtc clock + general support

[Paul Brook <paul@codesourcery.com>]

On Sunday 23 March 2008, Dor Laor wrote:
> On Sun, 2008-03-23 at 16:19 +0000, Paul Brook wrote:
> > On Sunday 23 March 2008, Dor Laor wrote:
> > > --- a/qemu/hw/irq.c
> > > +++ b/qemu/hw/irq.c
> > > @@ -30,6 +30,8 @@ struct IRQState {
> > >      int n;
> > >  };
> > >
> > > +uint32_t qemu_irq_acked[NR_IRQ_WORDS];
> >
> > This is absolute rubbish. The whole point of the IRQ framework is that it
> > doesn't assume a single flat IRQ controller.
>
> Thanks for the compliments & the review ...
> I specifically said that I'll move this variable into per-cpu var.

Per-cpu is no better.

> Moreover, the translation between irq line to vector is handled by the
> 'qemu_get_irq_vector' that calls 'irq_controller_get_vector' should take
> care of the translation.
> It works for ioapic, I'm not sure if it works for the flat pic case yet.

Which shows you've completely missed the point.  irq->n is not a globally 
unique identifier. It's a local per-controller index. qemu has targets with 
multiple nested interrupt controllers, anything trying to maintain global or 
per-cpu IRQ lists is fundamentally broken.

> Anyway you're welcome to drift without the patch or provide constructive
> comments.

Well, the patch doesn't even build on non-x86 targets.

> > a new timer will be fired to try inject it again soon (==0.1msec)

If the guest is missing interrupts, the chances of a 0.1ms interval working 
are not great.  Most likely It's either going trigger immediately, or be 
delayed significantly and you're going to end up even further behind. 

If triggering immediately is OK then why not do that all the time?
If triggering immediately is not acceptable then you're still going to loose 
interrupts.

Paul

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/