From: "Pavel Dovgaluk" <Pavel.Dovgaluk@ispras.ru>
References: <20141107103123.6136.18545.stgit@PASHA-ISP>
	<20141107103207.6136.22294.stgit@PASHA-ISP>
	<545CAAC0.7060904@redhat.com>
	<003b01cffa7f$06618a40$13249ec0$@Dovgaluk@ispras.ru>
	<545CB0C5.5080107@greensocs.com>
In-Reply-To: <545CB0C5.5080107@greensocs.com>
Date: Tue, 11 Nov 2014 12:41:40 +0300
Message-ID: <000c01cffd93$b2225cd0$16671670$@Dovgaluk@ispras.ru>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: 7bit
Content-Language: ru
Subject: Re: [Qemu-devel] [RFC PATCH v4 07/25] icount: implement icount
	requesting
To: 'Frederic Konrad' <fred.konrad@greensocs.com>, 'Paolo Bonzini' <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, alex.bennee@linaro.org, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, afaerber@suse.de

> From: Frederic Konrad [mailto:fred.konrad@greensocs.com]
> On 07/11/2014 12:36, Pavel Dovgaluk wrote:
> >> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> >> On 07/11/2014 11:32, Pavel Dovgalyuk wrote:
> >>> Replay uses number of executed instructions to determine corrent events
> >>> injection moments. This patch introduces new function for querying the
> >>> instructions counter.
> >>>
> >>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> >>> ---
> >>>   cpus.c               |   26 +++++++++++++++++++++++---
> >>>   include/qemu/timer.h |    1 +
> >>>   2 files changed, 24 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/cpus.c b/cpus.c
> >>> index 7e8c507..2ec6d75 100644
> >>> --- a/cpus.c
> >>> +++ b/cpus.c
> >>> @@ -136,8 +136,7 @@ typedef struct TimersState {
> >>>
> >>>   static TimersState timers_state;
> >>>
> >>> -/* Return the virtual CPU time, based on the instruction counter.  */
> >>> -static int64_t cpu_get_icount_locked(void)
> >>> +static int64_t cpu_get_instructions_counter_locked(void)
> >>>   {
> >>>       int64_t icount;
> >>>       CPUState *cpu = current_cpu;
> >>> @@ -145,10 +144,31 @@ static int64_t cpu_get_icount_locked(void)
> >>>       icount = timers_state.qemu_icount;
> >>>       if (cpu) {
> >>>           if (!cpu_can_do_io(cpu)) {
> >>> -            fprintf(stderr, "Bad clock read\n");
> >>> +            fprintf(stderr, "Bad icount read\n");
> >>> +            exit(1);
> >>>           }
> >>>           icount -= (cpu->icount_decr.u16.low + cpu->icount_extra);
> >>>       }
> >>> +    return icount;
> >>> +}
> >>> +
> >>> +int64_t cpu_get_instructions_counter(void)
> >>> +{
> >>> +    /* This function calls are synchnonized to timer changes,
> >>> +       calling cpu_get_instructions_counter_locked without lock is safe */
> >>> +    int64_t icount = timers_state.qemu_icount;
> >>> +    CPUState *cpu = current_cpu;
> >>> +
> >>> +    if (cpu) {
> >>> +        icount -= (cpu->icount_decr.u16.low + cpu->icount_extra);
> >>> +    }
> >>> +    return icount;
> >>> +}
> >> This is the same as Frederic's QEMU_CLOCK_ICOUNT clock, right?  Any
> >> reason why one is better than the other?
> > Not really. Frederic's QEMU_CLOCK_ICOUNT is measured in nanoseconds.
> > And this counter is measured in instructions.
> >
> So you transformed your implementation to use icount?

Right, new implementation uses icount.

> I guess you replay event's according to the current instruction?
> I implemented icount clock with the QEMUTimer API so a callback can be
> notified during the replay at the right instruction.

We cannot use timers, because there are synchronization events 
(like exceptions, interrupts, timer reads) that control the execution correctness.

> By the way do you have a git tree somewhere where I can get the patches?

No, there is no public repository. There are only the patches in the mail list.

Pavel Dovgalyuk