| Message ID | 20200731141232.618-1-trini@konsulko.com |
|---|---|
| State | Accepted |
| Commit | 8a5cdf601f8da6704c9146a253c1bcbfcd0e6286 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | test: efi_selftest: Do not force serial# setting | expand |
On Fri, Jul 31, 2020 at 10:12:32AM -0400, Tom Rini wrote: > As part of the EFI self test we set and check the serial# variable. > However, we should not be forcing this setting. In the case where we > are allowed to change the variable it will change, and we will pass the > test. In the case where we cannot change it, force may or may not be > allowed, depending on further environment restrictions. Drop the -f > flag here as we do not need it. > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Tom Rini <trini@konsulko.com> Applied to u-boot/master, thanks!
On 7/31/20 11:42 PM, Tom Rini wrote: > On Fri, Jul 31, 2020 at 10:12:32AM -0400, Tom Rini wrote: > >> As part of the EFI self test we set and check the serial# variable. >> However, we should not be forcing this setting. In the case where we >> are allowed to change the variable it will change, and we will pass the >> test. In the case where we cannot change it, force may or may not be >> allowed, depending on further environment restrictions. Drop the -f >> flag here as we do not need it. >> >> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> >> Signed-off-by: Tom Rini <trini@konsulko.com> > > Applied to u-boot/master, thanks! > With this patch on the Pine64 LTS: => => setenv efi_selftest device tree => => setenv serial# Testing DT ## Error: Can't overwrite "serial#" ## Error inserting "serial#" variable, errno=1 => This worked without your patch. This patch does not solve any problem. Best regards Heinrich
On Fri, Aug 07, 2020 at 09:34:06PM +0200, Heinrich Schuchardt wrote: > On 7/31/20 11:42 PM, Tom Rini wrote: > > On Fri, Jul 31, 2020 at 10:12:32AM -0400, Tom Rini wrote: > > > >> As part of the EFI self test we set and check the serial# variable. > >> However, we should not be forcing this setting. In the case where we > >> are allowed to change the variable it will change, and we will pass the > >> test. In the case where we cannot change it, force may or may not be > >> allowed, depending on further environment restrictions. Drop the -f > >> flag here as we do not need it. > >> > >> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> Signed-off-by: Tom Rini <trini@konsulko.com> > > > > Applied to u-boot/master, thanks! > > > > > With this patch on the Pine64 LTS: > > => => setenv efi_selftest device tree > => => setenv serial# Testing DT > ## Error: Can't overwrite "serial#" > ## Error inserting "serial#" variable, errno=1 > => > > This worked without your patch. > > This patch does not solve any problem. This patch resolved the problems trying to use "env set -f" on platforms where CONFIG_ENV_ACCESS_IGNORE_FORCE is unset and the variable is not protected. With: commit 0f036bf4b87e6416f5c4d23865a62a62d9073c20 Author: Marek Vasut <marex@denx.de> Date: Tue Jul 7 20:51:33 2020 +0200 env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set applied those platforms fail efi_selftest. The answer I believe is that on your platform serial# is protected so you do have to force overwriting it in order to change it. This is not the case of all platforms with a serial# variable.
On 8/7/20 9:50 PM, Tom Rini wrote: > On Fri, Aug 07, 2020 at 09:34:06PM +0200, Heinrich Schuchardt wrote: > >> On 7/31/20 11:42 PM, Tom Rini wrote: >>> On Fri, Jul 31, 2020 at 10:12:32AM -0400, Tom Rini wrote: >>> >>>> As part of the EFI self test we set and check the serial# variable. >>>> However, we should not be forcing this setting. In the case where we >>>> are allowed to change the variable it will change, and we will pass the >>>> test. In the case where we cannot change it, force may or may not be >>>> allowed, depending on further environment restrictions. Drop the -f >>>> flag here as we do not need it. >>>> >>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> >>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> >>> Applied to u-boot/master, thanks! >>> >> >> >> With this patch on the Pine64 LTS: >> >> => => setenv efi_selftest device tree >> => => setenv serial# Testing DT >> ## Error: Can't overwrite "serial#" >> ## Error inserting "serial#" variable, errno=1 >> => >> >> This worked without your patch. >> >> This patch does not solve any problem. > > This patch resolved the problems trying to use "env set -f" on platforms > where CONFIG_ENV_ACCESS_IGNORE_FORCE is unset and the variable is not > protected. With: > commit 0f036bf4b87e6416f5c4d23865a62a62d9073c20 > Author: Marek Vasut <marex@denx.de> > Date: Tue Jul 7 20:51:33 2020 +0200 > > env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set > > applied those platforms fail efi_selftest. The answer I believe is that > on your platform serial# is protected so you do have to force > overwriting it in order to change it. This is not the case of all > platforms with a serial# variable. So no problem is solved except Marek's fancy warning. Best regards Heinrich
On Fri, Aug 07, 2020 at 09:56:58PM +0200, Heinrich Schuchardt wrote: > On 8/7/20 9:50 PM, Tom Rini wrote: > > On Fri, Aug 07, 2020 at 09:34:06PM +0200, Heinrich Schuchardt wrote: > > > >> On 7/31/20 11:42 PM, Tom Rini wrote: > >>> On Fri, Jul 31, 2020 at 10:12:32AM -0400, Tom Rini wrote: > >>> > >>>> As part of the EFI self test we set and check the serial# variable. > >>>> However, we should not be forcing this setting. In the case where we > >>>> are allowed to change the variable it will change, and we will pass the > >>>> test. In the case where we cannot change it, force may or may not be > >>>> allowed, depending on further environment restrictions. Drop the -f > >>>> flag here as we do not need it. > >>>> > >>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> > >>>> Signed-off-by: Tom Rini <trini@konsulko.com> > >>> > >>> Applied to u-boot/master, thanks! > >>> > >> > >> > >> With this patch on the Pine64 LTS: > >> > >> => => setenv efi_selftest device tree > >> => => setenv serial# Testing DT > >> ## Error: Can't overwrite "serial#" > >> ## Error inserting "serial#" variable, errno=1 > >> => > >> > >> This worked without your patch. > >> > >> This patch does not solve any problem. > > > > This patch resolved the problems trying to use "env set -f" on platforms > > where CONFIG_ENV_ACCESS_IGNORE_FORCE is unset and the variable is not > > protected. With: > > commit 0f036bf4b87e6416f5c4d23865a62a62d9073c20 > > Author: Marek Vasut <marex@denx.de> > > Date: Tue Jul 7 20:51:33 2020 +0200 > > > > env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set > > > > applied those platforms fail efi_selftest. The answer I believe is that > > on your platform serial# is protected so you do have to force > > overwriting it in order to change it. This is not the case of all > > platforms with a serial# variable. > > So no problem is solved except Marek's fancy warning. It's not a "fancy warning". It's an important bit of user feedback. In fact, the next question I keep coming back to is, how was, or is, this working before? Without Marek's patch, we silently return 0 and don't change the variable. With Marek's patch we tell the user we aren't doing anything and return 0. So are we actually changing serial# when it's supposed to be unchangable?
diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py index 971c9f6053a3..9b520c2070b2 100644 --- a/test/py/tests/test_efi_selftest.py +++ b/test/py/tests/test_efi_selftest.py @@ -36,7 +36,7 @@ def test_efi_selftest_device_tree(u_boot_console): output = u_boot_console.run_command('bootefi selftest') assert '\'device tree\'' in output u_boot_console.run_command(cmd='setenv efi_selftest device tree') - u_boot_console.run_command(cmd='setenv -f serial# Testing DT') + u_boot_console.run_command(cmd='setenv serial# Testing DT') u_boot_console.run_command(cmd='bootefi selftest ${fdtcontroladdr}', wait_for_prompt=False) m = u_boot_console.p.expect(['serial-number: Testing DT', 'U-Boot']) if m != 0:
As part of the EFI self test we set and check the serial# variable. However, we should not be forcing this setting. In the case where we are allowed to change the variable it will change, and we will pass the test. In the case where we cannot change it, force may or may not be allowed, depending on further environment restrictions. Drop the -f flag here as we do not need it. Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Tom Rini <trini@konsulko.com> --- test/py/tests/test_efi_selftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)