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 8A4E6C001E0 for ; Fri, 28 Jul 2023 01:55:22 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A4351868AF; Fri, 28 Jul 2023 03:55:20 +0200 (CEST) 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="kG/NtoL5"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 719A0868B2; Fri, 28 Jul 2023 03:55:17 +0200 (CEST) Received: from mail-pf1-x430.google.com (mail-pf1-x430.google.com [IPv6:2607:f8b0:4864:20::430]) (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 CF17A8686F for ; Fri, 28 Jul 2023 03:55:10 +0200 (CEST) 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-pf1-x430.google.com with SMTP id d2e1a72fcca58-66d6a9851f3so347907b3a.0 for ; Thu, 27 Jul 2023 18:55:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690509309; x=1691114109; 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=PtkdhPNdIbHRB5n1tzC6naQb4OEZ91eSFuYKdDEgQgQ=; b=kG/NtoL5P5YYLQLFFZfmf4HxlUnZdrkVIj+1qGPwJs4MNbmMeHQrcpRRRyaA73GK1E 3h9ZT0mxQFi4PMLTlpgUnGbvruwekCRkzSg7GMuAYQjHCTq0dLjTeA55VgbzoJtKOcXk vaM5fwe8v7bI4lQ/lbpLwjUggcYh5mXm3AwHknfSP9GvfWn9YsEaa9Jcs0qkmYUV9L0V XnxvEC4rMP6BGOt2HzKPZiCQFkjOOU3TAiFiB8n8BFlhvhfCL49e4tenMegQ4gutk+3z 3J+mCVkC0SaJnPNtKtbFg9LZzN+bsIlvtUeTgyELY+uNCbhjeLSBtvz4UYLO1Yjx2rOL s+ZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690509309; x=1691114109; 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=PtkdhPNdIbHRB5n1tzC6naQb4OEZ91eSFuYKdDEgQgQ=; b=O8wt0nLlN0u6upNxXB1zx2sl3GbgHIJI7ahvAAgIUPGdhc2shRTnShHGYjCuVJu49U prBphlsMWBOUxXz9nNvr9z0NN5NDUjOz6ju/P4UXz+SvRpA62yDL8dRcyp/0vl3A+VNn FqmSTeP/Z58rQvAUlWgDrggy71kRQmclcG17q8TJnThLSxv7CbZmfjlOoNnI0pImOvPe 5GpGSbGaUyrCjPQHmx/39OqtPHUNoNafI6B4WK4/daidVkeyLEdgTdWLDd+NcbbG64R0 8MEGj/YGqBwopOd9lha6WcECBPmO+hITbPEOhFOuPGh4aYki0DSlCY6Vv4M+AYRb7pP/ 6UbA== X-Gm-Message-State: ABy/qLYI9aw7O682eFP2X4RNkD2WoqL1/mv1zx7IAt3g/3etgsqpBLjl hkwNcWaLTEA64idMKDiwm30LLg== X-Google-Smtp-Source: APBJJlE2b0xx92CHBEmN/HU7/AO8f+VWcTMj9aoxmJL6um3f5+v0B0icDLXwnNesLFw8kezvsCfM6A== X-Received: by 2002:a05:6a20:8422:b0:137:30db:bc1e with SMTP id c34-20020a056a20842200b0013730dbbc1emr990779pzd.3.1690509308807; Thu, 27 Jul 2023 18:55:08 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:25dd:d673:efea:dbcc]) by smtp.gmail.com with ESMTPSA id z4-20020a63ac44000000b0055b553157a5sm2186610pgn.71.2023.07.27.18.55.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 18:55:08 -0700 (PDT) Date: Fri, 28 Jul 2023 10:55:05 +0900 From: AKASHI Takahiro To: Michal Simek Cc: xypron.glpk@gmx.de, ilias.apalodimas@linaro.org, sughosh.ganu@linaro.org, u-boot@lists.denx.de Subject: Re: [PATCH] efi_loader: capsule: enforce guid check in api and capsule_on_disk Message-ID: Mail-Followup-To: AKASHI Takahiro , Michal Simek , xypron.glpk@gmx.de, ilias.apalodimas@linaro.org, sughosh.ganu@linaro.org, u-boot@lists.denx.de References: <20230727003800.25105-1-takahiro.akashi@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.8 at phobos.denx.de X-Virus-Status: Clean Hi Michal, On Thu, Jul 27, 2023 at 10:53:44AM +0200, Michal Simek wrote: > > > On 7/27/23 02:38, AKASHI Takahiro wrote: > > While UPDATE_CAPSULE api is not fully implemented, this interface and > > capsule-on-disk feature should behave in the same way, especially in > > handling an empty capsule for fwu multibank, for future enhancement. > > > > So move the guid check into efi_capsule_update_firmware(). > > > > Fixed: commit a6aafce494ab ("efi_loader: use efi_update_capsule_firmware() > > for capsule on disk") > > just fyi: b4 mess this. > You should likely put it on the same line and ignore line limit. > > This is how this ends up. > > handling an empty capsule for fwu multibank, for future enhancement. > > So move the guid check into efi_capsule_update_firmware(). > > for capsule on disk") > > Fixed: commit a6aafce494ab ("efi_loader: use efi_update_capsule_firmware() > Reported-by: Michal Simek > Signed-off-by: AKASHI Takahiro > Link: https://lore.kernel.org/r/20230727003800.25105-1-takahiro.akashi@linaro.org > > > > > > Reported-by: Michal Simek > > Signed-off-by: AKASHI Takahiro > > --- > > lib/efi_loader/efi_capsule.c | 17 ++++++++--------- > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 7a6f195cbc02..ddf8153e0982 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -581,6 +581,13 @@ static efi_status_t efi_capsule_update_firmware( > > fw_accept_os = capsule_data->flags & FW_ACCEPT_OS ? 0x1 : 0x0; > > } > > + if (guidcmp(&capsule_data->capsule_guid, > > + &efi_guid_firmware_management_capsule_id)) { > > + log_err("Unsupported capsule type: %pUs\n", > > + &capsule_data->capsule_guid); > > + return EFI_UNSUPPORTED; > > + } > > + > > /* sanity check */ > > if (capsule_data->header_size < sizeof(*capsule) || > > capsule_data->header_size >= capsule_data->capsule_image_size) > > @@ -751,15 +758,7 @@ efi_status_t EFIAPI efi_update_capsule( > > log_debug("Capsule[%d] (guid:%pUs)\n", > > i, &capsule->capsule_guid); > > - if (!guidcmp(&capsule->capsule_guid, > > - &efi_guid_firmware_management_capsule_id)) { > > - ret = efi_capsule_update_firmware(capsule); > > - } else { > > - log_err("Unsupported capsule type: %pUs\n", > > - &capsule->capsule_guid); > > - ret = EFI_UNSUPPORTED; > > - } > > - > > + ret = efi_capsule_update_firmware(capsule); > > if (ret != EFI_SUCCESS) > > goto out; > > } > > I have no problem with this patch because it works as the previous one. When > commit message is fixed feel free to add > Tested-by: Michal Simek I hope that the maintainer would take care of it when he tries to merge the patch. > And regarding empty capsule functionality with A/B. > I boot from A. Download capsules and run disk-update to get to Image B and > trial state and I can download and apply acceptance capsule by hand via > efidebug capsule update . That works fine for acceptance capsule is > reflected via fwu in mdata. > When I apply revert capsule there is nothing visible in mdata and I think it > should. The only visibility is that it resets to A system. Is this the only > intention of revert capsules? > (keep in mind that I use two images per bank). > > Empty capsules are just accepted only in trial state which is understandable. > > And I also see that with latest master branch capsule on disk feature is not > working properly. Capsule are not processed at all. Can you please double > check it? I locally ran the pytest with v2023.10-rc1, and - test_capsule_firmware_raw passed - test_capsule_firmware_signed_raw failed but it seems to me that 'signed_raw' failure is not directly related to efi implementation (I didn't dig into details, though). Can you describe more about your environment? -Takahiro Akashi > Thanks, > Michal > >