From owner-freebsd-hackers@FreeBSD.ORG Wed Jun 4 17:32:15 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9293FD8A for ; Wed, 4 Jun 2014 17:32:15 +0000 (UTC) Received: from elf.torek.net (50-73-42-1-utah.hfc.comcastbusiness.net [50.73.42.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6B94C2B7E for ; Wed, 4 Jun 2014 17:32:14 +0000 (UTC) Received: from elf.torek.net (localhost [127.0.0.1]) by elf.torek.net (8.14.5/8.14.5) with ESMTP id s54HWDlE032048 for ; Wed, 4 Jun 2014 11:32:13 -0600 (MDT) (envelope-from torek@torek.net) Message-Id: <201406041732.s54HWDlE032048@elf.torek.net> From: Chris Torek cc: freebsd-hackers@freebsd.org Subject: Re: kern.timecounter.smp_tsc_adjust In-reply-to: Your message of "Tue, 03 Jun 2014 14:57:02 -0600." <201406032057.s53Kv2YT016755@elf.torek.net> Date: Wed, 04 Jun 2014 11:32:13 -0600 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (elf.torek.net [127.0.0.1]); Wed, 04 Jun 2014 11:32:13 -0600 (MDT) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Jun 2014 17:32:15 -0000 >In any case, at least one, and maybe two, routines need their >types changed (and maybe the TSC_READ macro as well). Is it OK >to just assume the upper 32 bits are in sync? For whatever it may be worth, the following patch (to change everything to 64 bits) is boot-tested and no longer crashes the system that did crash with the unpatched version. It also successfully synchronized TSCs at least once (Intel based board with the "invariant TSC" bit set, 40 CPUs). Chris x86/tsc: fix SMP TSC adjustment code On SMP systems, if kern.timecounter.smp_tsc is set, the kernel attempts to determine whether the TSCs on the processors are sufficiently in-sync to be used for time counting. If not, and kern.timecounter.smp_tsc_adjust is set, we then attempt to adjust all the TSCs. The adjustment code assumed we kept a full 64-bit value for each TSC, but the data-gathering and comparison code used 32-bit values. As a result, if the timecounters were out of sync, the adjustment code could crash (depending on the number of CPUs). This converts everything to full 64-bit values. diff --git a/x86/x86/tsc.c b/x86/x86/tsc.c index 30b8a30..cf05e94 100644 --- a/x86/x86/tsc.c +++ b/x86/x86/tsc.c @@ -373,11 +373,11 @@ init_TSC(void) static void \ tsc_read_##x(void *arg) \ { \ - uint32_t *tsc = arg; \ + uint64_t *tsc = arg; \ u_int cpu = PCPU_GET(cpuid); \ \ __asm __volatile("cpuid" : : : "eax", "ebx", "ecx", "edx"); \ - tsc[cpu * 3 + x] = rdtsc32(); \ + tsc[cpu * 3 + x] = rdtsc(); \ } TSC_READ(0) TSC_READ(1) @@ -389,8 +389,8 @@ TSC_READ(2) static void comp_smp_tsc(void *arg) { - uint32_t *tsc; - int32_t d1, d2; + uint64_t *tsc; + int64_t d1, d2; u_int cpu = PCPU_GET(cpuid); u_int i, j, size; @@ -454,7 +454,7 @@ adj_smp_tsc(void *arg) static int test_tsc(void) { - uint32_t *data, *tsc; + uint64_t *data, *tsc; u_int i, size, adj; if ((!smp_tsc && !tsc_is_invariant) || vm_guest)