On 11/16/23 02:42, Simon Glass wrote:
Hi Heinrich,
On Wed, 15 Nov 2023 at 18:34, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
On 11/15/23 17:23, Heinrich Schuchardt wrote:
On 11/15/23 16:50, Simon Glass wrote:
Hi Heinrich,
On Sun, 12 Nov 2023 at 16:35, Heinrich Schuchardt <xypron.g...@gmx.de>
wrote:
Am 12. November 2023 22:20:50 MEZ schrieb Simon Glass
<s...@chromium.org>:
Hi Heinrich,
On Sun, 12 Nov 2023 at 13:32, Heinrich Schuchardt
<xypron.g...@gmx.de> wrote:
Am 12. November 2023 21:02:42 MEZ schrieb Simon Glass
<s...@chromium.org>:
When a USB device is unbound, it causes any bootflows attached to
it to
be removed, via a call to bootdev_clear_bootflows() from
bootdev_pre_unbind(). This obviously makes it impossible to boot the
bootflow.
However, when booting a bootflow that relies on USB, usb_stop() is
called, which unbinds the device. For EFI, this happens in
efi_exit_boot_services() which means that the bootflow disappears
before it is finished with.
After ExitBootServices() no driver of U-Boot can be used as the
operating system is in charge.
Any bootflow inside U-Boot is terminated by definition when
reaching ExitBootServices.
There is no need to unbind all the USB devices just to quiesce them.
Add a new usb_pause() call which removes them but leaves them bound.
As U-Boot must not access any device after ExitBootServices() it is
unclear to me what you want to achieve.
I can't remember exactly what is needed from the bootflow, but
something is. Perhaps the kernel, or the cmdline, or fdt? It would
have been better if I had mentioned this explicitly, But then this
patch has been sitting around for ages...
In any case, the intent of exit-boot-services is not to free all the
memory used, since some of it may have been used to hold data that the
app continues to use.
The EFI application reads the memory map and receives an ID which it
passes to ExitBootServiced() after this point any memory not marked
as EFI runtime can be used by the EFI app. This includes the memory
that harbors the U-Boot USB drivers. Therefore no drivers can be used
here.
Starting the EFI app via StartImage() must terminate any running
U-Boot "boot flow".
After ExitBootServices() the EFI application cannot return to U-Boot
except for SetVirtualAdressMspsetting which relocates the EFI runtime.
Bootflows and U-Boot drivers have no meaning after ExitBootServices().
Also there is U-Boot code between when the devices are unbound and
when U-Boot actually exits back to the app.
This hang was 100% repeatable on brya (an x86 Chromebook) and it took
quite a while to find.
We need a better understanding of the problem that you experience to
find an adequate solution. Why does removing all devices lead to
hanging the system?
Are you able to test this? With your better knowledge of EFI you might
be able to figure out something else that is going on. But in my case
it causes some memory to be freed (perhaps the memory holding the EFI
app?), which is then overwritten by something else being malloc()'d,
The memory for the EFI app is not assigned via malloc(). It is allocated
by AllocatedPages().
Reading "some memory freed" does not give me confidence that the problem
is sufficiently analyzed.
so the boot hangs. It is hard to see what is going on after the app
starts.
This patch was sent almost two months ago and fixes a real bug. The
first few versions attracted no comment now it is being blocked
because you don't understand how it fixes anything.
Do you understand why unbinding the devices causes the problem?
I can get the hardware up again and try this but it will take a while.
Digging a bit deeper seems to be the right approach.
Re: bug - bootflow: grub efi crashes when bootflow selected explicitly
https://lore.kernel.org/u-boot/CAHc5_t1H39YV=hssa7tcedzrctax+zibkx1vsuwew-raol9...@mail.gmail.com/
points to a probable root cause:
https://github.com/u-boot/u-boot/blob/master/boot/bootflow.c#L470
free(bflow->buf)
In the EFI boot bflow->buf points to $kernel_addr_r which never was
allocated and therefore must not be freed.
Yes, this is the root cause of the crash.
However, this patch should still be applied. I can update the commit
message if you like.
Freeing the FDT and kernel before booting them is just not safe,
whether EFI or anything else. Freed memory is not guaranteed to hang
around for any length of time. For example, with truetype fonts,
malloc() is called during any console output and will likely corrupt
the images just as they are being booted.
Please, observe that malloc() and efi_allocate_pages() use completely
separate parts of the memory.
When reaching ExitBootServices() the memory used by the EFI binary
(relocated in efi_load_pe()) and for the configuration table with the
device-tree have been allocated by efi_allocate_pages(). These addresses
cannot neither allocated by malloc() nor freed with free().
Best regards
Heinrich
We should leave the devices bound when booting.
Regards,
Simon
