diff mbox series

sandbox: Reduce keyed autoboot delay

Message ID 20210724211439.303802-1-sjg@chromium.org
State Accepted
Commit cb8970092f2cd5f1eaadf7d15d28cb905067e07f
Delegated to: Simon Glass
Headers show
Series sandbox: Reduce keyed autoboot delay | expand

Commit Message

Simon Glass July 24, 2021, 9:14 p.m. UTC
The autoboot tests are a recent addition to U-Boot, providing much-needed
coverage in this area.

A side effect of the keyed autoboot test is that this feature is enabled
in sandbox always. This changes the autoboot prompt and confuses the
pytests. Some tests become slower, for example the vboot tests take about
27s now instead of 3s.

We don't actually need this feature enabled to be able to run the tests.
Add a switch to allow sandbox to turn it on and off as needed. Use this
in the one test that needs it.

Add a command-line flag in case this is desired in normal use.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 25c8b9f298e ("test: add first autoboot unit tests")
---

 arch/sandbox/cpu/start.c         |  9 ++++++++
 arch/sandbox/cpu/state.c         | 18 ++++++++++++++++
 arch/sandbox/include/asm/state.h |  1 +
 common/autoboot.c                |  6 +++---
 include/autoboot.h               | 36 ++++++++++++++++++++++++++++++++
 test/common/test_autoboot.c      |  6 ++++++
 6 files changed, 73 insertions(+), 3 deletions(-)

Comments

Steffen Jaeckel July 25, 2021, 1:32 p.m. UTC | #1
Hi Simon,

On 7/24/21 11:14 PM, Simon Glass wrote:
> The autoboot tests are a recent addition to U-Boot, providing much-needed
> coverage in this area.
> 
> A side effect of the keyed autoboot test is that this feature is enabled
> in sandbox always. This changes the autoboot prompt and confuses the
> pytests. Some tests become slower, for example the vboot tests take about
> 27s now instead of 3s.
> 
> We don't actually need this feature enabled to be able to run the tests.
> Add a switch to allow sandbox to turn it on and off as needed. Use this
> in the one test that needs it.
> 
> Add a command-line flag in case this is desired in normal use.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 25c8b9f298e ("test: add first autoboot unit tests")

Reviewed-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>

Cheers
Steffen
Simon Glass July 31, 2021, 11:03 p.m. UTC | #2
Hi Simon,

On 7/24/21 11:14 PM, Simon Glass wrote:
> The autoboot tests are a recent addition to U-Boot, providing much-needed
> coverage in this area.
>
> A side effect of the keyed autoboot test is that this feature is enabled
> in sandbox always. This changes the autoboot prompt and confuses the
> pytests. Some tests become slower, for example the vboot tests take about
> 27s now instead of 3s.
>
> We don't actually need this feature enabled to be able to run the tests.
> Add a switch to allow sandbox to turn it on and off as needed. Use this
> in the one test that needs it.
>
> Add a command-line flag in case this is desired in normal use.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 25c8b9f298e ("test: add first autoboot unit tests")

Reviewed-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>

Cheers
Steffen

Applied to u-boot-dm, thanks!
diff mbox series

Patch

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 777db4e9522..a74f5ec7ba0 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -400,6 +400,15 @@  static int sandbox_cmdline_cb_signals(struct sandbox_state *state,
 SANDBOX_CMDLINE_OPT_SHORT(signals, 'S', 0,
 			  "Handle signals (such as SIGSEGV) in sandbox");
 
+static int sandbox_cmdline_cb_autoboot_keyed(struct sandbox_state *state,
+					     const char *arg)
+{
+	state->autoboot_keyed = true;
+
+	return 0;
+}
+SANDBOX_CMDLINE_OPT(autoboot_keyed, 0, "Allow keyed autoboot");
+
 static void setup_ram_buf(struct sandbox_state *state)
 {
 	/* Zero the RAM buffer if we didn't read it, to keep valgrind happy */
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index a4d99bade41..4e822538baf 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -4,6 +4,7 @@ 
  */
 
 #include <common.h>
+#include <autoboot.h>
 #include <bloblist.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -378,6 +379,23 @@  void state_reset_for_test(struct sandbox_state *state)
 	state->next_tag = state->ram_size;
 }
 
+bool autoboot_keyed(void)
+{
+	struct sandbox_state *state = state_get_current();
+
+	return IS_ENABLED(CONFIG_AUTOBOOT_KEYED) && state->autoboot_keyed;
+}
+
+bool autoboot_set_keyed(bool autoboot_keyed)
+{
+	struct sandbox_state *state = state_get_current();
+	bool old_val = state->autoboot_keyed;
+
+	state->autoboot_keyed = autoboot_keyed;
+
+	return old_val;
+}
+
 int state_init(void)
 {
 	state = &main_state;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 1c4c571e28d..10352a587e4 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -94,6 +94,7 @@  struct sandbox_state {
 	bool run_unittests;		/* Run unit tests */
 	const char *select_unittests;	/* Unit test to run */
 	bool handle_signals;		/* Handle signals within sandbox */
+	bool autoboot_keyed;		/* Use keyed-autoboot feature */
 
 	/* Pointer to information for each SPI bus/cs */
 	struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
diff --git a/common/autoboot.c b/common/autoboot.c
index 8b9e9aa8785..5bb2e190895 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -406,7 +406,7 @@  static int abortboot(int bootdelay)
 	int abort = 0;
 
 	if (bootdelay >= 0) {
-		if (IS_ENABLED(CONFIG_AUTOBOOT_KEYED))
+		if (autoboot_keyed())
 			abort = abortboot_key_sequence(bootdelay);
 		else
 			abort = abortboot_single_key(bootdelay);
@@ -481,7 +481,7 @@  void autoboot_command(const char *s)
 		bool lock;
 		int prev;
 
-		lock = IS_ENABLED(CONFIG_AUTOBOOT_KEYED) &&
+		lock = autoboot_keyed() &&
 			!IS_ENABLED(CONFIG_AUTOBOOT_KEYED_CTRLC);
 		if (lock)
 			prev = disable_ctrlc(1); /* disable Ctrl-C checking */
@@ -498,4 +498,4 @@  void autoboot_command(const char *s)
 		if (s)
 			run_command_list(s, -1, 0);
 	}
-}
\ No newline at end of file
+}
diff --git a/include/autoboot.h b/include/autoboot.h
index ac8157e5704..d6915dd0cc6 100644
--- a/include/autoboot.h
+++ b/include/autoboot.h
@@ -11,6 +11,42 @@ 
 #ifndef __AUTOBOOT_H
 #define __AUTOBOOT_H
 
+#include <stdbool.h>
+
+#ifdef CONFIG_SANDBOX
+
+/**
+ * autoboot_keyed() - check whether keyed autoboot should be used
+ *
+ * This is only implemented for sandbox since other platforms don't have a way
+ * of controlling the feature at runtime.
+ *
+ * @return true if enabled, false if not
+ */
+bool autoboot_keyed(void);
+
+/**
+ * autoboot_set_keyed() - set whether keyed autoboot should be used
+ *
+ * @autoboot_keyed: true to enable the feature, false to disable
+ * @return old value of the flag
+ */
+bool autoboot_set_keyed(bool autoboot_keyed);
+#else
+static inline bool autoboot_keyed(void)
+{
+	/* There is no runtime flag, so just use the CONFIG */
+	return IS_ENABLED(CONFIG_AUTOBOOT_KEYED);
+}
+
+static inline bool autoboot_set_keyed(bool autoboot_keyed)
+{
+	/* There is no runtime flag to set */
+	return false;
+}
+
+#endif
+
 #ifdef CONFIG_AUTOBOOT
 /**
  * bootdelay_process() - process the bootd delay
diff --git a/test/common/test_autoboot.c b/test/common/test_autoboot.c
index 6564ac70496..42a1e4ab1fa 100644
--- a/test/common/test_autoboot.c
+++ b/test/common/test_autoboot.c
@@ -16,13 +16,19 @@ 
 static int check_for_input(struct unit_test_state *uts, const char *in,
 			   bool correct)
 {
+	bool old_val;
 	/* The bootdelay is set to 1 second in test_autoboot() */
 	const char *autoboot_prompt =
 		"Enter password \"a\" in 1 seconds to stop autoboot";
 
 	console_record_reset_enable();
 	console_in_puts(in);
+
+	/* turn on keyed autoboot for the test, if possible */
+	old_val = autoboot_set_keyed(true);
 	autoboot_command("echo Autoboot password unlock not successful");
+	old_val = autoboot_set_keyed(old_val);
+
 	ut_assert_nextline(autoboot_prompt);
 	if (!correct)
 		ut_assert_nextline("Autoboot password unlock not successful");