Re: [RFC v3 3/4] cpu-timers, icount: new modules - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: "Claudio Fontana" <cfontana@suse.de>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>
Cc: Laurent Vivier <lvivier@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Roman Bolshakov <r.bolshakov@yadro.com>,
	haxm-team@intel.com, Wenchao Wang <wenchao.wang@intel.com>,
	Sunil Muthuswamy <sunilmut@microsoft.com>,
	Richard Henderson <rth@twiddle.net>,
	Colin Xu <colin.xu@intel.com>
Subject: Re: [RFC v3 3/4] cpu-timers, icount: new modules
Date: Mon, 25 May 2020 17:16:27 +0200	[thread overview]
Message-ID: <1a4eb7bb-9b92-7536-9de3-a7e56afd9da4@redhat.com> (raw)
In-Reply-To: <20200525145440.29728-4-cfontana@suse.de>

On 5/25/20 4:54 PM, Claudio Fontana wrote:
> refactoring of cpus.c continues with cpu timer state extraction.
> 
> cpu-timers: responsible for the cpu timers state, and for access to
> cpu clocks and ticks.
> 
> icount: counts the TCG instructions executed. As such it is specific to
> the TCG accelerator. Therefore, it is built only under CONFIG_TCG.
> 
> One complication is due to qtest, which misuses icount to warp time
> (qtest_clock_warp). In order to solve this problem, detach instead qtest
> from icount, and use a trivial separate counter for it.
> 
> This requires fixing assumptions scattered in the code that
> qtest_enabled() implies icount_enabled().
> 
> No functionality change.
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  accel/qtest.c                |   6 +-
>  accel/tcg/cpu-exec.c         |  43 ++-
>  accel/tcg/tcg-all.c          |   7 +-
>  accel/tcg/translate-all.c    |   3 +-
>  docs/replay.txt              |   6 +-
>  exec.c                       |   4 -
>  hw/core/ptimer.c             |   6 +-
>  hw/i386/x86.c                |   1 +
>  include/exec/cpu-all.h       |   4 +
>  include/exec/exec-all.h      |   4 +-
>  include/qemu/timer.h         |  22 +-
>  include/sysemu/cpu-timers.h  |  72 +++++
>  include/sysemu/cpus.h        |  12 +-
>  include/sysemu/qtest.h       |   2 +
>  include/sysemu/replay.h      |   4 +-
>  replay/replay.c              |   6 +-
>  softmmu/Makefile.objs        |   2 +
>  softmmu/cpu-timers.c         | 267 ++++++++++++++++
>  softmmu/cpus.c               | 731 +------------------------------------------
>  softmmu/icount.c             | 496 +++++++++++++++++++++++++++++
>  softmmu/qtest.c              |  34 +-
>  softmmu/timers-state.h       |  45 +++
>  softmmu/vl.c                 |   8 +-
>  stubs/Makefile.objs          |   3 +-
>  stubs/clock-warp.c           |   4 +-
>  stubs/cpu-get-clock.c        |   3 +-
>  stubs/cpu-get-icount.c       |  21 --
>  stubs/icount.c               |  22 ++
>  stubs/qemu-timer-notify-cb.c |   8 +
>  stubs/qtest.c                |   5 +
>  target/alpha/translate.c     |   3 +-
>  target/arm/helper.c          |   7 +-
>  target/riscv/csr.c           |   8 +-
>  tests/ptimer-test-stubs.c    |   7 +-
>  tests/test-timed-average.c   |   2 +-
>  util/main-loop.c             |   4 +-
>  util/qemu-timer.c            |  12 +-
>  37 files changed, 1062 insertions(+), 832 deletions(-)
>  create mode 100644 include/sysemu/cpu-timers.h
>  create mode 100644 softmmu/cpu-timers.c
>  create mode 100644 softmmu/icount.c
>  create mode 100644 softmmu/timers-state.h
>  delete mode 100644 stubs/cpu-get-icount.c
>  create mode 100644 stubs/icount.c
>  create mode 100644 stubs/qemu-timer-notify-cb.c

Similarly I'd rather see this patch comes before your current #1 "move
softmmu only files from root". Anyway wait for other reviewers before
spending time to reorder the series.