Hi Heinrich, On Wed, 15 Nov 2023 at 19:06, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > 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().
OK... To my reading, efi_load_pe() pulls the image apart into memory allocated with efi_allocate_pages(). So that is separate memory? In any case, it would end up in a 'struct bootflow'. IMO the EFI memory-allocation functions should be called only when booting, not before. I do worry about leakage... [..] Regards, Simon
