From: Janne Grunau Date: Sat, 23 Nov 2024 21:44:05 +0000 (+0100) Subject: dm: Add dm_remove_devices_active() for ordered device removal X-Git-Url: http://git.dujemihanovic.xyz/img/%7B%7B%20.RelPermalink%20%7D%7D?a=commitdiff_plain;h=dabaa4ae32062cb3f3d995e5c63e6cef54ad079b;p=u-boot.git dm: Add dm_remove_devices_active() for ordered device removal This replaces dm_remove_devices_flags() calls in all boot implementations to ensure non vital devices are consistently removed first. All boot implementation except arch/arm/lib/bootm.c currently just call dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL). This can result in crashes when dependencies between devices exists. The driver model's design document describes DM_FLAG_VITAL as "indicates that the device is 'vital' to the operation of other devices". Device removal at boot should follow this. Instead of adding dm_remove_devices_flags() with (DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL) everywhere add dm_remove_devices_active() which does this. Fixes a NULL pointer deref in the apple dart IOMMU driver during EFI boot. The xhci-pci (driver which depends on the IOMMU to work) removes its mapping on removal. This explodes when the IOMMU device was removed first. dm_remove_devices_flags() is kept since it is used for testing of device_remove() calls in dm. Signed-off-by: Janne Grunau --- diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 192c120a7d..974cbfe840 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -73,11 +73,10 @@ static void announce_and_cleanup(int fake) * Call remove function of all devices with a removal flag set. * This may be useful for last-stage operations, like cancelling * of DMA operation or releasing device internal buffers. + * dm_remove_devices_active() ensures that vital devices are removed in + * a second round. */ - dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL); - - /* Remove all active vital devices next */ - dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); + dm_remove_devices_active(); cleanup_before_linux(); } diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c index 82502972ee..76c610bcee 100644 --- a/arch/riscv/lib/bootm.c +++ b/arch/riscv/lib/bootm.c @@ -57,7 +57,7 @@ static void announce_and_cleanup(int fake) * This may be useful for last-stage operations, like cancelling * of DMA operation or releasing device internal buffers. */ - dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); + dm_remove_devices_active(); cleanup_before_linux(); } diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c index 55f581836d..0f79a5d549 100644 --- a/arch/x86/lib/bootm.c +++ b/arch/x86/lib/bootm.c @@ -49,7 +49,7 @@ void bootm_announce_and_cleanup(void) * This may be useful for last-stage operations, like cancelling * of DMA operation or releasing device internal buffers. */ - dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); + dm_remove_devices_active(); } #if defined(CONFIG_OF_LIBFDT) && !defined(CONFIG_OF_NO_KERNEL) diff --git a/drivers/core/root.c b/drivers/core/root.c index 7a714f5478..c7fb58285c 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -147,6 +147,13 @@ int dm_remove_devices_flags(uint flags) return 0; } + +void dm_remove_devices_active(void) +{ + /* Remove non-vital devices first */ + device_remove(dm_root(), DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL); + device_remove(dm_root(), DM_REMOVE_ACTIVE_ALL); +} #endif int dm_scan_plat(bool pre_reloc_only) diff --git a/include/dm/root.h b/include/dm/root.h index b2f30a842f..5651b868c8 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -167,8 +167,18 @@ int dm_uninit(void); * Return: 0 if OK, -ve on error */ int dm_remove_devices_flags(uint flags); + +/** + * dm_remove_devices_active - Call remove function of all active drivers heeding + * device dependencies as far as know, i.e. removing + * devices marked with DM_FLAG_VITAL last. + * + * All active devices will be removed + */ +void dm_remove_devices_active(void); #else static inline int dm_remove_devices_flags(uint flags) { return 0; } +static inline void dm_remove_devices_active(void) { } #endif /** diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f52284b4c..080e7f78ae 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2234,7 +2234,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, if (IS_ENABLED(CONFIG_USB_DEVICE)) udc_disconnect(); board_quiesce_devices(); - dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); + dm_remove_devices_active(); } /* Patch out unsupported runtime function */ diff --git a/test/dm/core.c b/test/dm/core.c index 7371d3ff42..c59ffc6f61 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -999,6 +999,56 @@ static int dm_test_remove_vital(struct unit_test_state *uts) } DM_TEST(dm_test_remove_vital, 0); +/* Test removal of 'active' devices */ +static int dm_test_remove_active(struct unit_test_state *uts) +{ + struct udevice *normal, *dma, *vital, *dma_vital; + + /* Skip the behaviour in test_post_probe() */ + uts->skip_post_probe = 1; + + ut_assertok(device_bind_by_name(uts->root, false, &driver_info_manual, + &normal)); + ut_assertnonnull(normal); + + ut_assertok(device_bind_by_name(uts->root, false, &driver_info_act_dma, + &dma)); + ut_assertnonnull(dma); + + ut_assertok(device_bind_by_name(uts->root, false, + &driver_info_vital_clk, &vital)); + ut_assertnonnull(vital); + + ut_assertok(device_bind_by_name(uts->root, false, + &driver_info_act_dma_vital_clk, + &dma_vital)); + ut_assertnonnull(dma_vital); + + /* Probe the devices */ + ut_assertok(device_probe(normal)); + ut_assertok(device_probe(dma)); + ut_assertok(device_probe(vital)); + ut_assertok(device_probe(dma_vital)); + + /* Check that devices are active right now */ + ut_asserteq(true, device_active(normal)); + ut_asserteq(true, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(true, device_active(dma_vital)); + + /* Remove active devices in an ordered way */ + dm_remove_devices_active(); + + /* Check that all devices are inactive right now */ + ut_asserteq(true, device_active(normal)); + ut_asserteq(false, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(false, device_active(dma_vital)); + + return 0; +} +DM_TEST(dm_test_remove_active, 0); + static int dm_test_uclass_before_ready(struct unit_test_state *uts) { struct uclass *uc;