Hi Heinrich,
On Sat, 26 Aug 2023 at 11:20, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> On 8/25/23 07:57, Masahisa Kojima wrote:
> > On Thu, 24 Aug 2023 at 09:11, AKASHI Takahiro
> > <takahiro.aka...@linaro.org> wrote:
> >>
> >> Hi Kojima-san,
> >>
> >> On Wed, Aug 23, 2023 at 05:37:19PM +0900, Masahisa Kojima wrote:
> >>> This adds the URI device path option for 'boot add' subcommand.
> >>> User can add the URI load option for downloading ISO image file
> >>> or EFI application through network(e.g. HTTP).
> >>>
> >>> Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> >>> ---
> >>> cmd/efidebug.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 39 insertions(+)
> >>>
> >>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> >>> index 0be3af3e76..62f867df2a 100644
> >>> --- a/cmd/efidebug.c
> >>> +++ b/cmd/efidebug.c
> >>> @@ -829,6 +829,44 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp,
> >>> int flag,
> >>> argc -= 1;
> >>> argv += 1;
> >>> break;
> >>> + case 'u':
> >>> + {
> >>> + char *pos;
> >>> + int uridp_len;
> >>> + struct efi_device_path_uri *uridp;
> >>> +
> >>> + if (argc < 3 || lo.label) {
> >>> + r = CMD_RET_USAGE;
> >>> + goto out;
> >>> + }
> >>> + id = (int)hextoul(argv[1], &endp);
> >>> + if (*endp != '\0' || id > 0xffff)
> >>> + return CMD_RET_USAGE;
> >>> +
> >>> + efi_create_indexed_name(var_name16,
> >>> sizeof(var_name16),
> >>> + "Boot", id);
> >>> +
> >>> + label = efi_convert_string(argv[2]);
> >>> + if (!label)
> >>> + return CMD_RET_FAILURE;
> >>> + lo.label = label;
> >>> +
> >>> + uridp_len = sizeof(struct efi_device_path) +
> >>> strlen(argv[3]) + 1;
> >>> + fp_free = efi_alloc(uridp_len + sizeof(END));
> >>> + uridp = (struct efi_device_path_uri *)fp_free;
> >>> + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> >>> + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI;
> >>> + uridp->dp.length = uridp_len;
> >>> + strcpy(uridp->uri, argv[3]);
> >>> + pos = (char *)uridp + uridp_len;
> >>> + memcpy(pos, &END, sizeof(END));
> >>> + fp_size += uridp_len + sizeof(END);
> >>> + file_path = (struct efi_device_path *)uridp;
> >>> + argc -= 3;
> >>> + argv += 3;
> >>> + break;
> >>> + }
> >>> +
> >>> default:
> >>> r = CMD_RET_USAGE;
> >>> goto out;
> >>> @@ -1492,6 +1530,7 @@ static char efidebug_help_text[] =
> >>> " -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file
> >>> path>\n"
> >>> " -i|-I <interface> <devnum>[:<part>] <initrd file path>\n"
> >>> " (-b, -i for short form device path)\n"
> >>> + " -u <bootid> <label> <uri>\n"
> >>
> >> It might be a matter of personal preference, but
> >> since the current syntax of efidebug is already much complex,
> >> I don't want to add more options unless it's necessary.
> >> How about re-using "-B" option, say
> >> => efidebug -B 3 debian-netinst uri - https://foo.com/baa
>
> You just replace one set of alternatives (-B -b -u) against another
> (mmc, nvme, uri, ...). I cannot see that this reduces complexity.
>
> >
> > I understand your concern. OK, I will add uri in -b|B and also update
> > documentation.
>
> It is unclear to me what the difference between -b and -B shall be for
> URIs. Would -B be with the device path of the network device
> (/VenHw()/MAC()/URI()) and -b without (/URI())?
Differentiating -b and -B for URI is not mandatory, I think.
To keep implementation simple, I would like to add '-u' option as the
original patch does.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> >
> > Thanks,
> > Masahisa Kojima
> >
> >>
> >> BTW, I think that <bootid> and <label> should have been moved out of "-b|B"
> >> when Ilias introduced this new syntax.
> >>
> >> -Takahiro Akashi
> >>
> >>
> >>> " -s '<optional data>'\n"
> >>> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> >>> " - delete UEFI BootXXXX variables\n"
> >>> --
> >>> 2.34.1
> >>>
>
