[PATCH v2 0/3] print befor sbi_console_init

[PATCH v2 0/3] print befor sbi_console_init

[Xiang W]

v2 changes:
- Moved the console initialization on crash to a separate patch as
suggested by Himanshu Chauhan
- Add console buffer size configuration in kconfig as suggested by
Himanshu Chauhan
- Fixed some code comments and formatting

This patch is used to output information to the console before
sbi_console_init

Xiang W (3):
  lib: sbi: print before sbi_console_init
  lib: sbi: dump logs when crash
  lib: sbi: Make the console print buffer size configurable

 lib/sbi/Kconfig       |  4 ++++
 lib/sbi/sbi_console.c | 31 ++++++++++++++++++++++++++++++-
 lib/sbi/sbi_hart.c    |  4 ++++
 3 files changed, 38 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 1/3] lib: sbi: print before sbi_console_init

[Xiang W]

The information from the print is cached via a ring buffer and
output to the console after initialization is complete.

Signed-off-by: Xiang W <wxjstz@126.com>
---
 lib/sbi/sbi_console.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
index d3ec461..1a91f88 100644
--- a/lib/sbi/sbi_console.c
+++ b/lib/sbi/sbi_console.c
@@ -19,6 +19,8 @@
 static const struct sbi_console_device *console_dev = NULL;
 static char console_tbuf[CONSOLE_TBUF_MAX];
 static u32 console_tbuf_len;
+static u32 console_tbuf_stat;
+static u32 console_tbuf_i;
 static spinlock_t console_out_lock	       = SPIN_LOCK_INITIALIZER;
 
 bool sbi_isprintable(char c)
@@ -135,6 +137,24 @@ static void printc(char **out, u32 *out_len, char ch, int flags)
 		return;
 	}
 
+	/* early print before sbi_console_init */
+	if (!console_dev && (flags & USE_TBUF)) {
+		/*
+		 * console_tbuf_stat
+		 *   0 buff is empty
+		 *   1 buff is not empty and does not overflow
+		 *   2 buff is overflow
+		 */
+		console_tbuf [console_tbuf_i++] = ch;
+		if (console_tbuf_stat == 0)
+			console_tbuf_stat = 1;
+		if (console_tbuf_i == CONSOLE_TBUF_MAX) {
+			console_tbuf_stat = 2;
+			console_tbuf_i = 0;
+		}
+		return;
+	}
+
 	/*
 	 * The *printf entry point functions have enforced that (*out) can
 	 * only be null when out_len is non-null and its value is zero.
@@ -476,6 +496,15 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
 		return;
 
 	console_dev = dev;
+
+	if (console_tbuf_stat == 2)
+		sbi_nputs(console_tbuf + console_tbuf_i,
+				CONSOLE_TBUF_MAX - console_tbuf_i);
+	if (console_tbuf_stat)
+		sbi_nputs(console_tbuf, console_tbuf_i);
+
+	console_tbuf_stat = 0;
+	console_tbuf_i = 0;
 }
 
 int sbi_console_init(struct sbi_scratch *scratch)
-- 
2.43.0

[PATCH v2 1/3] lib: sbi: print before sbi_console_init

[Andrew Jones]

On Mon, Jun 24, 2024 at 10:18:57PM GMT, Xiang W wrote:
> The information from the print is cached via a ring buffer and
> output to the console after initialization is complete.
> 
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/sbi_console.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
> index d3ec461..1a91f88 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -19,6 +19,8 @@
>  static const struct sbi_console_device *console_dev = NULL;
>  static char console_tbuf[CONSOLE_TBUF_MAX];
>  static u32 console_tbuf_len;
> +static u32 console_tbuf_stat;
> +static u32 console_tbuf_i;
>  static spinlock_t console_out_lock	       = SPIN_LOCK_INITIALIZER;
>  
>  bool sbi_isprintable(char c)
> @@ -135,6 +137,24 @@ static void printc(char **out, u32 *out_len, char ch, int flags)
>  		return;
>  	}
>  
> +	/* early print before sbi_console_init */
> +	if (!console_dev && (flags & USE_TBUF)) {
> +		/*
> +		 * console_tbuf_stat
> +		 *   0 buff is empty
> +		 *   1 buff is not empty and does not overflow
> +		 *   2 buff is overflow
> +		 */
> +		console_tbuf [console_tbuf_i++] = ch;
                            ^
                            ^ remove the blank here

> +		if (console_tbuf_stat == 0)
> +			console_tbuf_stat = 1;
> +		if (console_tbuf_i == CONSOLE_TBUF_MAX) {
> +			console_tbuf_stat = 2;
> +			console_tbuf_i = 0;
> +		}
> +		return;
> +	}
> +
>  	/*
>  	 * The *printf entry point functions have enforced that (*out) can
>  	 * only be null when out_len is non-null and its value is zero.
> @@ -476,6 +496,15 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
>  		return;
>  
>  	console_dev = dev;
> +
> +	if (console_tbuf_stat == 2)
> +		sbi_nputs(console_tbuf + console_tbuf_i,
> +				CONSOLE_TBUF_MAX - console_tbuf_i);
> +	if (console_tbuf_stat)
> +		sbi_nputs(console_tbuf, console_tbuf_i);
> +
> +	console_tbuf_stat = 0;
> +	console_tbuf_i = 0;
>  }
>  
>  int sbi_console_init(struct sbi_scratch *scratch)
> -- 
> 2.43.0
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH v2 1/3] lib: sbi: print before sbi_console_init

[Anup Patel]

On Mon, Jun 24, 2024 at 7:49?PM Xiang W <wxjstz@126.com> wrote:
>
> The information from the print is cached via a ring buffer and
> output to the console after initialization is complete.
>
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/sbi_console.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
> index d3ec461..1a91f88 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -19,6 +19,8 @@
>  static const struct sbi_console_device *console_dev = NULL;
>  static char console_tbuf[CONSOLE_TBUF_MAX];
>  static u32 console_tbuf_len;
> +static u32 console_tbuf_stat;
> +static u32 console_tbuf_i;
>  static spinlock_t console_out_lock            = SPIN_LOCK_INITIALIZER;
>
>  bool sbi_isprintable(char c)
> @@ -135,6 +137,24 @@ static void printc(char **out, u32 *out_len, char ch, int flags)
>                 return;
>         }
>
> +       /* early print before sbi_console_init */
> +       if (!console_dev && (flags & USE_TBUF)) {
> +               /*
> +                * console_tbuf_stat
> +                *   0 buff is empty
> +                *   1 buff is not empty and does not overflow
> +                *   2 buff is overflow
> +                */
> +               console_tbuf [console_tbuf_i++] = ch;
> +               if (console_tbuf_stat == 0)
> +                       console_tbuf_stat = 1;
> +               if (console_tbuf_i == CONSOLE_TBUF_MAX) {
> +                       console_tbuf_stat = 2;
> +                       console_tbuf_i = 0;
> +               }
> +               return;
> +       }
> +

Repurposing console_tbuf for caching early prints is only
going to make things complicated for maintenance. The
console_tbuf is only for temporarily caching in print() function
so that we can provide multiple characters to console_dev
in one call.

Himanshu had suggested implementing a common circular
buffer but I don't see that in this series.

I have an alternate series ready which incorporates both
my suggestions and Himanshu's suggestions. I will post
that soon.

Regards,
Anup


>         /*
>          * The *printf entry point functions have enforced that (*out) can
>          * only be null when out_len is non-null and its value is zero.
> @@ -476,6 +496,15 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
>                 return;
>
>         console_dev = dev;
> +
> +       if (console_tbuf_stat == 2)
> +               sbi_nputs(console_tbuf + console_tbuf_i,
> +                               CONSOLE_TBUF_MAX - console_tbuf_i);
> +       if (console_tbuf_stat)
> +               sbi_nputs(console_tbuf, console_tbuf_i);
> +
> +       console_tbuf_stat = 0;
> +       console_tbuf_i = 0;
>  }
>
>  int sbi_console_init(struct sbi_scratch *scratch)
> --
> 2.43.0
>

[PATCH v2 2/3] lib: sbi: dump logs when crash

[Xiang W]

When opensbi crashes, try to initialize the console in order to
output error messages.

Signed-off-by: Xiang W <wxjstz@126.com>
---
 lib/sbi/sbi_hart.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index c366701..48ec97c 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -1010,6 +1010,10 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
 
 void __attribute__((noreturn)) sbi_hart_hang(void)
 {
+	/* Try to initialize the console to output some error messages */
+	if (sbi_console_get_device() == NULL)
+		sbi_console_init(sbi_scratch_thishart_ptr());
+
 	while (1)
 		wfi();
 	__builtin_unreachable();
-- 
2.43.0

[PATCH v2 2/3] lib: sbi: dump logs when crash

[Himanshu Chauhan]

On Mon, Jun 24, 2024 at 10:18:58PM +0800, Xiang W wrote:
> When opensbi crashes, try to initialize the console in order to
> output error messages.
> 
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/sbi_hart.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index c366701..48ec97c 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -1010,6 +1010,10 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
>  
>  void __attribute__((noreturn)) sbi_hart_hang(void)
>  {
> +	/* Try to initialize the console to output some error messages */
> +	if (sbi_console_get_device() == NULL)
> +		sbi_console_init(sbi_scratch_thishart_ptr());
> +

Patch 2 and 3 must be in reverse order, i.e. Patch 2 should be 3 and 3 should be 2.
Otherwise, look good to me.

Please send the revision with correct order.

Regards
Himanshu

>  	while (1)
>  		wfi();
>  	__builtin_unreachable();
> -- 
> 2.43.0
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v2 2/3] lib: sbi: dump logs when crash

[Andrew Jones]

On Mon, Jun 24, 2024 at 10:18:58PM GMT, Xiang W wrote:
> When opensbi crashes, try to initialize the console in order to
> output error messages.
> 
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/sbi_hart.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index c366701..48ec97c 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -1010,6 +1010,10 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
>  
>  void __attribute__((noreturn)) sbi_hart_hang(void)
>  {
> +	/* Try to initialize the console to output some error messages */
> +	if (sbi_console_get_device() == NULL)
> +		sbi_console_init(sbi_scratch_thishart_ptr());
> +
>  	while (1)
>  		wfi();
>  	__builtin_unreachable();
> -- 
> 2.43.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH v2 2/3] lib: sbi: dump logs when crash

[Anup Patel]

On Mon, Jun 24, 2024 at 7:49?PM Xiang W <wxjstz@126.com> wrote:
>
> When opensbi crashes, try to initialize the console in order to
> output error messages.
>
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/sbi_hart.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index c366701..48ec97c 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -1010,6 +1010,10 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
>
>  void __attribute__((noreturn)) sbi_hart_hang(void)
>  {
> +       /* Try to initialize the console to output some error messages */
> +       if (sbi_console_get_device() == NULL)
> +               sbi_console_init(sbi_scratch_thishart_ptr());
> +

If a crash happened early then this will again change the order
in which platform console_init() is called as compared to other
functions in init_coldboot().

>         while (1)
>                 wfi();
>         __builtin_unreachable();
> --
> 2.43.0
>

Regards,
Anup

[PATCH v2 3/3] lib: sbi: Make the console print buffer size configurable

[Xiang W]

Before the console is initialized, the contents of the console print
will be cached. If there is a lot of information that needs to be
output before the console is initialized, can configure it through
make menuconfig.

Signed-off-by: Xiang W <wxjstz@126.com>
---
 lib/sbi/Kconfig       | 4 ++++
 lib/sbi/sbi_console.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
index 6cf54ce..fb6852e 100644
--- a/lib/sbi/Kconfig
+++ b/lib/sbi/Kconfig
@@ -62,4 +62,8 @@ config SBI_ECALL_SSE
 	bool "SSE extension"
 	default y
 
+config SBI_CONSOLE_TBUF_MAX
+	int "console print buffer size (bytes)"
+	default 256
+
 endmenu
diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
index 1a91f88..acf9f6b 100644
--- a/lib/sbi/sbi_console.c
+++ b/lib/sbi/sbi_console.c
@@ -14,7 +14,7 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
-#define CONSOLE_TBUF_MAX 256
+#define CONSOLE_TBUF_MAX CONFIG_SBI_CONSOLE_TBUF_MAX
 
 static const struct sbi_console_device *console_dev = NULL;
 static char console_tbuf[CONSOLE_TBUF_MAX];
-- 
2.43.0

[PATCH v2 3/3] lib: sbi: Make the console print buffer size configurable

[Andrew Jones]

On Mon, Jun 24, 2024 at 10:18:59PM GMT, Xiang W wrote:
> Before the console is initialized, the contents of the console print
> will be cached. If there is a lot of information that needs to be
> output before the console is initialized, can configure it through
> make menuconfig.
> 
> Signed-off-by: Xiang W <wxjstz@126.com>
> ---
>  lib/sbi/Kconfig       | 4 ++++
>  lib/sbi/sbi_console.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
> index 6cf54ce..fb6852e 100644
> --- a/lib/sbi/Kconfig
> +++ b/lib/sbi/Kconfig
> @@ -62,4 +62,8 @@ config SBI_ECALL_SSE
>  	bool "SSE extension"
>  	default y
>  
> +config SBI_CONSOLE_TBUF_MAX
> +	int "console print buffer size (bytes)"
> +	default 256
> +
>  endmenu
> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
> index 1a91f88..acf9f6b 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -14,7 +14,7 @@
>  #include <sbi/sbi_scratch.h>
>  #include <sbi/sbi_string.h>
>  
> -#define CONSOLE_TBUF_MAX 256
> +#define CONSOLE_TBUF_MAX CONFIG_SBI_CONSOLE_TBUF_MAX
>  
>  static const struct sbi_console_device *console_dev = NULL;
>  static char console_tbuf[CONSOLE_TBUF_MAX];
> -- 
> 2.43.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>