Re: [Qemu-devel] [PATCH] Don't enable a HPET timer if HPET is disabled

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Feb 21, 2014 at 09:37:23PM -0700, Matt Lupfer wrote:
> A HPET timer can be started when HPET is not yet
> enabled. This will not generate an interrupt
> to the guest, but causes problems when HPET is later
> enabled.
> 
> A timer that is created and expires at least once before
> HPET is enabled will have an initialized comparator based
> on a hpet_offset of 0 (uninitialized). When HPET is
> enabled, hpet_set_timer() is called a second time, which
> modifies the timer expiry to a time based on the
> difference between current ticks (measured with the
> newly initialized hpet_offset) and the timer's
> comparator (which was generated before hpet_offset was
> initialized). This results in a long period of no HPET
> timer ticks.
> 
> When this occurs with a CentOS 5.x guest, the guest
> may not receive timer interrupts during its narrow
> timer check window and panic on boot.
> 
> Signed-off-by: Matt Lupfer <mlupfer@ddn.com>

Queued for 2.0, thanks everyone.

> ---
>  hw/timer/hpet.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 1264dfd..e15d6bc 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -506,7 +506,8 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
>                  timer->cmp = (uint32_t)timer->cmp;
>                  timer->period = (uint32_t)timer->period;
>              }
> -            if (activating_bit(old_val, new_val, HPET_TN_ENABLE)) {
> +            if (activating_bit(old_val, new_val, HPET_TN_ENABLE) &&
> +                hpet_enabled(s)) {
>                  hpet_set_timer(timer);
>              } else if (deactivating_bit(old_val, new_val, HPET_TN_ENABLE)) {
>                  hpet_del_timer(timer);
> -- 
> 1.8.5.3