[coreboot-gerrit] Change in coreboot[master]: libpayload/keyboard: Revise keyboard_cmd() error handling

[Nico Huber (Code Review)]

Hello  Felix Singer, Matt Delco, Patrick Georgi, Furquan Shaikh, Julius Werner, Angel Pons, Patrick Rudolph, EricR Lai,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/47083

to review the following change.

Change subject: libpayload/keyboard: Revise keyboard_cmd() error handling
......................................................................

libpayload/keyboard: Revise keyboard_cmd() error handling

Even if we are careful, it's still possible that we read spurious
data from the keyboard, e.g. keystrokes. Namely, when we send the
reset/disable command, there is a race before the command is pro-
cessed.

So we should always process data from the keyboard in a loop. We
break it, when an ACK (0xfa) or a NAK (0xfe) is received, and warn
on unexpected data unless it might be due to the mentioned race.

Change-Id: I60a2643a8ff4b9231c63bf970c8749c97c7d8926
Signed-off-by: Nico Huber 
---
M payloads/libpayload/drivers/i8042/keyboard.c
1 file changed, 24 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47083/1

diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c
index a9b864b..c26babb 100644
--- a/payloads/libpayload/drivers/i8042/keyboard.c
+++ b/payloads/libpayload/drivers/i8042/keyboard.c
@@ -28,6 +28,7 @@
  */

#include <stdbool.h>
+#include <stdint.h>

#include <keycodes.h>
 #include <libpayload-config.h>
@@ -173,9 +174,31 @@

static bool keyboard_cmd(unsigned char cmd)
 {
+	const uint64_t timeout_us = 1*1000*1000;
+	const uint64_t start_time = timer_us(0);
+
    i8042_write_data(cmd);

-	return i8042_wait_read_ps2() == 0xfa;
+	do {
+		const int ret = i8042_wait_read_ps2();
+		switch (ret) {
+		case -1:
+			/* We have our own timout handling. */
+			continue;
+		case 0xfa:
+			return true;
+		case 0xfe:
+			return false;
+		default:
+			/* Warn only if we already disabled keyboard input. */
+			if (cmd != I8042_KBCMD_DEFAULT_DIS)
+				printf("WARNING: Keyboard sent spurious 0x%02x.\n", ret);
+			continue;
+		}
+	} while (timer_us(start_time) < timeout_us);
+
+	printf("ERROR: Keyboard command timed out.\n");
+	return false;
 }

bool keyboard_havechar(void)