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 000E6C433F5 for ; Fri, 25 Mar 2022 10:41:44 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id C757484064; Fri, 25 Mar 2022 11:41:42 +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="JQvfVKM9"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 84E278407A; Fri, 25 Mar 2022 11:41:40 +0100 (CET) Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) (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 627C884031 for ; Fri, 25 Mar 2022 11:41:36 +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-x535.google.com with SMTP id o8so6092652pgf.9 for ; Fri, 25 Mar 2022 03:41:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=fg3EZRT6ROBVIVvkNTRes4iEYZVyp5GGbVwlvKk300s=; b=JQvfVKM9uP8oh6wsCMNW+J/UICyeSEA3+Emhr+EEMC2M/0o3xVIdatYz3wJA/eHbyU PpPF/NnOOJDyTJvcyUzdLhdtD9aChTpRzo5gM/s/xUhrYDp9/8rEKdM8CetSyDhT2AIF alNjD2OH+EDPMolwhh6mbGgsTpY7Hqbgz01fKZx6Z4AOTe9K3R3f8xvbWJk3MIqNPPzF FN9cVThs+q/ndQIjohqlY5DNZfD9GLdbabmbqGfBEPcAgdKHxULTrkHjl6ZUe54fUXhb 8gS1Ek5faFdaEp3nvq5W006Gh/NCEzedX4GgbF6sd5dBJuWI4xDN52+/8n1Wck1SqWOz 7Psg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=fg3EZRT6ROBVIVvkNTRes4iEYZVyp5GGbVwlvKk300s=; b=VsV/5/JL0SclOnNLYVylWu7XTz5OYNZUqK/y3j9f0Y55zPxHzESZrivzaIGAUIjdVD hkQTn0ooi1TdfUe3rV9NxFYyd4E4v1lPkg0u1hYrfXggj50hR+6gQWOgoc9qsB7sB/Ln 7sUH6qEF4NtjWVMgN/o2d9ObT1cGaW9xTOViOfh40L3SWFZ1/FgEPS+TYXhxVjfZpoNz bPSAEDjZAdsvkSK2xQC1KbgG1YzU9Qm4J8JkMF+vqpDIDAbqHHsTapYJdliYNYNfLNWe 6yb/DFgK1WEQ8LPqVE2YycJ299aZ9LVh5a6KjR+0eSHPIwv0USlchHKV8+w2yHx4ga9u FDbQ== X-Gm-Message-State: AOAM533q6amfmvLN8S1nuC82WY5uT/wVO6SFJXb1kbXM6uWcF1odiaHa NpeLt8jp6erm5WNXpP94ZK3jXA== X-Google-Smtp-Source: ABdhPJzAhxNjdNWx0o/D0H0KONHmTSBuiD51l7OioMpX+YNSqF4MN4AEJ6ZXE6/jkxrUKPQ2tbigRg== X-Received: by 2002:a63:3d0b:0:b0:37f:ef34:1431 with SMTP id k11-20020a633d0b000000b0037fef341431mr7391768pga.547.1648204894695; Fri, 25 Mar 2022 03:41:34 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:15a6:15e9:1820:73]) by smtp.gmail.com with ESMTPSA id f66-20020a62db45000000b004fa8a7b8ad3sm6196296pfg.77.2022.03.25.03.41.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Mar 2022 03:41:34 -0700 (PDT) Date: Fri, 25 Mar 2022 19:41:29 +0900 From: AKASHI Takahiro To: Sughosh Ganu Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Ying-Chun Liu , Tuomas Tynkkynen , Heiko Thiery , Frieder Schrempf , Michael Walle , Masami Hiramatsu , Jassi Brar , Michal Simek , Michal Simek Subject: Re: [RFC PATCH 0/6] efi: capsule: Image GUID usage cleanup Message-ID: <20220325104129.GE50025@laputa> Mail-Followup-To: AKASHI Takahiro , Sughosh Ganu , u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Ying-Chun Liu , Tuomas Tynkkynen , Heiko Thiery , Frieder Schrempf , Michael Walle , Masami Hiramatsu , Jassi Brar , Michal Simek , Michal Simek References: <20220324123901.429472-1-sughosh.ganu@linaro.org> <20220325045304.GA50025@laputa> 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.5 at phobos.denx.de X-Virus-Status: Clean Sughosh, On Fri, Mar 25, 2022 at 03:14:20PM +0530, Sughosh Ganu wrote: > hi Takahiro, > > On Fri, 25 Mar 2022 at 10:23, AKASHI Takahiro > wrote: > > > > Sughosh, > > > > On Thu, Mar 24, 2022 at 06:08:55PM +0530, Sughosh Ganu wrote: > > > > > > This series is cleaning up the usage of the image GUIDs that are used > > > in capsule update and the EFI System Resource Table(ESRT). > > > > > > Currently, there are two instances of the Firmware Management > > > Protocol(FMP), one defined for updating the FIT images, and the other > > > for updating raw images. The FMP code defines two GUID values, one for > > > all FIT images, and one for raw images. Depending on the FMP instance > > > used on a platform, the platform needs to use the corresponding image > > > GUID value for all images on the platform, and also across platforms. > > > > > > A few issues are being fixed through the patch series. One, that an > > > image for a different platform can be flashed on another platform if > > > both the platforms are using the same FMP instance. So, for e.g. a > > > capsule generated for the Socionext DeveloperBox platform can be > > > flashed on the ZynqMP platform, since both the platforms use the > > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW instance of the FMP. This can be > > > corrected if each firmware image that can be updated through the > > > capsule update mechanism has it's own unique image GUID. > > > > Ok although the specification doesn't explicitly require the uniqueness > > "across platforms". > > Yes, but unless we have a single image running on multiple platforms, > we do need different GUID values across platforms. > > > > > > The second issue that this patch series fixes is the value of FwClass > > > in the ESRT. With the current logic, all firmware image entries in the > > > ESRT display the same GUID value -- either the FIT GUID or the raw > > > GUID. This is not in compliance with the UEFI specification, as the > > > specification requires all entries to have unique GUID values. > > > > Well, this is *not* a problem of fit FMP driver, but a matter of how > > ESRT is populated. Table 23-16 "ESRT and FMP Fields" says, > > The ImageTypeId GUID from the Firmware > > Management Protocol instance for a device is > > used as the Firmware Class GUID in the ESRT. > > Where there are multiple identical devices in > > the system, system firmware must create a > > mapping to ensure that the ESRT FwClass > > GUIDs are unique and consistent. > > The second sentence suggests that UEFI implementation, i.e. > > efi_esrt_populate(), may and should allow some *mapping*. > > > > That said, I basically accept the requirement that you mention above. > > > > > The third issue being fixed is the population of the > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR array. The current code uses the dfu > > > framework for populating the image descriptor array. However, there > > > might be other images that are not to be updated through the capsule > > > update mechanism also registered with the dfu framework. As a result > > > of this, the ESRT will show up entries of images that are not to be > > > targeted by the capsule update mechanism. > > > > > > These issues are being fixed by defining a structure, efi_fw_images. A > > > platform can then define image related information like the image GUID > > > and image name. Every platform that uses capsule update mechanism > > > needs to define fw_images array. This array will then be used to > > > populate the image descriptor array, and also in determining if a > > > particular capsule's payload can be used for updating an image on the > > > platform. > > > > When ESRT support patch was posted, I proposed that we should have > > a kind of configuration table that defines all the firmware on the system > > for ESRT, but this proposal was rejected. > > Your efi_fw_images[] looks quite similar as what I had in mind. > > Why not define efi_fw_images[] as ESRT itself. > > (Of course, some fields in an entry can still be populated through > > GetImageInfo API.) > > Currently, with this patch series, that is what is happening. The > efi_fw_images array gets read by the GetImageInfo function, and that > information gets used for populating the ESRT. Btw, I also want to > extend this structure as part of a separate task, where we have > information related to the dfu framework as part of the structure. > Then the capsule related dfu information can be populated from this > structure, instead of the dfu_alt_info env variable. This is what > Ilias has suggested. But I will take this up as a separate exercise. > > > > > > The first patch of this series adds the fw_images array in all > > > platforms which are using UEFI capsule updates > > > > > > The second patch of the series changes the logic for populating the > > > image descriptor array, using the information from the fw_images array > > > defined by the platform. > > > > So a FIT image can still work as a single binary of firmware > > under FIT FMP driver. Correct? > > Yes, this will still work. Only change is that every platform will > have a separate image GUID for the FIT image. Ok. > > > > > The third patch of the series removes the test cases using the --raw > > > and --fit parameters, removes test case for FIT images, and adds a > > > test case for checking that the update happens only with the correct > > > image GUID value in the capsule. > > > > Your change in test_capsule_firmware.py tries to remove FIT FMP > > driver test and it eventually hides the fact that FIT driver may get broken. > > Yes, I am aware of this. I was required to do this since we cannot > have both instances of the FMP. I will move the FIT test into a > separate script. Instead, please add a *new* test script, or separate test "class", for your changes. I think that you should mention remaining issues or TODO items in your cover letter. > > > > I expect that you should maintain FIT FMP driver properly and it be > > tested as before. > > Okay. Although, I wanted to check if you think we need to have support > for updating the FIT image. Yes, I think so. FIT format has been long used, recognized as a standard on U-Boot and well integrated with DFU framework. That is why we can simply call fit_update() in SetImage API. So having FIT FMP driver is a good migration path. Moreover, treating binaries as a single capsule ensures that all the firmware can and should be updated atomically and simultaneously. -Takahiro Akashi > The individual images can very much be > updated through the raw FMP instance. And the raw images also map to > the ESRT, which is not the case with the FIT image. > > > # Moreover, you have not yet added capsule authentication support > > to FIT FMP driver. > > Yes, I have not used an FIT image in my testing up until now, although > it should not be very difficult. I will keep this on my Todo list and > will add support once this gets upstreamed. > > -sughosh > > > > > -Takahiro Akashi > > > > > > > > The fourth patch of the series makes corresponding changes in the > > > capsule update related documentation. > > > > > > The fifth patch of the series removes the now unused FIT and raw image > > > GUID values from the FMP module. > > > > > > The sixth patch of the series removes the --raw and --fit command line > > > parameters in the mkeficapsule utility. > > > > > > > > > Sughosh Ganu (6): > > > capsule: Add Image GUIDs for platforms using capsule updates > > > capsule: FMP: Populate the image descriptor array from platform data > > > test: capsule: Modify the capsule tests to use GUID values for sandbox > > > doc: uefi: Update the capsule update related documentation > > > FMP: Remove GUIDs for FIT and raw images > > > mkeficapsule: Remove raw and FIT GUID types > > > > > > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 19 ++ > > > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 18 ++ > > > board/emulation/qemu-arm/qemu-arm.c | 20 +++ > > > board/kontron/pitx_imx8m/pitx_imx8m.c | 15 +- > > > board/kontron/sl-mx8mm/sl-mx8mm.c | 14 ++ > > > board/kontron/sl28/sl28.c | 14 ++ > > > board/sandbox/sandbox.c | 17 ++ > > > board/socionext/developerbox/developerbox.c | 23 +++ > > > board/xilinx/common/board.h | 18 ++ > > > board/xilinx/zynq/board.c | 18 ++ > > > board/xilinx/zynqmp/zynqmp.c | 18 ++ > > > configs/sandbox64_defconfig | 1 - > > > configs/sandbox_defconfig | 1 - > > > doc/develop/uefi/uefi.rst | 10 +- > > > include/configs/imx8mm-cl-iot-gate.h | 10 ++ > > > include/configs/imx8mp_rsb3720.h | 10 ++ > > > include/configs/kontron-sl-mx8mm.h | 6 + > > > include/configs/kontron_pitx_imx8m.h | 6 + > > > include/configs/kontron_sl28.h | 6 + > > > include/configs/qemu-arm.h | 10 ++ > > > include/configs/sandbox.h | 10 ++ > > > include/configs/synquacer.h | 14 ++ > > > include/efi_api.h | 8 - > > > include/efi_loader.h | 18 ++ > > > lib/efi_loader/efi_firmware.c | 95 +++------- > > > test/py/tests/test_efi_capsule/conftest.py | 20 +-- > > > .../test_efi_capsule/test_capsule_firmware.py | 167 ++++++------------ > > > tools/eficapsule.h | 8 - > > > tools/mkeficapsule.c | 26 +-- > > > 29 files changed, 384 insertions(+), 236 deletions(-) > > > > > > -- > > > 2.25.1 > > > > > >