Hi Jason, Thank you for the review. Comments inline.
----- Original Message ----- > From: "Jason Wessel" <[email protected]> > To: "Andrei Warkentin" <[email protected]> > Cc: [email protected], [email protected] > Sent: Sunday, February 26, 2012 8:10:56 AM > Subject: Re: [PATCH] KDB: Fix usability issues relating to the 'enter' key. > > I did not write the original code, but I can explain why there was > some special logic. > > When you restore the system back to the running state you do not want > to send any other key scan codes back to the kernel. The idea being > that you type "go" and press enter to resume kernel execution. At > that point you do not want to send a random scan code back to the > kernel, ideally you want to leave everything as it was. This also > handled the case where there was a PS/2 style mouse attached. The input device is already reset on kdb exit to a safe state, however, by the code inside kgdboc_post_exp_handler, which calls kgdboc_restore_input(). The existing logic also already correctly will handle the case of a PS/2 mouse, i.e.: /* * Ignore mouse events. */ if (scanstatus & KBD_STAT_MOUSE_OBF) return -1; ...so there is nothing that the non-ENTER-specific code already doesn't deal with. That special ENTER code also doesn't correctly handle typematic repeat, nor does it handle key-pad enter (as I mentioned before in the patch description, this has a different make and break codes, prefixed with 0xe0). > > I do have a question about part of the section you deleted. > > > > - if ((scancode & 0x7f) == 0x1c) { > > - /* > > - * enter key. All done. Absorb the release scancode. > > - */ > > - while ((inb(KBD_STATUS_REG) & KBD_STAT_OBF) == 0) > > - ; > > Seems there is a bug here. There is a cpu_relax() missing. Right, that's exactly what I said in the patch :-). > > > - > > - /* > > - * Fetch the scancode > > - */ > > - scancode = inb(KBD_DATA_REG); > > - scanstatus = inb(KBD_STATUS_REG); > > - > > - while (scanstatus & KBD_STAT_MOUSE_OBF) { > > There should also be a cpu_relax() right here. Correct. > > > - scancode = inb(KBD_DATA_REG); > > - scanstatus = inb(KBD_STATUS_REG); > > - } > > - > > > If you put the two cpu_relax() pieces in do you still end up with a > problem? If this does not work for you the possibility to exists to > clear the keyboard/mouse state on the kdb exit. You will solve the multi-second hangs on ESX. However, this still doesn't fix the bogus messages during typematic repeat (holding down ENTER) or keypad enter key use. Basically, if you nuke the entire ENTER-key special case block, the existing key will correctly handle both typematic, break codes, and KP ENTER key use (as well as any mouse events). Cleanup of mouse/keyboard state is done already on kdb exit by kgdboc, which is the only consumer of the kdb_keyboard code. A ------------------------------------------------------------------------------ Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 _______________________________________________ Kgdb-bugreport mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
