Hi,
My last attempt to fix resync problem for KVM users whise devices don't send
0xAA 0x00 but resetting mouse into bare PS/2 mode when switching was clearly
bogus, so here is a new version. It tries to do:
- strict PS/2 protocol checking (controlled by psmouse.strict option, turned
on by default in this patch)
- flag "suspect" packets - 1st bytes with overflow bits set and bytes that
indicate that Left + Middle or Right + Middle buttons are pressed at the
same time which is unusial combination. Suspect packets, together with
timeouts, are treated as "soft" errors.
- use rate limiting algorithm to detect whether reconnect request should be
ussued. psmouse.resetafter now accepts 2 values:
psmouse.resetafter=<limit>,<burst>
<limit> controls hard error limit, if psmouse detects so many hard errors
in row it will issue reconnect.
<burst> controls how many errors (both hard and soft) is considered too
much if they come within 1 second of each other when they are mixed with
packets that are considered ok.
Default values for now are 10,20.
Having {Left|Right} + Middle button combination considered "suspect" also
allows to force reconnect by holding these buttong and moving mouse quickly.
Please let me know if you are having problems using KVM and if the patch
makes any improvements.
Thanks.
--
Dmitry
diff -urN 2.6.7/drivers/input/mouse/psmouse-base.c linux-2.6.7/drivers/input/mouse/psmouse-base.c
--- 2.6.7/drivers/input/mouse/psmouse-base.c 2004-06-22 01:23:15.000000000 -0500
+++ linux-2.6.7/drivers/input/mouse/psmouse-base.c 2004-06-23 01:23:19.000000000 -0500
@@ -43,9 +43,14 @@
module_param_named(smartscroll, psmouse_smartscroll, bool, 0);
MODULE_PARM_DESC(smartscroll, "Logitech Smartscroll autorepeat, 1 = enabled (default), 0 = disabled.");
-static unsigned int psmouse_resetafter;
-module_param_named(resetafter, psmouse_resetafter, uint, 0);
-MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets (0 = never).");
+static unsigned int psmouse_strict = 1;
+module_param_named(strict, psmouse_strict, bool, 0);
+MODULE_PARM_DESC(strict, "Strict PS/2 protocol enforcement.");
+
+static unsigned int psmouse_resetafter[] = { 10, 20 };
+static unsigned int psmouse_resetafter_nargs;
+module_param_array_named(resetafter, psmouse_resetafter, uint, psmouse_resetafter_nargs, 0);
+MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets <limit, burst> (0 = never).");
__obsolete_setup("psmouse_noext");
__obsolete_setup("psmouse_resolution=");
@@ -65,11 +70,49 @@
struct input_dev *dev = &psmouse->dev;
unsigned char *packet = psmouse->packet;
+ if (psmouse->pktcnt == 1) {
+/*
+ * If go by the letter of PS/2 protcol 4th bit in 1st byte should always be set.
+ */
+ if (psmouse_strict && !(packet[0] & 0x08))
+ return PSMOUSE_BAD_DATA;
+
+/*
+ * Overflows should happen rarely.
+ */
+ if (packet[0] & 0xc0)
+ return PSMOUSE_SUSPECT_DATA;
+
+/*
+ * Having left or right button together with middle is somewhat unusual.
+ */
+ if ((packet[0] & 0x07) > 4)
+ return PSMOUSE_SUSPECT_DATA;
+ }
+
if (psmouse->pktcnt < 3 + (psmouse->type >= PSMOUSE_GENPS))
return PSMOUSE_GOOD_DATA;
/*
- * Full packet accumulated, process it
+ * Full packet accumulated, do final validation
+ */
+ if (psmouse_strict) {
+/*
+ * Explorer PS/2 protocol specifies that 2 most significant bits in 4th
+ * byte should be 0
+ */
+ if (psmouse->type == PSMOUSE_IMEX && (packet[3] & 0xc0))
+ return PSMOUSE_BAD_DATA;
+/*
+ * Intellimouse PS/2 protocol specifies that z4 to z8 should be the same
+ */
+ if (psmouse->type == PSMOUSE_IMPS &&
+ (packet[3] & 0xf8) != 0xf8 && (packet[3] & 0xf8) != 0x00)
+ return PSMOUSE_BAD_DATA;
+ }
+
+/*
+ * Now process the packet
*/
input_regs(dev, regs);
@@ -123,6 +166,26 @@
return PSMOUSE_FULL_PACKET;
}
+static int psmouse_check_resync(struct psmouse *psmouse)
+{
+ int now = jiffies;
+
+ psmouse->err_toks += now - psmouse->last_error;
+ psmouse->last_error = now;
+ if (psmouse->err_toks > psmouse->err_limit_burst * PSMOUSE_ERRLIMIT_JIFFIES)
+ psmouse->err_toks = psmouse->err_limit_burst * PSMOUSE_ERRLIMIT_JIFFIES;
+ if ((psmouse->err_limit_burst && psmouse->err_toks < PSMOUSE_ERRLIMIT_JIFFIES) ||
+ (psmouse->err_limit_hard && psmouse->out_of_sync == psmouse->err_limit_hard)) {
+ psmouse->state = PSMOUSE_IGNORE;
+ printk(KERN_NOTICE "psmouse.c: %s on %s reports too many errors, issuing reconnect request\n",
+ psmouse->name, psmouse->serio->phys);
+ serio_reconnect(psmouse->serio);
+ return 1;
+ }
+ psmouse->err_toks -= PSMOUSE_ERRLIMIT_JIFFIES;
+ return 0;
+}
+
/*
* psmouse_interrupt() handles incoming characters, either gathering them into
* packets or passing them to the command routine as command output.
@@ -178,6 +241,8 @@
printk(KERN_WARNING "psmouse.c: %s at %s lost synchronization, throwing %d bytes away.\n",
psmouse->name, psmouse->phys, psmouse->pktcnt);
psmouse->pktcnt = 0;
+ if (psmouse_check_resync(psmouse))
+ goto out;
}
psmouse->last = jiffies;
@@ -210,12 +275,13 @@
printk(KERN_WARNING "psmouse.c: %s at %s lost sync at byte %d\n",
psmouse->name, psmouse->phys, psmouse->pktcnt);
psmouse->pktcnt = 0;
+ psmouse->out_of_sync++;
+ psmouse_check_resync(psmouse);
- if (++psmouse->out_of_sync == psmouse_resetafter) {
- psmouse->state = PSMOUSE_IGNORE;
- printk(KERN_NOTICE "psmouse.c: issuing reconnect request\n");
- serio_reconnect(psmouse->serio);
- }
+ break;
+
+ case PSMOUSE_SUSPECT_DATA:
+ psmouse_check_resync(psmouse);
break;
case PSMOUSE_FULL_PACKET:
@@ -707,6 +773,9 @@
psmouse->dev.id.product = psmouse->type;
psmouse->dev.id.version = psmouse->model;
+ psmouse->err_limit_hard = psmouse_resetafter[0];
+ psmouse->err_limit_burst = psmouse_resetafter[1];
+
input_register_device(&psmouse->dev);
printk(KERN_INFO "input: %s on %s\n", psmouse->devname, serio->phys);
diff -urN 2.6.7/drivers/input/mouse/psmouse.h linux-2.6.7/drivers/input/mouse/psmouse.h
--- 2.6.7/drivers/input/mouse/psmouse.h 2004-06-22 01:23:15.000000000 -0500
+++ linux-2.6.7/drivers/input/mouse/psmouse.h 2004-06-23 01:23:19.000000000 -0500
@@ -22,9 +22,12 @@
#define PSMOUSE_ACTIVATED 1
#define PSMOUSE_IGNORE 2
+#define PSMOUSE_ERRLIMIT_JIFFIES (1 * HZ)
+
/* psmouse protocol handler return codes */
typedef enum {
PSMOUSE_BAD_DATA,
+ PSMOUSE_SUSPECT_DATA,
PSMOUSE_GOOD_DATA,
PSMOUSE_FULL_PACKET
} psmouse_ret_t;
@@ -52,7 +55,6 @@
unsigned char type;
unsigned char model;
unsigned long last;
- unsigned long out_of_sync;
unsigned char state;
char acking;
volatile char ack;
@@ -60,6 +62,13 @@
char devname[64];
char phys[32];
+ /* resync handling */
+ unsigned long err_limit_hard;
+ unsigned long err_limit_burst;
+ unsigned long out_of_sync;
+ unsigned long err_toks;
+ unsigned long last_error;
+
psmouse_ret_t (*protocol_handler)(struct psmouse *psmouse, struct pt_regs *regs);
int (*reconnect)(struct psmouse *psmouse);
void (*disconnect)(struct psmouse *psmouse);
Hi!
I really like this approach, and it's something I wanted to implement,
but never found the time to do it.
Another heuristic I planned to implement would be interbyte timing - not
just timeouts, but qualify the validity of a packet both on how close
its bytes arrived together and whether it's separated from other
packets by some time. We could even keep an estimate on both the
interbyte and interpacket timing and classify each byte as based on the
probability of a packet boundary. It might be a little overkill, though.
> + if (psmouse->pktcnt == 1) {
> +/*
> + * If go by the letter of PS/2 protcol 4th bit in 1st byte should always be set.
> + */
> + if (psmouse_strict && !(packet[0] & 0x08))
> + return PSMOUSE_BAD_DATA;
This bit actually says 'internal/external', and can be used to
differentiate between data from an internal trackpoint and an external
mouse on older notebooks.
All newer devices have it at '1'.
> +/*
> + * Overflows should happen rarely.
> + */
> + if (packet[0] & 0xc0)
> + return PSMOUSE_SUSPECT_DATA;
Very rarely indeed. It should be next to impossible to generate an
overflow at a reasonable report rate (> 80 reports/sec). For higher
resolutions (> 400 dpi) it might be reasonable to up the report rate
accordingly.
> +/*
> + * Having left or right button together with middle is somewhat unusual.
> + */
> + if ((packet[0] & 0x07) > 4)
> + return PSMOUSE_SUSPECT_DATA;
> + }
> +
> if (psmouse->pktcnt < 3 + (psmouse->type >= PSMOUSE_GENPS))
> return PSMOUSE_GOOD_DATA;
--
Vojtech Pavlik
SuSE Labs, SuSE CR
> - flag "suspect" packets - 1st bytes with overflow bits set and bytes that
> indicate that Left + Middle or Right + Middle buttons are pressed at the
> same time which is unusial combination. Suspect packets, together with
> timeouts, are treated as "soft" errors.
I'm a little unhappy with this. They are *currently* unusual, but
I hate to preclude anyone from using them.
Just a reminder, the PS/2 mouse protocol is 3 bytes:
Byte 0, bit 7: Y overflow
Byte 0, bit 6: X overflow
Byte 0, bit 5: Y sign bit (Y delta bit 8)
Byte 0, bit 4: X sign bit (X delta bit 8)
Byte 0, bit 3: always 1 (apparently, internal/external)
Byte 0, bit 2: Middle button
Byte 0, bit 1: Right button
Byte 0, bit 0: left button
Byte 1: X movement (X delta bits 7..0)
Byte 2: Y movement (Y delta bits 7..0)
You can put the mouse in an extended mode where it adds a 4th byte, the
Z axis motion of the scroll wheel. There's an extra-extended mode
which steals the high bots of that to encode the 4th and 5th button
presses.
Anyway, rather than a static button combination, I'd flag as "suspect"
any case where two buttons change state in opposite directions in
the same report.
I.e.
buttonsdown = packet[0] & ~prevbyte0;
buttonsup = ~packet[0] & prevbyte0;
prevbyte0 = packet[0]
if (buttonsup != 0 && buttonsdown != 0)
-> Suspect
You can press two buttons at the same time, but to switch buttons
that fast is virtually impossible. But normal mouse motion will
produce exactly that effect in the lsbits of the movement bytes.
There's probably a neat bit-banging trick to compute that more simply...
changed = packet[0] ^ prevbyte0;
if ((changed & packet[0]) != 0 && (changed & prevbyte0) != 0)
-> Suspect
... maybe someone can do better?
Also, although motions are 9-bit numbers and can encode values
up to -256/+255, you might consider motions outside -128/+127
(the sign bit does not match the msbit of the movement byte)
to be suspect as well.
You could also use the jiffies count to judge acceleration
as well and flag unrealisticly fast direction changes.
E.g. suppose we store prev_x, prev_y and prev_jiffies when we got
the last mouse packet. The current movement values are x, y and
jiffies.
accel**2 = ((x - prev_x)**2 + (y - prev_y)**2) / (jiffies - prev_jiffies)**2
If accel**2 is too large, the packet is suspect. (Perhaps even
have a degree of suspiscion based on how large it is.)
If jiffies - prev_jiffies is large, you can consider instead acceleration
from a stop (prev_x = prev_y = 0), with jiffies - prev_jiffies being
your reporting rate (the minimum commonly observed inter-packet interval.)
And finally, there's just looking for pauses in the data stream.
If you get <pause> nbytes <pause> and nbytes is divisible by 3 and
not 4, you can suspect the mouse got reset to 3-byte mode.
[email protected] wrote:
> > - flag "suspect" packets - 1st bytes with overflow bits set and bytes
> that
> > indicate that Left + Middle or Right + Middle buttons are pressed at
> the
> > same time which is unusial combination. Suspect packets, together with
> > timeouts, are treated as "soft" errors.
>
> I'm a little unhappy with this. They are *currently* unusual, but
> I hate to preclude anyone from using them.
I also had some concerns, it's not the final patch version by any
means... Your suggestions are very interesting and I will try to
implement some of them (acceleration and "suspicion scoring" is a
bit of overkill I think), but I like the "unusial combination" idea
because you can trigger reconnect _at will_. Although with my sysfs
patches one could do it by echoing reconnect to driver attribute of
corresponding serio port ability just hold couple of buttons and
wiggle mouse and have it restore connection seems to be nice.
What if we trigger packets with all 3 buttons pressed as suspect?
--
Dmitry
Hi!
> I also had some concerns, it's not the final patch version by any
> means... Your suggestions are very interesting and I will try to
> implement some of them (acceleration and "suspicion scoring" is a
> bit of overkill I think), but I like the "unusial combination" idea
> because you can trigger reconnect _at will_. Although with my sysfs
> patches one could do it by echoing reconnect to driver attribute of
> corresponding serio port ability just hold couple of buttons and
> wiggle mouse and have it restore connection seems to be nice.
>
> What if we trigger packets with all 3 buttons pressed as suspect?
>
Well... what if windowmanager people get same idea and use
that combination to implwment gestures or something similar?
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms