dm: usb: Deal with USB keyboard persisting across tests

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>

diff --git a/common/console.c b/common/console.c
index 09d4b51f675e286a47535440f370d90ccbb19e2f..0a5bf6cc4fb8a837f3e62f335005033c58950532 100644 (file)
--- a/common/console.c
+++ b/common/console.c
@@ -1243,3 +1243,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 1d9845b01ede6814ec35f746e3c97f4cc2a83994..bbfee23bc2698d989e3d49efcc0f452f502e6a07 100644 (file)
--- 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 6b6d0f9de736175d119689cca0ad8924e8240166..57fdb0834c14ac915e4c6348491463772a15f03f 100644 (file)
--- 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 e37f853482cc0117b8e7cddb88932bbfd11094a1..be37ed272e15438825eee135c207d4bce243959c 100644 (file)
--- 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 63e8be0ccd17980d23414320f4f432133a531536..b3d3e24cdcea9777731736a02e2c48420fbda5df 100644 (file)
--- 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 & UTF_DM)
                ut_assertok(dm_test_pre_run(uts));