| Message ID | 20170405132503.32125-10-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 05/04/2017 15:25, Alex Bennée wrote: > +{ > + int64_t executed = cpu_get_icount_executed(cpu); > + cpu->icount_budget -= executed; > + atomic_add(&timers_state.qemu_icount, executed); > +} Since there's only one writer, it's also okay to do atomic_set(&timers_state.qemu_icount, atomic_read(&timers_state.qemu_icount) + executed); (also not just faster, but also simpler to turn into TCG code if we do the update in gen_io_start). Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 05/04/2017 15:25, Alex Bennée wrote: >> +{ >> + int64_t executed = cpu_get_icount_executed(cpu); >> + cpu->icount_budget -= executed; >> + atomic_add(&timers_state.qemu_icount, executed); >> +} > > Since there's only one writer, it's also okay to do > > atomic_set(&timers_state.qemu_icount, > atomic_read(&timers_state.qemu_icount) + executed); > > (also not just faster, but also simpler to turn into TCG code if we do > the update in gen_io_start). OK fair enough. Annoyingly I've just noticed this breaks for 32 bit hosts because we might not have CONFIG_ATOMIC64. What's the best approach? #ifdef and handwave? -- Alex Bennée
On 05/04/2017 16:34, Alex Bennée wrote: >> On 05/04/2017 15:25, Alex Bennée wrote: >>> +{ >>> + int64_t executed = cpu_get_icount_executed(cpu); >>> + cpu->icount_budget -= executed; >>> + atomic_add(&timers_state.qemu_icount, executed); >>> +} >> Since there's only one writer, it's also okay to do >> >> atomic_set(&timers_state.qemu_icount, >> atomic_read(&timers_state.qemu_icount) + executed); >> >> (also not just faster, but also simpler to turn into TCG code if we do >> the update in gen_io_start). > > OK fair enough. Annoyingly I've just noticed this breaks for 32 bit > hosts because we might not have CONFIG_ATOMIC64. What's the best > approach? #ifdef and handwave? For 2.9 yes... For 2.10 I'm going to add a "Stat64" type that can compute max or add of 63-bit values, and blocks on the read side if a doubleword change is in progress. Paolo
diff --git a/cpus.c b/cpus.c index 88eabdc19f..71c3baba87 100644 --- a/cpus.c +++ b/cpus.c @@ -232,12 +232,24 @@ static int64_t cpu_get_icount_executed(CPUState *cpu) return cpu->icount_budget - (cpu->icount_decr.u16.low + cpu->icount_extra); } +/* + * Update the global shared timer_state.qemu_icount to take into + * account executed instructions. This is done by the TCG vCPU + * thread so the main-loop can see time has moved forward. + */ +void cpu_update_icount(CPUState *cpu) +{ + int64_t executed = cpu_get_icount_executed(cpu); + cpu->icount_budget -= executed; + atomic_add(&timers_state.qemu_icount, executed); +} + int64_t cpu_get_icount_raw(void) { int64_t icount; CPUState *cpu = current_cpu; - icount = timers_state.qemu_icount; + icount = atomic_read(&timers_state.qemu_icount); if (cpu && cpu->running) { if (!cpu->can_do_io) { fprintf(stderr, "Bad icount read\n"); @@ -1220,7 +1232,7 @@ static void process_icount_data(CPUState *cpu) { if (use_icount) { /* Account for executed instructions */ - timers_state.qemu_icount += cpu_get_icount_executed(cpu); + cpu_update_icount(cpu); /* Reset the counters */ cpu->icount_decr.u16.low = 0; diff --git a/include/qemu/timer.h b/include/qemu/timer.h index e1742f2f3d..8a1eb74839 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -869,6 +869,7 @@ int64_t cpu_get_icount_raw(void); int64_t cpu_get_icount(void); int64_t cpu_get_clock(void); int64_t cpu_icount_to_ns(int64_t icount); +void cpu_update_icount(CPUState *cpu); /*******************************************/ /* host CPU ticks (if available) */
By holding off updates to timer_state.qemu_icount we can run into trouble when the non-vCPU thread needs to know the time. This helper ensures we atomically update timers_state.qemu_icount based on what has been currently executed. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- cpus.c | 16 ++++++++++++++-- include/qemu/timer.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-)