Re: [PATCH] at_keyboard: Fix keyboards that report IBM PC AT scan codes

2020-12-14 Thread Javier Martinez Canillas
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

2020-12-14 Thread Daniel Kiper
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

2020-12-14 Thread Daniel Kiper
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

2020-12-14 Thread John Paul Adrian Glaubitz
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

2020-12-14 Thread Michael Chang via Grub-devel
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

2020-12-14 Thread Renaud Métrich

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