> From: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > Date: Sun, 6 Apr 2025 07:14:49 +0200 > > Simon Glass <s...@chromium.org> schrieb am Sa., 5. Apr. 2025, 22:46: > > > Hi, > > > > On Sat, 5 Apr 2025 at 19:58, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > > > > > > It has been reported that memory corruption can occurred because network > > > packages where received after EXIT_BOOT_SERVICES. See the thread > > > starting at [1]. > > > > > > We try to remove all drivers when EXIT_BOOT_SERVICES is called. But > > > > > > * Some network drivers don't call their own stop method when removed. > > > * Some network drivers don't have a remove method. > > > * Some devices have CONFIG_DM_DEVICE_REMOVE=n. > > > > A few points: > > 1. Only three boards disable DM_DEVICE_REMOVE and enable EFI_LOADER; > > EFI_LOADER must depend on it or select it > > > > Why does that option exist at all?
Presumably it shaves a few bytes off the final binaries? And if you don't have any drivers that do uncontrolled DMA you don't really need to do any driver tear down. > How would a driver live on when U-Boot is not around? > > My two cents: That option should vanish as it is incompatible with the > driver model. > > 2. If devices don't enable DM_DEVICE_REMOVE we'll have similar > > problems with USB, etc. > > 3. The code in efi_boottime.c should be unified with > > announce_and_cleanup() since otherwise this will happen on the non-EFI > > path > > > > Please do at least (1) now, if this patch is targeted at the imminent > > release. > > > > Also, we need a fix in the affected network driver, although I suspect > > the reporter will do that. > > > > You just sent patches for the network drivers. > > Best regards > > Heinrich > > > > Regards, > > Simon > > > > > > > > > > > > > > Let's call eth_halt() in EXIT_BOOT_SERVICES explicitly. > > > > > > [1] > > > > > https://lore.kernel.org/all/c101b675-eee6-44cb-8a44-83f72182f...@kohlschutter.com/ > > > > > > Cc: Michael Brown <mc...@ipxe.org> > > > Reported-by: Christian Kohlschütter <christ...@kohlschutter.com> > > > Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > > > --- > > > lib/efi_loader/efi_boottime.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/lib/efi_loader/efi_boottime.c > > b/lib/efi_loader/efi_boottime.c > > > index 5164cb15986..eaa6464fa39 100644 > > > --- a/lib/efi_loader/efi_boottime.c > > > +++ b/lib/efi_loader/efi_boottime.c > > > @@ -15,6 +15,7 @@ > > > #include <irq_func.h> > > > #include <log.h> > > > #include <malloc.h> > > > +#include <net-common.h> > > > #include <pe.h> > > > #include <time.h> > > > #include <u-boot/crc.h> > > > @@ -2235,6 +2236,8 @@ static efi_status_t EFIAPI > > efi_exit_boot_services(efi_handle_t image_handle, > > > bootm_disable_interrupts(); > > > if (IS_ENABLED(CONFIG_USB_DEVICE)) > > > udc_disconnect(); > > > + if (IS_ENABLED(CONFIG_DM_ETH)) > > > + eth_halt(); > > > board_quiesce_devices(); > > > dm_remove_devices_active(); > > > } > > > -- > > > 2.48.1 > > > > > >
