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 79A77C38142 for ; Mon, 23 Jan 2023 12:47:48 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id BB73385740; Mon, 23 Jan 2023 13:47:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="CKkec0vA"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8FBF985707; Mon, 23 Jan 2023 13:47:43 +0100 (CET) Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) (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 3626485735 for ; Mon, 23 Jan 2023 13:47:40 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pg1-x531.google.com with SMTP id s67so8857860pgs.3 for ; Mon, 23 Jan 2023 04:47:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=+o3c6FHMwkxHn5qLWfNx8c9bA0s6Fz+GZpd/aTE5HIM=; b=CKkec0vArtS50Q0VVMppxNXJRmt9tWpCuhrcwpV8H8IdWfQMyc1egLNjNzl8iKh7qy oFVDqLo5qIc0Z0hAaDqPrOCsfMM7xnkIR+/p0v/J/OExrPiwV/80ANCmGtSfZ972U/Qi C/9a9H01Uxwt5RLVfdKJPqUUbIl+hua5pjyv1y7rGBGxVoX2eW7AkZWACpe6ZyN4uJHB PGighA7L+QcpQJ+iAS62C5RCmhIuTS1em365EykSzWtq5G7fwfTacUyItFZtEONoDswr iXyl1DE20ncodYBn+ZNspr5nud4nyacRxXHr4WUz7HwY/+7QISKgaDBrR3ejY9KY3deB D/rQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+o3c6FHMwkxHn5qLWfNx8c9bA0s6Fz+GZpd/aTE5HIM=; b=yy4qAuOBteeWW36JQDRbIIvxlOv6Wu9A2qfyGeBAh5+g6v+vQFj1Yx7Nwsi/Bq19NB YPfUljNZn/V8SlVbm1Ru4hzWETjWBR5SFQGYgXERVRcFFwofLAErZRz/vDGka8vwXkWn kceugtw7ZKa9R+q2nGwjOGtCSTEOOXykwGnfzCnKye8rGmVB9NFSGHT3BpNyAbZ8g0eo gh7otX0tqG7OEjgtzAOgd6kbCNARmWIJu3Bv5uY+oxFtzxJuSboSCe4kyC8rpfqqY6pz 4onlBQ93cIB40FdfaFgI2TLebctRO1cPIUFz+Fm7zcQMJBXGV7a0L/Ke37hrU7Gi8jOC d+Gw== X-Gm-Message-State: AFqh2krvbPC0GT/x6hNszMqro/c8LWYbMU0LEa/12JLMIVH1YLk4eERb XypfUfYnZBBSrXNJX0Lr9dsaGA== X-Google-Smtp-Source: AMrXdXup+ZL/WTTvsPiZq36leV0B8o4wTw53NsgmVIhcrN8Pz0hJtTdAXRX+9asrA7zR7GPuzjeU9A== X-Received: by 2002:a05:6a00:189a:b0:58d:e33b:d588 with SMTP id x26-20020a056a00189a00b0058de33bd588mr4934813pfh.2.1674478058069; Mon, 23 Jan 2023 04:47:38 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:f14f:7ef3:e505:dac0]) by smtp.gmail.com with ESMTPSA id m1-20020aa79001000000b00580fb018e4bsm759505pfo.211.2023.01.23.04.47.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Jan 2023 04:47:37 -0800 (PST) Date: Mon, 23 Jan 2023 21:47:33 +0900 From: AKASHI Takahiro To: Ilias Apalodimas Cc: Simon Glass , Heinrich Schuchardt , u-boot@lists.denx.de, Pali Roh??r , Masahisa Kojima , Sughosh Ganu , Etienne Carriere , Patrick Delaunay Subject: Re: [RFC PATCH 1/1] efi_loader: get rid of ad-hoc EFI subsystem init Message-ID: <20230123124733.GA29571@laputa> Mail-Followup-To: AKASHI Takahiro , Ilias Apalodimas , Simon Glass , Heinrich Schuchardt , u-boot@lists.denx.de, Pali Roh??r , Masahisa Kojima , Sughosh Ganu , Etienne Carriere , Patrick Delaunay References: <20230120123120.1014589-1-ilias.apalodimas@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Jan 23, 2023 at 10:15:40AM +0200, Ilias Apalodimas wrote: > Hi Simon, Heinrich > > On Fri, Jan 20, 2023 at 03:11:13PM -0700, Simon Glass wrote: > > Hi Heinrich, > > > > On Fri, 20 Jan 2023 at 13:36, Heinrich Schuchardt wrote: > > > > > > On 1/20/23 20:19, Simon Glass wrote: > > > > Hi, > > > > > > > > On Fri, 20 Jan 2023 at 06:03, Heinrich Schuchardt wrote: > > > >> > > > >> Am 20. Januar 2023 13:31:19 MEZ schrieb Ilias Apalodimas : > > > >>> Up to now the EFI subsystem was left out of the main U-Boot init > > > >>> process. This has led to various hacks over the years, with the most > > > >>> notable one being sprinkling around the efi init call to various places > > > >>> such as U-Boot commands, the early boot code etc. > > > >>> > > > >>> Since EFI has it's own Kconfig option and people can remove it, let's > > > >>> wire up the EFI init call on an event for EVT_MAIN_LOOP. > > > >>> > > > >>> This will also get rid of ad-hoc code in the main event loop, which was > > > >>> trying to initialize the subsystem early and perform capsule updates. > > > >>> > > > >>> TODO: > > > >>> - The efi_tcg protocol implicitly initializes the TPM, as a result > > > >>> some of the tpm selftests will fail with the RFC. If everyone > > > >>> agrees that this is a good idea, I'll clean up the TPM hacks as well > > > >>> - We still need to run capsule updates on the main_loop() code since > > > >>> in some cases (e.g sandbox) we need preboot commands. > > > >>> - wider tests, I've only run QEMU for now > > > >>> > > > >>> Signed-off-by: Ilias Apalodimas > > > >> > > > >> > > > >> In case of efi_init_obj_list() failing we should still reach the U-Boot console but each of the EFI commands should abort early. > > > >> > > > >> Please, put the Kconfig related capsule change into a separate patch. > > > >> > > > >> Otherwise looks good to me. > > > >> > > > > > > > > I am OK with this change too. > > > > > > > > Two points from my side: > > > > > > > > 1. The main loop capsule update is (still) a mistake, unfortunately. > > > > It should be a command which is run on boot. For sandbox testing, that > > > > command should be run *without* rebooting. I am sure I asked for that > > > > > > Capsule updates must run outside of any command as this is required by > > > the UEFI specification. > > Yes it's still a mistake but we can't get rid of it easily. What I was I don't think it's a mistake. When I implemented the feature, update_tftp() was already placed in the main loop. I followed this tradition. > going to try is add another event notifier which would run post boot. That > would work and allow us to define events, after the preboot commands have > executed. It's an idea, but it's a matter of machanism, but not a matter of timing. -Takahiro Akashi > Simon there *is* a command do that. It's documented in [0]. The tl;dr is > run: > => efidebug boot add 0 Boot0000 virtio 0:1 > => efidebug boot next 0 > => setenv -e -nv -bs -rt -v OsIndications =0x0000000000000004 > => efidebug capsule disk-update > > The command exists for testing, but as Heinrich explains, we also need to > test the automatic upgrade after a reboot since that's what the EFI > specification expects. > > > > > What do you mean? Does the specification mention U-Boot commands? > > > > > > > > Capsule updates require a reboot and the sandbox must behave as a > > > regular system so that we can run the same tests on all systems. > > > > How can I get you past this thinking? We need to 'design for test'. > > The current test is a mess, sorry. Perhaps we could have a call to go > > through it? > > > > > > > > > at the time but for various reasons it didn't happen. Please can you > > > > make that change also? > > > > > > > > 2. EFI should not be maintaining its own separate data structures, but > > > > should keep them attached to driver model. They should be created as > > > > needed, dynamically, not all at the start. Is anyone looking at this? > > > > I am happy to help suggest initial target for this refactoring. > > This patch doesn't change anything wrt to structures or how it interacts > with DM. That's a different topic, but since there has been no progress > apart from block devices for a while, I'll start looking into it myself. > FWIW I agree we should refactor the protocol registration and match what > U-Boot does with the rest of the DM. > > > > > > > We have started with such a link for block devices. Once those block > > > devices are really UEFI compliant we should refactor the other devices > > > currently supported by EFI sub-system: > > > > > > UART, network, RNG, video, TPM. > > > > > > Some initialization like setting up UEFI variables will not have any > > > equivalence in the driver model. > > > > One way to handle that is to: > > > > - set up 'global' EFI structures attached to an EFI uclass > > - set up 'per device' EFI structures when the device is bound or probed > > > > Regards, > > Simon > > Can we do this is small steps since I prefer testing everything > thoroughly and making sure the EFI init doesn't break? > So my plan is to resend this, fixing any TPM selftest regressions and then > start cleaning up the protocol registration. > > [0] https://u-boot.readthedocs.io/en/v2021.04/board/emulation/qemu_capsule_update.html > > Regards > /Ilias