On Mon, Mar 14, 2022 at 04:35:45PM +0900, Masami Hiramatsu wrote: > Hi Simon, > > 2022年3月14日(月) 15:45 Simon Glass <s...@chromium.org>: > > > > Hi Takahiro, > > > > On Sun, 13 Mar 2022 at 20:43, AKASHI Takahiro > > <takahiro.aka...@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.aka...@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.aka...@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 <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. > > > > > > > > > > > > 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
