Re: [Xen-devel] [PATCH 1/1] hpet: Act more like real hardware

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.14 at 19:00, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
> Currently in 32 bit mode the routine hpet_set_timer() will convert a
> time in the past to a time in future.  This is done by the uint32_t
> cast of diff.
> 
> Even without this issue, hpet_tick_to_ns() does not support past
> times.
> 
> Real hardware does not support past times.
> 
> So just do the same thing in 32 bit mode as 64 bit mode.

While the change looks valid at the first glance, what I'm missing
is an explanation of how the problem that the introduction of this
fixed (c/s 13495:e2539ab3580a "[HVM] Fix slow wallclock in x64
Vista") is now being taken care of (or why this is of no concern).
That's pretty relevant considering for how long this code has been
there without causing (known) problems to anyone.

Jan

> Without this change it is possible for an HVM guest running linux to
> get the message:
> 
> ..MP-BIOS bug: 8254 timer not connected to IO-APIC
> 
> On the guest console(s); and will panic.
> 
> Also Xen hypervisor console with be flooded with:
> 
> vioapic.c:352:d1 Unsupported delivery mode 7
> vioapic.c:352:d1 Unsupported delivery mode 7
> vioapic.c:352:d1 Unsupported delivery mode 7
> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/hpet.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
> index 4324b52..14b1a39 100644
> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -197,10 +197,6 @@ static void hpet_stop_timer(HPETState *h, unsigned int 
> tn)
>      hpet_get_comparator(h, tn);
>  }
>  
> -/* the number of HPET tick that stands for
> - * 1/(2^10) second, namely, 0.9765625 milliseconds */
> -#define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
> -
>  static void hpet_set_timer(HPETState *h, unsigned int tn)
>  {
>      uint64_t tn_cmp, cur_tick, diff;
> @@ -231,14 +227,11 @@ static void hpet_set_timer(HPETState *h, unsigned int 
> tn)
>      diff = tn_cmp - cur_tick;
>  
>      /*
> -     * Detect time values set in the past. This is hard to do for 32-bit
> -     * comparators as the timer does not have to be set that far in the 
> future
> -     * for the counter difference to wrap a 32-bit signed integer. We fudge
> -     * by looking for a 'small' time value in the past.
> +     * Detect time values set in the past. Since hpet_tick_to_ns() does
> +     * not handle this, use 0 for both 64 and 32 bit mode.
>       */
>      if ( (int64_t)diff < 0 )
> -        diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
> -            ? (uint32_t)diff : 0;
> +        diff = 0;
>  
>      if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
>          /* if LegacyReplacementRoute bit is set, HPET specification requires
> -- 
> 1.8.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel