From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: Thu, 14 Jan 2021 11:46:29 +0200
Subject: [RFC PATCH 1/3] efi_loader: Introduce helper functions for EFI
In-Reply-To: <20210114044828.GB25252@laputa>
References: <20210113111149.64567-1-ilias.apalodimas@linaro.org>
 <20210113111149.64567-2-ilias.apalodimas@linaro.org>
 <20210114044828.GB25252@laputa>
Message-ID: <YAAS9dwU66seybsC@apalos.home>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Akashi-san,

> > +					efi_uintn_t *size, int idx)
[...]
> 
> The type of "idx" should be 'enum load_option_dp_type'.
> 
> Currently, "idx" is used as an index into the array of device paths,
> but given each device path is set to have its own guid, "idx" should be
> unlinked from the 'order' within the array.
> 
> Even if you don't want so, this function should at least take care of
> some special cases like
> <execution image> + (no initrd) + <dtb>
> Alternatively, "END device path" can be put at the second place.
> 
> I expect that handling those corner cases should be described explicitly.
> 

True, I'll refactor this a bit and allow us to extend it to load DTBs in the
future.

> > +{
> > +	struct efi_device_path *instance = NULL;
> > +

[...]

> > +/**
> > + * efi_get_fp_from_var() - Retrieve and return a device path from an EFI
> > + *			   Boot### variable
> > + *
> > + * @idx:	index of the instance to retrieve
> > + *
> > + * Return:	device path of NULL. Caller must free the returned value
> > + */
> > +struct efi_device_path *efi_get_fp_from_boot(int idx)
> 
> Are you sure that this function is called only when "BootCurrent"
> is appropriately set?
> 

Yea, this is either used on the function called by the EFI-stub or
try_load_entry() where the BootCurrent value is set explicitly.

Regards
/Ilias