Hi Shantur, On Thu, 23 Nov 2023 at 04:35, Shantur Rathore <i...@shantur.com> wrote: > > Hi Simon, > > On Tue, Nov 21, 2023 at 2:17 AM Simon Glass <s...@chromium.org> wrote: > > > > Hi Shantur, > > > > On Sun, 19 Nov 2023 at 15:47, Shantur Rathore <i...@shantur.com> wrote: > > > > > > Hi Simon, > > > > > > On Sun, Nov 19, 2023 at 7:12 PM Simon Glass <s...@chromium.org> wrote: > > > > > > > > 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. At that point any information > > > > attached to the bootflow is dropped. > > > > > > > > This is quite risky since the contents of freed memory are not > > > > guaranteed to remain unchanged. Depending on what other options are > > > > done before boot, a hard-to-find bug may crop up. > > > > > > > > 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. > > > > > > > > > > I am no UEFI / bootloader expert and while trying to gather more info > > > on EFI ExitBootServies I came across this > > > https://github.com/tianocore-docs/edk2-UefiDriverWritersGuide/blob/master/7_driver_entry_point/77_adding_the_exit_boot_services_feature.md > > > > > > If I understand this correctly, EFI ( U-boot in this case) should be > > > halting all USB controllers like done in usb_stop() > > > Is my understanding correct? > > > > Yes, that is correct. The dm_remove_devices_flags() call should do > > that. The code in bootm_disable_interrupts() is a hangover from the > > pre-driver model days, I suspect. > > > > Perhaps we should also remove the eth_halt() and usb_pause() from > > bootm_disable_interrupts() ? > > > > Apologies for delayed response. > In this case, I think we should remove eth_halt() and > usb_stop() (there is no usb_pause() ) from bootm_disable_interrupts(). > > No point stopping things twice.
Yes, I agree. I will take a look. Regards, Simon
