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 98520C433F5 for ; Mon, 24 Jan 2022 02:34:02 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2DD1981109; Mon, 24 Jan 2022 03:34:00 +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="mZ2DLzto"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 35802811DF; Mon, 24 Jan 2022 03:33:58 +0100 (CET) Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) (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 4E2A98083E for ; Mon, 24 Jan 2022 03:33:54 +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-pl1-x630.google.com with SMTP id z5so239444plg.8 for ; Sun, 23 Jan 2022 18:33:54 -0800 (PST) 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=blOWhtjN88Q44vHtFfQzit4advqSdLTa0QQlsjWHN/k=; b=mZ2DLztorsAphHN57fxMbrf7i6QTp3pDwJRT9FmWt2erm+9B/J+7/Mqg+mbXxTfhO8 4lQlwyA+gD9fs/wkFRqQFgRSb+vFVK2QsKfTxODqa2kaLI8Kptbd5LWmxGupMhtrgyuG NRImzvzbcTMnW7AqYWUz7PFLbXsCAaxlFr9KqZG9BvmdYh7SWL2Qe02oDPFknxgj3wtj Q5Xo8WjucBD8z6vDHVtDXxuWRbpqa5leXt0qN61PfMuuWVRcm9I9s/TWhtdZYpKOlXZ+ i9lxUgJVw3PwZ6BA1gQvourfp8YtAcrW7k26I9EcAmB+r0ExsAOAOegFcpHo48awH3yS ywYw== 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=blOWhtjN88Q44vHtFfQzit4advqSdLTa0QQlsjWHN/k=; b=fxbB0q4+xV+LqdQsbPlM+ijzmo9c6XS45mONtrbDnVgKtDDaK9LYkvn+uNmamJmT+j CXftyR07Dt3Ktlrsf5gJeuk1KQS0JQLqjPSC7GWFb25XdLBXxLzZ3rv3c5fIAi7/IILV BuxBls0vZlkTHGinTg+j5avFQ7Elfhx3jmAH7WGPPigZ8q4i+wtJ446rznba1dDR3XKY Z7cviQmuV2um+QAMYizVPWmjrCk02Cu6B0B3hKXfate1n+vjOIBgNvYaELdaNXHZcGPQ v25BM2duk+MHp3bEnmdsHxU2G0ZMS6b15xkyVPGMIzsiR0pi6oSbugD6czrPiDIvWIlt x6ZQ== X-Gm-Message-State: AOAM530WnNrmShIJtJzIeRtSAd44d1AgsIQTCp6jeuvyYaJz87s06/z/ uvMwn7bjLhBd6wIF/8Q6mg9hxg== X-Google-Smtp-Source: ABdhPJwX/Y93dohXY9UwaNSUiKw+ZpvIe3D4i7UVNqbW4+oybBdGDEJ60who0jv3yvJUfGKSkO8ZQA== X-Received: by 2002:a17:90b:4f44:: with SMTP id pj4mr6265115pjb.216.1642991632577; Sun, 23 Jan 2022 18:33:52 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:db1:8b95:c482:df85]) by smtp.gmail.com with ESMTPSA id x12sm10403696pge.58.2022.01.23.18.33.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Jan 2022 18:33:52 -0800 (PST) Date: Mon, 24 Jan 2022 11:33:46 +0900 From: AKASHI Takahiro To: Sughosh Ganu Cc: u-boot@lists.denx.de, Masami Hiramatsu , Patrick Delaunay , Patrice Chotard , Heinrich Schuchardt , Alexander Graf , Simon Glass , Bin Meng , Ilias Apalodimas , Jose Marinho , Grant Likely , Tom Rini , Etienne Carriere Subject: Re: [RFC PATCH v3 5/9] EFI: FMP: Add provision to update image's ImageTypeId in image descriptor Message-ID: <20220124023346.GB48616@laputa> Mail-Followup-To: AKASHI Takahiro , Sughosh Ganu , u-boot@lists.denx.de, Masami Hiramatsu , Patrick Delaunay , Patrice Chotard , Heinrich Schuchardt , Alexander Graf , Simon Glass , Bin Meng , Ilias Apalodimas , Jose Marinho , Grant Likely , Tom Rini , Etienne Carriere References: <20220119185548.16730-1-sughosh.ganu@linaro.org> <20220119185548.16730-6-sughosh.ganu@linaro.org> <20220120052415.GD42867@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 On Fri, Jan 21, 2022 at 12:32:04PM +0530, Sughosh Ganu wrote: > hi Takahiro, > > On Thu, 20 Jan 2022 at 10:54, AKASHI Takahiro > wrote: > > > > On Thu, Jan 20, 2022 at 12:25:44AM +0530, Sughosh Ganu wrote: > > > The FWU Multi Banks Update feature allows updating different types of > > > updatable firmware images on the platform. These image types are > > > identified using the ImageTypeId GUID value. Add support in the > > > GetImageInfo function of the FMP protocol to get the GUID values for > > > the individual images and populate these in the image descriptor for > > > the corresponding images. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > > > > Changes since V2: None > > > > > > lib/efi_loader/efi_firmware.c | 90 ++++++++++++++++++++++++++++++++--- > > > 1 file changed, 83 insertions(+), 7 deletions(-) > > > > > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > > > index a1b88dbfc2..648342ae72 100644 > > > --- a/lib/efi_loader/efi_firmware.c > > > +++ b/lib/efi_loader/efi_firmware.c > > > @@ -10,6 +10,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > > > > @@ -96,6 +97,46 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported( > > > return EFI_EXIT(EFI_UNSUPPORTED); > > > } > > > > > > +static efi_status_t fill_part_guid_array(const efi_guid_t *guid, > > > + efi_guid_t **part_guid_arr) > > > +{ > > > + int i; > > > + int dfu_num = 0; > > > + efi_guid_t *guid_arr; > > > + struct dfu_entity *dfu; > > > + efi_status_t ret = EFI_SUCCESS; > > > + > > > + dfu_init_env_entities(NULL, NULL); > > > + > > > + dfu_num = 0; > > > + list_for_each_entry(dfu, &dfu_list, list) { > > > + dfu_num++; > > > + } > > > + > > > + if (!dfu_num) { > > > + log_warning("Probably dfu_alt_info not defined\n"); > > > + ret = EFI_NOT_READY; > > > + goto out; > > > + } > > > + > > > + *part_guid_arr = malloc(sizeof(efi_guid_t) * dfu_num); > > > + if (!*part_guid_arr) { > > > + ret = EFI_OUT_OF_RESOURCES; > > > + goto out; > > > + } > > > + > > > + guid_arr = *part_guid_arr; > > > + for (i = 0; i < dfu_num; i++) { > > > + guidcpy(guid_arr, guid); > > > + ++guid_arr; > > > + } > > > + > > > +out: > > > + dfu_free_entities(); > > > + > > > + return ret; > > > +} > > > + > > > /** > > > * efi_get_dfu_info - return information about the current firmware image > > > * @this: Protocol instance > > > @@ -104,9 +145,9 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported( > > > * @descriptor_version: Pointer to version number > > > * @descriptor_count: Pointer to number of descriptors > > > * @descriptor_size: Pointer to descriptor size > > > - * package_version: Package version > > > - * package_version_name: Package version's name > > > - * image_type: Image type GUID > > > + * @package_version: Package version > > > + * @package_version_name: Package version's name > > > + * @guid_array: Image type GUID array > > > * > > > * Return information bout the current firmware image in @image_info. > > > * @image_info will consist of a number of descriptors. > > > @@ -122,7 +163,7 @@ static efi_status_t efi_get_dfu_info( > > > efi_uintn_t *descriptor_size, > > > u32 *package_version, > > > u16 **package_version_name, > > > - const efi_guid_t *image_type) > > > + const efi_guid_t *guid_array) > > > { > > > struct dfu_entity *dfu; > > > size_t names_len, total_size; > > > @@ -172,7 +213,7 @@ static efi_status_t efi_get_dfu_info( > > > next = name; > > > list_for_each_entry(dfu, &dfu_list, list) { > > > image_info[i].image_index = dfu->alt + 1; > > > - image_info[i].image_type_id = *image_type; > > > + image_info[i].image_type_id = guid_array[i]; > > > image_info[i].image_id = dfu->alt; > > > > > > /* copy the DFU entity name */ > > > @@ -250,6 +291,7 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( > > > u16 **package_version_name) > > > { > > > efi_status_t ret; > > > + efi_guid_t *part_guid_arr = NULL; > > > > > > EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, > > > image_info_size, image_info, > > > @@ -264,12 +306,19 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( > > > !descriptor_size || !package_version || !package_version_name)) > > > return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > > > + ret = fill_part_guid_array(&efi_firmware_image_type_uboot_fit, > > > + &part_guid_arr); > > > + if (ret != EFI_SUCCESS) > > > + goto out; > > > > Why do you not call fwu_plat_fill_partition_guids() for FIT FMP driver? > > If you have a specific reason, please describe it. > > The idea here was to retain the current behaviour for the non FWU use > case, where for both raw and FIT images, the same ImageTypeId is being > used. What I meant to say was fwu_plat_fill_partition_guids() is not called even in case of CONFIG_FWU_MULTI_BANK_UPDATE. > Do you want to have a weak function which fills the array with a > specific ImageTypeId value. Then, if any platform wants to have a > different implementation, they can define a function which fills the > array with relevant GUID values. > > > > > > + > > > ret = efi_get_dfu_info(image_info_size, image_info, > > > descriptor_version, descriptor_count, > > > descriptor_size, > > > package_version, package_version_name, > > > - &efi_firmware_image_type_uboot_fit); > > > + part_guid_arr); > > > > > > +out: > > > + free(part_guid_arr); > > > return EFI_EXIT(ret); > > > } > > > > > > @@ -358,7 +407,10 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info( > > > u32 *package_version, > > > u16 **package_version_name) > > > { > > > + int status; > > > efi_status_t ret = EFI_SUCCESS; > > > + const efi_guid_t null_guid = NULL_GUID; > > > + efi_guid_t *part_guid_arr = NULL; > > > > > > EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, > > > image_info_size, image_info, > > > @@ -373,12 +425,36 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info( > > > !descriptor_size || !package_version || !package_version_name)) > > > return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > > > + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) { > > > + ret = fill_part_guid_array(&null_guid, &part_guid_arr); > > > + if (ret != EFI_SUCCESS) > > > + goto out; > > > + > > > + /* > > > + * Call the platform function to fill the GUID array > > > + * with the corresponding partition GUID values > > > + */ > > > + status = fwu_plat_fill_partition_guids(&part_guid_arr); > > > + if (status < 0) { > > > + log_err("Unable to get partiion guid's\n"); > > > + ret = EFI_DEVICE_ERROR; > > > + goto out; > > > + } > > > + } else { > > > + ret = fill_part_guid_array(&efi_firmware_image_type_uboot_raw, > > > + &part_guid_arr); > > > + if (ret != EFI_SUCCESS) > > > + goto out; > > > + } > > > > The code: > > ret = fill_part_guid_array(&efi_firmware_image_type_uboot_raw, > > &part_guid_arr); > > if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) > > status = fwu_plat_fill_partition_guids(&part_guid_arr); > > > > would be much simpler here. > > Yep, will change. > > > > > But I don't know why you want to call fwu_plat_fill_partition_guids() > > only in case of CONFIG_FWU_MULTI_BANK_UPDATE. The functionality should > > be the same whether A/B update or not. > > Let me know if you are okay with what I have proposed above. Yes. The weak "plat" function will be expected to be optimized away. -Takahiro Akashi > -sughosh > > > > > -Takahiro Akashi > > > > > + > > > ret = efi_get_dfu_info(image_info_size, image_info, > > > descriptor_version, descriptor_count, > > > descriptor_size, > > > package_version, package_version_name, > > > - &efi_firmware_image_type_uboot_raw); > > > + part_guid_arr); > > > > > > +out: > > > + free(part_guid_arr); > > > return EFI_EXIT(ret); > > > } > > > > > > -- > > > 2.17.1 > > >