Re: [Qemu-devel] [RFC PATCH V4 0/6] icount: Implement delay algorithm between guest and host clocks

[Sebastian Tanase <sebastian.tanase@openwide.fr>]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mst@redhat.com, "camille begue"
> <camille.begue@openwide.fr>, qemu-devel@nongnu.org
> Envoyé: Mardi 22 Juillet 2014 16:59:18
> Objet: Re: [RFC PATCH V4 0/6] icount: Implement delay algorithm between guest and host clocks
> 
> Il 22/07/2014 16:02, Sebastian Tanase ha scritto:
> > Yes, QEMU_CLOCK_VIRTUAL counts up from
> > qemu_clock_get_ns(QEMU_CLOCK_REALTIME)
> > on ARM (I have only tested with the versatilepb and vexpress
> > boards).
> 
> That's a bug to fix indeed, then---it should count up from 0 without
> icount, and icount shouldn't affect this.  Thanks for investigating
> it.

Just to be sure I don't missunderstand, when you say "without icount"
you refer to qemu_icount_bias (aka when the vcpu is inactive), right?

Sebastian
> 
> > Supposing the patch that changes vm_clock_warp_start from 0 to -1
> > is accepted,
> 
> ... which shouldn't be a problem,... :)
> 
> > I could use the information in timers_state.cpu_clock_offset
> > instead of recalculating
> > the offset. Besides, given that I only need this particular field
> > from the whole
> > structure, I think I don't have to make timers_state public; I
> > could add a function
> > in cpus.c, for example:
> > 
> >     int64_t cpu_get_clock_offset(void)
> >     {
> >         int64_t ti;
> >         unsigned start;
> > 
> >         do {
> >             start =
> >             seqlock_read_begin(&timers_state.vm_clock_seqlock);
> >             ti = -timers_state.cpu_clock_offset;
> >         } while (seqlock_read_retry(&timers_state.vm_clock_seqlock,
> >         start));
> > 
> >         return ti;
> >     }
> > 
> > that will return the cpu_clock_offset field.
> 
> Indeed what I was proposing is a bit more sloppy.  If you do that,
> you
> have to make the function a bit more general:
> 
>     ti = timers_state.cpu_clock_offset;
>     if (!timers_state.cpu_ticks_enabled) {
>         ti -= get_clock();
>     }
>     ...
> 
>     return -ti;
> 
> even though in cpus.c you'll only be using it when cpu_ticks_enabled
> is
> true.  See cpu_enable_ticks() and cpu_disable_ticks().
> 
> Paolo
>