Re: [Qemu-devel] [PATCH for-2.12] cpus.c: ensure running CPU recalculates icount deadlines on timer expiry

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> When we run in TCG icount mode, we calculate the number of instructions
> to execute using tcg_get_icount_limit(), which ensures that we stop
> execution at the next timer deadline. However there is a bug where
> currently we do not recalculate that limit if the guest reprograms
> a timer so that the next deadline moves closer, and so we will
> continue execution until the original limit and fire the timer
> later than we should.
>
> Fix this bug in qemu_timer_notify_cb(): if we are currently running
> a VCPU in icount mode, we simply need to kick it out of the main
> loop and back to tcg_cpu_exec(), where it will recalculate the
> icount limit. If we are not currently running a VCPU, then we
> retain the existing logic for waking up a halted CPU.
>
> Cc: qemu-stable@nongnu.org
> Fixes: https://bugs.launchpad.net/qemu/+bug/1754038
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Thanks to Paolo for tracking down which function needed fixing!
>
>  cpus.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 2e6701795b..38eba8bff3 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -892,11 +892,19 @@ void qemu_timer_notify_cb(void *opaque, QEMUClockType type)
>          return;
>      }
>
> -    if (!qemu_in_vcpu_thread() && first_cpu) {
> +    if (qemu_in_vcpu_thread()) {
> +        /* A CPU is currently running; kick it back out to the
> +         * tcg_cpu_exec() loop so it will recalculate its
> +         * icount deadline immediately.
> +         */
> +        qemu_cpu_kick(current_cpu);

This is only totally true if whatever has caused the re-calculation has
ended the Translation Block, otherwise we won't finish until we've run a
few more instructions. I don't know if this could cause a problem if the
timeout was suddenly less than the remaining instructions in the block.

Anyway wording aside:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> +    } else if (first_cpu) {
>          /* qemu_cpu_kick is not enough to kick a halted CPU out of
>           * qemu_tcg_wait_io_event.  async_run_on_cpu, instead,
>           * causes cpu_thread_is_idle to return false.  This way,
>           * handle_icount_deadline can run.
> +         * If we have no CPUs at all for some reason, we don't
> +         * need to do anything.
>           */
>          async_run_on_cpu(first_cpu, do_nothing, RUN_ON_CPU_NULL);
>      }


--
Alex Bennée