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. Moreover, I wonder - why you want to move efi_console_register() after efi_disk_register(). - why python script can see such escape sequences. > > > > >> 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.g...@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', > >> 'fatload host 0:1 4000000 PK.auth', > >> 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) > >> assert(not re.search('Failed to set EFI variable', > >> ''.join(output))) > >> @@ -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', > >> 'fatload host 0:1 4000000 KEK.auth', > >> 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', > >> 'fatload host 0:1 4000000 PK.auth', > >> @@ -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', > >> 'fatload host 0:1 4000000 KEK.auth', > >> 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', > >> 'fatload host 0:1 4000000 PK.auth', > >> -- > >> 2.26.2 > >> >