Re: [Qemu-devel] [RFC PATCH V4 6/6] monitor: Add drift info to 'info jit'

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <address@hidden>
> À: "Sebastian Tanase" <address@hidden>
> Cc: address@hidden, address@hidden, address@hidden, "peter maydell" 
> <address@hidden>,
> address@hidden, address@hidden, address@hidden, address@hidden, 
> address@hidden,
> address@hidden, address@hidden, address@hidden, address@hidden, 
> address@hidden, "camille begue"
> <address@hidden>, address@hidden
> Envoyé: Mardi 22 Juillet 2014 12:19:42
> Objet: Re: [RFC PATCH V4 6/6] monitor: Add drift info to 'info jit'
> 
> Il 22/07/2014 11:58, Sebastian Tanase ha scritto:
> > 
> > -timers_state.cpu_clock_offset contains the offset between the real
> > and virtual clocks.
> > However, when using the value of the virtual clock
> > (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)),
> > qemu_icount_bias already includes this offset because, on ARM,
> > qemu_clock_warp (which
> > then calls icount_warp_rt) is called for the first time in
> > tcg_exec_all, making
> > qemu_icount_bias take the value of
> > qemu_clock_get_ns(QEMU_CLOCK_REALTIME)
> 
> Does this means that QEMU_CLOCK_VIRTUAL counts up from
> qemu_clock_get_ns(QEMU_CLOCK_REALTIME) rather than 0?  This would be
> a
> bug, and it would be good to fix it (by initializing
> vm_clock_warp_start
> to -1).

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

> 
> > A solution to not compute the initial offset in qemu_icount_bias
> > would be to initialize
> > vm_clock_warp_start to -1. The result  will be that
> > qemu_icount_bias will start counting when
> > the vcpu goes from active to inactive. At that time,
> > vm_clock_warp_start will already store the realtime clock
> > value and a timer on the real clock will be set to expire at clock
> > + deadline, making qemu_icount_bias increment
> > by deadline.
> 
> That would be correct.
> 
> > A consequence of initializing vm_clock_warp_start to -1 is also
> > the fact that we'll skip the first check for a virtual expired
> > timer. As I mentioned above, in ARM case,
> > it's not dangerous because there are no timers active the first
> > time we perform this check. However, this
> > is just a potential scenario and I cannot guarantee that on other
> > target architectures there won't be
> > an expired timer pending the first time we check.
> 
> Like a timer that expired in the past?  That would be caught in
> tcg_cpu_exec, when it initializes the decrementer to
> qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL).
> 
> > So, do you think it is worth taking this solution into account or
> > it will cause more harm than good?
> 
> Yes, even more so.  If we add workarounds to complicated code, it
> will
> become even more complicated.
> 
> Paolo
> 

I'll make a small patch that initializes vm_clock_warp_start to -1.


Sebastian