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 384A3C433EF for ; Wed, 23 Mar 2022 08:18:49 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id E48B683894; Wed, 23 Mar 2022 09:18:46 +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="gCtSuu7j"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id B6A0A838BA; Wed, 23 Mar 2022 09:18:44 +0100 (CET) Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) (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 A7E4782133 for ; Wed, 23 Mar 2022 09:18: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-pj1-x102f.google.com with SMTP id n7-20020a17090aab8700b001c6aa871860so1045691pjq.2 for ; Wed, 23 Mar 2022 01:18:40 -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=SZBsRHJDPvf8iKlGOrsP5fUJVlSeDvtRBA5S3A+xzF0=; b=gCtSuu7j7K3p7wd6ICsMeFQcB/RsoVzJx51tPnyCYWdjZmWxz9bZtiHMXU/WO8ntYR /2/pzZolY3NCO8+chzz4YCdPTuJR70pzUTxVpVEDwCCmY+OrsARj+VvRfLW/EZcRbbP7 iHa2bEaODEdXrX92z44JtCqk6p94IoX3t9hu/kM7W1yNG8qmmO5BNgyxCCFekSl8szAd uccZy1kQRCtFwJMzbuO+1ykpEK/h1yzx1UYSjLN94I6mJg6sfCNUmJr1bII+EABQbdeb RNWR3u5krpqn6Q+QV69qZu98oL+fTXgyDZG90dGz8QidN8s0F0G8zJtXf2CH1oH5vc7s 9BOg== 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=SZBsRHJDPvf8iKlGOrsP5fUJVlSeDvtRBA5S3A+xzF0=; b=30RkaTIvnZ213dXbsWPXtJ3Gq8vTG1Zc5/Gp97UgROhMyHpWus6iACktVxzVxyuQ4y 25ktq+4OYOEIrjl6JSxrF2FgFl9R84yhv+B6IqEv6/yLVXtJCuKEK1ObJsG72xlZbi/Z 7XNf9+HA3qeg7xkxwZ6lYnemoVbti+0KiAiBnI++AE7dwx7FfWJatCoImINIuahChHtU U4wMshLCIAUy+mQ5JHgaXn+7VbgBZwVVIN/y0HtY87C7h8WrgyDtokwq2QU1oGDZvMRw 67Ym7rJgeD8DdbRreMy9R6x9lRsZ/OVGH2RoEK+KdRnmRCI4WjcY9OwW9W7ygm3yc9vx 0z0g== X-Gm-Message-State: AOAM531XyEcpraEBVEGnU1bizKL9GzcXO4XcKLQwzhJSTvVF1o55b0Jy gmsFqhVgAEwKU42R5UBVq5ilPQ== X-Google-Smtp-Source: ABdhPJxKR/ujoNcQ3pUU5p1e5lRwOwn57Us4U3qHdSOac+1zmVGStDo3FqIMq6cgmiUJudbQCk10Qg== X-Received: by 2002:a17:902:ce91:b0:154:16c2:63b3 with SMTP id f17-20020a170902ce9100b0015416c263b3mr22893664plg.22.1648023519008; Wed, 23 Mar 2022 01:18:39 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:2199:5d47:ea3f:788f]) by smtp.gmail.com with ESMTPSA id ep2-20020a17090ae64200b001c6a7c22aedsm4831605pjb.37.2022.03.23.01.18.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Mar 2022 01:18:38 -0700 (PDT) Date: Wed, 23 Mar 2022 17:18:35 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, Ilias Apalodimas , Heinrich Schuchardt Subject: Re: [PATCH v2 5/9] efi_loader: use short-form DP for load options Message-ID: <20220323081835.GG49108@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , u-boot@lists.denx.de, Ilias Apalodimas , Heinrich Schuchardt References: <20220319091148.142036-1-heinrich.schuchardt@canonical.com> <20220319091148.142036-6-heinrich.schuchardt@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220319091148.142036-6-heinrich.schuchardt@canonical.com> 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 Sat, Mar 19, 2022 at 10:11:44AM +0100, Heinrich Schuchardt wrote: > From: Heinrich Schuchardt > > The GUID of partitions is sufficient for identification and will stay > constant in the lifetime of a boot option. The preceding path of the > device-path may change due to changes in the enumeration of devices. > Therefore it is preferable to use the short-form of device-paths in load > options. Adjust the 'efidebug boot add' command accordingly. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > support both short and long form device paths > split off exporting efi_dp_shorten() into a separate patch > --- > cmd/efidebug.c | 70 ++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 48 insertions(+), 22 deletions(-) > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index 401d13cc4c..51e2850d21 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -734,20 +734,20 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > } > > /** > - * create_initrd_dp() - Create a special device for our Boot### option > - * > - * @dev: Device > - * @part: Disk partition > - * @file: Filename > - * Return: Pointer to the device path or ERR_PTR > + * create_initrd_dp() - create a special device for our Boot### option > * > + * @dev: device > + * @part: disk partition > + * @file: filename > + * @shortform: create short form device path > + * Return: pointer to the device path or ERR_PTR > */ > static > struct efi_device_path *create_initrd_dp(const char *dev, const char *part, > - const char *file) > + const char *file, int shortform) > > { > - struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; > + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL, *short_fp = NULL; > struct efi_device_path *initrd_dp = NULL; > efi_status_t ret; > const struct efi_initrd_dp id_dp = { > @@ -771,9 +771,13 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, > printf("Cannot create device path for \"%s %s\"\n", part, file); > goto out; > } > + if (shortform) > + short_fp = efi_dp_shorten(tmp_fp); > + if (!short_fp) > + short_fp = tmp_fp; > > initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp, > - tmp_fp); > + short_fp); > > out: > efi_free_pool(tmp_dp); > @@ -807,6 +811,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > size_t label_len, label_len16; > u16 *label; > struct efi_device_path *device_path = NULL, *file_path = NULL; > + struct efi_device_path *fp_free = NULL; > struct efi_device_path *final_fp = NULL; > struct efi_device_path *initrd_dp = NULL; > struct efi_load_option lo; > @@ -826,7 +831,18 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > argc--; > argv++; /* 'add' */ > for (; argc > 0; argc--, argv++) { > - if (!strcmp(argv[0], "-b")) { > + int shortform; > + > + if (*argv[0] != '-' || strlen(argv[0]) != 2) { > + r = CMD_RET_USAGE; > + goto out; > + } > + shortform = 0; > + switch (argv[0][1]) { > + case 'b': > + shortform = 1; > + /* fallthrough */ > + case 'B': > if (argc < 5 || lo.label) { > r = CMD_RET_USAGE; > goto out; > @@ -849,24 +865,33 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > /* file path */ > ret = efi_dp_from_name(argv[3], argv[4], argv[5], > - &device_path, &file_path); > + &device_path, &fp_free); The semantics of efi_dp_from_name() seems odd as both "device_path" and "file_path" (or "fp_free") may partially contain a duplicated device path. That is why "device_path" is not used after this line. Although this behavior has not changed since my initial implementation, "file_path" should not have included preceding device path components other than the file path media device path. Anyhow, we can pass NULL instead of "&device_path" for now. > if (ret != EFI_SUCCESS) { > printf("Cannot create device path for \"%s %s\"\n", > argv[3], argv[4]); > r = CMD_RET_FAILURE; > goto out; > } > + if (shortform) > + file_path = efi_dp_shorten(fp_free); > + if (!file_path) > + file_path = fp_free; > fp_size += efi_dp_size(file_path) + > sizeof(struct efi_device_path); > argc -= 5; > argv += 5; > - } else if (!strcmp(argv[0], "-i")) { > + break; > + case 'i': > + shortform = 1; > + /* fallthrough */ > + case 'I': > if (argc < 3 || initrd_dp) { > r = CMD_RET_USAGE; > goto out; > } > > - initrd_dp = create_initrd_dp(argv[1], argv[2], argv[3]); > + initrd_dp = create_initrd_dp(argv[1], argv[2], argv[3], > + shortform); > if (!initrd_dp) { > printf("Cannot add an initrd\n"); > r = CMD_RET_FAILURE; > @@ -876,7 +901,8 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > argv += 3; > fp_size += efi_dp_size(initrd_dp) + > sizeof(struct efi_device_path); > - } else if (!strcmp(argv[0], "-s")) { > + break; > + case 's': > if (argc < 1 || lo.optional_data) { > r = CMD_RET_USAGE; > goto out; > @@ -884,7 +910,8 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > lo.optional_data = (const u8 *)argv[1]; > argc -= 1; > argv += 1; > - } else { > + break; > + default: > r = CMD_RET_USAGE; > goto out; > } > @@ -927,7 +954,7 @@ out: > efi_free_pool(final_fp); > efi_free_pool(initrd_dp); > efi_free_pool(device_path); > - efi_free_pool(file_path); > + efi_free_pool(fp_free); > free(lo.label); > > return r; > @@ -1571,12 +1598,11 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > static char efidebug_help_text[] = > " - UEFI Shell-like interface to configure UEFI environment\n" > "\n" > - "efidebug boot add " > - "-b