Heinrich,
On Fri, May 08, 2020 at 08:10:28AM +0900, AKASHI Takahiro wrote:
On Thu, May 07, 2020 at 11:14:17PM +0200, Heinrich Schuchardt wrote:
On 5/7/20 2:36 AM, AKASHI Takahiro wrote:
Heinrich,
On Mon, May 04, 2020 at 12:33:26PM +0200, Heinrich Schuchardt wrote:
When setting up the console via function efi_console_register() we call query_console_serial(). This functions sends an escape sequence to the terminal to query the display size. The response is another escape sequence.
console.run_command_list() is looking for a regular expression '^==>'. If the escape sequence for the screen size precedes the prompt without a line break, no match is found.
When efi_disk_register() is called before efi_console_register() this leads to a test failuere of the UEFI secure boot tests.
Why does the order of those calls affect test results?
Please, have a look at function query_console_serial() and at run_command_list().
As described above: '\e7\e[r\e[999;999H\e[6n\e8==>' cannot be matched by regular expression '^==>'.
(Probably) right, but what I don't get here is why efi_disk_register() have an impact on efi_console_register(). The former function has nothing to do with any console behaviors.
You have merged your patch without replying to my comment.
Moreover, I wonder
- why you want to move efi_console_register() after efi_disk_register().
- why python script can see such escape sequences.
I don't like your fix. With your fixes, my secure boot pytest now fails to run on sandbox locally.
Instead, I propose: 1. revert your commits commit 16ad946f41d3 ("efi_loader: change setup sequence") commit 5827c2545849 ("test: stabilize test_efi_secboot") 2. move efi_console_register() forward *before* efi_console_register()
Then my secure boot test should work again without any modification. I believe that my solution is much better than your workaround.
-Takahiro Akashi
We can avoid the problem if the first UEFI command passed to u_boot_console.run_command_list() produces output. This patch achieves this by appending '; echo' to the first UEFI related command of the problematic tests.
It looks to be a workaround. We'd better have another approach to fix the problem. Otherwise, similar issues will occur again.
I would not like to change the logic in Python to detect the U-Boot prompt for something UEFI specific. And we need a method to determine the size of a serial console.
The current approach allowed me to merge your patch series which otherwise might not have reached the v2020.07 release. Did the problem not show up in your Travis CI testing?
No. If your saying is correct, this can happen only if efi_console_register() is moved after efi_disk_register(). Right?
If you have a better solution, we can easily merge your patch.
First, I want to understand what's happening.
-Takahiro Akashi
Best regards
Heinrich
Thanks, -Takahiro Akashi
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
test/py/tests/test_efi_secboot/test_authvar.py | 8 ++++---- test/py/tests/test_efi_secboot/test_signed.py | 4 ++-- test/py/tests/test_efi_secboot/test_unsigned.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py index 55dcaa95f1..9912694a3e 100644 --- a/test/py/tests/test_efi_secboot/test_authvar.py +++ b/test/py/tests/test_efi_secboot/test_authvar.py @@ -133,7 +133,7 @@ class TestEfiAuthVar(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 PK.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK; echo', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 db.auth',@@ -174,7 +174,7 @@ class TestEfiAuthVar(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 PK.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK; echo', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 db.auth',@@ -215,7 +215,7 @@ class TestEfiAuthVar(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 PK.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK; echo', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 db.auth',@@ -249,7 +249,7 @@ class TestEfiAuthVar(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 PK.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK; echo', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 db.auth',diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 584282b338..fc722ab506 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -29,7 +29,7 @@ class TestEfiSignedImage(object): # Test Case 1a, run signed image if no db/dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""; echo', 'efidebug boot next 1', 'bootefi bootmgr']) assert(re.search('Hello, world!', ''.join(output)))@@ -81,7 +81,7 @@ class TestEfiSignedImage(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 db.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx; echo', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth',diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py index 22d849afb8..a4af845c51 100644 --- a/test/py/tests/test_efi_secboot/test_unsigned.py +++ b/test/py/tests/test_efi_secboot/test_unsigned.py @@ -30,7 +30,7 @@ class TestEfiUnsignedImage(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 KEK.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK; echo',@@ -58,7 +58,7 @@ class TestEfiUnsignedImage(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 db_hello.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize db; echo',@@ -82,7 +82,7 @@ class TestEfiUnsignedImage(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 db_hello.auth',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx; echo',-- 2.26.2