[Xen-devel] [PATCH v4 03/10] hvm/hpet: Only set comparator or period not both.

[Don Slutz <dslutz@xxxxxxxxxxx>]

The current code sets both.  If setting the comparator also set
comparator64 (the hidden version).

Based on:

software-developers-hpet-spec-1-0a.pdf

A write call should only change comparator or period, not both.

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/hvm/hpet.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 4fd6470..910d87d 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -401,20 +401,8 @@ static int hpet_write(
     case HPET_Tn_CMP(1):
     case HPET_Tn_CMP(2):
         tn = HPET_TN(CMP, addr);
-        if ( timer_is_32bit(h, tn) )
-            new_val = (uint32_t)new_val;
-        h->hpet.timers[tn].cmp = new_val;
-        if ( h->hpet.timers[tn].config & HPET_TN_SETVAL )
-            /*
-             * When SETVAL is one, software is able to "directly set a periodic
-             * timer's accumulator."  That is, set the comparator without
-             * adjusting the period.  Much the same as just setting the
-             * comparator on an enabled one-shot timer.
-             *
-             * This configuration bit clears when the comparator is written.
-             */
-            h->hpet.timers[tn].config &= ~HPET_TN_SETVAL;
-        else if ( timer_is_periodic(h, tn) )
+        if ( timer_is_periodic(h, tn) &&
+             !(h->hpet.timers[tn].config & HPET_TN_SETVAL) )
         {
             /*
              * Clamp period to reasonable min/max values:
@@ -426,7 +414,25 @@ static int hpet_write(
             new_val &= (timer_is_32bit(h, tn) ? ~0u : ~0ull) >> 1;
             h->hpet.period[tn] = new_val;
         }
-        h->hpet.comparator64[tn] = new_val;
+        else
+        {
+            /*
+             * When SETVAL is one, software is able to "directly set
+             * a periodic timer's accumulator."  That is, set the
+             * comparator without adjusting the period.  Much the
+             * same as just setting the comparator on an enabled
+             * one-shot timer.
+             *
+             * This configuration bit clears when the comparator is
+             * written.
+             */
+            h->hpet.timers[tn].config &= ~HPET_TN_SETVAL;
+            h->hpet.comparator64[tn] = new_val;
+            /* truncate if timer is in 32 bit mode */
+            if ( timer_is_32bit(h, tn) )
+                new_val = (uint32_t)new_val;
+            h->hpet.timers[tn].cmp = new_val;
+        }
         if ( hpet_enabled(h) && timer_enabled(h, tn) )
             set_restart_timer(tn);
         break;
-- 
1.8.4


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