On Fri, Feb 18, 2022 at 02:48:54PM +0100, Heinrich Schuchardt wrote: > On 2/18/22 03:16, Masami Hiramatsu wrote: > > Hi Simon, > > > > Thank you for your reply. > > > > 2022年2月18日(金) 2:56 Simon Glass <s...@chromium.org>: > > > > > > > > Hi Masami, > > > > > > On Wed, 16 Feb 2022 at 18:11, Masami Hiramatsu > > > <masami.hirama...@linaro.org> wrote: > > > > > > > > Hi Simon, > > > > > > > > Let me confirm your point. > > > > So are you concerning the 'real' reset for the capsule update test > > > > case itself or this patch? > > > > > > > > I'm actually learning how the test is working, so please help me to > > > > understand how I can solve it. > > > > > > > > There are 3 environments to run the test, sandbox, Qemu, and a real > > > > board. > > > > If we reset a sandbox, it will continue to run (just restart itself), > > > > > > Here you should be able to avoid doing a reset. See > > > dm_test_sysreset_base() which tests sysreset drivers on sandbox. > > > > Would you mean that reset-after-capsule-on-disk itself should not > > make a reset on sandbox? > > We have several tests that do resets by calling do_reset(), e.g. the > UEFI unit tests. There is nothing wrong about it. > > We want the sandbox to behave like any other board where capsule updates > lead to resets. > > > > > In dm_test_sysreset_base(), I can see the below code, this means > > sysreset_request() > > will not execute real reset, but just mimic the reset, right? > > > > state->sysreset_allowed[SYSRESET_WARM] = true; > > ut_asserteq(-EINPROGRESS, sysreset_request(dev, SYSRESET_WARM)); > > state->sysreset_allowed[SYSRESET_WARM] = false; > > > > > > but Qemu and real board will cause a real reset and it will terminate > > > > the qemu or stop the board (depends on how it is implemented). Thus, > > > > if a command or boot process will cause a reset, it will need a > > > > special care (maybe respawn?). > > > > > > Here you need to worry about the surrounding automation logic which > > > could be tbot of the U-Boot pytest hooks. I suggest you avoid this and > > > handle it some other way, without reset. > > The sandbox should run through exactly the same code path as all other > boards to get a meaningful test results. Therefore don't put in any > quirks on C level. Your Python test changes are all that is needed.
+1, I have the same opinion here. To exercise capsule-on-disk code, we need a "real" reset because pytest/CI loop is basically a black-box test. -Takahiro Akashi > > Best regards > > Heinrich > > > > > Hmm, would you mean adding a runtime flag to sandbox so that > > it will not does real reset but just showing some token on console like > > "sandbox fake reset done." ? > > > > > > > > Since the capsule update testcase only runs on sandbox, it will not > > > > cause real reset. But maybe it is possible to support running on Qemu. > > > > > > Maybe, but I don't think you should worry about that, at least for > > > now. The sandbox test is enough. > > > > > > > > > > > Current my test patch (and capsule update testcase itself) doesn't > > > > handle the real reset case correctly even on Qemu. The Qemu needs > > > > spawn a new instance and re-connect the console when the reset > > > > happens. > > > > > > Indeed. > > > > > > > > > > > If so, I think there are 2 issues to be solved. > > > > 1. change the capsule update testcase runable on Qemu > > > > 2. change my patch to handle the real reset correctly (not only > > > > waiting for the next boot, but also respawn it again) > > > > > > > > Do I understand correctly? > > > > > > I think the best approach is to get your test running on sandbox, with > > > the faked reset. Don't worry about the other cases as we don't support > > > them. > > > > OK. > > > > Thank you, > > > > > > > > Regards, > > > Simon > > > > > > > > > > > > > > Thank you, > > > > > > > > 2022年2月17日(木) 2:53 Simon Glass <s...@chromium.org>: > > > > > > > > > > Hi Heinrich, > > > > > > > > > > On Wed, 16 Feb 2022 at 10:50, Heinrich Schuchardt > > > > > <xypron.g...@gmx.de> wrote: > > > > > > > > > > > > On 2/16/22 16:46, Tom Rini wrote: > > > > > > > On Wed, Feb 16, 2022 at 04:32:40PM +0100, Heinrich Schuchardt > > > > > > > wrote: > > > > > > > > On 2/16/22 16:26, Simon Glass wrote: > > > > > > > > > Hi Masami, > > > > > > > > > > > > > > > > > > On Tue, 15 Feb 2022 at 02:05, Masami Hiramatsu > > > > > > > > > <masami.hirama...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > Since now the capsule_on_disk will restart the u-boot > > > > > > > > > > sandbox right > > > > > > > > > > after the capsule update, if > > > > > > > > > > CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y, the > > > > > > > > > > boot with a new capsule file will repeat reboot sequence. > > > > > > > > > > On the > > > > > > > > > > other hand, if CONFIG_EFI_CAPSULE_ON_DISK_EARLY=n, the 'env > > > > > > > > > > print -e' > > > > > > > > > > command will execute the capsule update on disk and reboot. > > > > > > > > > > > > > > > > > > > > Thus this update the uboot_console for those 2 cases; > > > > > > > > > > > > > > > > > > > > - restart_uboot(): Add expect_earlyreset optional > > > > > > > > > > parameter so that > > > > > > > > > > it can handle the reboot while booting. > > > > > > > > > > - run_command(): Add wait_for_reboot optional parameter > > > > > > > > > > so that it > > > > > > > > > > can handle the reboot after executing a command. > > > > > > > > > > > > > > > > > > > > And enable those options in the test_capsule_firmware.py > > > > > > > > > > test cases. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Masami Hiramatsu > > > > > > > > > > <masami.hirama...@linaro.org> > > > > > > > > > > --- > > > > > > > > > > .../test_efi_capsule/test_capsule_firmware.py | > > > > > > > > > > 39 ++++++-- > > > > > > > > > > test/py/u_boot_console_base.py | > > > > > > > > > > 95 +++++++++++++++----- > > > > > > > > > > test/py/u_boot_console_sandbox.py | > > > > > > > > > > 6 + > > > > > > > > > > 3 files changed, 102 insertions(+), 38 deletions(-) > > > > > > > > > > > > > > > > > > We have a means to avoid actually doing the reset, see the > > > > > > > > > reset driver. > > > > > > > > > > > > > > > > The UEFI specification requires a cold reset after a capsule is > > > > > > > > updated > > > > > > > > and before the console is reached. How could the reset driver > > > > > > > > help to > > > > > > > > fix the Python tests? > > > > > > > > > > > > > > Is this test going to be able to run on qemu, sandbox, real > > > > > > > hardware, or > > > > > > > all 3? The tests may well end up having to know a bit more, > > > > > > > sadly, > > > > > > > about the type of system they're testing. > > > > > > > > > > > > > Currently the test will only run on the sandbox in Gitlab (see > > > > > > usage of > > > > > > @pytest.mark.boardspec('sandbox') in > > > > > > test/py/tests/test_efi_capsule/). > > > > > > > > > > Let me know if you need help reworking this patch to operate on > > > > > sandbox without a 'real' reset. > > > > > > > > > > Regards, > > > > > Simon > > > > > > > > > > > > > > > > -- > > > > Masami Hiramatsu > > > > > > > > -- > > Masami Hiramatsu >