Re: [Xen-devel] [PATCH 1/4] x86/APIC: adjust types and comments in calibrate_APIC_clock()

2020-03-13 Thread Andrew Cooper
On 13/03/2020 09:25, Jan Beulich wrote:
> First and foremost the comment talking about potential underflow being
> taken care of by using signed long type variables was true only on
> 32-bit, which we've not been supporting for quite some time. Drop the
> comment and change all involved types to unsigned. Take the opportunity
> and also replace bus_cycle's fixed width type.
>
> Additionally there's no point using an "arbitrary (but long enough)
> timeout" here. Just use the maximum possible value; Linux does so too,
> just as an additional data point.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/4] x86/APIC: adjust types and comments in calibrate_APIC_clock()

2020-03-13 Thread Jan Beulich
First and foremost the comment talking about potential underflow being
taken care of by using signed long type variables was true only on
32-bit, which we've not been supporting for quite some time. Drop the
comment and change all involved types to unsigned. Take the opportunity
and also replace bus_cycle's fixed width type.

Additionally there's no point using an "arbitrary (but long enough)
timeout" here. Just use the maximum possible value; Linux does so too,
just as an additional data point.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1207,21 +1207,19 @@ static void wait_tick_pvh(void)
 static int __init calibrate_APIC_clock(void)
 {
 unsigned long long t1, t2;
-long tt1, tt2;
-long result;
-int i;
+unsigned long tt1, tt2, result;
+unsigned int i;
 unsigned long bus_freq; /* KAF: pointer-size avoids compile warns. */
-u32 bus_cycle;  /* length of one bus cycle in pico-seconds */
-const int LOOPS = HZ/10;
+unsigned int bus_cycle; /* length of one bus cycle in pico-seconds */
+const unsigned int LOOPS = HZ/10;
 
 apic_printk(APIC_VERBOSE, "calibrating APIC timer ...\n");
 
 /*
- * Put whatever arbitrary (but long enough) timeout
- * value into the APIC clock, we just want to get the
- * counter running for calibration.
+ * Setup the APIC counter to maximum. There is no way the lapic
+ * can underflow in the 100ms detection time frame.
  */
-__setup_APIC_LVTT(10);
+__setup_APIC_LVTT(0x);
 
 if ( !xen_guest )
 /*
@@ -1251,14 +1249,6 @@ static int __init calibrate_APIC_clock(v
 tt2 = apic_read(APIC_TMCCT);
 t2 = rdtsc_ordered();
 
-/*
- * The APIC bus clock counter is 32 bits only, it
- * might have overflown, but note that we use signed
- * longs, thus no extra care needed.
- *
- * underflown to be exact, as the timer counts down ;)
- */
-
 result = (tt1-tt2)*APIC_DIVISOR/LOOPS;
 
 apic_printk(APIC_VERBOSE, ". CPU clock speed is %ld.%04ld MHz.\n",


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel