[PATCH] test/py: efi_capsule: Handle expected reset after capsule on disk

[Simon Glass]

Hi Takahiro,

On Sun, 13 Mar 2022 at 20:43, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> On Sun, Mar 13, 2022 at 08:15:02PM -0600, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Sun, 13 Mar 2022 at 19:08, AKASHI Takahiro
> > <takahiro.akashi at linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Mar 11, 2022 at 07:24:39PM -0700, Simon Glass wrote:
> > > > Hi Takahiro,
> > > >
> > > > On Fri, 18 Feb 2022 at 18:16, AKASHI Takahiro
> > > > <takahiro.akashi at linaro.org> wrote:
> > > > >
> > > > > 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 <sjg at chromium.org>:
> > > > > > >
> > > > > > > >
> > > > > > > > Hi Masami,
> > > > > > > >
> > > > > > > > On Wed, 16 Feb 2022 at 18:11, Masami Hiramatsu
> > > > > > > > <masami.hiramatsu at 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.
> > > >
> > > > I don't see why you need the reset at all to test the code.
> > >
> > > As I repeatedly said, I believe that this is a black-box test and
> > > a system test. The purpose of the test is to make sure the firmware
> > > update be performed in real operations as expected, that is,
> > > a *reset* triggers the action *at the beginning of* system reboot.
> >
> > I understand you are frustrated with this, but I just don't agree, or
> > perhaps don't understand.
> >
> > What specific mechanism is used to initiate the firmware update? Is
> > this happening in U-Boot or somewhere else?
>
> There are two ways:
> a. CapsuleUpdate runtime service
> b. capsule delivery on disk
>
> My original patch provides only (b), partly, because runtime
> service is a bit hard to implement under the current framework.
>
> UEFI specification requires that (b) can/should be initiated
> by a *reset by a user* and another reset be automatically triggered by UEFI
> when the update has been completed either successfully or in vain.
> The latter behavior has been enforced on U-BOOT UEFI implementation
> by Masami's patch (not this series).

OK, well 'reset by a user' presumably starts the board up and then
runs some code to do the update in U-Boot? Is that right? If so, we
just need to trigger that update from the test. We don't need to test
the actual reset, at least not with sandbox. As I said, we need to
write the code so that it is easy to test.

>
> Masami's patch (this series) fixes issues around those two resets
> in pytest.

Yes and that is the problem I have, at least on sandbox.

Regards,
Simon

>
> > >
> > > > You should
> > > > be able to run a command to make the update happen. How does the
> > > > updata actually get triggered when you reset?
> > >
> > > It's not the purpose of this test.
> >
> > Then drop the reset and design the feature with testing in mind.
>
> So it's just beyond of my scope.
>
> -Takahiro Akashi
>
> > Regards,
> > SImon