[2/2] x86/time: report correct frequency of Xen PV clocksource
diff mbox series

Message ID 1580830825-18767-3-git-send-email-igor.druzhinin@citrix.com
State New
Headers show
Series
  • PV shim timekeeping fixes
Related show

Commit Message

Igor Druzhinin Feb. 4, 2020, 3:40 p.m. UTC
The value of the counter represents the number of nanoseconds
since host boot. That means the correct frequency is always 1GHz.

This inconsistency caused time to go slower in PV shim on most
platforms.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
 xen/arch/x86/time.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

Comments

Roger Pau Monné Feb. 4, 2020, 5:28 p.m. UTC | #1
On Tue, Feb 04, 2020 at 03:40:25PM +0000, Igor Druzhinin wrote:
> The value of the counter represents the number of nanoseconds
> since host boot. That means the correct frequency is always 1GHz.
> 
> This inconsistency caused time to go slower in PV shim on most
> platforms.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

With one nit below.

> ---
>  xen/arch/x86/time.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 7e7a62e..95840c4 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -567,27 +567,12 @@ static struct platform_timesource __initdata plt_tsc =
>   */
>  static uint64_t xen_timer_last;
>  
> -static uint64_t xen_timer_cpu_frequency(void)
> -{
> -    struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
> -    uint64_t freq;
> -
> -    freq = 1000000000ULL << 32;
> -    do_div(freq, info->tsc_to_system_mul);
> -    if ( info->tsc_shift < 0 )
> -        freq <<= -info->tsc_shift;
> -    else
> -        freq >>= info->tsc_shift;
> -
> -    return freq;
> -}
> -
>  static int64_t __init init_xen_timer(struct platform_timesource *pts)
>  {
>      if ( !xen_guest )
>          return 0;
>  
> -    pts->frequency = xen_timer_cpu_frequency();
> +    pts->frequency = 1000000000ULL;

You can init this field below at declaration, since it's a static
value.

Thanks, Roger.
Igor Druzhinin Feb. 4, 2020, 9:27 p.m. UTC | #2
On 04/02/2020 17:28, Roger Pau Monné wrote:
> On Tue, Feb 04, 2020 at 03:40:25PM +0000, Igor Druzhinin wrote:
>> The value of the counter represents the number of nanoseconds
>> since host boot. That means the correct frequency is always 1GHz.
>>
>> This inconsistency caused time to go slower in PV shim on most
>> platforms.
>>
>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Actually, this patch is buggy and causes a system to always detect 1GHz
processor. I'll send a v2.

Igor

Patch
diff mbox series

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 7e7a62e..95840c4 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -567,27 +567,12 @@  static struct platform_timesource __initdata plt_tsc =
  */
 static uint64_t xen_timer_last;
 
-static uint64_t xen_timer_cpu_frequency(void)
-{
-    struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
-    uint64_t freq;
-
-    freq = 1000000000ULL << 32;
-    do_div(freq, info->tsc_to_system_mul);
-    if ( info->tsc_shift < 0 )
-        freq <<= -info->tsc_shift;
-    else
-        freq >>= info->tsc_shift;
-
-    return freq;
-}
-
 static int64_t __init init_xen_timer(struct platform_timesource *pts)
 {
     if ( !xen_guest )
         return 0;
 
-    pts->frequency = xen_timer_cpu_frequency();
+    pts->frequency = 1000000000ULL;
 
     return pts->frequency;
 }