[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