[PATCH v3 4/4] fs: remove explicit efi configuration dependency

[Tom Rini]

On Tue, Dec 26, 2023 at 09:46:52AM +0000, Simon Glass wrote:
> Hi,
> 
> On Wed, Dec 20, 2023 at 1:25 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Dec 19, 2023 at 09:46:27PM -0700, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Mon, 18 Dec 2023 at 15:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >
> > > >
> > > >
> > > > Am 18. Dezember 2023 16:01:40 MEZ schrieb Simon Glass <sjg at chromium.org>:
> > > > >Hi AKASHI,
> > > > >
> > > > >On Sun, 17 Dec 2023 at 19:39, AKASHI Takahiro
> > > > ><takahiro.akashi at linaro.org> wrote:
> > > > >>
> > > > >> Now it is clear that the feature actually depends on efi interfaces,
> > > > >> not "bootefi" command. efi_set_bootdev() will automatically be nullified
> > > > >> if necessary efi component is disabled.
> > > > >>
> > > > >> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > > >> ---
> > > > >>  fs/fs.c | 7 +++----
> > > > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > > > >>
> > > > >> diff --git a/fs/fs.c b/fs/fs.c
> > > > >> index f33b85f92b61..82ee03b160e9 100644
> > > > >> --- a/fs/fs.c
> > > > >> +++ b/fs/fs.c
> > > > >> @@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> > > > >>                 return 1;
> > > > >>         }
> > > > >>
> > > > >> -       if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> > > > >> -               efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > > >> -                               (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> > > > >> -                               len_read);
> > > > >> +       efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > > >> +                       (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> > > > >> +                       len_read);
> > > > >
> > > > >As I understand it, this is setting the boot device so that (if it
> > > > >happens to be an efi application) it will know which device it came
> > > > >from. But this is a hack. For bootstd, the device is known as it loads
> > > > >the kernel.
> > > >
> > > > Please, consider what happens when the user interactively executes the load and bootefi command from the console.
> 
> Yes, that case needs to be handled.
> 
> > > >
> > > > >
> > > > >Also it does not deal with memory allocation (nor can it).
> > > > >
> > > > >Where are we using the 'load' command to load a kernel? The distro
> > > > >scripts are deprecated.
> > > >
> > > > Some users use boot.scr scripts
> 
> We will need to figure out that. But we should make bootstd do the right thing.
> 
> > > >
> > > >
> > > > >
> > > > >At some point this code should be removed. Is it too early for that?
> > > >
> > > > Yes, as long as we allow users to invoke the bootefi command with an address pointer.
> > >
> > > So how about we create the new path, with the info passed in as part
> > > of the bootefi call? Then we can remove the call from bootstd, at
> > > least.
> >
> > Why? It's not clear to me we'd have this information available at that
> > point unless we stored things to pass along at that point too.
> 
> For the bootstd case, we know the device where the kernel came from.
> See distro_efi_boot() for this code. It calls efiload_read_file()
> which calls set_efi_bootdev().
> 
> We should instead pass the device information to efi_binary_run().
> 
> With that done, we can perhaps make the 'load' and 'tftp' commands
> record the device information somewhere, so that bootm can pick it up
> when it starts and put it in 'images'. Then at some point we can clean
> up the 'devnr' parameter, which should really be a partition number,
> since the 'dev' parameter tells you the device.
> 
> Anyway, this series is fine, so long as we can agree to the above.

I'm getting very worried that we're spending a lot of time deciding how
users are going to use the system, without getting input from them on
what their use cases are, and are not, and why they've chosen what
they've chosen. Talking with some of the Debian people about why they're
using script + extlinux, and Armbian people about what they're doing
(which is just scripts?) should be the first priority before we decide
how booting things is going to work, outside of the EFI cases (which to
be clear, are having their interests / needs represented already I
believe).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231226/048b124a/attachment.sig>