From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1jebwt-0005V2-I8 for mharc-grub-devel@gnu.org; Fri, 29 May 2020 06:09:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54008) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jebwr-0005TI-Dg for grub-devel@gnu.org; Fri, 29 May 2020 06:09:09 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:34693 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1jebwp-0001CM-MR for grub-devel@gnu.org; Fri, 29 May 2020 06:09:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590746946; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Mls0297D9fUE2hlfXSMsiyJVC00nsB7yvWKyRLHan98=; b=PxDEuQmWW3XJf9nEHjv7JXog7Xyeppb8Dn21obUfVlRF6RM5O2xUDTPDsE7aKtGC+ceBJX pknkMW+UDYvM9FOtNcBjBjq1BI9tVXXblnpHq7UyXbo4cGhn+QAlHvHp7itks3/nBLk03T 8FX+6XJRKbhTrP8DiS8ntSlP0veiDXg= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-235-pUrw4JTWNnyFaNrMtmnYdQ-1; Fri, 29 May 2020 06:09:02 -0400 X-MC-Unique: pUrw4JTWNnyFaNrMtmnYdQ-1 Received: by mail-wr1-f72.google.com with SMTP id 3so843953wrs.10 for ; Fri, 29 May 2020 03:09:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Mls0297D9fUE2hlfXSMsiyJVC00nsB7yvWKyRLHan98=; b=eSFQscFtClk5FHGQsZFb6VU56FBthPxrBZPSL/+yOGvx9vgT6NzQa3N22vnKxpsa0N ORj9Tr8PuCOtDTcmB2eiuSFF/615Da2V8OfQmYHXF46le25Vo6MuMPaQ7yJU6wuBHqW6 NY/qKnitezuAx3xK2JJjLT+zzVWwGHZHaS34aJ7ekmCKftrObB4X+zwNTZKp7mYuWfXt 4HNgjH1lRKleVGpqq6QQprLyoOdMDgo3ZiqwSbud2nEgnd4AFz/JxD3kgeKEDftHkzQU GXNU5fifPjN5b+TysgFlV7dB3vPGkbFUCl/mo0plE25jnI+LyEKCjI6gs9FQzlfOaO+t wNiQ== X-Gm-Message-State: AOAM532H7Pueq3IvySIA72vcXTHY8cdkKvRx69AE+U2rfoduuIve3lzw FfarMM0OuI0ubCoe77jG6J9yZJfUp5f4omB4fnOAVtvRsjkQKMUOu0yZ/jy/rD2rfRRoNmLI/Nz Di0Xd0xY8ngk= X-Received: by 2002:a1c:7f44:: with SMTP id a65mr8257058wmd.53.1590746940872; Fri, 29 May 2020 03:09:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzaY3cH0pMU5DNoK7PVbWDxsIeWfX0caBC7GeO1NQWzFetyvQhxMWGlRFkFsk23BadpM7gPCg== X-Received: by 2002:a1c:7f44:: with SMTP id a65mr8257031wmd.53.1590746940498; Fri, 29 May 2020 03:09:00 -0700 (PDT) Received: from minerva.redhat.com ([2a01:c50f:ab80:6700:527f:85b3:5615:ff0b]) by smtp.gmail.com with ESMTPSA id 5sm9982084wmd.19.2020.05.29.03.08.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 May 2020 03:08:59 -0700 (PDT) From: Javier Martinez Canillas To: grub-devel@gnu.org Cc: Peter Jones , Javier Martinez Canillas , Daniel Kiper Subject: [PATCH v2] i386: Make pmtimer tsc calibration not take 51 seconds to fail Date: Fri, 29 May 2020 12:08:37 +0200 Message-Id: <20200529100837.1054806-1-javierm@redhat.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=207.211.31.120; envelope-from=javierm@redhat.com; helo=us-smtp-1.mimecast.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/05/29 01:34:27 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 May 2020 10:09:09 -0000 From: Peter Jones 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 ~0x2626aa0 TSCs, aka ~2.4 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. If "debug" is includes "pmtimer", you will see one of the following three outcomes. If pmtimer gives all 0 or all 1 bits, you will see: pmtimer: 0xffffff bad_reads: 1 pmtimer: 0xffffff bad_reads: 2 pmtimer: 0xffffff bad_reads: 3 pmtimer: 0xffffff bad_reads: 4 pmtimer: 0xffffff bad_reads: 5 pmtimer: 0xffffff bad_reads: 6 pmtimer: 0xffffff bad_reads: 7 pmtimer: 0xffffff bad_reads: 8 pmtimer: 0xffffff bad_reads: 9 pmtimer: 0xffffff bad_reads: 10 timer is broken; giving up. This outcome was tested using qemu+kvm with UEFI (OVMF) firmware and these options: -machine pc-q35-2.10 -cpu Broadwell-noTSX If pmtimer gives any other bit patterns but is not actually marching forward fast enough to use for clock calibration, you will see: pmtimer delta is 0x0 (1904 iterations) tsc delta is implausible: 0x2626aa0 This outcome was tested using grub compiled with GRUB_PMTIMER_IGNORE_BAD_READS defined (so as not to trip the bad read test) using qemu+kvm with UEFI (OVMF) firmware, and these options: -machine pc-q35-2.10 -cpu Broadwell-noTSX If pmtimer actually works, you'll see something like: pmtimer delta is 0xdff tsc delta is 0x278756 This outcome was tested using qemu+kvm with UEFI (OVMF) firmware, and these options: -machine pc-i440fx-2.4 -cpu Broadwell-noTSX I've also tested this outcome on a real Intel Xeon E3-1275v3 on an Intel Server Board S1200V3RPS using the SDV.RP.B8 "Release" build here: https://firmware.intel.com/sites/default/files/UEFIDevKit_S1200RP_vB8.zip Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas --- Hello Daniel, I think that addressed all the issues you pointed out to Peter in https://lists.gnu.org/archive/html/grub-devel/2018-02/msg00078.html Please let me know if I missed anything. Best regards, Javier Changes in v2: - Address issues pointed out by Daniel Kiper on previously posted version. grub-core/kern/i386/tsc_pmtimer.c | 112 ++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 20 deletions(-) diff --git a/grub-core/kern/i386/tsc_pmtimer.c b/grub-core/kern/i386/tsc_pmtimer.c index c9c36169978..d9b3765b018 100644 --- a/grub-core/kern/i386/tsc_pmtimer.c +++ b/grub-core/kern/i386/tsc_pmtimer.c @@ -28,40 +28,104 @@ #include #include +/* + * Define GRUB_PMTIMER_IGNORE_BAD_READS if you're trying to test a timer that's + * present but doesn't keep time well. + */ +// #define GRUB_PMTIMER_IGNORE_BAD_READS + grub_uint64_t grub_pmtimer_wait_count_tsc (grub_port_t pmtimer, grub_uint16_t num_pm_ticks) { grub_uint32_t start; - grub_uint32_t last; - grub_uint32_t cur, end; + grub_uint64_t cur, end; grub_uint64_t start_tsc; grub_uint64_t end_tsc; - int num_iter = 0; + unsigned int num_iter = 0; +#ifndef GRUB_PMTIMER_IGNORE_BAD_READS + int bad_reads = 0; +#endif - start = grub_inl (pmtimer) & 0xffffff; - last = start; + /* + * Some timers are 24-bit and some are 32-bit, but it doesn't make much + * difference to us. Caring which one we have isn't really worth it since + * the low-order digits will give us enough data to calibrate TSC. So just + * mask the top-order byte off. + */ + cur = start = grub_inl (pmtimer) & 0x00ffffffUL; end = start + num_pm_ticks; start_tsc = grub_get_tsc (); while (1) { - cur = grub_inl (pmtimer) & 0xffffff; - if (cur < last) - cur |= 0x1000000; - num_iter++; + cur &= 0xffffffffff000000ULL; + /* + * Only take the low-order 24-bit for the reason explained above. + */ + cur |= grub_inl (pmtimer) & 0x00ffffffUL; + + end_tsc = grub_get_tsc(); + +#ifndef GRUB_PMTIMER_IGNORE_BAD_READS + /* + * 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 == 0xffffffUL || cur == 0) + { + bad_reads++; + grub_dprintf ("pmtimer", + "pmtimer: 0x%"PRIxGRUB_UINT64_T" bad_reads: %d\n", + cur, bad_reads); + grub_dprintf ("pmtimer", "timer is broken; giving up.\n"); + + if (bad_reads == 10) + return 0; + } +#endif + + if (cur < start) + cur += 0x1000000; + if (cur >= end) { - end_tsc = grub_get_tsc (); + grub_dprintf ("pmtimer", "pmtimer delta is 0x%"PRIxGRUB_UINT64_T"\n", + cur - start); + grub_dprintf ("pmtimer", "tsc delta is 0x%"PRIxGRUB_UINT64_T"\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. 1ms at 10GHz should be 1E+7 TSCs; at + * 250MHz it should be 2.5E5. So if after 4E+7 TSCs on a 10GHz machine, + * we should have seen pmtimer show 4ms of change (i.e. cur =~ + * start+14320); on a 250MHz machine that should be 160ms (start+572800). + * If after this a time we still don't have 1ms on pmtimer, then pmtimer + * is broken. + * + * Likewise, if our code is perfectly efficient and introduces no delays + * whatsoever, on a 10GHz system we should see a TSC delta of 3580 in + * ~3580 iterations. On a 250MHz machine that should be ~900 iterations. + * + * With those factors in mind, there are two limits here. There's a hard + * limit here at 8x our desired pm timer delta. This limit was picked as + * an arbitrarily large value that's still not a lot of time to humans, + * because if we get that far this is either an implausibly fast machine + * or the pmtimer is not running. And there is another limit on a 4 ms TSC + * delta on a 10 GHz clock, without seeing cur converge on our target value. */ - if ((num_iter & 0xffffff) == 0 && grub_get_tsc () - start_tsc > 5000000) { - return 0; - } + if ((++num_iter > (grub_uint32_t)num_pm_ticks << 3UL) || + end_tsc - start_tsc > 40000000) + { + grub_dprintf ("pmtimer", + "pmtimer delta is 0x%"PRIxGRUB_UINT64_T" (%u iterations)\n", + cur - start, num_iter); + grub_dprintf ("pmtimer", + "tsc delta is implausible: 0x%"PRIxGRUB_UINT64_T"\n", + end_tsc - start_tsc); + return 0; + } } } @@ -74,12 +138,20 @@ grub_tsc_calibrate_from_pmtimer (void) fadt = grub_acpi_find_fadt (); if (!fadt) - return 0; + { + grub_dprintf ("pmtimer", "No FADT found; not using pmtimer.\n"); + return 0; + } pmtimer = fadt->pmtimer; if (!pmtimer) - return 0; + { + grub_dprintf ("pmtimer", "FADT does not specify pmtimer; skipping.\n"); + return 0; + } - /* It's 3.579545 MHz clock. Wait 1 ms. */ + /* + * It's 3.579545 MHz clock. Wait 1 ms. + */ tsc_diff = grub_pmtimer_wait_count_tsc (pmtimer, 3580); if (tsc_diff == 0) return 0; -- 2.26.2