[PATCH v5 05/12] usb: Avoid unbinding devices in use by bootflows
Shantur Rathore
i at shantur.com
Thu Nov 23 12:35:40 CET 2023
Hi Simon,
On Tue, Nov 21, 2023 at 2:17 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Shantur,
>
> On Sun, 19 Nov 2023 at 15:47, Shantur Rathore <i at shantur.com> wrote:
> >
> > Hi Simon,
> >
> > On Sun, Nov 19, 2023 at 7:12 PM Simon Glass <sjg at 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.
Kind regards,
Shantur
More information about the U-Boot
mailing list