From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0C1EBC433EF for ; Wed, 16 Mar 2022 03:13:37 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0A16D8394D; Wed, 16 Mar 2022 04:13:35 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="MR7Oic6V"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BD8D08394C; Wed, 16 Mar 2022 04:13:32 +0100 (CET) Received: from mail-vs1-xe29.google.com (mail-vs1-xe29.google.com [IPv6:2607:f8b0:4864:20::e29]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 014DD838AB for ; Wed, 16 Mar 2022 04:13:27 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-vs1-xe29.google.com with SMTP id j128so906624vsc.9 for ; Tue, 15 Mar 2022 20:13:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=+nriQFOtoOP+XYGeP1pZzTFA6ScGV4Em2aTUBQMZEqk=; b=MR7Oic6VccSoPJEQe0rIRJUMAc9IxvE7mMBL8SEf4ESGt6X5uTFLvYzr5k5OCq5JVy coWRhHwiVDTKfsVfBPfw6IaOisLIeTortnZpwpTA0N+UU0L/CXoN211EndnpaN/cO255 e+VbIOcrPwy3LTcwYOd+lrZahuDvjheMFWdxs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=+nriQFOtoOP+XYGeP1pZzTFA6ScGV4Em2aTUBQMZEqk=; b=oKJTr+mNHKbqXZVw+dmJRLBA5SYnWV/tu/Vp9yXzVJu22ZmS2pVftnZP6TnZZK5spk VCp5n9uevm9sN3KUa+TjHJiWTvemGPec2AoDE5xteqGkGxLg1ORxG2kTDOb5LK6c2gX1 OI3FDdRHb9EUcOOuw1Obad/g0qmNt9b6pc/nxUtsIXU353b6KimUPKJliRK1mW43o/1W tMUy7jV6Zzl9LEawORmcR8IuU9ayG3N/HG1jvR1kqA++p3ERvv8qw4d0xW8/Kaq79RVI rCsFJXdZCDu34LEZR7ZB6dd3PmoLQMCAxFDRqRXaU/nKuW3euttsPVIdRIoZdPaJu+T/ xqXQ== X-Gm-Message-State: AOAM5322LLC1tQThr9nxmdP3/sctKM/GQsKYPWAwcUxMfllQ9qRzRcp/ +zOJE1OOPSliqkXgrTcHeXl4PVs/g30k0nS5mWfAxw== X-Google-Smtp-Source: ABdhPJwG3GVB+lB2S7aK2vVdZourNQ/DC50g8JtqzI5xGB8CI2LpxMt4Ycs3Mznu8053nSq0lWGCYz7buNU/E7ufi98= X-Received: by 2002:a05:6102:114b:b0:320:a22e:e7c with SMTP id j11-20020a056102114b00b00320a22e0e7cmr12796975vsg.51.1647400406428; Tue, 15 Mar 2022 20:13:26 -0700 (PDT) MIME-Version: 1.0 References: <4ba143f8-6114-db0e-08e2-d97ac9dc6e13@gmx.de> <20220219011550.GB6672@laputa> <20220314010813.GA37492@laputa> <20220314024258.GC37492@laputa> In-Reply-To: From: Simon Glass Date: Tue, 15 Mar 2022 21:13:15 -0600 Message-ID: Subject: Re: [PATCH] test/py: efi_capsule: Handle expected reset after capsule on disk To: Masami Hiramatsu Cc: AKASHI Takahiro , Heinrich Schuchardt , Tom Rini , U-Boot Mailing List , Patrick Delaunay , Patrice Chotard , Alexander Graf , Bin Meng , Ilias Apalodimas , Jose Marinho , Grant Likely , Etienne Carriere , Sughosh Ganu , Paul Liu Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Hi Masami, On Tue, 15 Mar 2022 at 02:36, Masami Hiramatsu wrote: > > Hi Simon, > > 2022=E5=B9=B43=E6=9C=8815=E6=97=A5(=E7=81=AB) 14:04 Simon Glass : > > > > Hi Masami, > > > > On Mon, 14 Mar 2022 at 18:40, Masami Hiramatsu > > wrote: > > > > > > Hi Simon, > > > > > > 2022=E5=B9=B43=E6=9C=8815=E6=97=A5(=E7=81=AB) 3:24 Simon Glass : > > > > > > > > > > OK, well 'reset by a user' presumably starts the board up and t= hen > > > > > > 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 t= o 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-up= date" > > > > > 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. > > > > > > > > > > However, the 'capsule update on-disk' feature is also expected (a= nd > > > > > defined in the spec?) to run when the UEFI subsystem is initializ= ed. > > > > > 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. > > > > > > > > The 'UEFI subsystem is intialised' is a problem, actually, since if= it > > > > were better integrated into driver model, it would not have separat= e > > > > structures or they would be present and enabled when driver model i= s. > > > > I hope that it can be fixed and Takahiro's series is a start in tha= t > > > > direction. > > > > > > OK. > > > > > > > But as to a test that an update is called when UEFI starts, that se= ems > > > > like a single line of code. Sure it is nice to test it, but it is m= uch > > > > more important to test the installation of the update and the > > > > execution of the update. I suppose another way to test that is to > > > > shut down the UEFI subsystem and start it up? > > > > > > Yes, currently we call do_reset() after install the capsule file. > > > (This reset can be avoided if we replace it with > > > sysreset_walk_halt(SYSRESET_COLD) as you said, right?) > > > > > > Here is how I tested it on my machine; > > > > > > > usb start > > > > fatload usb 0 $kernel_addr_r test.cap > > > > fatwrite mmc 0 $fileaddr EFI/UpdateCapsule/test.cap $filesize > > > > efidebug capsule disk-update > > > (run install process and reboot the machine) > > > > > > So, if we can avoid the last reset, we can test the below without > > > reset on sandbox (depends on scenarios). > > > - confirm that the capsule update on disk can find the capsule file > > > from ESP specified by the BOOTXXXX EFI variable. > > > - confirm that the capsule update on disk writes the firmware > > > correctly to the storage which specified by DFU. > > > - confirm that the capsule update on disk success if the capsule imag= e > > > type is supported. > > > - confirm that the capsule update on disk fails if the capsule image > > > type is not supported. > > > - confirm that the capsule update on disk will reboot after update > > > even if the update is failed. > > > > > > The only spec we can not test is > > > - confirm that the capsule update on disk is kicked when the UEFI is > > > initialized. > > > > Even that could be tested, by installing an update and then initing UEF= I? > > yeah, if the UEFI is not initialized yet, we can run some UEFI related > command (e.g. printenv -e) instead of efidebug capsule... to execute > the capsule update on disk. > But anyway, this is only available at the first time. We need a way to > reset UEFI subsystem without system reset. Yes. It is certainly possible, but I'm not sure how easy it is. Perhaps just drop all the EFI data structures and run the EFI init again? We have something similar with driver model. See dm_test_pre_run() > > > > > > > > > > Anyway we should design subsystems so they are easy to test. > > > > > > Here I guess you mean the unit test, not system test, am I correct? > > > > Yes. Easy testing is so important for developer productivity and > > happiness. It is fine to have large system/functional tests as a fall > > back or catch-all, but they tend to test the happy path only. When > > they fail, they are hard to debug because they cover such as large > > area of the code and they often have complex setup requirements so are > > hard to run manually. > > > > My hope is that all the functionality should be covered by unit tests > > or integration tests, so that system/functional almost never fail. > > My another question is how small is the granularity of the unit test. > As I showed, the UEFI capsule update needs to prepare a capsule file > installed in the storage. > That seems to be very system-level. But you think that is still be a unit= test? > (I expected that the 'Unit test' is something like KUnit in Linux) Well I am using your terminology here. Technically many of the U-Boot tests (executed by 'ut' command) are not really unit tests. They bring in a lot of code and run one test case using it. For example, one of the tests brings up the USB subsystem, including a fake USB stick, then checks it can read data from the stick, using the USB stack. Another one writes some things to the emulated display and then checks that the correct pixels are there. Perhaps a better name would be integration test. But the point is that we can run these tests very, very quickly and (setup aside) without outside influence, or without restarting the executable, etc. > > > > > > > > > > > > > > > > > > > > > Masami's patch (this series) fixes issues around those two re= sets > > > > > > > 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 up= date, > > > > > it could help? > > > > > > > > Yes that can help, because sandbox can detect that and turn it into= a nop. > > > > > > OK, let me submit a patch to update it. > > > > OK thank you. Regards, SImon