[PATCH v5 05/12] usb: Avoid unbinding devices in use by bootflows

[Simon Glass]

Hi Shantur,

On Thu, 23 Nov 2023 at 04:35, Shantur Rathore <i at shantur.com> wrote:
>
> 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.

Yes, I agree. I will take a look.

Regards,
Simon