* [PATCH] test: efi_selftest: Do not force serial# setting
@ 2020-07-31 14:12 Tom Rini
2020-07-31 21:42 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2020-07-31 14:12 UTC (permalink / raw)
To: u-boot
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(-)
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:
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] test: efi_selftest: Do not force serial# setting
2020-07-31 14:12 [PATCH] test: efi_selftest: Do not force serial# setting Tom Rini
@ 2020-07-31 21:42 ` Tom Rini
2020-08-07 19:34 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2020-07-31 21:42 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200731/fd5ac547/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] test: efi_selftest: Do not force serial# setting
2020-07-31 21:42 ` Tom Rini
@ 2020-08-07 19:34 ` Heinrich Schuchardt
2020-08-07 19:50 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2020-08-07 19:34 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] test: efi_selftest: Do not force serial# setting
2020-08-07 19:34 ` Heinrich Schuchardt
@ 2020-08-07 19:50 ` Tom Rini
2020-08-07 19:56 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2020-08-07 19:50 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200807/2c1b0219/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] test: efi_selftest: Do not force serial# setting
2020-08-07 19:50 ` Tom Rini
@ 2020-08-07 19:56 ` Heinrich Schuchardt
2020-08-07 20:12 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2020-08-07 19:56 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] test: efi_selftest: Do not force serial# setting
2020-08-07 19:56 ` Heinrich Schuchardt
@ 2020-08-07 20:12 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2020-08-07 20:12 UTC (permalink / raw)
To: u-boot
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?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200807/16824adb/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-07 20:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-31 14:12 [PATCH] test: efi_selftest: Do not force serial# setting Tom Rini
2020-07-31 21:42 ` Tom Rini
2020-08-07 19:34 ` Heinrich Schuchardt
2020-08-07 19:50 ` Tom Rini
2020-08-07 19:56 ` Heinrich Schuchardt
2020-08-07 20:12 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox