* [PATCH 1/6] platform: Setup serial console device in early_init()
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
@ 2024-07-05 7:14 ` Anup Patel
2024-07-24 5:40 ` Himanshu Chauhan
2024-07-05 7:15 ` [PATCH 2/6] lib: sbi: Remove sbi_console_init() and console_init() platform callback Anup Patel
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:14 UTC (permalink / raw)
To: opensbi
The sbi_console_init() does not do any special initialization so
setup serial console device in early_init() so that console prints
work as early as possible.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
platform/fpga/ariane/platform.c | 26 +++++++++-----------------
platform/fpga/openpiton/platform.c | 20 ++++----------------
platform/generic/platform.c | 21 +++++++++++----------
platform/kendryte/k210/platform.c | 18 ++++++------------
platform/nuclei/ux600/platform.c | 18 +++++++-----------
platform/template/platform.c | 18 ++++++------------
6 files changed, 43 insertions(+), 78 deletions(-)
diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
index 8be5e6c..8d46d88 100644
--- a/platform/fpga/ariane/platform.c
+++ b/platform/fpga/ariane/platform.c
@@ -7,7 +7,6 @@
#include <sbi/riscv_asm.h>
#include <sbi/riscv_encoding.h>
#include <sbi/riscv_io.h>
-#include <sbi/sbi_console.h>
#include <sbi/sbi_const.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_platform.h>
@@ -67,8 +66,15 @@ static struct aclint_mtimer_data mtimer = {
*/
static int ariane_early_init(bool cold_boot)
{
- /* For now nothing to do. */
- return 0;
+ if (!cold_boot)
+ return 0;
+
+ return uart8250_init(ARIANE_UART_ADDR,
+ ARIANE_UART_FREQ,
+ ARIANE_UART_BAUDRATE,
+ ARIANE_UART_REG_SHIFT,
+ ARIANE_UART_REG_WIDTH,
+ ARIANE_UART_REG_OFFSET);
}
/*
@@ -87,19 +93,6 @@ static int ariane_final_init(bool cold_boot)
return 0;
}
-/*
- * Initialize the ariane console.
- */
-static int ariane_console_init(void)
-{
- return uart8250_init(ARIANE_UART_ADDR,
- ARIANE_UART_FREQ,
- ARIANE_UART_BAUDRATE,
- ARIANE_UART_REG_SHIFT,
- ARIANE_UART_REG_WIDTH,
- ARIANE_UART_REG_OFFSET);
-}
-
static int plic_ariane_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
{
int ret;
@@ -175,7 +168,6 @@ static int ariane_timer_init(bool cold_boot)
const struct sbi_platform_operations platform_ops = {
.early_init = ariane_early_init,
.final_init = ariane_final_init,
- .console_init = ariane_console_init,
.irqchip_init = ariane_irqchip_init,
.ipi_init = ariane_ipi_init,
.timer_init = ariane_timer_init,
diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
index 2317a89..dd74a0c 100644
--- a/platform/fpga/openpiton/platform.c
+++ b/platform/fpga/openpiton/platform.c
@@ -6,7 +6,6 @@
#include <sbi/riscv_asm.h>
#include <sbi/riscv_encoding.h>
#include <sbi/riscv_io.h>
-#include <sbi/sbi_console.h>
#include <sbi/sbi_const.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_platform.h>
@@ -103,7 +102,10 @@ static int openpiton_early_init(bool cold_boot)
ACLINT_DEFAULT_MTIMECMP_OFFSET;
}
- return 0;
+ return uart8250_init(uart.addr, uart.freq, uart.baud,
+ OPENPITON_DEFAULT_UART_REG_SHIFT,
+ OPENPITON_DEFAULT_UART_REG_WIDTH,
+ OPENPITON_DEFAULT_UART_REG_OFFSET);
}
/*
@@ -122,19 +124,6 @@ static int openpiton_final_init(bool cold_boot)
return 0;
}
-/*
- * Initialize the openpiton console.
- */
-static int openpiton_console_init(void)
-{
- return uart8250_init(uart.addr,
- uart.freq,
- uart.baud,
- OPENPITON_DEFAULT_UART_REG_SHIFT,
- OPENPITON_DEFAULT_UART_REG_WIDTH,
- OPENPITON_DEFAULT_UART_REG_OFFSET);
-}
-
static int plic_openpiton_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
{
int ret;
@@ -210,7 +199,6 @@ static int openpiton_timer_init(bool cold_boot)
const struct sbi_platform_operations platform_ops = {
.early_init = openpiton_early_init,
.final_init = openpiton_final_init,
- .console_init = openpiton_console_init,
.irqchip_init = openpiton_irqchip_init,
.ipi_init = openpiton_ipi_init,
.timer_init = openpiton_timer_init,
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index 1f46b76..3470474 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -221,9 +221,19 @@ static int generic_nascent_init(void)
static int generic_early_init(bool cold_boot)
{
- if (cold_boot)
+ int rc;
+
+ if (cold_boot) {
fdt_reset_init();
+ if (semihosting_enabled())
+ rc = semihosting_init();
+ else
+ rc = fdt_serial_init();
+ if (rc)
+ return rc;
+ }
+
if (!generic_plat || !generic_plat->early_init)
return 0;
@@ -378,14 +388,6 @@ static uint64_t generic_pmu_xlate_to_mhpmevent(uint32_t event_idx,
return evt_val;
}
-static int generic_console_init(void)
-{
- if (semihosting_enabled())
- return semihosting_init();
- else
- return fdt_serial_init();
-}
-
const struct sbi_platform_operations platform_ops = {
.cold_boot_allowed = generic_cold_boot_allowed,
.nascent_init = generic_nascent_init,
@@ -395,7 +397,6 @@ const struct sbi_platform_operations platform_ops = {
.final_exit = generic_final_exit,
.extensions_init = generic_extensions_init,
.domains_init = generic_domains_init,
- .console_init = generic_console_init,
.irqchip_init = fdt_irqchip_init,
.irqchip_exit = fdt_irqchip_exit,
.ipi_init = fdt_ipi_init,
diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
index 27b23f7..531adcd 100644
--- a/platform/kendryte/k210/platform.c
+++ b/platform/kendryte/k210/platform.c
@@ -9,7 +9,6 @@
#include <sbi/riscv_asm.h>
#include <sbi/riscv_encoding.h>
-#include <sbi/sbi_console.h>
#include <sbi/sbi_const.h>
#include <sbi/sbi_platform.h>
#include <sbi/sbi_system.h>
@@ -109,10 +108,13 @@ static struct sbi_system_reset_device k210_reset = {
static int k210_early_init(bool cold_boot)
{
- if (cold_boot)
- sbi_system_reset_add_device(&k210_reset);
+ if (!cold_boot)
+ return 0;
- return 0;
+ sbi_system_reset_add_device(&k210_reset);
+
+ return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
+ K210_UART_BAUDRATE);
}
static int k210_final_init(bool cold_boot)
@@ -130,12 +132,6 @@ static int k210_final_init(bool cold_boot)
return 0;
}
-static int k210_console_init(void)
-{
- return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
- K210_UART_BAUDRATE);
-}
-
static int k210_irqchip_init(bool cold_boot)
{
int rc;
@@ -181,8 +177,6 @@ const struct sbi_platform_operations platform_ops = {
.final_init = k210_final_init,
- .console_init = k210_console_init,
-
.irqchip_init = k210_irqchip_init,
.ipi_init = k210_ipi_init,
diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
index f688b50..2289366 100644
--- a/platform/nuclei/ux600/platform.c
+++ b/platform/nuclei/ux600/platform.c
@@ -11,7 +11,6 @@
#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
#include <sbi/riscv_encoding.h>
-#include <sbi/sbi_console.h>
#include <sbi/sbi_const.h>
#include <sbi/sbi_platform.h>
#include <sbi/sbi_system.h>
@@ -150,8 +149,10 @@ static int ux600_early_init(bool cold_boot)
{
u32 regval;
- if (cold_boot)
- sbi_system_reset_add_device(&ux600_reset);
+ if (!cold_boot)
+ return 0;
+
+ sbi_system_reset_add_device(&ux600_reset);
/* Measure CPU Frequency using Timer */
ux600_clk_freq = ux600_get_clk_freq();
@@ -163,7 +164,9 @@ static int ux600_early_init(bool cold_boot)
regval = readl((void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS)) |
UX600_GPIO_IOF_UART0_MASK;
writel(regval, (void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS));
- return 0;
+
+ return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
+ UX600_UART_BAUDRATE);
}
static void ux600_modify_dt(void *fdt)
@@ -184,12 +187,6 @@ static int ux600_final_init(bool cold_boot)
return 0;
}
-static int ux600_console_init(void)
-{
- return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
- UX600_UART_BAUDRATE);
-}
-
static int ux600_irqchip_init(bool cold_boot)
{
int rc;
@@ -234,7 +231,6 @@ static int ux600_timer_init(bool cold_boot)
const struct sbi_platform_operations platform_ops = {
.early_init = ux600_early_init,
.final_init = ux600_final_init,
- .console_init = ux600_console_init,
.irqchip_init = ux600_irqchip_init,
.ipi_init = ux600_ipi_init,
.timer_init = ux600_timer_init,
diff --git a/platform/template/platform.c b/platform/template/platform.c
index 4b3f2ac..b4d30a5 100644
--- a/platform/template/platform.c
+++ b/platform/template/platform.c
@@ -64,7 +64,12 @@ static struct aclint_mtimer_data mtimer = {
*/
static int platform_early_init(bool cold_boot)
{
- return 0;
+ if (!cold_boot)
+ return 0;
+
+ /* Example if the generic UART8250 driver is used */
+ return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
+ PLATFORM_UART_BAUDRATE, 0, 1, 0);
}
/*
@@ -75,16 +80,6 @@ static int platform_final_init(bool cold_boot)
return 0;
}
-/*
- * Initialize the platform console.
- */
-static int platform_console_init(void)
-{
- /* Example if the generic UART8250 driver is used */
- return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
- PLATFORM_UART_BAUDRATE, 0, 1, 0);
-}
-
/*
* Initialize the platform interrupt controller for current HART.
*/
@@ -143,7 +138,6 @@ static int platform_timer_init(bool cold_boot)
const struct sbi_platform_operations platform_ops = {
.early_init = platform_early_init,
.final_init = platform_final_init,
- .console_init = platform_console_init,
.irqchip_init = platform_irqchip_init,
.ipi_init = platform_ipi_init,
.timer_init = platform_timer_init
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 1/6] platform: Setup serial console device in early_init()
2024-07-05 7:14 ` [PATCH 1/6] platform: Setup serial console device in early_init() Anup Patel
@ 2024-07-24 5:40 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:40 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The sbi_console_init() does not do any special initialization so
> setup serial console device in early_init() so that console prints
> work as early as possible.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> platform/fpga/ariane/platform.c | 26 +++++++++-----------------
> platform/fpga/openpiton/platform.c | 20 ++++----------------
> platform/generic/platform.c | 21 +++++++++++----------
> platform/kendryte/k210/platform.c | 18 ++++++------------
> platform/nuclei/ux600/platform.c | 18 +++++++-----------
> platform/template/platform.c | 18 ++++++------------
> 6 files changed, 43 insertions(+), 78 deletions(-)
>
> diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
> index 8be5e6c..8d46d88 100644
> --- a/platform/fpga/ariane/platform.c
> +++ b/platform/fpga/ariane/platform.c
> @@ -7,7 +7,6 @@
> #include <sbi/riscv_asm.h>
> #include <sbi/riscv_encoding.h>
> #include <sbi/riscv_io.h>
> -#include <sbi/sbi_console.h>
> #include <sbi/sbi_const.h>
> #include <sbi/sbi_hart.h>
> #include <sbi/sbi_platform.h>
> @@ -67,8 +66,15 @@ static struct aclint_mtimer_data mtimer = {
> */
> static int ariane_early_init(bool cold_boot)
> {
> - /* For now nothing to do. */
> - return 0;
> + if (!cold_boot)
> + return 0;
> +
> + return uart8250_init(ARIANE_UART_ADDR,
> + ARIANE_UART_FREQ,
> + ARIANE_UART_BAUDRATE,
> + ARIANE_UART_REG_SHIFT,
> + ARIANE_UART_REG_WIDTH,
> + ARIANE_UART_REG_OFFSET);
> }
>
> /*
> @@ -87,19 +93,6 @@ static int ariane_final_init(bool cold_boot)
> return 0;
> }
>
> -/*
> - * Initialize the ariane console.
> - */
> -static int ariane_console_init(void)
> -{
> - return uart8250_init(ARIANE_UART_ADDR,
> - ARIANE_UART_FREQ,
> - ARIANE_UART_BAUDRATE,
> - ARIANE_UART_REG_SHIFT,
> - ARIANE_UART_REG_WIDTH,
> - ARIANE_UART_REG_OFFSET);
> -}
> -
> static int plic_ariane_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
> {
> int ret;
> @@ -175,7 +168,6 @@ static int ariane_timer_init(bool cold_boot)
> const struct sbi_platform_operations platform_ops = {
> .early_init = ariane_early_init,
> .final_init = ariane_final_init,
> - .console_init = ariane_console_init,
> .irqchip_init = ariane_irqchip_init,
> .ipi_init = ariane_ipi_init,
> .timer_init = ariane_timer_init,
> diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> index 2317a89..dd74a0c 100644
> --- a/platform/fpga/openpiton/platform.c
> +++ b/platform/fpga/openpiton/platform.c
> @@ -6,7 +6,6 @@
> #include <sbi/riscv_asm.h>
> #include <sbi/riscv_encoding.h>
> #include <sbi/riscv_io.h>
> -#include <sbi/sbi_console.h>
> #include <sbi/sbi_const.h>
> #include <sbi/sbi_hart.h>
> #include <sbi/sbi_platform.h>
> @@ -103,7 +102,10 @@ static int openpiton_early_init(bool cold_boot)
> ACLINT_DEFAULT_MTIMECMP_OFFSET;
> }
>
> - return 0;
> + return uart8250_init(uart.addr, uart.freq, uart.baud,
> + OPENPITON_DEFAULT_UART_REG_SHIFT,
> + OPENPITON_DEFAULT_UART_REG_WIDTH,
> + OPENPITON_DEFAULT_UART_REG_OFFSET);
> }
>
> /*
> @@ -122,19 +124,6 @@ static int openpiton_final_init(bool cold_boot)
> return 0;
> }
>
> -/*
> - * Initialize the openpiton console.
> - */
> -static int openpiton_console_init(void)
> -{
> - return uart8250_init(uart.addr,
> - uart.freq,
> - uart.baud,
> - OPENPITON_DEFAULT_UART_REG_SHIFT,
> - OPENPITON_DEFAULT_UART_REG_WIDTH,
> - OPENPITON_DEFAULT_UART_REG_OFFSET);
> -}
> -
> static int plic_openpiton_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
> {
> int ret;
> @@ -210,7 +199,6 @@ static int openpiton_timer_init(bool cold_boot)
> const struct sbi_platform_operations platform_ops = {
> .early_init = openpiton_early_init,
> .final_init = openpiton_final_init,
> - .console_init = openpiton_console_init,
> .irqchip_init = openpiton_irqchip_init,
> .ipi_init = openpiton_ipi_init,
> .timer_init = openpiton_timer_init,
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index 1f46b76..3470474 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -221,9 +221,19 @@ static int generic_nascent_init(void)
>
> static int generic_early_init(bool cold_boot)
> {
> - if (cold_boot)
> + int rc;
> +
> + if (cold_boot) {
> fdt_reset_init();
>
> + if (semihosting_enabled())
> + rc = semihosting_init();
> + else
> + rc = fdt_serial_init();
> + if (rc)
> + return rc;
> + }
> +
> if (!generic_plat || !generic_plat->early_init)
> return 0;
>
> @@ -378,14 +388,6 @@ static uint64_t generic_pmu_xlate_to_mhpmevent(uint32_t event_idx,
> return evt_val;
> }
>
> -static int generic_console_init(void)
> -{
> - if (semihosting_enabled())
> - return semihosting_init();
> - else
> - return fdt_serial_init();
> -}
> -
> const struct sbi_platform_operations platform_ops = {
> .cold_boot_allowed = generic_cold_boot_allowed,
> .nascent_init = generic_nascent_init,
> @@ -395,7 +397,6 @@ const struct sbi_platform_operations platform_ops = {
> .final_exit = generic_final_exit,
> .extensions_init = generic_extensions_init,
> .domains_init = generic_domains_init,
> - .console_init = generic_console_init,
> .irqchip_init = fdt_irqchip_init,
> .irqchip_exit = fdt_irqchip_exit,
> .ipi_init = fdt_ipi_init,
> diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
> index 27b23f7..531adcd 100644
> --- a/platform/kendryte/k210/platform.c
> +++ b/platform/kendryte/k210/platform.c
> @@ -9,7 +9,6 @@
>
> #include <sbi/riscv_asm.h>
> #include <sbi/riscv_encoding.h>
> -#include <sbi/sbi_console.h>
> #include <sbi/sbi_const.h>
> #include <sbi/sbi_platform.h>
> #include <sbi/sbi_system.h>
> @@ -109,10 +108,13 @@ static struct sbi_system_reset_device k210_reset = {
>
> static int k210_early_init(bool cold_boot)
> {
> - if (cold_boot)
> - sbi_system_reset_add_device(&k210_reset);
> + if (!cold_boot)
> + return 0;
>
> - return 0;
> + sbi_system_reset_add_device(&k210_reset);
> +
> + return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
> + K210_UART_BAUDRATE);
> }
>
> static int k210_final_init(bool cold_boot)
> @@ -130,12 +132,6 @@ static int k210_final_init(bool cold_boot)
> return 0;
> }
>
> -static int k210_console_init(void)
> -{
> - return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
> - K210_UART_BAUDRATE);
> -}
> -
> static int k210_irqchip_init(bool cold_boot)
> {
> int rc;
> @@ -181,8 +177,6 @@ const struct sbi_platform_operations platform_ops = {
>
> .final_init = k210_final_init,
>
> - .console_init = k210_console_init,
> -
> .irqchip_init = k210_irqchip_init,
>
> .ipi_init = k210_ipi_init,
> diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
> index f688b50..2289366 100644
> --- a/platform/nuclei/ux600/platform.c
> +++ b/platform/nuclei/ux600/platform.c
> @@ -11,7 +11,6 @@
> #include <sbi/riscv_asm.h>
> #include <sbi/riscv_io.h>
> #include <sbi/riscv_encoding.h>
> -#include <sbi/sbi_console.h>
> #include <sbi/sbi_const.h>
> #include <sbi/sbi_platform.h>
> #include <sbi/sbi_system.h>
> @@ -150,8 +149,10 @@ static int ux600_early_init(bool cold_boot)
> {
> u32 regval;
>
> - if (cold_boot)
> - sbi_system_reset_add_device(&ux600_reset);
> + if (!cold_boot)
> + return 0;
> +
> + sbi_system_reset_add_device(&ux600_reset);
>
> /* Measure CPU Frequency using Timer */
> ux600_clk_freq = ux600_get_clk_freq();
> @@ -163,7 +164,9 @@ static int ux600_early_init(bool cold_boot)
> regval = readl((void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS)) |
> UX600_GPIO_IOF_UART0_MASK;
> writel(regval, (void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS));
> - return 0;
> +
> + return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
> + UX600_UART_BAUDRATE);
> }
>
> static void ux600_modify_dt(void *fdt)
> @@ -184,12 +187,6 @@ static int ux600_final_init(bool cold_boot)
> return 0;
> }
>
> -static int ux600_console_init(void)
> -{
> - return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
> - UX600_UART_BAUDRATE);
> -}
> -
> static int ux600_irqchip_init(bool cold_boot)
> {
> int rc;
> @@ -234,7 +231,6 @@ static int ux600_timer_init(bool cold_boot)
> const struct sbi_platform_operations platform_ops = {
> .early_init = ux600_early_init,
> .final_init = ux600_final_init,
> - .console_init = ux600_console_init,
> .irqchip_init = ux600_irqchip_init,
> .ipi_init = ux600_ipi_init,
> .timer_init = ux600_timer_init,
> diff --git a/platform/template/platform.c b/platform/template/platform.c
> index 4b3f2ac..b4d30a5 100644
> --- a/platform/template/platform.c
> +++ b/platform/template/platform.c
> @@ -64,7 +64,12 @@ static struct aclint_mtimer_data mtimer = {
> */
> static int platform_early_init(bool cold_boot)
> {
> - return 0;
> + if (!cold_boot)
> + return 0;
> +
> + /* Example if the generic UART8250 driver is used */
> + return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
> + PLATFORM_UART_BAUDRATE, 0, 1, 0);
> }
>
> /*
> @@ -75,16 +80,6 @@ static int platform_final_init(bool cold_boot)
> return 0;
> }
>
> -/*
> - * Initialize the platform console.
> - */
> -static int platform_console_init(void)
> -{
> - /* Example if the generic UART8250 driver is used */
> - return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
> - PLATFORM_UART_BAUDRATE, 0, 1, 0);
> -}
> -
> /*
> * Initialize the platform interrupt controller for current HART.
> */
> @@ -143,7 +138,6 @@ static int platform_timer_init(bool cold_boot)
> const struct sbi_platform_operations platform_ops = {
> .early_init = platform_early_init,
> .final_init = platform_final_init,
> - .console_init = platform_console_init,
> .irqchip_init = platform_irqchip_init,
> .ipi_init = platform_ipi_init,
> .timer_init = platform_timer_init
> --
> 2.34.1
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/6] lib: sbi: Remove sbi_console_init() and console_init() platform callback
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
2024-07-05 7:14 ` [PATCH 1/6] platform: Setup serial console device in early_init() Anup Patel
@ 2024-07-05 7:15 ` Anup Patel
2024-07-24 5:40 ` Himanshu Chauhan
2024-07-05 7:15 ` [PATCH 3/6] lib: sbi: Optimize fifo enqueue/dequeue for basic data types Anup Patel
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:15 UTC (permalink / raw)
To: opensbi
Now that all platforms have been updated to initialize serial console
device in early_init(), the sbi_console_init() and console_init()
platform callback are redundant hence remove them.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_console.h | 2 --
include/sbi/sbi_platform.h | 17 -----------------
lib/sbi/sbi_console.c | 11 -----------
lib/sbi/sbi_init.c | 4 ----
4 files changed, 34 deletions(-)
diff --git a/include/sbi/sbi_console.h b/include/sbi/sbi_console.h
index 0979765..3097db1 100644
--- a/include/sbi/sbi_console.h
+++ b/include/sbi/sbi_console.h
@@ -58,8 +58,6 @@ void sbi_console_set_device(const struct sbi_console_device *dev);
struct sbi_scratch;
-int sbi_console_init(struct sbi_scratch *scratch);
-
#define SBI_ASSERT(cond, args) do { \
if (unlikely(!(cond))) \
sbi_panic args; \
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index 581935a..7b3ac4b 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -111,9 +111,6 @@ struct sbi_platform_operations {
/** Get platform specific mhpmevent value */
uint64_t (*pmu_xlate_to_mhpmevent)(uint32_t event_idx, uint64_t data);
- /** Initialize the platform console */
- int (*console_init)(void);
-
/** Initialize the platform interrupt controller for current HART */
int (*irqchip_init)(bool cold_boot);
/** Exit the platform interrupt controller for current HART */
@@ -549,20 +546,6 @@ static inline uint64_t sbi_platform_pmu_xlate_to_mhpmevent(const struct sbi_plat
return 0;
}
-/**
- * Initialize the platform console
- *
- * @param plat pointer to struct sbi_platform
- *
- * @return 0 on success and negative error code on failure
- */
-static inline int sbi_platform_console_init(const struct sbi_platform *plat)
-{
- if (plat && sbi_platform_ops(plat)->console_init)
- return sbi_platform_ops(plat)->console_init();
- return 0;
-}
-
/**
* Initialize the platform interrupt controller for current HART
*
diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
index d3ec461..194529d 100644
--- a/lib/sbi/sbi_console.c
+++ b/lib/sbi/sbi_console.c
@@ -477,14 +477,3 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
console_dev = dev;
}
-
-int sbi_console_init(struct sbi_scratch *scratch)
-{
- int rc = sbi_platform_console_init(sbi_platform_ptr(scratch));
-
- /* console is not a necessary device */
- if (rc == SBI_ENODEV)
- return 0;
-
- return rc;
-}
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index 0f9e14c..d80efe9 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -261,10 +261,6 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
if (rc)
sbi_hart_hang();
- rc = sbi_console_init(scratch);
- if (rc)
- sbi_hart_hang();
-
rc = sbi_sse_init(scratch, true);
if (rc) {
sbi_printf("%s: sse init failed (error %d)\n", __func__, rc);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/6] lib: sbi: Remove sbi_console_init() and console_init() platform callback
2024-07-05 7:15 ` [PATCH 2/6] lib: sbi: Remove sbi_console_init() and console_init() platform callback Anup Patel
@ 2024-07-24 5:40 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:40 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Now that all platforms have been updated to initialize serial console
> device in early_init(), the sbi_console_init() and console_init()
> platform callback are redundant hence remove them.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_console.h | 2 --
> include/sbi/sbi_platform.h | 17 -----------------
> lib/sbi/sbi_console.c | 11 -----------
> lib/sbi/sbi_init.c | 4 ----
> 4 files changed, 34 deletions(-)
>
> diff --git a/include/sbi/sbi_console.h b/include/sbi/sbi_console.h
> index 0979765..3097db1 100644
> --- a/include/sbi/sbi_console.h
> +++ b/include/sbi/sbi_console.h
> @@ -58,8 +58,6 @@ void sbi_console_set_device(const struct sbi_console_device *dev);
>
> struct sbi_scratch;
>
> -int sbi_console_init(struct sbi_scratch *scratch);
> -
> #define SBI_ASSERT(cond, args) do { \
> if (unlikely(!(cond))) \
> sbi_panic args; \
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 581935a..7b3ac4b 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -111,9 +111,6 @@ struct sbi_platform_operations {
> /** Get platform specific mhpmevent value */
> uint64_t (*pmu_xlate_to_mhpmevent)(uint32_t event_idx, uint64_t data);
>
> - /** Initialize the platform console */
> - int (*console_init)(void);
> -
> /** Initialize the platform interrupt controller for current HART */
> int (*irqchip_init)(bool cold_boot);
> /** Exit the platform interrupt controller for current HART */
> @@ -549,20 +546,6 @@ static inline uint64_t sbi_platform_pmu_xlate_to_mhpmevent(const struct sbi_plat
> return 0;
> }
>
> -/**
> - * Initialize the platform console
> - *
> - * @param plat pointer to struct sbi_platform
> - *
> - * @return 0 on success and negative error code on failure
> - */
> -static inline int sbi_platform_console_init(const struct sbi_platform *plat)
> -{
> - if (plat && sbi_platform_ops(plat)->console_init)
> - return sbi_platform_ops(plat)->console_init();
> - return 0;
> -}
> -
> /**
> * Initialize the platform interrupt controller for current HART
> *
> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
> index d3ec461..194529d 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -477,14 +477,3 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
>
> console_dev = dev;
> }
> -
> -int sbi_console_init(struct sbi_scratch *scratch)
> -{
> - int rc = sbi_platform_console_init(sbi_platform_ptr(scratch));
> -
> - /* console is not a necessary device */
> - if (rc == SBI_ENODEV)
> - return 0;
> -
> - return rc;
> -}
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 0f9e14c..d80efe9 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -261,10 +261,6 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
> if (rc)
> sbi_hart_hang();
>
> - rc = sbi_console_init(scratch);
> - if (rc)
> - sbi_hart_hang();
> -
> rc = sbi_sse_init(scratch, true);
> if (rc) {
> sbi_printf("%s: sse init failed (error %d)\n", __func__, rc);
> --
> 2.34.1
>
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/6] lib: sbi: Optimize fifo enqueue/dequeue for basic data types
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
2024-07-05 7:14 ` [PATCH 1/6] platform: Setup serial console device in early_init() Anup Patel
2024-07-05 7:15 ` [PATCH 2/6] lib: sbi: Remove sbi_console_init() and console_init() platform callback Anup Patel
@ 2024-07-05 7:15 ` Anup Patel
2024-07-24 5:37 ` Himanshu Chauhan
2024-07-05 7:15 ` [PATCH 4/6] lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue() Anup Patel
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:15 UTC (permalink / raw)
To: opensbi
Don't use sbi_memcpy() for basic data types in fifo enqueue/dequeue
instead use direct type-cast and assignment.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/sbi_fifo.c | 43 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/lib/sbi/sbi_fifo.c b/lib/sbi/sbi_fifo.c
index 72c6d39..9199a30 100644
--- a/lib/sbi/sbi_fifo.c
+++ b/lib/sbi/sbi_fifo.c
@@ -66,7 +66,26 @@ static inline void __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
if (head >= fifo->num_entries)
head = head - fifo->num_entries;
- sbi_memcpy((char *)fifo->queue + head * fifo->entry_size, data, fifo->entry_size);
+ switch (fifo->entry_size) {
+ case 1:
+ *(char *)(fifo->queue + head * fifo->entry_size) = *(char *)data;
+ break;
+ case 2:
+ *(u16 *)(fifo->queue + head * fifo->entry_size) = *(u16 *)data;
+ break;
+ case 4:
+ *(u32 *)(fifo->queue + head * fifo->entry_size) = *(u32 *)data;
+ break;
+#if __riscv_xlen > 32
+ case 8:
+ *(u64 *)(fifo->queue + head * fifo->entry_size) = *(u64 *)data;
+ break;
+#endif
+ default:
+ sbi_memcpy(fifo->queue + head * fifo->entry_size,
+ data, fifo->entry_size);
+ break;
+ }
fifo->avail++;
}
@@ -184,8 +203,26 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
return SBI_ENOENT;
}
- sbi_memcpy(data, (char *)fifo->queue + (u32)fifo->tail * fifo->entry_size,
- fifo->entry_size);
+ switch (fifo->entry_size) {
+ case 1:
+ *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+ case 2:
+ *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+ case 4:
+ *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+#if __riscv_xlen > 32
+ case 8:
+ *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+#endif
+ default:
+ sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
+ fifo->entry_size);
+ break;
+ }
fifo->avail--;
fifo->tail++;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/6] lib: sbi: Optimize fifo enqueue/dequeue for basic data types
2024-07-05 7:15 ` [PATCH 3/6] lib: sbi: Optimize fifo enqueue/dequeue for basic data types Anup Patel
@ 2024-07-24 5:37 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:37 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Don't use sbi_memcpy() for basic data types in fifo enqueue/dequeue
> instead use direct type-cast and assignment.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/sbi_fifo.c | 43 ++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/lib/sbi/sbi_fifo.c b/lib/sbi/sbi_fifo.c
> index 72c6d39..9199a30 100644
> --- a/lib/sbi/sbi_fifo.c
> +++ b/lib/sbi/sbi_fifo.c
> @@ -66,7 +66,26 @@ static inline void __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
> if (head >= fifo->num_entries)
> head = head - fifo->num_entries;
>
> - sbi_memcpy((char *)fifo->queue + head * fifo->entry_size, data, fifo->entry_size);
> + switch (fifo->entry_size) {
> + case 1:
> + *(char *)(fifo->queue + head * fifo->entry_size) = *(char *)data;
> + break;
> + case 2:
> + *(u16 *)(fifo->queue + head * fifo->entry_size) = *(u16 *)data;
> + break;
> + case 4:
> + *(u32 *)(fifo->queue + head * fifo->entry_size) = *(u32 *)data;
> + break;
> +#if __riscv_xlen > 32
> + case 8:
> + *(u64 *)(fifo->queue + head * fifo->entry_size) = *(u64 *)data;
> + break;
> +#endif
> + default:
> + sbi_memcpy(fifo->queue + head * fifo->entry_size,
> + data, fifo->entry_size);
> + break;
> + }
>
> fifo->avail++;
> }
> @@ -184,8 +203,26 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
> return SBI_ENOENT;
> }
>
> - sbi_memcpy(data, (char *)fifo->queue + (u32)fifo->tail * fifo->entry_size,
> - fifo->entry_size);
> + switch (fifo->entry_size) {
> + case 1:
> + *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> + case 2:
> + *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> + case 4:
> + *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> +#if __riscv_xlen > 32
> + case 8:
> + *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> +#endif
> + default:
> + sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
> + fifo->entry_size);
> + break;
> + }
>
> fifo->avail--;
> fifo->tail++;
> --
> 2.34.1
>
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/6] lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
` (2 preceding siblings ...)
2024-07-05 7:15 ` [PATCH 3/6] lib: sbi: Optimize fifo enqueue/dequeue for basic data types Anup Patel
@ 2024-07-05 7:15 ` Anup Patel
2024-07-24 5:38 ` Himanshu Chauhan
2024-07-05 7:15 ` [PATCH 5/6] include: sbi: Add macros to create FIFO as local or global variable Anup Patel
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:15 UTC (permalink / raw)
To: opensbi
Extend sbi_fifo_enqueue() to allow forceful queueing by droping
data from the tail.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_fifo.h | 2 +-
lib/sbi/sbi_fifo.c | 69 +++++++++++++++++++++++++-----------------
lib/sbi/sbi_sse.c | 2 +-
lib/sbi/sbi_tlb.c | 3 +-
4 files changed, 45 insertions(+), 31 deletions(-)
diff --git a/include/sbi/sbi_fifo.h b/include/sbi/sbi_fifo.h
index 1a85f07..af1632a 100644
--- a/include/sbi/sbi_fifo.h
+++ b/include/sbi/sbi_fifo.h
@@ -30,7 +30,7 @@ enum sbi_fifo_inplace_update_types {
};
int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data);
-int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data);
+int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force);
void sbi_fifo_init(struct sbi_fifo *fifo, void *queue_mem, u16 entries,
u16 entry_size);
int sbi_fifo_is_empty(struct sbi_fifo *fifo);
diff --git a/lib/sbi/sbi_fifo.c b/lib/sbi/sbi_fifo.c
index 9199a30..d07ebff 100644
--- a/lib/sbi/sbi_fifo.c
+++ b/lib/sbi/sbi_fifo.c
@@ -90,6 +90,39 @@ static inline void __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
fifo->avail++;
}
+/* Note: must be called with fifo->qlock held */
+static inline void __sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
+{
+ if (!data)
+ goto skip_data_copy;
+
+ switch (fifo->entry_size) {
+ case 1:
+ *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+ case 2:
+ *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+ case 4:
+ *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+#if __riscv_xlen > 32
+ case 8:
+ *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+ break;
+#endif
+ default:
+ sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
+ fifo->entry_size);
+ break;
+ }
+
+skip_data_copy:
+ fifo->avail--;
+ fifo->tail++;
+ if (fifo->tail >= fifo->num_entries)
+ fifo->tail = 0;
+}
/* Note: must be called with fifo->qlock held */
static inline bool __sbi_fifo_is_empty(struct sbi_fifo *fifo)
@@ -173,7 +206,7 @@ int sbi_fifo_inplace_update(struct sbi_fifo *fifo, void *in,
return ret;
}
-int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
+int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force)
{
if (!fifo || !data)
return SBI_EINVAL;
@@ -181,9 +214,13 @@ int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
spin_lock(&fifo->qlock);
if (__sbi_fifo_is_full(fifo)) {
- spin_unlock(&fifo->qlock);
- return SBI_ENOSPC;
+ if (!force) {
+ spin_unlock(&fifo->qlock);
+ return SBI_ENOSPC;
+ }
+ __sbi_fifo_dequeue(fifo, NULL);
}
+
__sbi_fifo_enqueue(fifo, data);
spin_unlock(&fifo->qlock);
@@ -203,31 +240,7 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
return SBI_ENOENT;
}
- switch (fifo->entry_size) {
- case 1:
- *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
- break;
- case 2:
- *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
- break;
- case 4:
- *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
- break;
-#if __riscv_xlen > 32
- case 8:
- *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
- break;
-#endif
- default:
- sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
- fifo->entry_size);
- break;
- }
-
- fifo->avail--;
- fifo->tail++;
- if (fifo->tail >= fifo->num_entries)
- fifo->tail = 0;
+ __sbi_fifo_dequeue(fifo, data);
spin_unlock(&fifo->qlock);
diff --git a/lib/sbi/sbi_sse.c b/lib/sbi/sbi_sse.c
index e39963f..fe36a64 100644
--- a/lib/sbi/sbi_sse.c
+++ b/lib/sbi/sbi_sse.c
@@ -667,7 +667,7 @@ static int sse_ipi_inject_send(unsigned long hartid, uint32_t event_id)
sse_inject_fifo_r =
sbi_scratch_offset_ptr(remote_scratch, sse_inject_fifo_off);
- ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt);
+ ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt, false);
if (ret)
return SBI_EFAIL;
diff --git a/lib/sbi/sbi_tlb.c b/lib/sbi/sbi_tlb.c
index cca319f..01b31f4 100644
--- a/lib/sbi/sbi_tlb.c
+++ b/lib/sbi/sbi_tlb.c
@@ -351,7 +351,8 @@ static int tlb_update(struct sbi_scratch *scratch,
ret = sbi_fifo_inplace_update(tlb_fifo_r, data, tlb_update_cb);
- if (ret == SBI_FIFO_UNCHANGED && sbi_fifo_enqueue(tlb_fifo_r, data) < 0) {
+ if (ret == SBI_FIFO_UNCHANGED &&
+ sbi_fifo_enqueue(tlb_fifo_r, data, false) < 0) {
/**
* For now, Busy loop until there is space in the fifo.
* There may be case where target hart is also
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/6] lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
2024-07-05 7:15 ` [PATCH 4/6] lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue() Anup Patel
@ 2024-07-24 5:38 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:38 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Extend sbi_fifo_enqueue() to allow forceful queueing by droping
> data from the tail.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_fifo.h | 2 +-
> lib/sbi/sbi_fifo.c | 69 +++++++++++++++++++++++++-----------------
> lib/sbi/sbi_sse.c | 2 +-
> lib/sbi/sbi_tlb.c | 3 +-
> 4 files changed, 45 insertions(+), 31 deletions(-)
>
> diff --git a/include/sbi/sbi_fifo.h b/include/sbi/sbi_fifo.h
> index 1a85f07..af1632a 100644
> --- a/include/sbi/sbi_fifo.h
> +++ b/include/sbi/sbi_fifo.h
> @@ -30,7 +30,7 @@ enum sbi_fifo_inplace_update_types {
> };
>
> int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data);
> -int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data);
> +int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force);
> void sbi_fifo_init(struct sbi_fifo *fifo, void *queue_mem, u16 entries,
> u16 entry_size);
> int sbi_fifo_is_empty(struct sbi_fifo *fifo);
> diff --git a/lib/sbi/sbi_fifo.c b/lib/sbi/sbi_fifo.c
> index 9199a30..d07ebff 100644
> --- a/lib/sbi/sbi_fifo.c
> +++ b/lib/sbi/sbi_fifo.c
> @@ -90,6 +90,39 @@ static inline void __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
> fifo->avail++;
> }
>
> +/* Note: must be called with fifo->qlock held */
> +static inline void __sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
> +{
> + if (!data)
> + goto skip_data_copy;
> +
> + switch (fifo->entry_size) {
> + case 1:
> + *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> + case 2:
> + *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> + case 4:
> + *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> +#if __riscv_xlen > 32
> + case 8:
> + *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> + break;
> +#endif
> + default:
> + sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
> + fifo->entry_size);
> + break;
> + }
> +
> +skip_data_copy:
> + fifo->avail--;
> + fifo->tail++;
> + if (fifo->tail >= fifo->num_entries)
> + fifo->tail = 0;
> +}
>
> /* Note: must be called with fifo->qlock held */
> static inline bool __sbi_fifo_is_empty(struct sbi_fifo *fifo)
> @@ -173,7 +206,7 @@ int sbi_fifo_inplace_update(struct sbi_fifo *fifo, void *in,
> return ret;
> }
>
> -int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
> +int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force)
> {
> if (!fifo || !data)
> return SBI_EINVAL;
> @@ -181,9 +214,13 @@ int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
> spin_lock(&fifo->qlock);
>
> if (__sbi_fifo_is_full(fifo)) {
> - spin_unlock(&fifo->qlock);
> - return SBI_ENOSPC;
> + if (!force) {
> + spin_unlock(&fifo->qlock);
> + return SBI_ENOSPC;
> + }
> + __sbi_fifo_dequeue(fifo, NULL);
> }
> +
> __sbi_fifo_enqueue(fifo, data);
>
> spin_unlock(&fifo->qlock);
> @@ -203,31 +240,7 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
> return SBI_ENOENT;
> }
>
> - switch (fifo->entry_size) {
> - case 1:
> - *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> - break;
> - case 2:
> - *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> - break;
> - case 4:
> - *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> - break;
> -#if __riscv_xlen > 32
> - case 8:
> - *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
> - break;
> -#endif
> - default:
> - sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
> - fifo->entry_size);
> - break;
> - }
> -
> - fifo->avail--;
> - fifo->tail++;
> - if (fifo->tail >= fifo->num_entries)
> - fifo->tail = 0;
> + __sbi_fifo_dequeue(fifo, data);
>
> spin_unlock(&fifo->qlock);
>
> diff --git a/lib/sbi/sbi_sse.c b/lib/sbi/sbi_sse.c
> index e39963f..fe36a64 100644
> --- a/lib/sbi/sbi_sse.c
> +++ b/lib/sbi/sbi_sse.c
> @@ -667,7 +667,7 @@ static int sse_ipi_inject_send(unsigned long hartid, uint32_t event_id)
> sse_inject_fifo_r =
> sbi_scratch_offset_ptr(remote_scratch, sse_inject_fifo_off);
>
> - ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt);
> + ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt, false);
> if (ret)
> return SBI_EFAIL;
>
> diff --git a/lib/sbi/sbi_tlb.c b/lib/sbi/sbi_tlb.c
> index cca319f..01b31f4 100644
> --- a/lib/sbi/sbi_tlb.c
> +++ b/lib/sbi/sbi_tlb.c
> @@ -351,7 +351,8 @@ static int tlb_update(struct sbi_scratch *scratch,
>
> ret = sbi_fifo_inplace_update(tlb_fifo_r, data, tlb_update_cb);
>
> - if (ret == SBI_FIFO_UNCHANGED && sbi_fifo_enqueue(tlb_fifo_r, data) < 0) {
> + if (ret == SBI_FIFO_UNCHANGED &&
> + sbi_fifo_enqueue(tlb_fifo_r, data, false) < 0) {
> /**
> * For now, Busy loop until there is space in the fifo.
> * There may be case where target hart is also
> --
> 2.34.1
>
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/6] include: sbi: Add macros to create FIFO as local or global variable
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
` (3 preceding siblings ...)
2024-07-05 7:15 ` [PATCH 4/6] lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue() Anup Patel
@ 2024-07-05 7:15 ` Anup Patel
2024-07-24 5:38 ` Himanshu Chauhan
2024-07-05 7:15 ` [PATCH 6/6] lib: sbi: Introduce an early console buffer for caching early prints Anup Patel
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:15 UTC (permalink / raw)
To: opensbi
The FIFO data structure is quite handy of variety of use-case so add
SBI_FIFO_INITIALIZER() and SBI_FIFO_DEFINE() helper macros to create
FIFO as local or global variable.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_fifo.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/sbi/sbi_fifo.h b/include/sbi/sbi_fifo.h
index af1632a..89a2ea2 100644
--- a/include/sbi/sbi_fifo.h
+++ b/include/sbi/sbi_fifo.h
@@ -23,6 +23,18 @@ struct sbi_fifo {
u16 tail;
};
+#define SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size) \
+{ .queue = __queue_mem, \
+ .qlock = SPIN_LOCK_INITIALIZER, \
+ .num_entries = __entries, \
+ .entry_size = __entry_size, \
+ .avail = 0, \
+ .tail = 0, \
+}
+
+#define SBI_FIFO_DEFINE(__name, __queue_mem, __entries, __entry_size) \
+struct sbi_fifo __name = SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size)
+
enum sbi_fifo_inplace_update_types {
SBI_FIFO_SKIP,
SBI_FIFO_UPDATED,
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/6] include: sbi: Add macros to create FIFO as local or global variable
2024-07-05 7:15 ` [PATCH 5/6] include: sbi: Add macros to create FIFO as local or global variable Anup Patel
@ 2024-07-24 5:38 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:38 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The FIFO data structure is quite handy of variety of use-case so add
> SBI_FIFO_INITIALIZER() and SBI_FIFO_DEFINE() helper macros to create
> FIFO as local or global variable.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_fifo.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/sbi/sbi_fifo.h b/include/sbi/sbi_fifo.h
> index af1632a..89a2ea2 100644
> --- a/include/sbi/sbi_fifo.h
> +++ b/include/sbi/sbi_fifo.h
> @@ -23,6 +23,18 @@ struct sbi_fifo {
> u16 tail;
> };
>
> +#define SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size) \
> +{ .queue = __queue_mem, \
> + .qlock = SPIN_LOCK_INITIALIZER, \
> + .num_entries = __entries, \
> + .entry_size = __entry_size, \
> + .avail = 0, \
> + .tail = 0, \
> +}
> +
> +#define SBI_FIFO_DEFINE(__name, __queue_mem, __entries, __entry_size) \
> +struct sbi_fifo __name = SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size)
> +
> enum sbi_fifo_inplace_update_types {
> SBI_FIFO_SKIP,
> SBI_FIFO_UPDATED,
> --
> 2.34.1
>
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/6] lib: sbi: Introduce an early console buffer for caching early prints
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
` (4 preceding siblings ...)
2024-07-05 7:15 ` [PATCH 5/6] include: sbi: Add macros to create FIFO as local or global variable Anup Patel
@ 2024-07-05 7:15 ` Anup Patel
2024-07-24 5:39 ` Himanshu Chauhan
2024-07-08 16:48 ` [PATCH 0/6] Early console buffer for OpenSBI Xiang W
2024-07-24 6:50 ` Anup Patel
7 siblings, 1 reply; 16+ messages in thread
From: Anup Patel @ 2024-07-05 7:15 UTC (permalink / raw)
To: opensbi
The console device is registered by platform only in early_init()
callback so any prints before this point will be lost. Introduce an
early console buffer for caching prints before platform early_init().
For crashes before platform early_init(), users can simply dump the
contents of the console_early_buffer[] string using a debugger. The
relative address of the console_early_buffer[] string can be found
using following two commands:
CONSOLE_EARLY_FIFO_ADDR=`${CROSS_COMPILE}objdump -D \
build/platform/generic/firmware/fw_dynamic.elf | \
grep "<console_early_fifo>:" | awk '{print $1}'`
${CROSS_COMPILE}objdump -R build/platform/generic/firmware/fw_dynamic.elf | \
grep $CONSOLE_EARLY_FIFO_ADDR | awk '{print $3}'
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/Kconfig | 6 +++++-
lib/sbi/sbi_console.c | 27 +++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
index 6cf54ce..bd8ba2b 100644
--- a/lib/sbi/Kconfig
+++ b/lib/sbi/Kconfig
@@ -1,6 +1,10 @@
# SPDX-License-Identifier: BSD-2-Clause
-menu "SBI Extension Support"
+menu "Generic SBI Support"
+
+config CONSOLE_EARLY_BUFFER_SIZE
+ int "Early console buffer size (bytes)"
+ default 256
config SBI_ECALL_TIME
bool "Timer extension"
diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
index 194529d..d760885 100644
--- a/lib/sbi/sbi_console.c
+++ b/lib/sbi/sbi_console.c
@@ -9,6 +9,7 @@
#include <sbi/riscv_locks.h>
#include <sbi/sbi_console.h>
+#include <sbi/sbi_fifo.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_platform.h>
#include <sbi/sbi_scratch.h>
@@ -21,6 +22,15 @@ static char console_tbuf[CONSOLE_TBUF_MAX];
static u32 console_tbuf_len;
static spinlock_t console_out_lock = SPIN_LOCK_INITIALIZER;
+#ifdef CONFIG_CONSOLE_EARLY_BUFFER_SIZE
+#define CONSOLE_EARLY_BUFFER_SIZE CONFIG_CONSOLE_EARLY_BUFFER_SIZE
+#else
+#define CONSOLE_EARLY_BUFFER_SIZE 256
+#endif
+static char console_early_buffer[CONSOLE_EARLY_BUFFER_SIZE] = { 0 };
+static SBI_FIFO_DEFINE(console_early_fifo, console_early_buffer, \
+ CONSOLE_EARLY_BUFFER_SIZE, sizeof(char));
+
bool sbi_isprintable(char c)
{
if (((31 < c) && (c < 127)) || (c == '\f') || (c == '\r') ||
@@ -39,6 +49,7 @@ int sbi_getc(void)
static unsigned long nputs(const char *str, unsigned long len)
{
+ char ch;
unsigned long i;
if (console_dev) {
@@ -51,6 +62,11 @@ static unsigned long nputs(const char *str, unsigned long len)
console_dev->console_putc(str[i]);
}
}
+ } else {
+ for (i = 0; i < len; i++) {
+ ch = str[i];
+ sbi_fifo_enqueue(&console_early_fifo, &ch, true);
+ }
}
return len;
}
@@ -472,8 +488,19 @@ const struct sbi_console_device *sbi_console_get_device(void)
void sbi_console_set_device(const struct sbi_console_device *dev)
{
+ char ch;
+ bool flush_early_fifo = false;
+
if (!dev)
return;
+ if (!console_dev)
+ flush_early_fifo = true;
+
console_dev = dev;
+
+ if (flush_early_fifo) {
+ while (!sbi_fifo_dequeue(&console_early_fifo, &ch))
+ sbi_putc(ch);
+ }
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/6] lib: sbi: Introduce an early console buffer for caching early prints
2024-07-05 7:15 ` [PATCH 6/6] lib: sbi: Introduce an early console buffer for caching early prints Anup Patel
@ 2024-07-24 5:39 ` Himanshu Chauhan
0 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2024-07-24 5:39 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The console device is registered by platform only in early_init()
> callback so any prints before this point will be lost. Introduce an
> early console buffer for caching prints before platform early_init().
>
> For crashes before platform early_init(), users can simply dump the
> contents of the console_early_buffer[] string using a debugger. The
> relative address of the console_early_buffer[] string can be found
> using following two commands:
>
> CONSOLE_EARLY_FIFO_ADDR=`${CROSS_COMPILE}objdump -D \
> build/platform/generic/firmware/fw_dynamic.elf | \
> grep "<console_early_fifo>:" | awk '{print $1}'`
>
> ${CROSS_COMPILE}objdump -R build/platform/generic/firmware/fw_dynamic.elf | \
> grep $CONSOLE_EARLY_FIFO_ADDR | awk '{print $3}'
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/Kconfig | 6 +++++-
> lib/sbi/sbi_console.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
> index 6cf54ce..bd8ba2b 100644
> --- a/lib/sbi/Kconfig
> +++ b/lib/sbi/Kconfig
> @@ -1,6 +1,10 @@
> # SPDX-License-Identifier: BSD-2-Clause
>
> -menu "SBI Extension Support"
> +menu "Generic SBI Support"
> +
> +config CONSOLE_EARLY_BUFFER_SIZE
> + int "Early console buffer size (bytes)"
> + default 256
>
> config SBI_ECALL_TIME
> bool "Timer extension"
> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c
> index 194529d..d760885 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -9,6 +9,7 @@
>
> #include <sbi/riscv_locks.h>
> #include <sbi/sbi_console.h>
> +#include <sbi/sbi_fifo.h>
> #include <sbi/sbi_hart.h>
> #include <sbi/sbi_platform.h>
> #include <sbi/sbi_scratch.h>
> @@ -21,6 +22,15 @@ static char console_tbuf[CONSOLE_TBUF_MAX];
> static u32 console_tbuf_len;
> static spinlock_t console_out_lock = SPIN_LOCK_INITIALIZER;
>
> +#ifdef CONFIG_CONSOLE_EARLY_BUFFER_SIZE
> +#define CONSOLE_EARLY_BUFFER_SIZE CONFIG_CONSOLE_EARLY_BUFFER_SIZE
> +#else
> +#define CONSOLE_EARLY_BUFFER_SIZE 256
> +#endif
> +static char console_early_buffer[CONSOLE_EARLY_BUFFER_SIZE] = { 0 };
> +static SBI_FIFO_DEFINE(console_early_fifo, console_early_buffer, \
> + CONSOLE_EARLY_BUFFER_SIZE, sizeof(char));
> +
> bool sbi_isprintable(char c)
> {
> if (((31 < c) && (c < 127)) || (c == '\f') || (c == '\r') ||
> @@ -39,6 +49,7 @@ int sbi_getc(void)
>
> static unsigned long nputs(const char *str, unsigned long len)
> {
> + char ch;
> unsigned long i;
>
> if (console_dev) {
> @@ -51,6 +62,11 @@ static unsigned long nputs(const char *str, unsigned long len)
> console_dev->console_putc(str[i]);
> }
> }
> + } else {
> + for (i = 0; i < len; i++) {
> + ch = str[i];
> + sbi_fifo_enqueue(&console_early_fifo, &ch, true);
> + }
> }
> return len;
> }
> @@ -472,8 +488,19 @@ const struct sbi_console_device *sbi_console_get_device(void)
>
> void sbi_console_set_device(const struct sbi_console_device *dev)
> {
> + char ch;
> + bool flush_early_fifo = false;
> +
> if (!dev)
> return;
>
> + if (!console_dev)
> + flush_early_fifo = true;
> +
> console_dev = dev;
> +
> + if (flush_early_fifo) {
> + while (!sbi_fifo_dequeue(&console_early_fifo, &ch))
> + sbi_putc(ch);
> + }
> }
> --
> 2.34.1
>
LGTM
Reviewed-By: Himanshu Chauhan <hchauhan@ventanamicro.com>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/6] Early console buffer for OpenSBI
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
` (5 preceding siblings ...)
2024-07-05 7:15 ` [PATCH 6/6] lib: sbi: Introduce an early console buffer for caching early prints Anup Patel
@ 2024-07-08 16:48 ` Xiang W
2024-07-09 3:43 ` Anup Patel
2024-07-24 6:50 ` Anup Patel
7 siblings, 1 reply; 16+ messages in thread
From: Xiang W @ 2024-07-08 16:48 UTC (permalink / raw)
To: opensbi
? 2024-07-05???? 12:44 +0530?Anup Patel???
> This series tries to improve OpenSBI debugging by:
> 1) Registering console device via platform early_init()
> 2) Introducing early console buffer
>
> These patches can also be found in the console_early_probe_v1
> branch at: https://github.com/avpatel/opensbi.git
>
> Anup Patel (6):
> ? platform: Setup serial console device in early_init()
> ? lib: sbi: Remove sbi_console_init() and console_init() platform
> ??? callback
> ? lib: sbi: Optimize fifo enqueue/dequeue for basic data types
> ? lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
> ? include: sbi: Add macros to create FIFO as local or global variable
> ? lib: sbi: Introduce an early console buffer for caching early prints
>
Hi, Anup
Before the console is initialized, sbi_printf is only called if an error
occurs, and an error usually causes the program to enter sbi_hart_hang,
resulting in no output. This makes adding an early console buffer less
useful. The program will crash anyway, and we don't need to care about
the order of function calls, we should just output some useful information
as much as possible.
Regards,
Xiang W
> ?include/sbi/sbi_console.h????????? |? 2 -
> ?include/sbi/sbi_fifo.h???????????? | 14 +++++-
> ?include/sbi/sbi_platform.h???????? | 17 -------
> ?lib/sbi/Kconfig??????????????????? |? 6 ++-
> ?lib/sbi/sbi_console.c????????????? | 36 ++++++++++-----
> ?lib/sbi/sbi_fifo.c???????????????? | 72 +++++++++++++++++++++++++-----
> ?lib/sbi/sbi_init.c???????????????? |? 4 --
> ?lib/sbi/sbi_sse.c????????????????? |? 2 +-
> ?lib/sbi/sbi_tlb.c????????????????? |? 3 +-
> ?platform/fpga/ariane/platform.c??? | 26 ++++-------
> ?platform/fpga/openpiton/platform.c | 20 ++-------
> ?platform/generic/platform.c??????? | 21 ++++-----
> ?platform/kendryte/k210/platform.c? | 18 +++-----
> ?platform/nuclei/ux600/platform.c?? | 18 +++-----
> ?platform/template/platform.c?????? | 18 +++-----
> ?15 files changed, 151 insertions(+), 126 deletions(-)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 0/6] Early console buffer for OpenSBI
2024-07-08 16:48 ` [PATCH 0/6] Early console buffer for OpenSBI Xiang W
@ 2024-07-09 3:43 ` Anup Patel
0 siblings, 0 replies; 16+ messages in thread
From: Anup Patel @ 2024-07-09 3:43 UTC (permalink / raw)
To: opensbi
On Mon, Jul 8, 2024 at 10:19?PM Xiang W <wxjstz@126.com> wrote:
>
> ? 2024-07-05???? 12:44 +0530?Anup Patel???
> > This series tries to improve OpenSBI debugging by:
> > 1) Registering console device via platform early_init()
> > 2) Introducing early console buffer
> >
> > These patches can also be found in the console_early_probe_v1
> > branch at: https://github.com/avpatel/opensbi.git
> >
> > Anup Patel (6):
> > platform: Setup serial console device in early_init()
> > lib: sbi: Remove sbi_console_init() and console_init() platform
> > callback
> > lib: sbi: Optimize fifo enqueue/dequeue for basic data types
> > lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
> > include: sbi: Add macros to create FIFO as local or global variable
> > lib: sbi: Introduce an early console buffer for caching early prints
> >
> Hi, Anup
>
> Before the console is initialized, sbi_printf is only called if an error
> occurs, and an error usually causes the program to enter sbi_hart_hang,
> resulting in no output. This makes adding an early console buffer less
> useful. The program will crash anyway, and we don't need to care about
> the order of function calls, we should just output some useful information
> as much as possible.
If the program crashes before early_init() then users can use HW debugger
to dump the contents of the early console. This practice has been followed
in other open source projects as well.
There is always a small boot time window across system level software
(such as bootloader, firmware, OS kernels and hypervisors) where if a
failure occurs in this window then it will result in no prints. The goal
should be to minimize this boot time window and make it robust
without working-around or hacking driver dependency.
Currently, the sbi_domain_init() must be completed before the serial
drivers are probed. You can certainly to do a ugly work-around to split
the serial driver init into two parts but what if the serial drivers also use
heap in the future. There is no end to such work-arounds.
Directly initializing the console from sbi_hart_hang() is
NOT A MAINTAINABLE approach.
Regards,
Anup
>
> Regards,
> Xiang W
> > include/sbi/sbi_console.h | 2 -
> > include/sbi/sbi_fifo.h | 14 +++++-
> > include/sbi/sbi_platform.h | 17 -------
> > lib/sbi/Kconfig | 6 ++-
> > lib/sbi/sbi_console.c | 36 ++++++++++-----
> > lib/sbi/sbi_fifo.c | 72 +++++++++++++++++++++++++-----
> > lib/sbi/sbi_init.c | 4 --
> > lib/sbi/sbi_sse.c | 2 +-
> > lib/sbi/sbi_tlb.c | 3 +-
> > platform/fpga/ariane/platform.c | 26 ++++-------
> > platform/fpga/openpiton/platform.c | 20 ++-------
> > platform/generic/platform.c | 21 ++++-----
> > platform/kendryte/k210/platform.c | 18 +++-----
> > platform/nuclei/ux600/platform.c | 18 +++-----
> > platform/template/platform.c | 18 +++-----
> > 15 files changed, 151 insertions(+), 126 deletions(-)
> >
> > --
> > 2.34.1
> >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/6] Early console buffer for OpenSBI
2024-07-05 7:14 [PATCH 0/6] Early console buffer for OpenSBI Anup Patel
` (6 preceding siblings ...)
2024-07-08 16:48 ` [PATCH 0/6] Early console buffer for OpenSBI Xiang W
@ 2024-07-24 6:50 ` Anup Patel
7 siblings, 0 replies; 16+ messages in thread
From: Anup Patel @ 2024-07-24 6:50 UTC (permalink / raw)
To: opensbi
On Fri, Jul 5, 2024 at 12:45?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> This series tries to improve OpenSBI debugging by:
> 1) Registering console device via platform early_init()
> 2) Introducing early console buffer
>
> These patches can also be found in the console_early_probe_v1
> branch at: https://github.com/avpatel/opensbi.git
>
> Anup Patel (6):
> platform: Setup serial console device in early_init()
> lib: sbi: Remove sbi_console_init() and console_init() platform
> callback
> lib: sbi: Optimize fifo enqueue/dequeue for basic data types
> lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
> include: sbi: Add macros to create FIFO as local or global variable
> lib: sbi: Introduce an early console buffer for caching early prints
Applied this series to the riscv/opensbi repo.
Regards,
Anup
>
> include/sbi/sbi_console.h | 2 -
> include/sbi/sbi_fifo.h | 14 +++++-
> include/sbi/sbi_platform.h | 17 -------
> lib/sbi/Kconfig | 6 ++-
> lib/sbi/sbi_console.c | 36 ++++++++++-----
> lib/sbi/sbi_fifo.c | 72 +++++++++++++++++++++++++-----
> lib/sbi/sbi_init.c | 4 --
> lib/sbi/sbi_sse.c | 2 +-
> lib/sbi/sbi_tlb.c | 3 +-
> platform/fpga/ariane/platform.c | 26 ++++-------
> platform/fpga/openpiton/platform.c | 20 ++-------
> platform/generic/platform.c | 21 ++++-----
> platform/kendryte/k210/platform.c | 18 +++-----
> platform/nuclei/ux600/platform.c | 18 +++-----
> platform/template/platform.c | 18 +++-----
> 15 files changed, 151 insertions(+), 126 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread