-
-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC : a x230 user has keyboard issue being raw translated 2 instead of AT translated 2 #1525
Conversation
…f AT translated 2. Deactivate coreboot ps2 init Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Well, last time we attempted to fix this was on 4.14 kernel and coreboot 4.13 at #1302 (comment) The fix above doesn't expect to fix issue with x230t, but maybe will fix issue that wasn't there for user prior of flashing from PB (coreboot 4.13 and kernel 4.14) to latest. Let's still check that first. |
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…ads behavior Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… 2 from coreboot to heads and on final kexec'ed os Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…2 from coreboot to heads and on final kexec'ed os Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… x230t and traces from users Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
f4296e3
to
f9b8e6e
Compare
Without coreboot ps2 init, user reports no joy. |
I have no clue what to do with that. We see that successful and failing behavior is linked with configuration of ps2 controller through i8042 which is not configuring the keyboard the same way. Observations:
Following logs have been stripped for comparison doing:
And then anything after dectection of keyboard has been removed to protect user's typed keystrokes related privacy/disclosure of irrelevant information. |
Let's analyse the debug logs above.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
I have thinkpad x230 with heads (not sure the version) and arch Linux as OS, I had this exact problem more or less a few months ago, I think it was a bug in systemd, I found the error appearing in versions from systemd 253.2+, later resolved (I don't know when exactly, about mid 254 or 255?) I am not really able to use my laptop now, but I'd say first watch if using systemd and its version and try a downgrade (first to 252.x, but all of them should work up to 253.1) |
…up exists. Might want to discuss that implementation. Some functions needed to be moved from functions to ash_functions so that gpg_auth can be called from recovery function. That might need to be discussed as well, recovery could be moved from ash_functions to functions instead. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…s linked to detach signing errors Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…not so and adapt further troubleshooting notes in code when keys cannot be accessed on media for whatever cause so user can understand what is happening when accessing GPG material on backup thumb drive Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…used with GPG key material thumb drive backup Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…should be written to file and reused since not all in same functions/files for TPM2 Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…o to recovery shell instead of rebooting Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…S=y and checks for it; the default of oem-factory-reset is now to propose user to use defaults first for simplicity of most common use case without allianating advanced users which can simply not accept the default and answer questionnaire Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Prompt for TPM owner password internally within tpm2_counter_create. Add tpm1_counter_create to prompt for password internally. Wipe the cache in either if the operation fails, in case the password was incorrect. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
…DEBUG. Put back console output as of master. TODO: decide what we do with tpmr extend output for the future. Hint: forward sealing of next flashed firmware measurements. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…nerating keys on smartcard without backup, not a wanring anymore Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…logic. Also move USB Security dongle capability detection under code already checking for USB Security Dongle's smartcard presence. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
config/linux-librem_common-6.1.8.config: passed to oldconfig format through 'make BOARD=librem_14 linux.modify_and_save_oldconfig_in_place' Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…storage size and loop until none is connected to exit loop. Warn user if connected usb block device is less then 128mb, since creating LUKS container of less then 8mb might cause issues. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Combine prompt to disconnect other devices with prompt to connect the desired device. Show block device sizes in MB/GB when selecting device so it is easier to select. file_selector now supports --show-size to include block device sizes in menu. Rework file_selector so menu options can contain spaces (use bash array) and to simplify logic. Prompt to select flash drive and LUKS percentage in OEM reset before actually taking any actions, so aborting doesn't half-reset the system. Abort OEM reset if user aborts the flash drive selection instead of looping forever. (Canceling the confirmation still loops to retry but it is possible to exit by aborting the repeated menu.) Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Move confirmation of formatting flash drive with LUKS percentage selection before any reset actions have been taken, so aborting does not result in a half-reset system. Combine with the more basic "confirm" prompt that existed after selecting the device (but did not include the LUKS size information). Split up prepare_flash_drive into interactive_prepare_flash_drive (both prompts and formats as before), confirm_thumb_drive_format (just confirms the selections), and prepare_thumb_drive (now noninteractive). Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Don't repeat this message if the user says "no" to the confirmation prompt. Go directly to the menu. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Allow configuring the ZIP-format update file extension with CONFIG_BRAND_UPDATE_PKG_EXT in board config. Default is 'zip'. Create update package in the default Makefile target. Delete create_npf.sh. Do not require /tmp/verified_rom in the update file package's sha256sum.txt (but allow it for backward compatibility). Show the integrity error if unzip fails instead of dying (which returns to main menu with no explanation, error is left on recovery console). This is the most likely way corruption would be detected as ZIP has CRCs. The sha256sum is still present for more robust detection. Don't require the ROM to be the first file in sha256sum.txt since it raises complexity of adding more files to the update archive in the future. Instead require that the package contains exactly one file matching '*.rom'. Restore confirmation prompt for the update-package flow, at some point this was lost. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
talos-2 (only) uses .tgz instead of .rom for updates. Currently, both are treated as alternatives to a ZIP-format update archive with SHA-256 integrity check, extend that to the prompts to reduce clutter. Reflow the "You will need ... your BIOS image" prompt to fit on fbwhiptail. The .tgz format could be better integrated with the ZIP updates, but this needs more work specific to talos-2. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
The only purpose of legacy flash boards is to be flashed over vendor firmware using an exploit, to then flash non-maximized Heads firmware. They are never upgraded to another legacy flash build, and they move the coreboot ROM from the build directory, so don't build an update package for those boards. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Only add the update package to all if it is actually being built, fixes default target with CONFIG_LEGACY_FLASH=y. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
talos-2 builds its own tgz update package that is not currently integrated with the zip method. While the zip method right now would theoretically if the tgz was inside it, this would have to be hooked up for talos-2 specifically. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
…sh prompt when talos-2 ato validate hashes against hashes provided under tgz through flash.sh validation (still offer zip and tgz, which tgz might change to zip later but only tgz offered through builds) Attempt to address linuxboot#1526 (comment) Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Nitrokey is going to switch from npf to zip per discussion. Remove this config. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… and selftest fails since we want AT translated to be setuped in all case, eaving coreboot to decide for proper implementation. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Its a different thing if OS applies udev/systemd hacks on detected hardware IDs then coreboot not being able to detect the keyboard correctly in the sense of talking with it correctly/iitializing it correctly creating different behaviors across reboot (warm reset: not all components being resetted) and cold boot (cold power on) or even on reset. I just pushed an ugly hack under 1a6fbfa for a user to test and report behavior from cold boot (which is source of issue) and reboot, giving info from the source (cbmem -1 giving ps2 probe/detection/errors) and Heads dmesg output. This patch is augmenting timeout of pc80 code, removing exit codes on two of the functions (leaving debugging info for coreboot devels to take decision) and continuing instead of abording, since pc80 code following is actually the one responsible to report of how ps2 keyboard communicates through IOs. This is definitely not production ready and a plain PoC on top of master, but the output should convince coreboot that there is an issue and that it should be resolved for x230t/x220t and now x230 users of coreboot+linux, where otherwise the keyboard is left in a weird state where workarounds need to be applied from OS (where otherwise seabios has workarounds that are applied automatically and hides the problem). |
Next steps QubesOS/qubes-issues#3306 (comment) |
Another attempt to discuss over coreboot channel with collected evidence from users testing of above ugly patch: https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$vgBbogOreZfS41d-yJInWqAUUgvUTvfJLzLlokfMUY8?via=matrix.org&via=libera.chat&via=sibnsk.net (self-tracking) icon on coreboot suggests to try to adapt libpayload quirks to pc80 or rewrite something else since enabling ps2 keyboard under coreboot is adding delay on boot. Also suggests that the payload (linux) should do it properly. Unfortunately, I'm not sure I get the details properly. As I currently understand the flow of pc80 code, even if everything goes wrong, we could bypass all timeouts and tests (exit 0 removed in patch) and set AT 2 translated mode and be happy. Otherwise quirks attempted (i8042.notimeout, i8042.reset, i8042.translate=1:kbd were not helpful having coldboot/warm boot having AT Translated 2 consistently for some users. Is it EC firmware fault, keyboard SKU, things are unclear and I can't replicate). |
Opened coreboot issue https://ticket.coreboot.org/issues/515 |
I would really prefer not going that path, but this is pseudo code generated by bing discussion that could be applied to i8042 and start discussion with Linux folks for linuxboot use cases. Disclaimer: Not tested, might be wrong and would not want to maintain that forever as patch under heads but if works with coreboot ps2 keyboard deactivated, might be useful to prove validity of new option and would permit boards that need it to just enable it under correhoot->Linux COMMAND_LINE to initialize ps2 keyboard properly from the "payload" (firmware) by simply adding "i8042.forceat2" there.
|
An optimized first approach to patch the i8042 Linux kernel code is to modify the i8042_kbd_init function in the i8042.c file, which is responsible for initializing and probing the PS/2 keyboard. This approach adds a code block that sends the command 0xF0 to the keyboard port, followed by the argument 0x02, which sets the keyboard to use the AT Translated Set 2 protocol. This is similar to what you did in the pc80 keyboard.c file in the heads project. This approach also adds a new kernel parameter, i8042.forceat2, that enables this modification. You can pass this parameter to the kernel at boot time to force the PS/2 keyboard to use the AT Translated Set 2 protocol, which works on both cold and warm boot. Here is what the code for this approach looks like: --- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -38,6 +38,7 @@
#define I8042_KBD_IRQ 1
#define I8042_AUX_IRQ 12
+#define I8042_CMD_SET_AT2 0xF0
#define I8042_CMD_GETID 0xF2
#define I8042_CMD_AUX_LOOP 0xD3
#define I8042_CMD_AUX_SEND 0xD4
@@ -105,6 +106,7 @@ static bool i8042_bypass_aux_irq_test;
static bool i8042_check_reset;
static bool i8042_dritek;
static bool i8042_dumbkbd;
+static bool i8042_forceat2;
static bool i8042_noaux;
static bool i8042_nokbd;
static bool i8042_nomux;
@@ -122,6 +124,7 @@ module_param_named(bypass_aux_irq_test, i8042_bypass_aux_irq_test, bool, 0);
module_param_named(check_reset, i8042_check_reset, bool, 0);
module_param_named(dritek, i8042_dritek, bool, 0);
module_param_named(dumbkbd, i8042_dumbkbd, bool, 0);
+module_param_named(forceat2, i8042_forceat2, bool, 0);
module_param_named(noaux, i8042_noaux, bool, 0);
module_param_named(nokbd, i8042_nokbd, bool, 0);
module_param_named(nomux, i8042_nomux, bool, 0);
@@ -1138,6 +1141,16 @@ static int __init i8042_kbd_init(void)
return -ENODEV;
}
+ if (i8042_forceat2) {
+ retval = i8042_wait_write();
+ if (retval)
+ return retval;
+
+ dbg("%02x -> i8042 (kbd init)", I8042_CMD_SET_AT2);
+ i8042_write_data(I8042_CMD_SET_AT2);
+ i8042_write_data(0x02);
+ }
+
if (i8042_dumbkbd) {
i8042_kbd_read_data = i8042_dumbkbd_read_data;
i8042_kbd_read_status = i8042_dumbkbd_read_status; This code adds a conditional statement that checks if the i8042.forceat2 kernel parameter is enabled. If the condition is true, it sends the command 0xF0 to the keyboard port, followed by the argument 0x02, which sets the keyboard to use the AT Translated Set 2 protocol. This code is executed only once, after the keyboard is initialized and probed. This avoids sending the command and the argument unnecessarily every time the i8042_command function is called. |
A second optimized approach to patch the i8042 Linux kernel code is to modify the i8042_kbd_get_id function in the i8042.c file, which is responsible for sending the probe command (0xF2) and reading the keyboard ID. This approach adds a fallback mechanism that retries the probe command or assumes a default ID for the keyboard (0xab83) if the keyboard does not respond or responds with an invalid ID. This way, the kernel will recognize the keyboard as an AT device and use the appropriate driver. This approach also uses the same kernel parameter, i8042.forceat2, to enable this modification. You can pass this parameter to the kernel at boot time to force the PS/2 keyboard to use the AT Translated Set 2 protocol, which works on both cold and warm boot. Here is what the code for this approach looks like: --- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -38,6 +38,7 @@
#define I8042_KBD_IRQ 1
#define I8042_AUX_IRQ 12
+#define I8042_CMD_SET_AT2 0xF0
#define I8042_CMD_GETID 0xF2
#define I8042_CMD_AUX_LOOP 0xD3
#define I8042_CMD_AUX_SEND 0xD4
@@ -105,6 +106,7 @@ static bool i8042_bypass_aux_irq_test;
static bool i8042_check_reset;
static bool i8042_dritek;
static bool i8042_dumbkbd;
+static bool i8042_forceat2;
static bool i8042_noaux;
static bool i8042_nokbd;
static bool i8042_nomux;
@@ -122,6 +124,7 @@ module_param_named(bypass_aux_irq_test, i8042_bypass_aux_irq_test, bool, 0);
module_param_named(check_reset, i8042_check_reset, bool, 0);
module_param_named(dritek, i8042_dritek, bool, 0);
module_param_named(dumbkbd, i8042_dumbkbd, bool, 0);
+module_param_named(forceat2, i8042_forceat2, bool, 0);
module_param_named(noaux, i8042_noaux, bool, 0);
module_param_named(nokbd, i8042_nokbd, bool, 0);
module_param_named(nomux, i8042_nomux, bool, 0);
@@ -1070,6 +1073,16 @@ static int i8042_kbd_get_id(struct i8042_port *port)
if (retval)
return retval;
+ if (i8042_forceat2) {
+ retval = i8042_wait_write();
+ if (retval)
+ return retval;
+
+ dbg("%02x -> i8042 (kbd get id)", I8042_CMD_SET_AT2);
+ i8042_write_data(I8042_CMD_SET_AT2);
+ i8042_write_data(0x02);
+ }
+
retval = i8042_wait_write();
if (retval)
return retval;
@@ -1081,6 +1094,15 @@ static int i8042_kbd_get_id(struct i8042_port *port)
if (retval)
return retval;
+ if (i8042_forceat2 && !port->exists) {
+ dbg("keyboard did not respond to probe command, retrying");
+ retval = i8042_wait_write();
+ if (retval)
+ return retval;
+
+ i8042_write_data(I8042_CMD_GETID);
+ }
+
if (port->exists) {
retval = i8042_wait_read();
if (retval == 0) {
@@ -1094,6 +1116,12 @@ static int i8042_kbd_get_id(struct i8042_port *port)
}
}
}
+
+ if (i8042_forceat2 && !port->exists) {
+ dbg("keyboard still did not respond to probe command, assuming AT Translated Set 2");
+ port->exists = true;
+ port->id = 0xab83;
+ }
return 0;
} This code adds a conditional statement that checks if the i8042.forceat2 kernel parameter is enabled. If the condition is true, it sends the command 0xF0 to the keyboard port, followed by the argument 0x02, which sets the keyboard to use the AT Translated Set 2 protocol. This code is executed before sending the probe command (0xF2) to the keyboard. This code also adds another conditional statement that checks if the i8042.forceat2 kernel parameter is enabled and if the keyboard does not exist (port->exists is false). If the condition is true, it retries the probe command (0xF2) or assumes a default ID for the keyboard (0xab83) if the keyboard does not respond or responds with an invalid ID. This code is executed after sending the probe command (0xF2) to the keyboard. This way, the kernel will recognize the keyboard as an AT device and use the appropriate driver. |
Not needed anymore see #1744 |
This is PoC testing with user having inconsistent issues. Randomly properly detected on boot and not, just like for x230t under #958 which was never completely resolved.
First things first