grub-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] Make pmtimer tsc calibration not take 51 seconds to fail.


From: Daniel Kiper
Subject: Re: [PATCH] Make pmtimer tsc calibration not take 51 seconds to fail.
Date: Thu, 18 Jan 2018 23:52:53 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Jan 16, 2018 at 01:16:17PM -0500, Peter Jones wrote:
> On my laptop running at 2.4GHz, if I run a VM where tsc calibration
> using pmtimer will fail presuming a broken pmtimer, it takes ~51 seconds
> to do so (as measured with the stopwatch on my phone), with a tsc delta
> of 0x1cd1c85300, or around 125 billion cycles.
>
> If instead of trying to wait for 5-200ms to show up on the pmtimer, we try
> to wait for 5-200us, it decides it's broken in ~0x7998f9e TSCs, aka ~2
> million cycles, or more or less instantly.
>
> Additionally, this reading the pmtimer was returning 0xffffffff anyway,
> and that's obviously an invalid return.  I've added a check for that and
> 0 so we don't bother waiting for the test if what we're seeing is dead
> pins with no response at all.
>
> Signed-off-by: Peter Jones <address@hidden>
> ---
>  grub-core/kern/i386/tsc_pmtimer.c | 43 
> ++++++++++++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/grub-core/kern/i386/tsc_pmtimer.c 
> b/grub-core/kern/i386/tsc_pmtimer.c
> index c9c36169978..609402b8376 100644
> --- a/grub-core/kern/i386/tsc_pmtimer.c
> +++ b/grub-core/kern/i386/tsc_pmtimer.c
> @@ -38,30 +38,53 @@ grub_pmtimer_wait_count_tsc (grub_port_t pmtimer,
>    grub_uint64_t start_tsc;
>    grub_uint64_t end_tsc;
>    int num_iter = 0;
> +  int bad_reads = 0;
>
> -  start = grub_inl (pmtimer) & 0xffffff;
> +  start = grub_inl (pmtimer) & 0x3fff;

I am not sure why you are changing this to 0x3fff...

>    last = start;
>    end = start + num_pm_ticks;
>    start_tsc = grub_get_tsc ();
>    while (1)
>      {
> -      cur = grub_inl (pmtimer) & 0xffffff;

What about 24-bit timers? I would leave this here...

> +      cur = grub_inl (pmtimer);
> +
> +      /* If we get 10 reads in a row that are obviously dead pins, there's no
> +      reason to do this thousands of times.
> +       */
> +      if (cur == 0xffffffff || cur == 0)

...and here I would check for 0xffffff and 0.

> +     {
> +       bad_reads++;
> +       grub_dprintf ("pmtimer", "cur: 0x%08x bad_reads: %d\n", cur, 
> bad_reads);
> +
> +       if (bad_reads == 10)
> +         return 0;
> +     }
> +      else if (bad_reads)
> +     bad_reads = 0;

Do we really need to reset this?

> +      cur &= 0x3fff;
> +
>        if (cur < last)
> -     cur |= 0x1000000;
> +     cur |= 0x4000;
>        num_iter++;
>        if (cur >= end)
>       {
>         end_tsc = grub_get_tsc ();
> +       grub_dprintf ("pmtimer", "tsc delta is 0x%016lx\n",
> +                     end_tsc - start_tsc);
>         return end_tsc - start_tsc;
>       }
> -      /* Check for broken PM timer.
> -      50000000 TSCs is between 5 ms (10GHz) and 200 ms (250 MHz)
> -      if after this time we still don't have 1 ms on pmtimer, then
> -      pmtimer is broken.
> +      /* Check for broken PM timer.  5000 TSCs is between 5us (10GHz) and
                                                             ^^^ 500ns?

> +      200us (250 MHz).  If after this time we still don't have 1us on
         ^^^^^ 20us?

> +      pmtimer, then pmtimer is broken.
>         */
> -      if ((num_iter & 0xffffff) == 0 && grub_get_tsc () - start_tsc > 
> 5000000) {
> -     return 0;
> -      }
> +      end_tsc = grub_get_tsc();
> +      if ((num_iter & 0x3fff) == 0 && end_tsc - start_tsc > 5000)

Why 0x3fff here which means, AIUI, 4000 iterations?

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]