From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: [PATCH v3 03/11] hvm/hpet: Only set comparator or period not both. Date: Thu, 17 Apr 2014 13:42:57 -0400 Message-ID: <1397756585-27091-4-git-send-email-dslutz@verizon.com> References: <1397756585-27091-1-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1397756585-27091-1-git-send-email-dslutz@verizon.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: Keir Fraser , Don Slutz , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 --- v3: Only have 2 blocks of code. Set comparator64 before truncation 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