* Theo Buehler <t...@theobuehler.org> le [14-08-2019 07:23:01 +0200]:
> On Mon, Aug 12, 2019 at 02:34:55PM +0200, p...@ybad.name wrote:
> > >Synopsis: key input repeat speed too fast.
> > >Category: cwm
> > >Environment:
> > System : OpenBSD 6.6
> > Details : OpenBSD 6.6-beta (GENERIC.MP) #201: Sun Aug 11 23:13:55
> > MDT 2019
> >
> > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> >
> > Architecture: OpenBSD.amd64
> > Machine : amd64
> > >Description:
> > Since last snapshot, when opening an X session with cwm, a pressed key
> > is
> > repeated multiple times.
> > Example : try to write "ls" in xterm produce "lllsss"
> >
> > Also, videos are played very fast (not sure it's related).
> > >How-To-Repeat:
> > Open a new X session under cwm and write anything.
> > >Fix:
>
> Thanks for the report.
>
> It would be helpful to know which commit triggered this issue. One of
> the suspicious commits is the TSC change. Could you apply the revert
> below on top of a -current tree, compile and install a new kernel
> (see release(8), step 2) and test if the problem persists?
>
> Index: sys/arch/amd64/amd64/cpu.c
> ===================================================================
> RCS file: /var/cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 cpu.c
> --- sys/arch/amd64/amd64/cpu.c 9 Aug 2019 15:20:04 -0000 1.139
> +++ sys/arch/amd64/amd64/cpu.c 12 Aug 2019 18:44:55 -0000
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: cpu.c,v 1.139 2019/08/09 15:20:04 pirofti Exp $ */
> +/* $OpenBSD: cpu.c,v 1.138 2019/08/07 18:53:28 guenther Exp $ */
> /* $NetBSD: cpu.c,v 1.1 2003/04/26 18:39:26 fvdl Exp $ */
>
> /*-
> @@ -800,10 +800,6 @@ cpu_init(struct cpu_info *ci)
> cr4 = rcr4();
> lcr4(cr4 & ~CR4_PGE);
> lcr4(cr4);
> -
> - /* Synchronize TSC */
> - if (cold && !CPU_IS_PRIMARY(ci))
> - tsc_sync_ap(ci);
> #endif
> }
>
> @@ -858,7 +854,6 @@ void
> cpu_start_secondary(struct cpu_info *ci)
> {
> int i;
> - u_long s;
>
> ci->ci_flags |= CPUF_AP;
>
> @@ -879,18 +874,6 @@ cpu_start_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> - } else {
> - /*
> - * Synchronize time stamp counters. Invalidate cache and
> - * synchronize twice (in tsc_sync_bp) to minimize possible
> - * cache effects. Disable interrupts to try and rule out any
> - * external interference.
> - */
> - s = intr_disable();
> - wbinvd();
> - tsc_sync_bp(ci);
> - intr_restore(s);
> - printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
> }
>
> if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
> @@ -915,8 +898,6 @@ void
> cpu_boot_secondary(struct cpu_info *ci)
> {
> int i;
> - int64_t drift;
> - u_long s;
>
> atomic_setbits_int(&ci->ci_flags, CPUF_GO);
>
> @@ -929,17 +910,6 @@ cpu_boot_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> - } else if (cold) {
> - /* Synchronize TSC again, check for drift. */
> - drift = ci->ci_tsc_skew;
> - s = intr_disable();
> - wbinvd();
> - tsc_sync_bp(ci);
> - intr_restore(s);
> - drift -= ci->ci_tsc_skew;
> - printf("TSC skew=%lld drift=%lld\n",
> - (long long)ci->ci_tsc_skew, (long long)drift);
> - tsc_sync_drift(drift);
> }
> }
>
> @@ -964,14 +934,7 @@ cpu_hatch(void *v)
> panic("%s: already running!?", ci->ci_dev->dv_xname);
> #endif
>
> - /*
> - * Synchronize the TSC for the first time. Note that interrupts are
> - * off at this point.
> - */
> - wbinvd();
> ci->ci_flags |= CPUF_PRESENT;
> - ci->ci_tsc_skew = 0; /* reset on resume */
> - tsc_sync_ap(ci);
>
> lapic_enable();
> lapic_startclock();
> Index: sys/arch/amd64/amd64/tsc.c
> ===================================================================
> RCS file: /var/cvs/src/sys/arch/amd64/amd64/tsc.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 tsc.c
> --- sys/arch/amd64/amd64/tsc.c 9 Aug 2019 15:20:05 -0000 1.13
> +++ sys/arch/amd64/amd64/tsc.c 12 Aug 2019 18:44:55 -0000
> @@ -1,10 +1,8 @@
> -/* $OpenBSD: tsc.c,v 1.13 2019/08/09 15:20:05 pirofti Exp $ */
> +/* $OpenBSD: tsc.c,v 1.12 2019/08/03 14:57:51 jcs Exp $ */
> /*
> - * Copyright (c) 2008 The NetBSD Foundation, Inc.
> * Copyright (c) 2016,2017 Reyk Floeter <r...@openbsd.org>
> * Copyright (c) 2017 Adam Steen <a...@adamsteen.com.au>
> * Copyright (c) 2017 Mike Belopuhov <m...@openbsd.org>
> - * Copyright (c) 2019 Paul Irofti <piro...@openbsd.org>
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -22,7 +20,6 @@
> #include <sys/param.h>
> #include <sys/systm.h>
> #include <sys/timetc.h>
> -#include <sys/atomic.h>
>
> #include <machine/cpu.h>
> #include <machine/cpufunc.h>
> @@ -36,12 +33,6 @@ int tsc_recalibrate;
> uint64_t tsc_frequency;
> int tsc_is_invariant;
>
> -#define TSC_DRIFT_MAX 250
> -int64_t tsc_drift_observed;
> -
> -volatile int64_t tsc_sync_val;
> -volatile struct cpu_info *tsc_sync_cpu;
> -
> uint tsc_get_timecount(struct timecounter *tc);
>
> #include "lapic.h"
> @@ -190,8 +181,10 @@ calibrate_tsc_freq(void)
> return;
> tsc_frequency = freq;
> tsc_timecounter.tc_frequency = freq;
> +#ifndef MULTIPROCESSOR
> if (tsc_is_invariant)
> tsc_timecounter.tc_quality = 2000;
> +#endif
> }
>
> void
> @@ -210,13 +203,14 @@ cpu_recalibrate_tsc(struct timecounter *
> uint
> tsc_get_timecount(struct timecounter *tc)
> {
> - return rdtsc() + curcpu()->ci_tsc_skew;
> + return rdtsc();
> }
>
> void
> tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
> {
> - if (!(ci->ci_flags & CPUF_CONST_TSC) ||
> + if (!(ci->ci_flags & CPUF_PRIMARY) ||
> + !(ci->ci_flags & CPUF_CONST_TSC) ||
> !(ci->ci_flags & CPUF_INVAR_TSC))
> return;
>
> @@ -226,7 +220,9 @@ tsc_timecounter_init(struct cpu_info *ci
> /* Newer CPUs don't require recalibration */
> if (tsc_frequency > 0) {
> tsc_timecounter.tc_frequency = tsc_frequency;
> +#ifndef MULTIPROCESSOR
> tsc_timecounter.tc_quality = 2000;
> +#endif
> } else {
> tsc_recalibrate = 1;
> tsc_frequency = cpufreq;
> @@ -234,103 +230,5 @@ tsc_timecounter_init(struct cpu_info *ci
> calibrate_tsc_freq();
> }
>
> - if (tsc_drift_observed > TSC_DRIFT_MAX) {
> - printf("ERROR: %lld cycle TSC drift observed\n",
> - (long long)tsc_drift_observed);
> - tsc_timecounter.tc_quality = -1000;
> - tsc_is_invariant = 0;
> - }
> -
> - printf("%s: TSC skew=%lld observed drift=%lld\n", __func__,
> - (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
> -
> - if (ci->ci_flags & CPUF_PRIMARY)
> - tc_init(&tsc_timecounter);
> -}
> -
> -/*
> - * Record drift (in clock cycles). Called during AP startup.
> - */
> -void
> -tsc_sync_drift(int64_t drift)
> -{
> - if (drift < 0)
> - drift = -drift;
> - if (drift > tsc_drift_observed)
> - tsc_drift_observed = drift;
> -}
> -
> -/*
> - * Called during startup of APs, by the boot processor. Interrupts
> - * are disabled on entry.
> - */
> -void
> -tsc_read_bp(struct cpu_info *ci, uint64_t *bptscp, uint64_t *aptscp)
> -{
> - uint64_t bptsc;
> -
> - if (atomic_swap_ptr(&tsc_sync_cpu, ci) != NULL)
> - panic("tsc_sync_bp: 1");
> -
> - /* Flag it and read our TSC. */
> - atomic_setbits_int(&ci->ci_flags, CPUF_SYNCTSC);
> - bptsc = (rdtsc() >> 1);
> -
> - /* Wait for remote to complete, and read ours again. */
> - while ((ci->ci_flags & CPUF_SYNCTSC) != 0)
> - membar_consumer();
> - bptsc += (rdtsc() >> 1);
> -
> - /* Wait for the results to come in. */
> - while (tsc_sync_cpu == ci)
> - CPU_BUSY_CYCLE();
> - if (tsc_sync_cpu != NULL)
> - panic("tsc_sync_bp: 2");
> -
> - *bptscp = bptsc;
> - *aptscp = tsc_sync_val;
> -}
> -
> -void
> -tsc_sync_bp(struct cpu_info *ci)
> -{
> - uint64_t bptsc, aptsc;
> -
> - tsc_read_bp(ci, &bptsc, &aptsc); /* discarded - cache effects */
> - tsc_read_bp(ci, &bptsc, &aptsc);
> -
> - /* Compute final value to adjust for skew. */
> - ci->ci_tsc_skew = bptsc - aptsc;
> -}
> -
> -/*
> - * Called during startup of AP, by the AP itself. Interrupts are
> - * disabled on entry.
> - */
> -void
> -tsc_post_ap(struct cpu_info *ci)
> -{
> - uint64_t tsc;
> -
> - /* Wait for go-ahead from primary. */
> - while ((ci->ci_flags & CPUF_SYNCTSC) == 0)
> - membar_consumer();
> - tsc = (rdtsc() >> 1);
> -
> - /* Instruct primary to read its counter. */
> - atomic_clearbits_int(&ci->ci_flags, CPUF_SYNCTSC);
> - tsc += (rdtsc() >> 1);
> -
> - /* Post result. Ensure the whole value goes out atomically. */
> - (void)atomic_swap_64(&tsc_sync_val, tsc);
> -
> - if (atomic_swap_ptr(&tsc_sync_cpu, NULL) != ci)
> - panic("tsc_sync_ap");
> -}
> -
> -void
> -tsc_sync_ap(struct cpu_info *ci)
> -{
> - tsc_post_ap(ci);
> - tsc_post_ap(ci);
> + tc_init(&tsc_timecounter);
> }
> Index: sys/arch/amd64/include/cpu.h
> ===================================================================
> RCS file: /var/cvs/src/sys/arch/amd64/include/cpu.h,v
> retrieving revision 1.132
> diff -u -p -r1.132 cpu.h
> --- sys/arch/amd64/include/cpu.h 9 Aug 2019 15:20:05 -0000 1.132
> +++ sys/arch/amd64/include/cpu.h 12 Aug 2019 18:44:55 -0000
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: cpu.h,v 1.132 2019/08/09 15:20:05 pirofti Exp $ */
> +/* $OpenBSD: cpu.h,v 1.131 2019/05/17 19:07:16 guenther Exp $ */
> /* $NetBSD: cpu.h,v 1.1 2003/04/26 18:39:39 fvdl Exp $ */
>
> /*-
> @@ -206,8 +206,6 @@ struct cpu_info {
> union vmm_cpu_cap ci_vmm_cap;
> paddr_t ci_vmxon_region_pa;
> struct vmxon_region *ci_vmxon_region;
> -
> - int64_t ci_tsc_skew; /* counter skew vs cpu0 */
> };
>
> #define CPUF_BSP 0x0001 /* CPU is the original BSP */
> @@ -223,7 +221,6 @@ struct cpu_info {
> #define CPUF_INVAR_TSC 0x0100 /* CPU has invariant TSC */
> #define CPUF_USERXSTATE 0x0200 /* CPU has curproc's xsave
> state */
>
> -#define CPUF_SYNCTSC 0x0800 /* Synchronize TSC */
> #define CPUF_PRESENT 0x1000 /* CPU is present */
> #define CPUF_RUNNING 0x2000 /* CPU is running */
> #define CPUF_PAUSE 0x4000 /* CPU is paused in DDB */
> Index: sys/arch/amd64/include/cpuvar.h
> ===================================================================
> RCS file: /var/cvs/src/sys/arch/amd64/include/cpuvar.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 cpuvar.h
> --- sys/arch/amd64/include/cpuvar.h 9 Aug 2019 15:20:05 -0000 1.10
> +++ sys/arch/amd64/include/cpuvar.h 12 Aug 2019 18:44:55 -0000
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: cpuvar.h,v 1.10 2019/08/09 15:20:05 pirofti Exp $ */
> +/* $OpenBSD: cpuvar.h,v 1.9 2017/10/06 13:33:53 mikeb Exp $ */
> /* $NetBSD: cpuvar.h,v 1.1 2003/03/01 18:29:28 fvdl Exp $ */
>
> /*-
> @@ -97,9 +97,5 @@ void identifycpu(struct cpu_info *);
> void cpu_init(struct cpu_info *);
> void cpu_init_first(void);
> void cpu_adjust_tsc_freq(uint64_t (*)());
> -
> -void tsc_sync_drift(int64_t);
> -void tsc_sync_bp(struct cpu_info *);
> -void tsc_sync_ap(struct cpu_info *);
>
> #endif
>
Patch applied, no more issue now.
However, even if /etc/sysctl.conf is empty, I notice :
# sysctl |grep timecoun.*hard
kern.timecounter.hardware=acpihpet0
So, by default, @phessler solution seems to be applied.
Of course, telle me if I can provide any further data.
--
prx
