[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: "Slutz, Donald Christopher" <dslutz@xxxxxxxxxxx>
  • Date: Wed, 12 Mar 2014 19:23:17 +0000
  • Accept-language: en-US
  • Cc: Keir Fraser <keir@xxxxxxx>, "xen-devel@xxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxx>
  • Delivery-date: Wed, 12 Mar 2014 19:23:49 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: AQHPMxywdnc9W1NXnEmyWxt8HgdCEZrJJ+qAgAChDwCAAO1JgP///s2AgBGiwYCAASTSAIAAaeV5
  • Thread-topic: [PATCH 1/1] hpet: Act more like real hardware

I am having e-mail issues and so may not be formatted right.
From: Jan Beulich [JBeulich@xxxxxxxx]
Sent: Wednesday, March 12, 2014 4:48 AM
To: Slutz, Donald Christopher
Cc: xen-devel@xxxxxxxxxxxxx; Keir Fraser
Subject: Re: [PATCH 1/1] hpet: Act more like real hardware

>>> On 11.03.14 at 20:20, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
> ping.

This hasn't been forgotten, but I am still waiting for at least on of
two things to happen:

- a v2 submission with an extended description
- comments/opinions by other people, particularly ones who
  were involved with the earlier modifications to the code in

If neither happens, my reservations stand against changing the
code in question which has been in place for over 6 years without
any complaints, and could only be eliminated by demonstrating in
practice that either Vista didn't work properly anymore after the
second of the changes you pointed out (as your extended
explanation seems to suggest) or that Vista as well as other
Windows versions continue to work flawlessly (or at least no
worse than before) with your change in place.


While working on v2, I found an error in my statements.  Digging into
it more leads me to a new patch:

From fa0e4f13237acff73443644e49ecabf549d72a40 Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@xxxxxxxxxxx>
Date: Wed, 12 Mar 2014 14:32:58 -0400
Subject: [PATCH] hpet: Only do HPET_TINY_TIME_SPAN check for one shot 32 bit

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

The only way that I can see the patch (c/s 13495:e2539ab3580a commit
f545359b1c54f59be9d7c27112a68c51c45b06b5 "[HVM] Fix slow wallclock
in x64 Vista") fixed the reported issue is by assuming that:

1) x64 Vista wallclock is using hpet in 32bit oneshot mode.

2) very offen the diff would be in the range (max 32 -) to
   -0.9765625 milliseconds.  This is the range that converts from a
   past time into a future time.

Linux is expecting to start a periodic timer and does not expect the
1 interrupt to be 68,718,500,160 ns or more and the rest to be
1,000,000 ns between.

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
 xen/arch/x86/hvm/hpet.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 4324b52..4b7a396 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -229,6 +229,7 @@ static void hpet_set_timer(HPETState *h, unsigned int tn)
     diff = tn_cmp - cur_tick;
+    oneshot = !timer_is_periodic(h, tn);
      * Detect time values set in the past. This is hard to do for 32-bit
@@ -237,7 +238,8 @@ static void hpet_set_timer(HPETState *h, unsigned int tn)
      * by looking for a 'small' time value in the past.
     if ( (int64_t)diff < 0 )
-        diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
+        diff = (timer_is_32bit(h, tn) && oneshot &&
+                (-diff > HPET_TINY_TIME_SPAN))
             ? (uint32_t)diff : 0;
     if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
@@ -254,7 +256,6 @@ static void hpet_set_timer(HPETState *h, unsigned int tn)
      * have elapsed between the time the comparator was written and the timer
      * being enabled (now).
-    oneshot = !timer_is_periodic(h, tn);
     create_periodic_time(vhpet_vcpu(h), &h->pt[tn],
                          hpet_tick_to_ns(h, diff),
                          oneshot ? 0 : hpet_tick_to_ns(h, h->hpet.period[tn]),

Which does fix the linux issue I found and should not break x64 Vista.  I could 
also do this another way (why this as a v2 has not been sent) by adding code 
that says the diff must be less then or equal to the periodic time.  However 
that has a wider impact and I am not 100% sure what real hardware does in this 

I will be doing some long term windows testing (Vista aka windows server 2003 
and Windows server 2012) to see how the wall clock time goes when w32_time is 
not enabled.

   -Don Slutz

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.