Clear any USB-keyboard devices before running a unit test, to avoid using a stale udevice pointer in stdio. Add a long comment to explain this situation and why this solution seems best, at least for now.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/console.c | 34 ++++++++++++++++++++++++++++++++++ common/usb_kbd.c | 5 +++++ include/console.h | 8 ++++++++ include/usb.h | 12 ++++++++++++ test/test-main.c | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+)
diff --git a/common/console.c b/common/console.c index 034942ec4a9..17dceba40b8 100644 --- a/common/console.c +++ b/common/console.c @@ -1241,3 +1241,37 @@ int console_init_r(void) }
#endif /* CONFIG_IS_ENABLED(SYS_CONSOLE_IS_IN_ENV) */ + +int console_remove_by_name(const char *name) +{ + int err = 0; + +#if CONFIG_IS_ENABLED(CONSOLE_MUX) + int fnum; + + log_debug("removing console device %s\n", name); + for (fnum = 0; fnum < MAX_FILES; fnum++) { + struct stdio_dev **src, **dest; + int i; + + log_debug("file %d: %d devices: ", fnum, cd_count[fnum]); + src = console_devices[fnum]; + dest = src; + for (i = 0; i < cd_count[fnum]; i++, src++) { + struct stdio_dev *sdev = *src; + int ret = 0; + + if (!strcmp(sdev->name, name)) + ret = stdio_deregister_dev(sdev, true); + else + *dest++ = *src; + if (ret && !err) + err = ret; + } + cd_count[fnum] = dest - console_devices[fnum]; + log_debug("now %d\n", cd_count[fnum]); + } +#endif /* CONSOLE_MUX */ + + return err; +} diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 1d9845b01ed..bbfee23bc26 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -137,6 +137,11 @@ extern int __maybe_unused net_busy_flag; /* The period of time between two calls of usb_kbd_testc(). */ static unsigned long kbd_testc_tms;
+int usb_kbd_remove_for_test(void) +{ + return console_remove_by_name(DEVNAME); +} + /* Puts character in the queue and sets up the in and out pointer. */ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c) { diff --git a/include/console.h b/include/console.h index 2617e160073..125f498facb 100644 --- a/include/console.h +++ b/include/console.h @@ -179,6 +179,14 @@ void console_puts_select_stderr(bool serial_only, const char *s); */ int console_clear(void);
+/** + * console_remove_by_name() - Remove a console by its stdio name + * + * This must only be used in tests. It removes any use of the named stdio device + * from the console tables. + */ +int console_remove_by_name(const char *name); + /* * CONSOLE multiplexing. */ diff --git a/include/usb.h b/include/usb.h index e37f853482c..be37ed272e1 100644 --- a/include/usb.h +++ b/include/usb.h @@ -1092,4 +1092,16 @@ struct usb_generic_descriptor **usb_emul_find_descriptor( */ void usb_show_tree(void);
+/** + * usb_kbd_remove_for_test() - Remove any USB keyboard + * + * This can only be called from test_pre_run(). It removes the USB keyboard from + * the console system so that the USB device can be dropped + */ +#if CONFIG_IS_ENABLED(USB_KEYBOARD) +int usb_kbd_remove_for_test(void); +#else +static inline int usb_kbd_remove_for_test(void) { return 0; } +#endif + #endif /*_USB_H_ */ diff --git a/test/test-main.c b/test/test-main.c index 3fa6f6e32ec..bfe9f707dc1 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -12,6 +12,7 @@ #include <net.h> #include <of_live.h> #include <os.h> +#include <usb.h> #include <dm/ofnode.h> #include <dm/root.h> #include <dm/test.h> @@ -289,6 +290,43 @@ static int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { ut_assertok(event_init());
+ /* + * Remove any USB keyboard, so that we can add and remove USB devices + * in tests. + * + * For UT_TESTF_DM tests, the old driver model state is saved and + * restored across each test. Within in each test there is therefore a + * new driver model state, which means that any USB keyboard device in + * stdio points to the old state. + * + * This is fine in most cases. But if a non-UT_TESTF_DM test starts up + * USB (thus creating a stdio record pointing to the USB keyboard + * device) then when the test finishes, the new driver model state is + * freed, meaning that there is now a stale pointer in stdio. + * + * This means that any future UT_TESTF_DM test which uses stdin will + * cause the console system to call tstc() on the stale device pointer, + * causing a crash. + * + * We don't want to fix this by enabling UT_TESTF_DM for all tests as + * this causes other problems. For example, bootflow_efi relies on + * U-Boot going through a proper init - without that we don't have the + * TCG measurement working and get an error + * 'tcg2 measurement fails(0x8000000000000007)'. Once we tidy up how EFI + * runs tests (e.g. get rid of all the restarting of U-Boot) we could + * potentially make the bootstd tests set UT_TESTF_DM, but other tests + * might do the same thing. + * + * We could add a test flag to declare that USB is being used, but that + * seems unnecessary, at least for now. We could detect USB being used + * in a test, but there is no obvious drawback to clearing out stale + * pointers always. + * + * So just remove any USB keyboards from the console tables. This allows + * UT_TESTF_DM and non-UT_TESTF_DM tests to coexist happily. + */ + usb_kbd_remove_for_test(); + if (test->flags & UT_TESTF_DM) ut_assertok(dm_test_pre_run(uts));