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 7EA3CC6FA8E for ; Thu, 2 Mar 2023 05:30:02 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CFEE685B16; Thu, 2 Mar 2023 06:29:59 +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="rYoTjnqE"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7FF2585B2A; Thu, 2 Mar 2023 06:29:58 +0100 (CET) Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) (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 8B84785A8C for ; Thu, 2 Mar 2023 06:29:55 +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-pj1-x1031.google.com with SMTP id y2so15704821pjg.3 for ; Wed, 01 Mar 2023 21:29:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1677734994; 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=OPnmpQmgc/Cwlwy0YPJyU4VkmA1Byw9HFdTmCTy6CTk=; b=rYoTjnqELo3f7XxcfKPmMD0rjTLHlyRK8CpezyBXPD/hAd10Plg6qC2DRQTes9oU4W 8T4TOASAxuO1FqzL1DRRZexPIDjcapEr74+cBkMwtK/qcIvXdjXdp6pO+UvSz9kZri47 HWhOzAnz5wlMyXSxDn0FUg1JpDPUVhVDSnOu6T+aETsGnlSfA0zFFJhFQu+H5EqLABBt qF/HbEaMZeNoSbOakRJq3mAc1CTrQDFa3v8z9jpy15vpvBk+pdteDyrBH/GQ6GKbZl5P t8uUlQpEp6xkVZvvOA4fwTkVdI7IqPG0GDWFKxjfnQK6UPh/Lg3KtCol4cKRoxKTKyaD SySQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677734994; 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=OPnmpQmgc/Cwlwy0YPJyU4VkmA1Byw9HFdTmCTy6CTk=; b=mSgFBV4hj6FtF3YU1pD06bXZJcd7Rcf+4LOmWSYBTBwaJymfd8+m9+38vOd9lcofn+ eV7/wjDSgKD+Wao9GqG0upSHQ8VcXjrasb+vgDTy1LNCesbkqsvWKKpvIYYXjqyOOUQk 96KxEM+3UmpeO+QByvvFgUJvP4DBET1duUIOCSGyO6O0irbOHEIq2QX7Et+QWwtL13Dr 1RqxyYAG2VNiJPeLYWZSUYtMYWpseSdIwhbxKnn8lSWIdTrwIIsczYG4zk+//Ve0pH27 D5eTsyP21+ZJGF8EQD6kSmbj+bLZ3/OtV1qfsozZC/20aqiIcQGCsNjKjALZSFFj7vwA jSZA== X-Gm-Message-State: AO0yUKU6/qV0x/1TQxoebqOm6rvgdWej8SN9lX/Lf24Wjk6bgDtsBTZT P/pRs8DzWrFDQkSJODSorJN93A== X-Google-Smtp-Source: AK7set84ZcoDguCtwXbMG2KWzkbLsdtmHIMp4dUy9+xLGvhd2trFyZhG5HyZjgEnGDUfCB09K4QlhA== X-Received: by 2002:a05:6a20:8e0b:b0:cd:c79:5124 with SMTP id y11-20020a056a208e0b00b000cd0c795124mr10475189pzj.6.1677734993829; Wed, 01 Mar 2023 21:29:53 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:91c7:d373:91a1:d173]) by smtp.gmail.com with ESMTPSA id x14-20020a63170e000000b00502e1c50af3sm8220225pgl.45.2023.03.01.21.29.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Mar 2023 21:29:53 -0800 (PST) Date: Thu, 2 Mar 2023 14:29:49 +0900 From: Takahiro Akashi To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas Subject: Re: [PATCH v2 4/4] mkeficapsule: add FMP Payload Header Message-ID: <20230302052949.GC44192@laputa> Mail-Followup-To: Takahiro Akashi , Masahisa Kojima , u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas References: <20230301091523.18384-1-masahisa.kojima@linaro.org> <20230301091523.18384-5-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230301091523.18384-5-masahisa.kojima@linaro.org> 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 Wed, Mar 01, 2023 at 06:15:22PM +0900, Masahisa Kojima wrote: > Current mkeficapsule tool does not provide firmware > version management. EDK2 reference implementation inserts > the FMP Payload Header right before the payload. > It coutains the fw_version and lowest supported version. > > This commit adds three new parameters required to generate > the FMP Payload Header for mkeficapsule tool. > '-f' indicates whether FMP Payload Header is inserted. > '-v' indicates the firmware version. > '-l' indicates the lowest supported version. from user's point of view, '-f' looks redundant since '-v' or '-l' implicitly means '-f'. > When mkeficapsule tool is invoked without '-f' option, > FMP Payload Header is not inserted, the behavior is same as > current implementation. > > Signed-off-by: Masahisa Kojima > --- > Newly created in v2 > > tools/mkeficapsule.c | 81 ++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 75 insertions(+), 6 deletions(-) > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > index b71537beee..e0a6948df8 100644 > --- a/tools/mkeficapsule.c > +++ b/tools/mkeficapsule.c > @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule"; > efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; > > -static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR"; > +static const char *opts_short = "g:i:I:v:l:p:c:m:o:dfhAR"; > > enum { > CAPSULE_NORMAL_BLOB = 0, > @@ -41,6 +41,9 @@ static struct option options[] = { > {"guid", required_argument, NULL, 'g'}, > {"index", required_argument, NULL, 'i'}, > {"instance", required_argument, NULL, 'I'}, > + {"fmp-payload-header", no_argument, NULL, 'f'}, > + {"fw-version", required_argument, NULL, 'v'}, > + {"lsv", required_argument, NULL, 'l'}, > {"private-key", required_argument, NULL, 'p'}, > {"certificate", required_argument, NULL, 'c'}, > {"monotonic-count", required_argument, NULL, 'm'}, > @@ -60,6 +63,9 @@ static void print_usage(void) > "\t-g, --guid guid for image blob type\n" > "\t-i, --index update image index\n" > "\t-I, --instance update hardware instance\n" > + "\t-f, --fmp-payload-header insert fmp payload header\n" > + "\t-v, --fw-version firmware version\n" > + "\t-l, --lsv lowest supported version\n" > "\t-p, --private-key private key file\n" > "\t-c, --certificate signer's certificate file\n" > "\t-m, --monotonic-count monotonic count\n" > @@ -71,6 +77,30 @@ static void print_usage(void) > tool_name); > } > > +#define SIGNATURE_16(A, B) ((A) | ((B) << 8)) > +#define SIGNATURE_32(A, B, C, D) \ > + (SIGNATURE_16(A, B) | (SIGNATURE_16(C, D) << 16)) > + > +#define FMP_PAYLOAD_HDR_SIGNATURE SIGNATURE_32('M', 'S', 'S', '1') > + > +/** > + * struct fmp_payload_header - EDK2 header for the FMP payload > + * > + * This structure describes the header which is preprended to the > + * FMP payload by the edk2 capsule generation scripts. > + * > + * @signature: Header signature used to identify the header > + * @header_size: Size of the structure > + * @fw_version: Firmware versions used > + * @lowest_supported_version: Lowest supported version > + */ > +struct fmp_payload_header { > + uint32_t signature; > + uint32_t header_size; > + uint32_t fw_version; > + uint32_t lowest_supported_version; > +}; > + > /** > * auth_context - authentication context > * @key_file: Path to a private key file > @@ -95,6 +125,12 @@ struct auth_context { > size_t sig_size; > }; > > +struct fmp_payload_header_params { > + bool have_header; > + uint32_t fw_version; > + uint32_t lowest_supported_version; > +}; > + You may put those definitions in tools/eficapsule.h. > static int dump_sig; > > /** > @@ -402,6 +438,7 @@ static void free_sig_data(struct auth_context *ctx) > */ > static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > unsigned long index, unsigned long instance, > + struct fmp_payload_header_params *fmp_ph_params, > uint64_t mcount, char *privkey_file, char *cert_file, > uint16_t oemflags) > { > @@ -410,10 +447,11 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > struct efi_firmware_management_capsule_image_header image; > struct auth_context auth_context; > FILE *f; > - uint8_t *data; > + uint8_t *data, *new_data, *buf; > off_t bin_size; > uint64_t offset; > int ret; > + struct fmp_payload_header payload_header; > > #ifdef DEBUG > fprintf(stderr, "For output: %s\n", path); > @@ -423,6 +461,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > auth_context.sig_size = 0; > f = NULL; > data = NULL; > + new_data = NULL; > ret = -1; > > /* > @@ -431,12 +470,31 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > if (read_bin_file(bin, &data, &bin_size)) > goto err; > > + buf = data; > + > + /* insert fmp payload header right before the payload */ > + if (fmp_ph_params->have_header) { > + new_data = malloc(bin_size + sizeof(payload_header)); > + if (!new_data) > + goto err; > + > + payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE; > + payload_header.header_size = sizeof(payload_header); > + payload_header.fw_version = fmp_ph_params->fw_version; > + payload_header.lowest_supported_version = > + fmp_ph_params->lowest_supported_version; > + memcpy(new_data, &payload_header, sizeof(payload_header)); > + memcpy(new_data + sizeof(payload_header), data, bin_size); > + buf = new_data; > + bin_size += sizeof(payload_header); > + } > + > /* first, calculate signature to determine its size */ > if (privkey_file && cert_file) { > auth_context.key_file = privkey_file; > auth_context.cert_file = cert_file; > auth_context.auth.monotonic_count = mcount; > - auth_context.image_data = data; > + auth_context.image_data = buf; > auth_context.image_size = bin_size; > > if (create_auth_data(&auth_context)) { > @@ -536,7 +594,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > /* > * firmware binary > */ > - if (write_capsule_file(f, data, bin_size, "Firmware binary")) > + if (write_capsule_file(f, buf, bin_size, "Firmware binary")) > goto err; > > ret = 0; > @@ -545,6 +603,7 @@ err: > fclose(f); > free_sig_data(&auth_context); > free(data); > + free(new_data); > > return ret; > } > @@ -644,6 +703,7 @@ int main(int argc, char **argv) > unsigned long oemflags; > char *privkey_file, *cert_file; > int c, idx; > + struct fmp_payload_header_params fmp_ph_params = { 0 }; > > guid = NULL; > index = 0; > @@ -679,6 +739,15 @@ int main(int argc, char **argv) > case 'I': > instance = strtoul(optarg, NULL, 0); > break; > + case 'f': > + fmp_ph_params.have_header = true; > + break; > + case 'v': > + fmp_ph_params.fw_version = strtoul(optarg, NULL, 0); > + break; > + case 'l': > + fmp_ph_params.lowest_supported_version = strtoul(optarg, NULL, 0); > + break; Should we check dependencies between two options? Let's say, '-v' is required if '-l' is provided, and 'fw_version' must be greater than 'lowest_supported_version'. -Takahiro Akashi > case 'p': > if (privkey_file) { > fprintf(stderr, > @@ -747,11 +816,11 @@ int main(int argc, char **argv) > if (capsule_type != CAPSULE_NORMAL_BLOB) { > if (create_empty_capsule(argv[argc - 1], guid, > capsule_type == CAPSULE_ACCEPT) < 0) { > - fprintf(stderr, "Creating empty capsule failed\n"); > + fprintf(stderr, "Creating empty capsulefailed\n"); > exit(EXIT_FAILURE); > } > } else if (create_fwbin(argv[argc - 1], argv[argc - 2], guid, > - index, instance, mcount, privkey_file, > + index, instance, &fmp_ph_params, mcount, privkey_file, > cert_file, (uint16_t)oemflags) < 0) { > fprintf(stderr, "Creating firmware capsule failed\n"); > exit(EXIT_FAILURE); > -- > 2.17.1 >