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

Re: [Xen-devel] [PATCH v3 11/11] hvm/hpet: handle 1st period special


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: "Slutz, Donald Christopher" <dslutz@xxxxxxxxxxx>
  • Date: Sat, 26 Apr 2014 14:10:13 +0000
  • Accept-language: en-US
  • Cc: Keir Fraser <keir@xxxxxxx>, "xen-devel@xxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxx>
  • Delivery-date: Sat, 26 Apr 2014 14:18:43 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: AQHPWmSEPOY59iaHnEOHKOYHKc53apsikyaAgAGtmoA=
  • Thread-topic: [PATCH v3 11/11] hvm/hpet: handle 1st period special

On 04/25/14 08:32, Jan Beulich wrote:
>>>> On 17.04.14 at 19:43, <dslutz@xxxxxxxxxxx> wrote:
>> v3:
>>    Better commit message.
>>    More setting of first_mc64 & first_enabled when needed.
>>    Switch to bool_t.
>>
>>   xen/arch/x86/hvm/hpet.c       | 83 
>> ++++++++++++++++++++++++++++++++++++-------
>>   xen/include/asm-x86/hvm/vpt.h |  2 ++
>>   2 files changed, 73 insertions(+), 12 deletions(-)
> I still can't help myself thinking that this must be achievable with less
> special casing - I just can't imagine that hardware also has this huge
> an amount of special case logic just for the first period.

The hardware is simple. And the code to simulate it:

do_count()
{
     mc++;
     if ( mc == comparator )
         handle_int();
         if ( periodic )
             comparator += period;
    }
}

is also simple.  However this way has a very high cost in that you have
to call do_count at the frequency of the master clock.  Instead of the
simple simulation, the current code uses arithmetic to compute the
current state of mc and comparator.  This is where the complexity comes
in.  During answering of comments on #8 (signed divide) and #10
(prevent mc equal), I noticed that what I coded here only works for
expected case where mc is close to zero and comparator starts
out > mc in unsigned.  I will be sending a v4 with this issue fixed.
New code (untested):

+        if ( h->hpet.first_enabled[tn] )
+        {
+            int64_t mc_diff = mc - h->hpet.first_mc64[tn];
+
+            if ( mc_diff >= 0 && mc_diff < comparator - h->hpet.first_mc64[tn] 
)


was:

+            if ( mc >= h->hpet.first_mc64[tn] && mc < comparator )


>> @@ -208,12 +216,18 @@ static void hpet_stop_timer(HPETState *h, unsigned int 
>> tn,
>>    * 1/(2^10) second, namely, 0.9765625 milliseconds */
>>   #define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
>>   
>> +static void hpet_time_fired(struct vcpu *v, void *priv)
>> +{
>> +    bool_t *first_enabled_p = (bool_t *)priv;
> Pointless cast.

Will drop.

    -Don Slutz

> Jan
>

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


 


Rackspace

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