[Bug 226968] IRQ storm on cpu0 timer when holding down key on USB keyboard

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=226968

--- Comment #3 from Hans Petter Selasky <hselasky at FreeBSD.org> ---
Hi,

I have been discussing some patches with Bruce Evans on this topic. The problem
is the keyboard repeat rate is zero, which cause the timer to trigger
instantly. This issue fell off my radar. Here is the last patch WIP:

Index: sys/arm/samsung/exynos/chrome_kb.c
===================================================================
--- sys/arm/samsung/exynos/chrome_kb.c
+++ sys/arm/samsung/exynos/chrome_kb.c
@@ -514,17 +514,8 @@
         if (!KBD_HAS_DEVICE(kbd)) {
             return (0);
         }
-        if (((int *)arg)[1] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 200)    /* fastest possible value */
-            kbd->kb_delay1 = 200;
-        else
-            kbd->kb_delay1 = ((int *)arg)[0];
-        kbd->kb_delay2 = ((int *)arg)[1];
+        kbd->kb_delay1 = KEYBOARD_REPEAT_GET(arg,0,200,1000);
+        kbd->kb_delay2 = KEYBOARD_REPEAT_GET(arg,1,34,504);
         return (0);

     case KDSETRAD:            /* set keyboard repeat rate (old
Index: sys/arm/versatile/pl050.c
===================================================================
--- sys/arm/versatile/pl050.c
+++ sys/arm/versatile/pl050.c
@@ -418,17 +418,8 @@
         if (!KBD_HAS_DEVICE(kbd)) {
             return (0);
         }
-        if (((int *)arg)[1] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 200)    /* fastest possible value */
-            kbd->kb_delay1 = 200;
-        else
-            kbd->kb_delay1 = ((int *)arg)[0];
-        kbd->kb_delay2 = ((int *)arg)[1];
+        kbd->kb_delay1 = KEYBOARD_REPEAT_GET(arg,0,200,1000);
+        kbd->kb_delay2 = KEYBOARD_REPEAT_GET(arg,1,34,504);
         return (0);

 #if defined(COMPAT_FREEBSD6) || defined(COMPAT_FREEBSD5) || \
Index: sys/dev/adb/adb_kbd.c
===================================================================
--- sys/dev/adb/adb_kbd.c
+++ sys/dev/adb/adb_kbd.c
@@ -780,16 +780,8 @@
     case KDSETREPEAT:
         if (!KBD_HAS_DEVICE(kbd))
             return 0;
-        if (((int *)data)[1] < 0)
-            return EINVAL;
-        if (((int *)data)[0] < 0)
-            return EINVAL;
-        else if (((int *)data)[0] == 0)  /* fastest possible value */
-            kbd->kb_delay1 = 200;
-        else
-            kbd->kb_delay1 = ((int *)data)[0];
-        kbd->kb_delay2 = ((int *)data)[1];
-
+        kbd->kb_delay1 = KEYBOARD_REPEAT_GET(data,0,200,1000);
+        kbd->kb_delay2 = KEYBOARD_REPEAT_GET(data,1,34,504);
         break;

     case KDSETRAD:
Index: sys/dev/gpio/gpiokeys.c
===================================================================
--- sys/dev/gpio/gpiokeys.c
+++ sys/dev/gpio/gpiokeys.c
@@ -822,17 +822,8 @@
         if (!KBD_HAS_DEVICE(kbd)) {
             return (0);
         }
-        if (((int *)arg)[1] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 0) {
-            return (EINVAL);
-        }
-        if (((int *)arg)[0] < 200)    /* fastest possible value */
-            kbd->kb_delay1 = 200;
-        else
-            kbd->kb_delay1 = ((int *)arg)[0];
-        kbd->kb_delay2 = ((int *)arg)[1];
+        kbd->kb_delay1 = KEYBOARD_REPEAT_GET(arg,0,200,1000);
+        kbd->kb_delay2 = KEYBOARD_REPEAT_GET(arg,1,34,504);
         return (0);

 #if defined(COMPAT_FREEBSD6) || defined(COMPAT_FREEBSD5) || \
Index: sys/dev/usb/input/ukbd.c
===================================================================
--- sys/dev/usb/input/ukbd.c
+++ sys/dev/usb/input/ukbd.c
@@ -1946,14 +1946,8 @@
         if (!KBD_HAS_DEVICE(kbd)) {
             return (0);
         }
-        /*
-         * Convert negative, zero and tiny args to the same limits
-         * as atkbd.  We could support delays of 1 msec, but
-         * anything much shorter than the shortest atkbd value
-         * of 250.34 is almost unusable as well as incompatible.
-         */
-        kbd->kb_delay1 = imax(((int *)arg)[0], 250);
-        kbd->kb_delay2 = imax(((int *)arg)[1], 34);
+        kbd->kb_delay1 = KEYBOARD_REPEAT_GET(arg,0,200,1000);
+        kbd->kb_delay2 = KEYBOARD_REPEAT_GET(arg,1,34,504);
 #ifdef EVDEV_SUPPORT
         if (sc->sc_evdev != NULL)
             evdev_push_repeats(sc->sc_evdev, kbd);
Index: sys/sys/kbio.h
===================================================================
--- sys/sys/kbio.h
+++ sys/sys/kbio.h
@@ -87,6 +87,15 @@
     int        kb_repeat[2];
 };
 typedef struct keyboard_repeat keyboard_repeat_t;
+
+#define    KEYBOARD_REPEAT_GET(p,i,min,max) ({    \
+    const struct keyboard_repeat *__ptr =    \
+    (const struct keyboard_repeat *)(p);    \
+    __ptr->kb_repeat[i] < (min) ? (int)(min) :    \
+    __ptr->kb_repeat[i] > (max) ? (int)(max) :    \
+    __ptr->kb_repeat[i];            \
+})
+
 #define KDSETREPEAT    _IOW('K', 102, keyboard_repeat_t)
 #define KDGETREPEAT    _IOR('K', 103, keyboard_repeat_t)

-- 
You are receiving this mail because:
You are the assignee for the bug.