On Mon, Mar 14, 2022 at 04:35:45PM +0900, Masami Hiramatsu wrote:
Hi Simon,
2022年3月14日(月) 15:45 Simon Glass sjg@chromium.org:
Hi Takahiro,
On Sun, 13 Mar 2022 at 20:43, AKASHI Takahiro takahiro.akashi@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@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@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@chromium.org: > > > > > > > > > > > Hi Masami, > > > > > > > > On Wed, 16 Feb 2022 at 18:11, Masami Hiramatsu > > > > masami.hiramatsu@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.
Actually, we already have that command, "efidebug capsule disk-update" which kicks the capsule update code even without the 'reset by a user'. So we can just kick this command for checking whether the U-Boot UEFI code correctly find the capsule file from ESP which specified by UEFI vars.
I oppose it simply
However, the 'capsule update on-disk' feature is also expected (and defined in the spec?) to run when the UEFI subsystem is initialized. This behavior will not be tested if we skip the 'reset by a user'. I guess Takahiro's current test case tries to check it.
because of this.
System test should not use an internal function, it should only exercise public interfaces from user's viewpoint.
-Takahiro Akashi
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.
So If I I call sysreset_walk_halt(SYSRESET_COLD) after capsule update, it could help?
Thank you,
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
-- Masami Hiramatsu