Re: [PATCH] at_keyboard: Fix keyboards that report IBM PC AT scan codes
Hello Vladimir, On 12/11/20 3:08 PM, Vladimir 'phcoder' Serbinenko wrote: > On Fri, Dec 4, 2020 at 5:53 AM Renaud Métrich wrote: >> >> Hi, >> >> Testing the proposed patch on my old Asus N53SN in Legacy failed: as soon as >> at_keyboard is selected, the keys are corrupted and it's impossible to do >> anything. >> >> Digging into this, it appears that query_mode() returns 2 (so set2 needs to >> be used), but in fact internally the keycode are the ones expected by set1. > > This is because the patch doesn't take into account that controller is > in "translate" mode. > @Javier Martinez Canillas : Can you make your patch check whether > KEYBOARD_AT_TRANSLATE is set ? > Thanks a lot for you feedback. Renaud is working on a new patch that takes this into account and will post to the mailing list after doing some testing on the machines that where having the problem. Best regards, Javier ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v8 00/18] Cryptodisk fixes for v2.06 redux
On Sat, Dec 12, 2020 at 06:36:58PM +0100, Patrick Steinhardt wrote: > On Sat, Dec 12, 2020 at 03:40:30PM +0100, Daniel Kiper wrote: > > On Sat, Dec 12, 2020 at 09:20:24AM +0100, Patrick Steinhardt wrote: > > > On Thu, Dec 10, 2020 at 05:20:59PM +0100, Daniel Kiper wrote: > > > > On Tue, Dec 08, 2020 at 04:45:31PM -0600, Glenn Washburn wrote: > > > > > This patch series, fixes issues with v7 series. Some of those patches > > > > > had > > > > > changes applied to the wrong patch. The added structure member was > > > > > renamed > > > > > again, this time to idx. And there is an added patch that renames > > > > > some json > > > > > index variables to note that they are such. > > > > > > > > > > Glenn > > > > > > > > > > Glenn Washburn (18): > > > > > disk: Rename grub_disk_get_size to grub_disk_native_sectors > > > > > misc: Add parentheses around ALIGN_UP and ALIGN_DOWN arguments > > > > > whitespace: convert 8 spaces to tabs > > > > > luks2: Remove unused argument in grub_error > > > > > luks2: Make sure all fields of output argument in > > > > > luks2_parse_digest() > > > > > are written to > > > > > luks2: Add idx member to struct grub_luks2_keyslot/segment/digest > > > > > luks2: Use more intuitive object name instead of json index in user > > > > > messages > > > > > luks2: Rename json index variables to names that they are obviously > > > > > json indexes > > > > > luks2: Add string "index" to user strings using a json index. > > > > > cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals > > > > > cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals > > > > > luks2: grub_cryptodisk_t->total_sectors is the max number of device > > > > > native sectors > > > > > cryptodisk: Properly handle non-512 byte sized sectors > > > > > luks2: Better error handling when setting up the cryptodisk > > > > > luks2: Error check segment.sector_size > > > > > mips: Enable __clzdi2() > > > > > misc: Add grub_log2ull macro for calculating log base 2 of 64-bit > > > > > integers > > > > > luks2: Use grub_log2ull to calculate log_sector_size and improve > > > > > readability > > > > > > > > I will get all the patches except #14 and maybe some changes after that > > > > one which does not apply due to lack of it. > > > > Thanks! However, I have pushed some of these patches before you did review. > > > > Daniel > > I thought as much, though I didn't check. I don't really mind, thought > it might make sense to send them out in any case. Yeah, at least I am more sure I did not miss any mistakes... :-) Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB 2.06 release
On Wed, Jul 29, 2020 at 07:46:43PM +0200, Daniel Kiper wrote: > On Fri, May 29, 2020 at 02:07:29PM +0200, Daniel Kiper wrote: > > On Wed, Apr 22, 2020 at 12:24:40PM +0200, Daniel Kiper wrote: > > > On Mon, Mar 16, 2020 at 05:41:29PM +0100, Daniel Kiper wrote: > > > > On Wed, Mar 11, 2020 at 11:47:35AM +0100, Daniel Kiper wrote: > > > > > On Tue, Mar 03, 2020 at 06:26:03PM +0100, Daniel Kiper wrote: > > > > > > On Wed, Feb 19, 2020 at 04:01:38PM +0100, Daniel Kiper wrote: > > > > > > > Hi all, > > > > > > > > > > > > > > As I told during my FOSDEM 2020 presentation we are preparing for > > > > > > > GRUB 2.06 release. Tentative schedule is below: > > > > > > > - code freeze: 15th of March, 23:59:59 UTC; everything posted > > > > > > > after > > > > > > > that date will not be considered as a release material, > > > > > > > > > > > > Just a kind reminder... Less than two weeks left... > > > > > > > > > > Less than a week left... This is final call... > > > > > > > > Freeze in force! No new features are accepted at this point! > > > > > > > > I will be reviewing all patches posted before the end of Sunday. All of > > > > them will be considered for inclusion in the release. Additionally, all > > > > features under active development and discussed on grub-devel still can > > > > be considered for inclusion. However, if there is a substantial danger > > > > of destabilizing the code or delaying the release I may reject any patch > > > > at any point. > > > > > > > > I am especially interested in fixes and cleanups at this point... > > > > > > > > I am going to cut an RC in 2-4 weeks. > > > > > > Bad news is that we have delays... :-( Good news is that everything is > > > going > > > forward... :-) I merged next portion of patches yesterday. I am working > > > on some fixes for issues which I found. I am also trying to work out > > > the best solution for other bugs which are lingering here and there. So, > > > please be patient... However, if you have any questions or suggestions > > > just drop me a line... > > > > Unfortunately there are still delays due to some unexpected things > > happening... Though I am still ploughing through backlog. I have > > recently posted a few fixes and cleanups required for rc1. Sadly there > > are still a few issues which have to be fixed. I am working on them. So, > > I hope that in the following weeks we will be able to cut rc1... > > > > If you are waiting for my review please bear with me. I will get back to > > you as soon as I can. Though I am mostly looking at important fixes in > > first order right now. > > I think this link [1] will explain my long absence... Sorry about that. > > I am going to go back to GRUB work next week. I will triage all the patches > and take all (obvious) fixes. Then I will release rc1 ASAP... All new features > will be taken after 2.06 release. > > Daniel > > [1] https://lists.gnu.org/archive/html/grub-devel/2020-07/msg00034.html Huh... This took longer than I wanted to. Anyway, I am going to cut 2.06-rc1 by the of this week. So, this is the final call to say what is missing in current master. I am going to take only critical code fixes after RC. Documentation, tests, etc. changes still will be welcome. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB 2.06 release
Hi Daniel! On 12/14/20 1:39 PM, Daniel Kiper wrote: > Huh... This took longer than I wanted to. Anyway, I am going to cut > 2.06-rc1 by the of this week. So, this is the final call to say what is > missing in current master. I am going to take only critical code fixes > after RC. Documentation, tests, etc. changes still will be welcome. Great! Kudos and thanks to everyone who has contributed to this effort! Let me know when you want me to run some basic tests on all Debian targets. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
On Thu, Dec 10, 2020 at 05:50:53PM +0100, Daniel Kiper wrote: > On Tue, Dec 08, 2020 at 10:20:03AM +0800, Michael Chang via Grub-devel wrote: > > On Thu, Dec 03, 2020 at 04:01:49PM +0100, Javier Martinez Canillas wrote: > > > The shim_lock module registers a verifier to call shim's verify, but the > > > handler is registered even when the shim_lock protocol was not installed. > > > > > > This doesn't cause a NULL pointer dereference in shim_lock_write() because > > > the shim_lock_init() function just returns GRUB_ERR_NONE if sl isn't set. > > > > > > But in that case there's no point to even register the shim_lock verifier > > > since won't do anything. Additionally, it is only useful when Secure Boot > > > is enabled. > > > > > > Finally, don't assume that the shim_lock protocol will always be present > > > when the shim_lock_write() function is called, and check for it on every > > > call to this function. > > > > > > Reported-by: Michael Chang > > > > To complete the information here, this fixed the problem I tried to > > solve before, but in a more elegant way. :) > > > > https://www.mail-archive.com/grub-devel@gnu.org/msg30738.html > > > > Thank you to work on the patch. > > You are welcome! > > May I add your Tested-by do this patch? Sure you can. I have verified that it solved the problem, despite for the unexpected build error. ../../grub-core/commands/efi/shim_lock.c:121:21: error: implicit declaration of function ‘grub_efi_get_secureboot’; did you mean ‘grub_efi_get_device_path’? [-Werror=implicit-function-declaration] 121 | if (sl == NULL || grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED) FWIW, the trivial patch I use to get around above build error is included. diff --git a/grub-core/commands/efi/shim_lock.c b/grub-core/commands/efi/shim_lock.c index 5259b27e8..b0c3cc178 100644 --- a/grub-core/commands/efi/shim_lock.c +++ b/grub-core/commands/efi/shim_lock.c @@ -24,6 +24,7 @@ #include #include #include +#include GRUB_MOD_LICENSE ("GPLv3+"); Thanks, Michael > > Daniel > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] at_keyboard: Fix keyboards that report IBM PC AT scan codes
Hi Vladimir, Thanks for the hint, this was obvious now. Please find attached the new patch which definitely fixes the issue. It has been tested on various hardware (see git commit details). In a nutshell the solution is to stick to set 1 if controller is in Translate mode, and use set X otherwise, X being the queried mode. Additionally, in controller_fini, nothing has to be restored, since nothing was changed. This fixes an issue when switching between at_keyboard, console, and at_keyboard again, in case queried set is not the actual used set. Renaud. On 12/11/20 3:08 PM, Vladimir 'phcoder' Serbinenko wrote: On Fri, Dec 4, 2020 at 5:53 AM Renaud Métrich wrote: Hi, Testing the proposed patch on my old Asus N53SN in Legacy failed: as soon as at_keyboard is selected, the keys are corrupted and it's impossible to do anything. Digging into this, it appears that query_mode() returns 2 (so set2 needs to be used), but in fact internally the keycode are the ones expected by set1. This is because the patch doesn't take into account that controller is in "translate" mode. @Javier Martinez Canillas : Can you make your patch check whether KEYBOARD_AT_TRANSLATE is set ? ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel >From eac16e0623fa99ae5a6642852321cb8f9c74b33d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Renaud=20M=C3=A9trich?= Date: Thu, 3 Dec 2020 09:13:24 +0100 Subject: [PATCH] at_keyboard: use set 1 when keyboard is in Translate mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When keyboard controller acts in Translate mode (0x40 mask), then use set 1 since translation is done. Otherwise use the mode queried from the controller (usually set 2). Added "atkeyb" debugging messages in at_keyboard module as well. Resolves: rhbz#1897587 Tested on: - Asus N53SN (set 1 used) - Dell Precision (set 1 used) - HP Elitebook (set 2 used) - HP G5430 (set 1 used, keyboard in XT mode!) - Lenovo P71 & Lenovo T460s (set 2 used) - QEMU/KVM (set 1 used) Signed-off-by: Renaud Métrich --- grub-core/term/at_keyboard.c | 29 - include/grub/at_keyboard.h | 4 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/grub-core/term/at_keyboard.c b/grub-core/term/at_keyboard.c index 597111077..260143826 100644 --- a/grub-core/term/at_keyboard.c +++ b/grub-core/term/at_keyboard.c @@ -135,20 +135,28 @@ query_mode (void) int e; e = write_mode (0); - if (!e) + if (!e) { +grub_dprintf("atkeyb", "query_mode: write_mode(0) failed\n"); return 0; + } do { keyboard_controller_wait_until_ready (); ret = grub_inb (KEYBOARD_REG_DATA); } while (ret == GRUB_AT_ACK); /* QEMU translates the set even in no-translate mode. */ - if (ret == 0x43 || ret == 1) + if (ret == 0x43 || ret == 1) { +grub_dprintf("atkeyb", "query_mode: returning 1 (ret=0x%x)\n", ret); return 1; - if (ret == 0x41 || ret == 2) + } + if (ret == 0x41 || ret == 2) { +grub_dprintf("atkeyb", "query_mode: returning 2 (ret=0x%x)\n", ret); return 2; - if (ret == 0x3f || ret == 3) + } + if (ret == 0x3f || ret == 3) { +grub_dprintf("atkeyb", "query_mode: returning 3 (ret=0x%x)\n", ret); return 3; + } return 0; } @@ -165,7 +173,13 @@ set_scancodes (void) } #if !USE_SCANCODE_SET - ps2_state.current_set = 1; + if ((grub_keyboard_controller_orig & KEYBOARD_AT_TRANSLATE) == KEYBOARD_AT_TRANSLATE) { +grub_dprintf ("atkeyb", "queried set is %d but keyboard in Translate mode, so actually in set 1\n", grub_keyboard_orig_set); +ps2_state.current_set = 1; + } else { +grub_dprintf ("atkeyb", "using queried set %d\n", grub_keyboard_orig_set); +ps2_state.current_set = grub_keyboard_orig_set; + } return; #else @@ -266,6 +280,7 @@ grub_keyboard_controller_init (void) grub_keyboard_orig_set = 2; #else grub_keyboard_controller_orig = grub_keyboard_controller_read (); + grub_dprintf ("atkeyb", "grub_keyboard_controller_orig = 0x%x\n", grub_keyboard_controller_orig); grub_keyboard_orig_set = query_mode (); #endif set_scancodes (); @@ -275,11 +290,15 @@ grub_keyboard_controller_init (void) static grub_err_t grub_keyboard_controller_fini (struct grub_term_input *term __attribute__ ((unused))) { +/* In !USE_SCANCODE_SET mode, we didn't change anything, so nothing to restore */ +#if USE_SCANCODE_SET if (ps2_state.current_set == 0) return GRUB_ERR_NONE; + grub_dprintf ("atkeyb", "restoring set %d, controller 0x%x\n", grub_keyboard_orig_set, grub_keyboard_controller_orig); if (grub_keyboard_orig_set) write_mode (grub_keyboard_orig_set); grub_keyboard_controller_write (grub_keyboard_controller_orig); +#endif return GRUB_ERR_NONE; } diff --git a/include/grub/at_keyboard.h b/include/grub/at_keyboard.h index bcb4d9ba7..9414dc1b9 100644 --- a/include/grub/at_keyboa