[PATCH] efi: Call bootm_disable_interrupts earlier in efi_exit_boot_services

[Ilias Apalodimas]

On Sat, 20 Nov 2021 at 00:09, Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Nov 19, 2021 at 10:52:27PM +0100, Heinrich Schuchardt wrote:
> >
> >
> > Am 19. November 2021 22:33:04 MEZ schrieb Tom Rini <trini at konsulko.com>:
> > >If we look at the path that bootm/booti take when preparing to boot the
> > >OS, we see that as part of (or prior to calling do_bootm_states,
> > >explicitly) the process, bootm_disable_interrupts() is called prior to
> > >announce_and_cleanup() which is where udc_disconnect() /
> > >board_quiesce_devices() / dm_remove_devices_flags() are called from.  In
> > >the EFI path, these are called afterwards.  In efi_exit_boot_services()
> > >however we have been calling bootm_disable_interrupts() after the above
> > >functions, as part of ensuring that we disable interrupts as required
> > >by the spec.  However, bootm_disable_interrupts() is also where we go
> > >and call usb_stop().  While this has been fine before, on the TI J721E
> > >platform this leads us to an exception.  This exception seems likely to
> > >be the case that we're trying to stop devices that we have already
> > >disabled clocks for.  The most direct way to handle this particular
> >
> > This patch may hide an error on your board but obviously does not address the real problem.

I don't think it 'hides' it.  I think it's the other way around, this
board exposes the problem.  In any case I think disabling irq's etc
make sense to run before we disable devices completely.

> >
> > If dependencies in the shut down sequence should exist, we need to consider them in the driver model.

Yes agree 100% here.

> >
> > What is your plan to analyze the problem?
>
> I'm not sure there is a different problem to solve here.  It's unsafe to
> call the "shut everything down" function, which is what usb_stop() is,
> after having shut everything down.  We may be able to stop calling
> usb_stop() as any sort of shutdown should already have happened via
> driver model, which is what we see now.

Linux has CONFIG_EFI_DISABLE_PCI_DMA to deal with similar problems on
PCI devices.  Basically it disables busmastering until linux properly
configures the SMMU.  I think disabling the devices before handing
over to the OS (when possible) is a good policy.  In any case I think
this patch is reasonable since it at lest makes EFI and bootm/i behave
similarly.  I'll go have a look on bootm/i and cleanup the whole thing
at some point.

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>
> --
> Tom