[PATCH v3 0/8] sandbox: implement cold reset

[PATCH v3 0/8] sandbox: implement cold reset

[Heinrich Schuchardt]

The command to shut down a device is 'poweroff'. It is a deficit of the
sandbox that it does not support resetting yet but shuts down upong seeing
the 'reset' command.

The patch series implements the cold reset function as a relaunch of the
U-Boot binary via execv().

Unit tests are adjusted.

A bug leading to closing the console input file descriptor is resolved.

Add some of the sandbox API to the HTML documentation.

v3:
	update commit message for patch 5
v2:
	use O_CLOEXEC in os_open()
	avoid longjmp()
	add more comments
	add the os.h to the HTML documentation

Heinrich Schuchardt (8):
  sandbox: eth-raw: do not close the console input
  sandbox: enable poweroff command
  test/py: test poweroff
  sandbox: use O_CLOEXEC in os_open()
  sandbox: implement reset
  test: adjust sysreset tests
  sandbox: update function descriptions in os.h
  doc: add sandbox API

 arch/Kconfig                              |   3 +-
 arch/sandbox/cpu/eth-raw-os.c             |   8 +-
 arch/sandbox/cpu/os.c                     |  16 +-
 arch/sandbox/cpu/start.c                  |  26 +++
 arch/sandbox/cpu/state.c                  |   1 +
 arch/sandbox/include/asm/u-boot-sandbox.h |  10 +
 doc/api/index.rst                         |   1 +
 doc/api/sandbox.rst                       |   9 +
 drivers/sysreset/sysreset_sandbox.c       |   3 +
 include/os.h                              | 236 +++++++++++++---------
 test/dm/sysreset.c                        |  11 +-
 test/py/tests/test_sandbox_exit.py        |   8 +-
 12 files changed, 227 insertions(+), 105 deletions(-)
 create mode 100644 doc/api/sandbox.rst

--
2.28.0

[PATCH v3 1/8] sandbox: eth-raw: do not close the console input

[Heinrich Schuchardt]

When the sandbox eth-raw device host_lo is removed this leads to closing
the console input.

Do not call close(0).

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 arch/sandbox/cpu/eth-raw-os.c | 8 ++++----
 arch/sandbox/cpu/os.c         | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/sandbox/cpu/eth-raw-os.c b/arch/sandbox/cpu/eth-raw-os.c
index da01d1addf..6a8d809756 100644
--- a/arch/sandbox/cpu/eth-raw-os.c
+++ b/arch/sandbox/cpu/eth-raw-os.c
@@ -53,7 +53,7 @@ int sandbox_eth_raw_os_is_local(const char *ifname)
 	}
 	ret = !!(ifr.ifr_flags & IFF_LOOPBACK);
 out:
-	close(fd);
+	os_close(fd);
 	return ret;
 }

@@ -220,7 +220,7 @@ int sandbox_eth_raw_os_send(void *packet, int length,
 		struct sockaddr_in addr;

 		if (priv->local_bind_sd != -1)
-			close(priv->local_bind_sd);
+			os_close(priv->local_bind_sd);

 		/* A normal UDP socket is required to bind */
 		priv->local_bind_sd = socket(AF_INET, SOCK_DGRAM, 0);
@@ -284,11 +284,11 @@ void sandbox_eth_raw_os_stop(struct eth_sandbox_raw_priv *priv)
 {
 	free(priv->device);
 	priv->device = NULL;
-	close(priv->sd);
+	os_close(priv->sd);
 	priv->sd = -1;
 	if (priv->local) {
 		if (priv->local_bind_sd != -1)
-			close(priv->local_bind_sd);
+			os_close(priv->local_bind_sd);
 		priv->local_bind_sd = -1;
 		priv->local_bind_udp_port = 0;
 	}
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index e7ec892bdf..c461fb0db0 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -86,7 +86,10 @@ int os_open(const char *pathname, int os_flags)

 int os_close(int fd)
 {
-	return close(fd);
+	/* Do not close the console input */
+	if (fd)
+		return close(fd);
+	return -1;
 }

 int os_unlink(const char *pathname)
--
2.28.0

[PATCH v3 2/8] sandbox: enable poweroff command

[Heinrich Schuchardt]

The command to shut down a device is 'poweroff'. It is a deficit of the
sandbox that it does not support resetting yet but shuts down upong seeing
the 'reset' command.

Once the sandbox properly supports reset we need the 'poweroff' command to
leave the sandbox.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 arch/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 683e384319..63e9d725b7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -87,6 +87,7 @@ config SANDBOX
 	bool "Sandbox"
 	select BOARD_LATE_INIT
 	select BZIP2
+	select CMD_POWEROFF
 	select DM
 	select DM_GPIO
 	select DM_I2C
@@ -102,7 +103,7 @@ config SANDBOX
 	select PCI_ENDPOINT
 	select SPI
 	select SUPPORT_OF_CONTROL
-	select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
+	select SYSRESET_CMD_POWEROFF
 	imply BITREVERSE
 	select BLOBLIST
 	imply CMD_DM
--
2.28.0

[PATCH v3 3/8] test/py: test poweroff

[Heinrich Schuchardt]

It is the 'poweroff' and not the 'reset' command that should shut down the
sandbox.

Adjust the unit test accordingly

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 test/py/tests/test_sandbox_exit.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/py/tests/test_sandbox_exit.py b/test/py/tests/test_sandbox_exit.py
index a301f4b559..2d242ae0f6 100644
--- a/test/py/tests/test_sandbox_exit.py
+++ b/test/py/tests/test_sandbox_exit.py
@@ -6,11 +6,11 @@ import pytest
 import signal

 @pytest.mark.boardspec('sandbox')
- at pytest.mark.buildconfigspec('sysreset')
-def test_reset(u_boot_console):
-    """Test that the "reset" command exits sandbox process."""
+ at pytest.mark.buildconfigspec('sysreset_cmd_poweroff')
+def test_poweroff(u_boot_console):
+    """Test that the "poweroff" command exits sandbox process."""

-    u_boot_console.run_command('reset', wait_for_prompt=False)
+    u_boot_console.run_command('poweroff', wait_for_prompt=False)
     assert(u_boot_console.validate_exited())

 @pytest.mark.boardspec('sandbox')
--
2.28.0

[PATCH v3 4/8] sandbox: use O_CLOEXEC in os_open()

[Heinrich Schuchardt]

During a cold reset execv() is used to relaunch the U-Boot binary.
We must ensure that all files are closed in this case.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v3:
	no change
v2:
	new patch
---
 arch/sandbox/cpu/os.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index c461fb0db0..7e474d6364 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -80,6 +80,11 @@ int os_open(const char *pathname, int os_flags)
 		flags |= O_CREAT;
 	if (os_flags & OS_O_TRUNC)
 		flags |= O_TRUNC;
+	/*
+	 * During a cold reset execv() is used to relaunch the U-Boot binary.
+	 * We must ensure that all files are closed in this case.
+	 */
+	flags |= O_CLOEXEC;

 	return open(pathname, flags, 0777);
 }
--
2.28.0

[PATCH v3 5/8] sandbox: implement reset

[Heinrich Schuchardt]

Up to now the sandbox would shutdown upon a cold reset request. Instead it
should be reset.

In our coding we use static variables like LIST_HEAD(efi_obj_list). A reset
can occur at any time, e.g. via an UEFI binary calling the reset service.
The only safe way to return to an initial state is to relaunch the U-Boot
binary.

The reset implementation uses execv() to relaunch U-Boot.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v3:
	update commit message
v2:
	avoid longjmp()
	add more comments
---
 arch/sandbox/cpu/os.c                     |  6 ++++++
 arch/sandbox/cpu/start.c                  | 26 +++++++++++++++++++++++
 arch/sandbox/cpu/state.c                  |  1 +
 arch/sandbox/include/asm/u-boot-sandbox.h | 10 +++++++++
 drivers/sysreset/sysreset_sandbox.c       |  3 +++
 include/os.h                              | 15 +++++++++++++
 6 files changed, 61 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 7e474d6364..0d8efd83f6 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -822,3 +822,9 @@ void *os_find_text_base(void)

 	return base;
 }
+
+void os_relaunch(char *argv[])
+{
+	execv(argv[0], argv);
+	os_exit(1);
+}
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index c6a2bbe468..cc47b28d1d 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -5,6 +5,7 @@

 #include <common.h>
 #include <command.h>
+#include <dm/root.h>
 #include <errno.h>
 #include <init.h>
 #include <os.h>
@@ -19,6 +20,8 @@

 DECLARE_GLOBAL_DATA_PTR;

+static char **os_argv;
+
 /* Compare two options so that they can be sorted into alphabetical order */
 static int h_compare_opt(const void *p1, const void *p2)
 {
@@ -394,12 +397,35 @@ void state_show(struct sandbox_state *state)
 	printf("\n");
 }

+void sandbox_reset(void)
+{
+	/* Do this here while it still has an effect */
+	os_fd_restore();
+	if (state_uninit())
+		os_exit(2);
+
+	if (dm_uninit())
+		os_exit(2);
+
+	/* Restart U-Boot */
+	os_relaunch(os_argv);
+}
+
 int main(int argc, char *argv[])
 {
 	struct sandbox_state *state;
 	gd_t data;
 	int ret;

+	/*
+	 * Copy argv[] so that we can pass the arguments in the original
+	 * sequence when resetting the sandbox.
+	 */
+	os_argv = calloc(argc + 1, sizeof(char *));
+	if (!os_argv)
+		os_exit(1);
+	memcpy(os_argv, argv, sizeof(char *) * (argc + 1));
+
 	memset(&data, '\0', sizeof(data));
 	gd = &data;
 	gd->arch.text_base = os_find_text_base();
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index 34b6fff7e7..59f37fab0b 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -358,6 +358,7 @@ void state_reset_for_test(struct sandbox_state *state)
 	/* No reset yet, so mark it as such. Always allow power reset */
 	state->last_sysreset = SYSRESET_COUNT;
 	state->sysreset_allowed[SYSRESET_POWER_OFF] = true;
+	state->sysreset_allowed[SYSRESET_COLD] = true;
 	state->allow_memio = false;

 	memset(&state->wdt, '\0', sizeof(state->wdt));
diff --git a/arch/sandbox/include/asm/u-boot-sandbox.h b/arch/sandbox/include/asm/u-boot-sandbox.h
index 798d003077..73b1897191 100644
--- a/arch/sandbox/include/asm/u-boot-sandbox.h
+++ b/arch/sandbox/include/asm/u-boot-sandbox.h
@@ -84,6 +84,16 @@ void sandbox_set_enable_pci_map(int enable);
  */
 int sandbox_read_fdt_from_file(void);

+/**
+ * sandbox_reset() - reset sandbox
+ *
+ * This functions implements the cold reboot of the sandbox. It relaunches the
+ * U-Boot binary with the same command line parameters as the original call.
+ * The PID of the process stays the same. All file descriptors that have not
+ * been opened with O_CLOEXEC stay open including stdin, stdout, stderr.
+ */
+void sandbox_reset(void);
+
 /* Exit sandbox (quit U-Boot) */
 void sandbox_exit(void);

diff --git a/drivers/sysreset/sysreset_sandbox.c b/drivers/sysreset/sysreset_sandbox.c
index 69c22a7000..c92132798c 100644
--- a/drivers/sysreset/sysreset_sandbox.c
+++ b/drivers/sysreset/sysreset_sandbox.c
@@ -56,6 +56,9 @@ static int sandbox_sysreset_request(struct udevice *dev, enum sysreset_t type)
 	switch (type) {
 	case SYSRESET_COLD:
 		state->last_sysreset = type;
+		if (!state->sysreset_allowed[type])
+			return -EACCES;
+		sandbox_reset();
 		break;
 	case SYSRESET_POWER_OFF:
 		state->last_sysreset = type;
diff --git a/include/os.h b/include/os.h
index 1874ae674f..88dfb71c1a 100644
--- a/include/os.h
+++ b/include/os.h
@@ -355,4 +355,19 @@ int os_read_file(const char *name, void **bufp, int *sizep);
  */
 void *os_find_text_base(void);

+/**
+ * os_relaunch() - restart the sandbox
+ *
+ * This functions is used to implement the cold reboot of the sand box.
+ * @argv[0] specifies the binary that is started while the calling process
+ * stops immediately. If the new binary cannot be started, the process is
+ * terminated and 1 is set as shell return code.
+ *
+ * The PID of the process stays the same. All file descriptors that have not
+ * been opened with O_CLOEXEC stay open including stdin, stdout, stderr.
+ *
+ * @argv:	NULL terminated list of command line parameters
+ */
+void os_relaunch(char *argv[]);
+
 #endif
--
2.28.0

[PATCH v3 6/8] test: adjust sysreset tests

[Heinrich Schuchardt]

As we have a working COLD_RESET on the sandbox the sysreset test has to be
adjusted.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 test/dm/sysreset.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/dm/sysreset.c b/test/dm/sysreset.c
index aec97b1cbb..691683c567 100644
--- a/test/dm/sysreset.c
+++ b/test/dm/sysreset.c
@@ -37,7 +37,9 @@ static int dm_test_sysreset_base(struct unit_test_state *uts)
 	/* Device 2 is the cold sysreset device */
 	ut_assertok(uclass_get_device(UCLASS_SYSRESET, 2, &dev));
 	ut_asserteq(-ENOSYS, sysreset_request(dev, SYSRESET_WARM));
+	state->sysreset_allowed[SYSRESET_COLD] = false;
 	ut_asserteq(-EACCES, sysreset_request(dev, SYSRESET_COLD));
+	state->sysreset_allowed[SYSRESET_COLD] = true;
 	state->sysreset_allowed[SYSRESET_POWER] = false;
 	ut_asserteq(-EACCES, sysreset_request(dev, SYSRESET_POWER));
 	state->sysreset_allowed[SYSRESET_POWER] = true;
@@ -71,22 +73,25 @@ static int dm_test_sysreset_walk(struct unit_test_state *uts)
 	struct sandbox_state *state = state_get_current();

 	/* If we generate a power sysreset, we will exit sandbox! */
+	state->sysreset_allowed[SYSRESET_WARM] = false;
+	state->sysreset_allowed[SYSRESET_COLD] = false;
 	state->sysreset_allowed[SYSRESET_POWER] = false;
 	state->sysreset_allowed[SYSRESET_POWER_OFF] = false;
 	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_WARM));
 	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_COLD));
 	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_POWER));
+	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_POWER_OFF));

 	/*
 	 * Enable cold system reset - this should make cold system reset work,
 	 * plus a warm system reset should be promoted to cold, since this is
 	 * the next step along.
 	 */
-	state->sysreset_allowed[SYSRESET_COLD] = true;
+	state->sysreset_allowed[SYSRESET_WARM] = true;
 	ut_asserteq(-EINPROGRESS, sysreset_walk(SYSRESET_WARM));
-	ut_asserteq(-EINPROGRESS, sysreset_walk(SYSRESET_COLD));
+	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_COLD));
 	ut_asserteq(-EACCES, sysreset_walk(SYSRESET_POWER));
-	state->sysreset_allowed[SYSRESET_COLD] = false;
+	state->sysreset_allowed[SYSRESET_COLD] = true;
 	state->sysreset_allowed[SYSRESET_POWER] = true;

 	return 0;
--
2.28.0

[PATCH v3 7/8] sandbox: update function descriptions in os.h

[Heinrich Schuchardt]

Use Sphinx style function descriptions.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v3:
	no change
v2:
	new patch
---
 include/os.h | 223 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 130 insertions(+), 93 deletions(-)

diff --git a/include/os.h b/include/os.h
index 88dfb71c1a..1fe44f3510 100644
--- a/include/os.h
+++ b/include/os.h
@@ -19,30 +19,30 @@ struct sandbox_state;
 /**
  * Access to the OS read() system call
  *
- * \param fd	File descriptor as returned by os_open()
- * \param buf	Buffer to place data
- * \param count	Number of bytes to read
- * \return number of bytes read, or -1 on error
+ * @fd:		File descriptor as returned by os_open()
+ * @buf:	Buffer to place data
+ * @count:	Number of bytes to read
+ * Return:	number of bytes read, or -1 on error
  */
 ssize_t os_read(int fd, void *buf, size_t count);

 /**
  * Access to the OS write() system call
  *
- * \param fd	File descriptor as returned by os_open()
- * \param buf	Buffer containing data to write
- * \param count	Number of bytes to write
- * \return number of bytes written, or -1 on error
+ * @fd:		File descriptor as returned by os_open()
+ * @buf:	Buffer containing data to write
+ * @count:	Number of bytes to write
+ * Return:	number of bytes written, or -1 on error
  */
 ssize_t os_write(int fd, const void *buf, size_t count);

 /**
  * Access to the OS lseek() system call
  *
- * \param fd	File descriptor as returned by os_open()
- * \param offset	File offset (based on whence)
- * \param whence	Position offset is relative to (see below)
- * \return new file offset
+ * @fd:		File descriptor as returned by os_open()
+ * @offset:	File offset (based on whence)
+ * @whence:	Position offset is relative to (see below)
+ * Return:	new file offset
  */
 off_t os_lseek(int fd, off_t offset, int whence);

@@ -54,9 +54,9 @@ off_t os_lseek(int fd, off_t offset, int whence);
 /**
  * Access to the OS open() system call
  *
- * \param pathname	Pathname of file to open
- * \param flags		Flags, like OS_O_RDONLY, OS_O_RDWR
- * \return file descriptor, or -1 on error
+ * @pathname:	Pathname of file to open
+ * @flags:	Flags, like OS_O_RDONLY, OS_O_RDWR
+ * Return:	file descriptor, or -1 on error
  */
 int os_open(const char *pathname, int flags);

@@ -68,42 +68,42 @@ int os_open(const char *pathname, int flags);
 #define OS_O_TRUNC	01000

 /**
- * Access to the OS close() system call
+ * os_close() - access to the OS close() system call
  *
- * \param fd	File descriptor to close
- * \return 0 on success, -1 on error
+ * @fd:		File descriptor to close
+ * Return:	0 on success, -1 on error
  */
 int os_close(int fd);

 /**
- * Access to the OS unlink() system call
+ * os_unlink() - access to the OS unlink() system call
  *
- * \param pathname Path of file to delete
- * \return 0 for success, other for error
+ * @pathname:	Path of file to delete
+ * Return:	0 for success, other for error
  */
 int os_unlink(const char *pathname);

 /**
- * Access to the OS exit() system call
+ * os_exit() - access to the OS exit() system call
  *
  * This exits with the supplied return code, which should be 0 to indicate
  * success.
  *
- * @param exit_code	exit code for U-Boot
+ * @exit_code:	exit code for U-Boot
  */
 void os_exit(int exit_code) __attribute__((noreturn));

 /**
- * Put tty into raw mode to mimic serial console better
+ * os_tty_raw() - put tty into raw mode to mimic serial console better
  *
- * @param fd		File descriptor of stdin (normally 0)
- * @param allow_sigs	Allow Ctrl-C, Ctrl-Z to generate signals rather than
- *			be handled by U-Boot
+ * @fd:		File descriptor of stdin (normally 0)
+ * @allow_sigs:	Allow Ctrl-C, Ctrl-Z to generate signals rather than
+ *		be handled by U-Boot
  */
 void os_tty_raw(int fd, bool allow_sigs);

 /**
- * Restore the tty to its original mode
+ * os_fs_restore() - restore the tty to its original mode
  *
  * Call this to restore the original terminal mode, after it has been changed
  * by os_tty_raw(). This is an internal function.
@@ -111,144 +111,180 @@ void os_tty_raw(int fd, bool allow_sigs);
 void os_fd_restore(void);

 /**
- * Acquires some memory from the underlying os.
+ * os_malloc() - aquires some memory from the underlying os.
  *
- * \param length	Number of bytes to be allocated
- * \return Pointer to length bytes or NULL on error
+ * @length:	Number of bytes to be allocated
+ * Return:	Pointer to length bytes or NULL on error
  */
 void *os_malloc(size_t length);

 /**
- * Free memory previous allocated with os_malloc()
+ * os_free() - free memory previous allocated with os_malloc()
  *
  * This returns the memory to the OS.
  *
- * \param ptr		Pointer to memory block to free
+ * @ptr:	Pointer to memory block to free
  */
 void os_free(void *ptr);

 /**
- * Access to the usleep function of the os
+ * os_usleep() - access to the usleep function of the os
  *
- * \param usec Time to sleep in micro seconds
+ * @usec:	time to sleep in micro seconds
  */
 void os_usleep(unsigned long usec);

 /**
  * Gets a monotonic increasing number of nano seconds from the OS
  *
- * \return A monotonic increasing time scaled in nano seconds
+ * Return:	a monotonic increasing time scaled in nano seconds
  */
 uint64_t os_get_nsec(void);

 /**
  * Parse arguments and update sandbox state.
  *
- * @param state		Sandbox state to update
- * @param argc		Argument count
- * @param argv		Argument vector
- * @return 0 if ok, and program should continue;
- *	1 if ok, but program should stop;
- *	-1 on error: program should terminate.
+ * @state:	sandbox state to update
+ * @argc:	argument count
+ * @argv:	argument vector
+ * Return:
+ * *  0 if ok, and program should continue
+ * *  1 if ok, but program should stop
+ * * -1 on error: program should terminate
  */
 int os_parse_args(struct sandbox_state *state, int argc, char *argv[]);

 /*
+ * enum os_dirent_t - type of directory entry
+ *
  * Types of directory entry that we support. See also os_dirent_typename in
  * the C file.
  */
 enum os_dirent_t {
-	OS_FILET_REG,		/* Regular file */
-	OS_FILET_LNK,		/* Symbolic link */
-	OS_FILET_DIR,		/* Directory */
-	OS_FILET_UNKNOWN,	/* Something else */
-
+	/**
+	 * @OS_FILET_REG:	regular file
+	 */
+	OS_FILET_REG,
+	/**
+	 * @OS_FILET_LNK:	symbolic link
+	 */
+	OS_FILET_LNK,
+	/**
+	 * @OS_FILET_DIR:	directory
+	 */
+	OS_FILET_DIR,
+	/**
+	 * @OS_FILET_UNKNOWN:	something else
+	 */
+	OS_FILET_UNKNOWN,
+	/**
+	 * @OS_FILET_COUNT:	number of directory entry types
+	 */
 	OS_FILET_COUNT,
 };

-/** A directory entry node, containing information about a single dirent */
+/**
+ * struct os_dirent_node - directory node
+ *
+ * A directory entry node, containing information about a single dirent
+ *
+ */
 struct os_dirent_node {
-	struct os_dirent_node *next;	/* Pointer to next node, or NULL */
-	ulong size;			/* Size of file in bytes */
-	enum os_dirent_t type;		/* Type of entry */
-	char name[0];			/* Name of entry */
+	/**
+	 * @next:	pointer to next node, or NULL
+	 */
+	struct os_dirent_node *next;
+	/**
+	 * @size:	size of file in bytes
+	 */
+	ulong size;
+	/**
+	 * @type:	type of entry
+	 */
+	enum os_dirent_t type;
+	/**
+	 * @name:	name of entry
+	 */
+	char name[0];
 };

 /**
- * Get a directionry listing
+ * os_dirent_ls() - get a directory listing
  *
  * This allocates and returns a linked list containing the directory listing.
  *
- * @param dirname	Directory to examine
- * @param headp		Returns pointer to head of linked list, or NULL if none
- * @return 0 if ok, -ve on error
+ * @dirname:	directory to examine
+ * @headp:	on return pointer to head of linked list, or NULL if none
+ * Return:	0 if ok, -ve on error
  */
 int os_dirent_ls(const char *dirname, struct os_dirent_node **headp);

 /**
- * Free directory list
+ * os_dirent_free() - free directory list
  *
  * This frees a linked list containing a directory listing.
  *
- * @param node		Pointer to head of linked list
+ * @node:	pointer to head of linked list
  */
 void os_dirent_free(struct os_dirent_node *node);

 /**
- * Get the name of a directory entry type
+ * os_dirent_get_typename() - get the name of a directory entry type
  *
- * @param type		Type to check
- * @return string containing the name of that type, or "???" if none/invalid
+ * @type:	type to check
+ * Return:
+ * string containing the name of that type,
+ * or "???" if none/invalid
  */
 const char *os_dirent_get_typename(enum os_dirent_t type);

 /**
- * Get the size of a file
+ * os_get_filesize() - get the size of a file
  *
- * @param fname		Filename to check
- * @param size		size of file is returned if no error
- * @return 0 on success or -1 if an error ocurred
+ * @fname:	filename to check
+ * @size:	size of file is returned if no error
+ * Return:	0 on success or -1 if an error ocurred
  */
 int os_get_filesize(const char *fname, loff_t *size);

 /**
- * Write a character to the controlling OS terminal
+ * os_putc() - write a character to the controlling OS terminal
  *
  * This bypasses the U-Boot console support and writes directly to the OS
  * stdout file descriptor.
  *
- * @param ch	Character to write
+ * @ch:		haracter to write
  */
 void os_putc(int ch);

 /**
- * Write a string to the controlling OS terminal
+ * os_puts() - write a string to the controlling OS terminal
  *
  * This bypasses the U-Boot console support and writes directly to the OS
  * stdout file descriptor.
  *
- * @param str	String to write (note that \n is not appended)
+ * @str:	string to write (note that \n is not appended)
  */
 void os_puts(const char *str);

 /**
- * Write the sandbox RAM buffer to a existing file
+ * os_write_ram_buf() - write the sandbox RAM buffer to a existing file
  *
- * @param fname		Filename to write memory to (simple binary format)
- * @return 0 if OK, -ve on error
+ * @fname:	filename to write memory to (simple binary format)
+ * Return:	0 if OK, -ve on error
  */
 int os_write_ram_buf(const char *fname);

 /**
- * Read the sandbox RAM buffer from an existing file
+ * os_read_ram_buf() - read the sandbox RAM buffer from an existing file
  *
- * @param fname		Filename containing memory (simple binary format)
- * @return 0 if OK, -ve on error
+ * @fname:	filename containing memory (simple binary format)
+ * Return:	0 if OK, -ve on error
  */
 int os_read_ram_buf(const char *fname);

 /**
- * Jump to a new executable image
+ * os_jump_to_image() - jump to a new executable image
  *
  * This uses exec() to run a new executable image, after putting it in a
  * temporary file. The same arguments and environment are passed to this
@@ -261,22 +297,23 @@ int os_read_ram_buf(const char *fname);
  *			have access to this. It also means that the original
  *			memory filename passed to U-Boot will be left intact.
  *
- * @param dest		Buffer containing executable image
- * @param size		Size of buffer
+ * @dest:	buffer containing executable image
+ * @size:	size of buffer
+ * Return:	0 if OK, -ve on error
  */
 int os_jump_to_image(const void *dest, int size);

 /**
- * os_find_u_boot() - Determine the path to U-Boot proper
+ * os_find_u_boot() - determine the path to U-Boot proper
  *
  * This function is intended to be called from within sandbox SPL. It uses
  * a few heuristics to find U-Boot proper. Normally it is either in the same
  * directory, or the directory above (since u-boot-spl is normally in an
  * spl/ subdirectory when built).
  *
- * @fname:	Place to put full path to U-Boot
- * @maxlen:	Maximum size of @fname
- * @return 0 if OK, -NOSPC if the filename is too large, -ENOENT if not found
+ * @fname:	place to put full path to U-Boot
+ * @maxlen:	maximum size of @fname
+ * Return:	0 if OK, -NOSPC if the filename is too large, -ENOENT if not found
  */
 int os_find_u_boot(char *fname, int maxlen);

@@ -286,23 +323,23 @@ int os_find_u_boot(char *fname, int maxlen);
  * When called from SPL, this runs U-Boot proper. The filename is obtained by
  * calling os_find_u_boot().
  *
- * @fname:	Full pathname to U-Boot executable
- * @return 0 if OK, -ve on error
+ * @fname:	full pathname to U-Boot executable
+ * Return:	0 if OK, -ve on error
  */
 int os_spl_to_uboot(const char *fname);

 /**
- * Read the current system time
+ * os_localtime() - read the current system time
  *
  * This reads the current Local Time and places it into the provided
  * structure.
  *
- * @param rt		Place to put system time
+ * @rt:		place to put system time
  */
 void os_localtime(struct rtc_time *rt);

 /**
- * os_abort() - Raise SIGABRT to exit sandbox (e.g. to debugger)
+ * os_abort() - raise SIGABRT to exit sandbox (e.g. to debugger)
  */
 void os_abort(void);

@@ -313,12 +350,12 @@ void os_abort(void);
  *
  * @start:	Region start
  * @len:	Region length in bytes
- * @return 0 if OK, -1 on error from mprotect()
+ * Return:	0 if OK, -1 on error from mprotect()
  */
 int os_mprotect_allow(void *start, size_t len);

 /**
- * os_write_file() - Write a file to the host filesystem
+ * os_write_file() - write a file to the host filesystem
  *
  * This can be useful when debugging for writing data out of sandbox for
  * inspection by external tools.
@@ -326,7 +363,7 @@ int os_mprotect_allow(void *start, size_t len);
  * @name:	File path to write to
  * @buf:	Data to write
  * @size:	Size of data to write
- * @return 0 if OK, -ve on error
+ * Return:	0 if OK, -ve on error
  */
 int os_write_file(const char *name, const void *buf, int size);

@@ -340,7 +377,7 @@ int os_write_file(const char *name, const void *buf, int size);
  * @name:	File path to read from
  * @bufp:	Returns buffer containing data read
  * @sizep:	Returns size of data
- * @return 0 if OK, -ve on error
+ * Return:	0 if OK, -ve on error
  */
 int os_read_file(const char *name, void **bufp, int *sizep);

@@ -351,7 +388,7 @@ int os_read_file(const char *name, void **bufp, int *sizep);
  * It can be useful to map the address of functions to the address listed in
  * the u-boot.map file.
  *
- * @return address if found, else NULL
+ * Return:	address if found, else NULL
  */
 void *os_find_text_base(void);

@@ -359,7 +396,7 @@ void *os_find_text_base(void);
  * os_relaunch() - restart the sandbox
  *
  * This functions is used to implement the cold reboot of the sand box.
- * @argv[0] specifies the binary that is started while the calling process
+ * @argv\[0] specifies the binary that is started while the calling process
  * stops immediately. If the new binary cannot be started, the process is
  * terminated and 1 is set as shell return code.
  *
--
2.28.0

[PATCH v3 8/8] doc: add sandbox API

[Heinrich Schuchardt]

Add sandbox API to generated HTML documentation

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v3:
	no change
v2:
	new patch
---
 doc/api/index.rst   | 1 +
 doc/api/sandbox.rst | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 doc/api/sandbox.rst

diff --git a/doc/api/index.rst b/doc/api/index.rst
index 787b6778e5..cc3bce494e 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -11,6 +11,7 @@ U-Boot API documentation
    linker_lists
    pinctrl
    rng
+   sandbox
    serial
    timer
    unicode
diff --git a/doc/api/sandbox.rst b/doc/api/sandbox.rst
new file mode 100644
index 0000000000..724776399b
--- /dev/null
+++ b/doc/api/sandbox.rst
@@ -0,0 +1,9 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Sandbox
+=======
+
+The following API routines are used to implement the U-Boot sandbox.
+
+.. kernel-doc:: include/os.h
+   :internal:
--
2.28.0

[PATCH v3 4/8] sandbox: use O_CLOEXEC in os_open()

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> During a cold reset execv() is used to relaunch the U-Boot binary.
> We must ensure that all files are closed in this case.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         no change
> v2:
>         new patch
> ---
>  arch/sandbox/cpu/os.c | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v3 5/8] sandbox: implement reset

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Up to now the sandbox would shutdown upon a cold reset request. Instead it
> should be reset.
>
> In our coding we use static variables like LIST_HEAD(efi_obj_list). A reset
> can occur at any time, e.g. via an UEFI binary calling the reset service.
> The only safe way to return to an initial state is to relaunch the U-Boot
> binary.
>
> The reset implementation uses execv() to relaunch U-Boot.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         update commit message
> v2:
>         avoid longjmp()
>         add more comments
> ---
>  arch/sandbox/cpu/os.c                     |  6 ++++++
>  arch/sandbox/cpu/start.c                  | 26 +++++++++++++++++++++++
>  arch/sandbox/cpu/state.c                  |  1 +
>  arch/sandbox/include/asm/u-boot-sandbox.h | 10 +++++++++
>  drivers/sysreset/sysreset_sandbox.c       |  3 +++
>  include/os.h                              | 15 +++++++++++++
>  6 files changed, 61 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v3 7/8] sandbox: update function descriptions in os.h

[Simon Glass]

Hi Heinrich,

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Use Sphinx style function descriptions.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         no change
> v2:
>         new patch
> ---
>  include/os.h | 223 ++++++++++++++++++++++++++++++---------------------
>  1 file changed, 130 insertions(+), 93 deletions(-)
>
> diff --git a/include/os.h b/include/os.h
> index 88dfb71c1a..1fe44f3510 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -19,30 +19,30 @@ struct sandbox_state;
>  /**
>   * Access to the OS read() system call
>   *
> - * \param fd   File descriptor as returned by os_open()
> - * \param buf  Buffer to place data
> - * \param count        Number of bytes to read
> - * \return number of bytes read, or -1 on error
> + * @fd:                File descriptor as returned by os_open()
> + * @buf:       Buffer to place data
> + * @count:     Number of bytes to read
> + * Return:     number of bytes read, or -1 on error
>   */
>  ssize_t os_read(int fd, void *buf, size_t count);
>
>  /**
>   * Access to the OS write() system call
>   *
> - * \param fd   File descriptor as returned by os_open()
> - * \param buf  Buffer containing data to write
> - * \param count        Number of bytes to write
> - * \return number of bytes written, or -1 on error
> + * @fd:                File descriptor as returned by os_open()
> + * @buf:       Buffer containing data to write
> + * @count:     Number of bytes to write
> + * Return:     number of bytes written, or -1 on error
>   */
>  ssize_t os_write(int fd, const void *buf, size_t count);
>
>  /**
>   * Access to the OS lseek() system call
>   *
> - * \param fd   File descriptor as returned by os_open()
> - * \param offset       File offset (based on whence)
> - * \param whence       Position offset is relative to (see below)
> - * \return new file offset
> + * @fd:                File descriptor as returned by os_open()
> + * @offset:    File offset (based on whence)
> + * @whence:    Position offset is relative to (see below)
> + * Return:     new file offset
>   */
>  off_t os_lseek(int fd, off_t offset, int whence);
>
> @@ -54,9 +54,9 @@ off_t os_lseek(int fd, off_t offset, int whence);
>  /**
>   * Access to the OS open() system call
>   *
> - * \param pathname     Pathname of file to open
> - * \param flags                Flags, like OS_O_RDONLY, OS_O_RDWR
> - * \return file descriptor, or -1 on error
> + * @pathname:  Pathname of file to open
> + * @flags:     Flags, like OS_O_RDONLY, OS_O_RDWR
> + * Return:     file descriptor, or -1 on error
>   */
>  int os_open(const char *pathname, int flags);
>
> @@ -68,42 +68,42 @@ int os_open(const char *pathname, int flags);
>  #define OS_O_TRUNC     01000
>
>  /**
> - * Access to the OS close() system call
> + * os_close() - access to the OS close() system call
>   *
> - * \param fd   File descriptor to close
> - * \return 0 on success, -1 on error
> + * @fd:                File descriptor to close
> + * Return:     0 on success, -1 on error
>   */
>  int os_close(int fd);
>
>  /**
> - * Access to the OS unlink() system call
> + * os_unlink() - access to the OS unlink() system call
>   *
> - * \param pathname Path of file to delete
> - * \return 0 for success, other for error
> + * @pathname:  Path of file to delete
> + * Return:     0 for success, other for error
>   */
>  int os_unlink(const char *pathname);
>
>  /**
> - * Access to the OS exit() system call
> + * os_exit() - access to the OS exit() system call
>   *
>   * This exits with the supplied return code, which should be 0 to indicate
>   * success.
>   *
> - * @param exit_code    exit code for U-Boot
> + * @exit_code: exit code for U-Boot
>   */
>  void os_exit(int exit_code) __attribute__((noreturn));
>
>  /**
> - * Put tty into raw mode to mimic serial console better
> + * os_tty_raw() - put tty into raw mode to mimic serial console better
>   *
> - * @param fd           File descriptor of stdin (normally 0)
> - * @param allow_sigs   Allow Ctrl-C, Ctrl-Z to generate signals rather than
> - *                     be handled by U-Boot
> + * @fd:                File descriptor of stdin (normally 0)
> + * @allow_sigs:        Allow Ctrl-C, Ctrl-Z to generate signals rather than
> + *             be handled by U-Boot
>   */
>  void os_tty_raw(int fd, bool allow_sigs);
>
>  /**
> - * Restore the tty to its original mode
> + * os_fs_restore() - restore the tty to its original mode
>   *
>   * Call this to restore the original terminal mode, after it has been changed
>   * by os_tty_raw(). This is an internal function.
> @@ -111,144 +111,180 @@ void os_tty_raw(int fd, bool allow_sigs);
>  void os_fd_restore(void);
>
>  /**
> - * Acquires some memory from the underlying os.
> + * os_malloc() - aquires some memory from the underlying os.
>   *
> - * \param length       Number of bytes to be allocated
> - * \return Pointer to length bytes or NULL on error
> + * @length:    Number of bytes to be allocated
> + * Return:     Pointer to length bytes or NULL on error
>   */
>  void *os_malloc(size_t length);
>
>  /**
> - * Free memory previous allocated with os_malloc()
> + * os_free() - free memory previous allocated with os_malloc()
>   *
>   * This returns the memory to the OS.
>   *
> - * \param ptr          Pointer to memory block to free
> + * @ptr:       Pointer to memory block to free
>   */
>  void os_free(void *ptr);
>
>  /**
> - * Access to the usleep function of the os
> + * os_usleep() - access to the usleep function of the os
>   *
> - * \param usec Time to sleep in micro seconds
> + * @usec:      time to sleep in micro seconds
>   */
>  void os_usleep(unsigned long usec);
>
>  /**
>   * Gets a monotonic increasing number of nano seconds from the OS
>   *
> - * \return A monotonic increasing time scaled in nano seconds
> + * Return:     a monotonic increasing time scaled in nano seconds
>   */
>  uint64_t os_get_nsec(void);
>
>  /**
>   * Parse arguments and update sandbox state.
>   *
> - * @param state                Sandbox state to update
> - * @param argc         Argument count
> - * @param argv         Argument vector
> - * @return 0 if ok, and program should continue;
> - *     1 if ok, but program should stop;
> - *     -1 on error: program should terminate.
> + * @state:     sandbox state to update
> + * @argc:      argument count
> + * @argv:      argument vector
> + * Return:
> + * *  0 if ok, and program should continue
> + * *  1 if ok, but program should stop
> + * * -1 on error: program should terminate
>   */
>  int os_parse_args(struct sandbox_state *state, int argc, char *argv[]);
>
>  /*
> + * enum os_dirent_t - type of directory entry
> + *
>   * Types of directory entry that we support. See also os_dirent_typename in
>   * the C file.
>   */
>  enum os_dirent_t {
> -       OS_FILET_REG,           /* Regular file */
> -       OS_FILET_LNK,           /* Symbolic link */
> -       OS_FILET_DIR,           /* Directory */
> -       OS_FILET_UNKNOWN,       /* Something else */
> -
> +       /**
> +        * @OS_FILET_REG:       regular file
> +        */
> +       OS_FILET_REG,

Maybe I missed your response about a similar thing with
global_data.h...can we not do this?

OS_FILET_REG,   /**< @OS_FILET_REG:       regular file */

or

OS_FILET_REG,   /**< regular file */

> +       /**
> +        * @OS_FILET_LNK:       symbolic link
> +        */

or even:

/** @OS_FILET_LNK:       symbolic link */

> +       OS_FILET_LNK,
> +       /**
> +        * @OS_FILET_DIR:       directory
> +        */

Regards,
Simon

[PATCH v3 8/8] doc: add sandbox API

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Add sandbox API to generated HTML documentation
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         no change
> v2:
>         new patch
> ---
>  doc/api/index.rst   | 1 +
>  doc/api/sandbox.rst | 9 +++++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 doc/api/sandbox.rst

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v3 7/8] sandbox: update function descriptions in os.h

[Heinrich Schuchardt]

Am 3. November 2020 01:06:00 MEZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de>
>wrote:
>>
>> Use Sphinx style function descriptions.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v3:
>>         no change
>> v2:
>>         new patch
>> ---
>>  include/os.h | 223
>++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 130 insertions(+), 93 deletions(-)
>>
>> diff --git a/include/os.h b/include/os.h
>> index 88dfb71c1a..1fe44f3510 100644
>> --- a/include/os.h
>> +++ b/include/os.h
>> @@ -19,30 +19,30 @@ struct sandbox_state;
>>  /**
>>   * Access to the OS read() system call
>>   *
>> - * \param fd   File descriptor as returned by os_open()
>> - * \param buf  Buffer to place data
>> - * \param count        Number of bytes to read
>> - * \return number of bytes read, or -1 on error
>> + * @fd:                File descriptor as returned by os_open()
>> + * @buf:       Buffer to place data
>> + * @count:     Number of bytes to read
>> + * Return:     number of bytes read, or -1 on error
>>   */
>>  ssize_t os_read(int fd, void *buf, size_t count);
>>
>>  /**
>>   * Access to the OS write() system call
>>   *
>> - * \param fd   File descriptor as returned by os_open()
>> - * \param buf  Buffer containing data to write
>> - * \param count        Number of bytes to write
>> - * \return number of bytes written, or -1 on error
>> + * @fd:                File descriptor as returned by os_open()
>> + * @buf:       Buffer containing data to write
>> + * @count:     Number of bytes to write
>> + * Return:     number of bytes written, or -1 on error
>>   */
>>  ssize_t os_write(int fd, const void *buf, size_t count);
>>
>>  /**
>>   * Access to the OS lseek() system call
>>   *
>> - * \param fd   File descriptor as returned by os_open()
>> - * \param offset       File offset (based on whence)
>> - * \param whence       Position offset is relative to (see below)
>> - * \return new file offset
>> + * @fd:                File descriptor as returned by os_open()
>> + * @offset:    File offset (based on whence)
>> + * @whence:    Position offset is relative to (see below)
>> + * Return:     new file offset
>>   */
>>  off_t os_lseek(int fd, off_t offset, int whence);
>>
>> @@ -54,9 +54,9 @@ off_t os_lseek(int fd, off_t offset, int whence);
>>  /**
>>   * Access to the OS open() system call
>>   *
>> - * \param pathname     Pathname of file to open
>> - * \param flags                Flags, like OS_O_RDONLY, OS_O_RDWR
>> - * \return file descriptor, or -1 on error
>> + * @pathname:  Pathname of file to open
>> + * @flags:     Flags, like OS_O_RDONLY, OS_O_RDWR
>> + * Return:     file descriptor, or -1 on error
>>   */
>>  int os_open(const char *pathname, int flags);
>>
>> @@ -68,42 +68,42 @@ int os_open(const char *pathname, int flags);
>>  #define OS_O_TRUNC     01000
>>
>>  /**
>> - * Access to the OS close() system call
>> + * os_close() - access to the OS close() system call
>>   *
>> - * \param fd   File descriptor to close
>> - * \return 0 on success, -1 on error
>> + * @fd:                File descriptor to close
>> + * Return:     0 on success, -1 on error
>>   */
>>  int os_close(int fd);
>>
>>  /**
>> - * Access to the OS unlink() system call
>> + * os_unlink() - access to the OS unlink() system call
>>   *
>> - * \param pathname Path of file to delete
>> - * \return 0 for success, other for error
>> + * @pathname:  Path of file to delete
>> + * Return:     0 for success, other for error
>>   */
>>  int os_unlink(const char *pathname);
>>
>>  /**
>> - * Access to the OS exit() system call
>> + * os_exit() - access to the OS exit() system call
>>   *
>>   * This exits with the supplied return code, which should be 0 to
>indicate
>>   * success.
>>   *
>> - * @param exit_code    exit code for U-Boot
>> + * @exit_code: exit code for U-Boot
>>   */
>>  void os_exit(int exit_code) __attribute__((noreturn));
>>
>>  /**
>> - * Put tty into raw mode to mimic serial console better
>> + * os_tty_raw() - put tty into raw mode to mimic serial console
>better
>>   *
>> - * @param fd           File descriptor of stdin (normally 0)
>> - * @param allow_sigs   Allow Ctrl-C, Ctrl-Z to generate signals
>rather than
>> - *                     be handled by U-Boot
>> + * @fd:                File descriptor of stdin (normally 0)
>> + * @allow_sigs:        Allow Ctrl-C, Ctrl-Z to generate signals
>rather than
>> + *             be handled by U-Boot
>>   */
>>  void os_tty_raw(int fd, bool allow_sigs);
>>
>>  /**
>> - * Restore the tty to its original mode
>> + * os_fs_restore() - restore the tty to its original mode
>>   *
>>   * Call this to restore the original terminal mode, after it has
>been changed
>>   * by os_tty_raw(). This is an internal function.
>> @@ -111,144 +111,180 @@ void os_tty_raw(int fd, bool allow_sigs);
>>  void os_fd_restore(void);
>>
>>  /**
>> - * Acquires some memory from the underlying os.
>> + * os_malloc() - aquires some memory from the underlying os.
>>   *
>> - * \param length       Number of bytes to be allocated
>> - * \return Pointer to length bytes or NULL on error
>> + * @length:    Number of bytes to be allocated
>> + * Return:     Pointer to length bytes or NULL on error
>>   */
>>  void *os_malloc(size_t length);
>>
>>  /**
>> - * Free memory previous allocated with os_malloc()
>> + * os_free() - free memory previous allocated with os_malloc()
>>   *
>>   * This returns the memory to the OS.
>>   *
>> - * \param ptr          Pointer to memory block to free
>> + * @ptr:       Pointer to memory block to free
>>   */
>>  void os_free(void *ptr);
>>
>>  /**
>> - * Access to the usleep function of the os
>> + * os_usleep() - access to the usleep function of the os
>>   *
>> - * \param usec Time to sleep in micro seconds
>> + * @usec:      time to sleep in micro seconds
>>   */
>>  void os_usleep(unsigned long usec);
>>
>>  /**
>>   * Gets a monotonic increasing number of nano seconds from the OS
>>   *
>> - * \return A monotonic increasing time scaled in nano seconds
>> + * Return:     a monotonic increasing time scaled in nano seconds
>>   */
>>  uint64_t os_get_nsec(void);
>>
>>  /**
>>   * Parse arguments and update sandbox state.
>>   *
>> - * @param state                Sandbox state to update
>> - * @param argc         Argument count
>> - * @param argv         Argument vector
>> - * @return 0 if ok, and program should continue;
>> - *     1 if ok, but program should stop;
>> - *     -1 on error: program should terminate.
>> + * @state:     sandbox state to update
>> + * @argc:      argument count
>> + * @argv:      argument vector
>> + * Return:
>> + * *  0 if ok, and program should continue
>> + * *  1 if ok, but program should stop
>> + * * -1 on error: program should terminate
>>   */
>>  int os_parse_args(struct sandbox_state *state, int argc, char
>*argv[]);
>>
>>  /*
>> + * enum os_dirent_t - type of directory entry
>> + *
>>   * Types of directory entry that we support. See also
>os_dirent_typename in
>>   * the C file.
>>   */
>>  enum os_dirent_t {
>> -       OS_FILET_REG,           /* Regular file */
>> -       OS_FILET_LNK,           /* Symbolic link */
>> -       OS_FILET_DIR,           /* Directory */
>> -       OS_FILET_UNKNOWN,       /* Something else */
>> -
>> +       /**
>> +        * @OS_FILET_REG:       regular file
>> +        */
>> +       OS_FILET_REG,
>
>Maybe I missed your response about a similar thing with
>global_data.h...can we not do this?
>
>OS_FILET_REG,   /**< @OS_FILET_REG:       regular file */
>
>or
>
>OS_FILET_REG,   /**< regular file */
>
>> +       /**
>> +        * @OS_FILET_LNK:       symbolic link
>> +        */
>
>or even:
>
>/** @OS_FILET_LNK:       symbolic link */
>
>> +       OS_FILET_LNK,
>> +       /**
>> +        * @OS_FILET_DIR:       directory
>> +        */
>
>Regards,
>Simon

We want to follow the Linux documentation style. Otherwise we cannot profit from the upstream scripts.

https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

Best regards

Heinrich

[PATCH v3 7/8] sandbox: update function descriptions in os.h

[Simon Glass]

Hi Heinrich,

On Mon, 2 Nov 2020 at 17:13, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 3. November 2020 01:06:00 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >wrote:
> >>
> >> Use Sphinx style function descriptions.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >> v3:
> >>         no change
> >> v2:
> >>         new patch
> >> ---
> >>  include/os.h | 223
> >++++++++++++++++++++++++++++++---------------------
> >>  1 file changed, 130 insertions(+), 93 deletions(-)
> >>
> >> diff --git a/include/os.h b/include/os.h
> >> index 88dfb71c1a..1fe44f3510 100644
> >> --- a/include/os.h
> >> +++ b/include/os.h
> >> @@ -19,30 +19,30 @@ struct sandbox_state;
> >>  /**
> >>   * Access to the OS read() system call
> >>   *
> >> - * \param fd   File descriptor as returned by os_open()
> >> - * \param buf  Buffer to place data
> >> - * \param count        Number of bytes to read
> >> - * \return number of bytes read, or -1 on error
> >> + * @fd:                File descriptor as returned by os_open()
> >> + * @buf:       Buffer to place data
> >> + * @count:     Number of bytes to read
> >> + * Return:     number of bytes read, or -1 on error
> >>   */
> >>  ssize_t os_read(int fd, void *buf, size_t count);
> >>
> >>  /**
> >>   * Access to the OS write() system call
> >>   *
> >> - * \param fd   File descriptor as returned by os_open()
> >> - * \param buf  Buffer containing data to write
> >> - * \param count        Number of bytes to write
> >> - * \return number of bytes written, or -1 on error
> >> + * @fd:                File descriptor as returned by os_open()
> >> + * @buf:       Buffer containing data to write
> >> + * @count:     Number of bytes to write
> >> + * Return:     number of bytes written, or -1 on error
> >>   */
> >>  ssize_t os_write(int fd, const void *buf, size_t count);
> >>
> >>  /**
> >>   * Access to the OS lseek() system call
> >>   *
> >> - * \param fd   File descriptor as returned by os_open()
> >> - * \param offset       File offset (based on whence)
> >> - * \param whence       Position offset is relative to (see below)
> >> - * \return new file offset
> >> + * @fd:                File descriptor as returned by os_open()
> >> + * @offset:    File offset (based on whence)
> >> + * @whence:    Position offset is relative to (see below)
> >> + * Return:     new file offset
> >>   */
> >>  off_t os_lseek(int fd, off_t offset, int whence);
> >>
> >> @@ -54,9 +54,9 @@ off_t os_lseek(int fd, off_t offset, int whence);
> >>  /**
> >>   * Access to the OS open() system call
> >>   *
> >> - * \param pathname     Pathname of file to open
> >> - * \param flags                Flags, like OS_O_RDONLY, OS_O_RDWR
> >> - * \return file descriptor, or -1 on error
> >> + * @pathname:  Pathname of file to open
> >> + * @flags:     Flags, like OS_O_RDONLY, OS_O_RDWR
> >> + * Return:     file descriptor, or -1 on error
> >>   */
> >>  int os_open(const char *pathname, int flags);
> >>
> >> @@ -68,42 +68,42 @@ int os_open(const char *pathname, int flags);
> >>  #define OS_O_TRUNC     01000
> >>
> >>  /**
> >> - * Access to the OS close() system call
> >> + * os_close() - access to the OS close() system call
> >>   *
> >> - * \param fd   File descriptor to close
> >> - * \return 0 on success, -1 on error
> >> + * @fd:                File descriptor to close
> >> + * Return:     0 on success, -1 on error
> >>   */
> >>  int os_close(int fd);
> >>
> >>  /**
> >> - * Access to the OS unlink() system call
> >> + * os_unlink() - access to the OS unlink() system call
> >>   *
> >> - * \param pathname Path of file to delete
> >> - * \return 0 for success, other for error
> >> + * @pathname:  Path of file to delete
> >> + * Return:     0 for success, other for error
> >>   */
> >>  int os_unlink(const char *pathname);
> >>
> >>  /**
> >> - * Access to the OS exit() system call
> >> + * os_exit() - access to the OS exit() system call
> >>   *
> >>   * This exits with the supplied return code, which should be 0 to
> >indicate
> >>   * success.
> >>   *
> >> - * @param exit_code    exit code for U-Boot
> >> + * @exit_code: exit code for U-Boot
> >>   */
> >>  void os_exit(int exit_code) __attribute__((noreturn));
> >>
> >>  /**
> >> - * Put tty into raw mode to mimic serial console better
> >> + * os_tty_raw() - put tty into raw mode to mimic serial console
> >better
> >>   *
> >> - * @param fd           File descriptor of stdin (normally 0)
> >> - * @param allow_sigs   Allow Ctrl-C, Ctrl-Z to generate signals
> >rather than
> >> - *                     be handled by U-Boot
> >> + * @fd:                File descriptor of stdin (normally 0)
> >> + * @allow_sigs:        Allow Ctrl-C, Ctrl-Z to generate signals
> >rather than
> >> + *             be handled by U-Boot
> >>   */
> >>  void os_tty_raw(int fd, bool allow_sigs);
> >>
> >>  /**
> >> - * Restore the tty to its original mode
> >> + * os_fs_restore() - restore the tty to its original mode
> >>   *
> >>   * Call this to restore the original terminal mode, after it has
> >been changed
> >>   * by os_tty_raw(). This is an internal function.
> >> @@ -111,144 +111,180 @@ void os_tty_raw(int fd, bool allow_sigs);
> >>  void os_fd_restore(void);
> >>
> >>  /**
> >> - * Acquires some memory from the underlying os.
> >> + * os_malloc() - aquires some memory from the underlying os.
> >>   *
> >> - * \param length       Number of bytes to be allocated
> >> - * \return Pointer to length bytes or NULL on error
> >> + * @length:    Number of bytes to be allocated
> >> + * Return:     Pointer to length bytes or NULL on error
> >>   */
> >>  void *os_malloc(size_t length);
> >>
> >>  /**
> >> - * Free memory previous allocated with os_malloc()
> >> + * os_free() - free memory previous allocated with os_malloc()
> >>   *
> >>   * This returns the memory to the OS.
> >>   *
> >> - * \param ptr          Pointer to memory block to free
> >> + * @ptr:       Pointer to memory block to free
> >>   */
> >>  void os_free(void *ptr);
> >>
> >>  /**
> >> - * Access to the usleep function of the os
> >> + * os_usleep() - access to the usleep function of the os
> >>   *
> >> - * \param usec Time to sleep in micro seconds
> >> + * @usec:      time to sleep in micro seconds
> >>   */
> >>  void os_usleep(unsigned long usec);
> >>
> >>  /**
> >>   * Gets a monotonic increasing number of nano seconds from the OS
> >>   *
> >> - * \return A monotonic increasing time scaled in nano seconds
> >> + * Return:     a monotonic increasing time scaled in nano seconds
> >>   */
> >>  uint64_t os_get_nsec(void);
> >>
> >>  /**
> >>   * Parse arguments and update sandbox state.
> >>   *
> >> - * @param state                Sandbox state to update
> >> - * @param argc         Argument count
> >> - * @param argv         Argument vector
> >> - * @return 0 if ok, and program should continue;
> >> - *     1 if ok, but program should stop;
> >> - *     -1 on error: program should terminate.
> >> + * @state:     sandbox state to update
> >> + * @argc:      argument count
> >> + * @argv:      argument vector
> >> + * Return:
> >> + * *  0 if ok, and program should continue
> >> + * *  1 if ok, but program should stop
> >> + * * -1 on error: program should terminate
> >>   */
> >>  int os_parse_args(struct sandbox_state *state, int argc, char
> >*argv[]);
> >>
> >>  /*
> >> + * enum os_dirent_t - type of directory entry
> >> + *
> >>   * Types of directory entry that we support. See also
> >os_dirent_typename in
> >>   * the C file.
> >>   */
> >>  enum os_dirent_t {
> >> -       OS_FILET_REG,           /* Regular file */
> >> -       OS_FILET_LNK,           /* Symbolic link */
> >> -       OS_FILET_DIR,           /* Directory */
> >> -       OS_FILET_UNKNOWN,       /* Something else */
> >> -
> >> +       /**
> >> +        * @OS_FILET_REG:       regular file
> >> +        */
> >> +       OS_FILET_REG,
> >
> >Maybe I missed your response about a similar thing with
> >global_data.h...can we not do this?
> >
> >OS_FILET_REG,   /**< @OS_FILET_REG:       regular file */
> >
> >or
> >
> >OS_FILET_REG,   /**< regular file */
> >
> >> +       /**
> >> +        * @OS_FILET_LNK:       symbolic link
> >> +        */
> >
> >or even:
> >
> >/** @OS_FILET_LNK:       symbolic link */
> >
> >> +       OS_FILET_LNK,
> >> +       /**
> >> +        * @OS_FILET_DIR:       directory
> >> +        */
> >
> >Regards,
> >Simon
>
> We want to follow the Linux documentation style. Otherwise we cannot profit from the upstream scripts.
>
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

There are so many different styles floating around that I am not even
sure what this new one is called.

But it does suggest there is a single-line option:

/** @foobar: Single line description. */
              int foobar;

It is quite a bit less verbose and use up a lot less vertical screen
real estate.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[PATCH v3 8/8] doc: add sandbox API

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Add sandbox API to generated HTML documentation
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         no change
> v2:
>         new patch
> ---
>  doc/api/index.rst   | 1 +
>  doc/api/sandbox.rst | 9 +++++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 doc/api/sandbox.rst

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[PATCH v3 6/8] test: adjust sysreset tests

[Simon Glass]

As we have a working COLD_RESET on the sandbox the sysreset test has to be
adjusted.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 test/dm/sysreset.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v3 7/8] sandbox: update function descriptions in os.h

[Simon Glass]

Hi Heinrich,

On Mon, 2 Nov 2020 at 17:13, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 3. November 2020 01:06:00 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >wrote:
> >>
> >> Use Sphinx style function descriptions.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >> v3:
> >>         no change
> >> v2:
> >>         new patch
> >> ---
> >>  include/os.h | 223
> >++++++++++++++++++++++++++++++---------------------
> >>  1 file changed, 130 insertions(+), 93 deletions(-)
> >>
Applied to u-boot-dm, thanks!

[PATCH v3 5/8] sandbox: implement reset

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Up to now the sandbox would shutdown upon a cold reset request. Instead it
> should be reset.
>
> In our coding we use static variables like LIST_HEAD(efi_obj_list). A reset
> can occur at any time, e.g. via an UEFI binary calling the reset service.
> The only safe way to return to an initial state is to relaunch the U-Boot
> binary.
>
> The reset implementation uses execv() to relaunch U-Boot.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         update commit message
> v2:
>         avoid longjmp()
>         add more comments
> ---
>  arch/sandbox/cpu/os.c                     |  6 ++++++
>  arch/sandbox/cpu/start.c                  | 26 +++++++++++++++++++++++
>  arch/sandbox/cpu/state.c                  |  1 +
>  arch/sandbox/include/asm/u-boot-sandbox.h | 10 +++++++++
>  drivers/sysreset/sysreset_sandbox.c       |  3 +++
>  include/os.h                              | 15 +++++++++++++
>  6 files changed, 61 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[PATCH v3 4/8] sandbox: use O_CLOEXEC in os_open()

[Simon Glass]

On Tue, 27 Oct 2020 at 13:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> During a cold reset execv() is used to relaunch the U-Boot binary.
> We must ensure that all files are closed in this case.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v3:
>         no change
> v2:
>         new patch
> ---
>  arch/sandbox/cpu/os.c | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[PATCH v3 3/8] test/py: test poweroff

[Simon Glass]

It is the 'poweroff' and not the 'reset' command that should shut down the
sandbox.

Adjust the unit test accordingly

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 test/py/tests/test_sandbox_exit.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v3 2/8] sandbox: enable poweroff command

[Simon Glass]

The command to shut down a device is 'poweroff'. It is a deficit of the
sandbox that it does not support resetting yet but shuts down upong seeing
the 'reset' command.

Once the sandbox properly supports reset we need the 'poweroff' command to
leave the sandbox.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 arch/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks!

[PATCH v3 1/8] sandbox: eth-raw: do not close the console input

[Simon Glass]

When the sandbox eth-raw device host_lo is removed this leads to closing
the console input.

Do not call close(0).

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v3:
	no change
v2:
	no change
---
 arch/sandbox/cpu/eth-raw-os.c | 8 ++++----
 arch/sandbox/cpu/os.c         | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

Applied to u-boot-dm, thanks!