* [PATCH 0/3] OpenSBI IPI device rating
@ 2025-09-02 12:17 Anup Patel
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Anup Patel @ 2025-09-02 12:17 UTC (permalink / raw)
To: Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi, Anup Patel
Introduce IPI device rating which further allows us to remove platform
specific IPI init and convert IPI drivers as early drivers.
These patches can also found in sbi_ipi_rating_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (3):
lib: sbi: Introduce IPI device rating
include: sbi: Remove platform specific IPI init
lib: utils/ipi: Convert IPI drivers as early drivers
include/sbi/sbi_ipi.h | 6 ++++-
include/sbi/sbi_platform.h | 17 ------------
include/sbi_utils/ipi/fdt_ipi.h | 26 -------------------
lib/sbi/sbi_init.c | 2 +-
lib/sbi/sbi_ipi.c | 33 ++++++++++++++----------
lib/utils/ipi/aclint_mswi.c | 1 +
lib/utils/ipi/andes_plicsw.c | 1 +
lib/utils/ipi/fdt_ipi.c | 22 ----------------
lib/utils/ipi/fdt_ipi_drivers.carray | 3 ---
lib/utils/ipi/fdt_ipi_mswi.c | 2 +-
lib/utils/ipi/fdt_ipi_plicsw.c | 2 +-
lib/utils/ipi/objects.mk | 7 ++---
lib/utils/irqchip/imsic.c | 1 +
platform/fpga/ariane/platform.c | 15 +++++------
platform/generic/openhwgroup/openpiton.c | 13 +++-------
platform/generic/platform.c | 2 --
platform/kendryte/k210/platform.c | 13 +++++-----
platform/nuclei/ux600/platform.c | 11 ++++----
platform/template/platform.c | 16 +++++-------
19 files changed, 60 insertions(+), 133 deletions(-)
delete mode 100644 include/sbi_utils/ipi/fdt_ipi.h
delete mode 100644 lib/utils/ipi/fdt_ipi.c
delete mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
--
2.43.0
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-02 12:17 [PATCH 0/3] OpenSBI IPI device rating Anup Patel
@ 2025-09-02 12:17 ` Anup Patel
2025-09-02 23:41 ` Samuel Holland
2025-09-03 7:00 ` Nick Hu
2025-09-02 12:17 ` [PATCH 2/3] include: sbi: Remove platform specific IPI init Anup Patel
2025-09-02 12:17 ` [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers Anup Patel
2 siblings, 2 replies; 14+ messages in thread
From: Anup Patel @ 2025-09-02 12:17 UTC (permalink / raw)
To: Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi, Anup Patel
A platform can have multiple IPI devices (such as ACLINT MSWI,
AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
the sbi_ipi_set_device() function in correct order and prefer
the first avaiable IPI device which is fragile.
Instead of the above, introdcue IPI device rating and prefer
the highest rated IPI device. This further allows extending
the sbi_ipi_raw_clear() to clear all available IPI devices.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_ipi.h | 6 +++++-
lib/sbi/sbi_init.c | 2 +-
lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
lib/utils/ipi/aclint_mswi.c | 1 +
lib/utils/ipi/andes_plicsw.c | 1 +
lib/utils/irqchip/imsic.c | 1 +
6 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
index 62d61304..0a9ae03d 100644
--- a/include/sbi/sbi_ipi.h
+++ b/include/sbi/sbi_ipi.h
@@ -14,6 +14,7 @@
/* clang-format off */
+#define SBI_IPI_DEVICE_MAX (4)
#define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
/* clang-format on */
@@ -23,6 +24,9 @@ struct sbi_ipi_device {
/** Name of the IPI device */
char name[32];
+ /** Ratings of the IPI device (higher is better) */
+ unsigned long rating;
+
/** Send IPI to a target HART index */
void (*ipi_send)(u32 hart_index);
@@ -87,7 +91,7 @@ void sbi_ipi_process(void);
int sbi_ipi_raw_send(u32 hartindex);
-void sbi_ipi_raw_clear(void);
+void sbi_ipi_raw_clear(bool all_devices);
const struct sbi_ipi_device *sbi_ipi_get_device(void);
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index 84a63748..663b486b 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
if (hstate == SBI_HSM_STATE_SUSPENDED) {
init_warm_resume(scratch, hartid);
} else {
- sbi_ipi_raw_clear();
+ sbi_ipi_raw_clear(true);
init_warm_startup(scratch, hartid);
}
}
diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
index 2de459b0..b57ec652 100644
--- a/lib/sbi/sbi_ipi.c
+++ b/lib/sbi/sbi_ipi.c
@@ -32,8 +32,9 @@ _Static_assert(
"type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
);
-static unsigned long ipi_data_off;
+static unsigned long ipi_data_off, ipi_dev_count;
static const struct sbi_ipi_device *ipi_dev = NULL;
+static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
@@ -248,7 +249,7 @@ void sbi_ipi_process(void)
sbi_scratch_offset_ptr(scratch, ipi_data_off);
sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
- sbi_ipi_raw_clear();
+ sbi_ipi_raw_clear(false);
ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
ipi_event = 0;
@@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
return 0;
}
-void sbi_ipi_raw_clear(void)
+void sbi_ipi_raw_clear(bool all_devices)
{
- if (ipi_dev && ipi_dev->ipi_clear)
- ipi_dev->ipi_clear();
+ int i;
+
+ if (all_devices) {
+ for (i = 0; i < ipi_dev_count; i++) {
+ if (ipi_dev_array[i]->ipi_clear)
+ ipi_dev_array[i]->ipi_clear();
+ }
+ } else {
+ if (ipi_dev && ipi_dev->ipi_clear)
+ ipi_dev->ipi_clear();
+ }
/*
* Ensure that memory or MMIO writes after this
@@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
{
- if (!dev || ipi_dev)
+ if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
return;
+ ipi_dev_array[ipi_dev_count++] = dev;
- ipi_dev = dev;
+ if (!ipi_dev || ipi_dev->rating < dev->rating)
+ ipi_dev = dev;
}
int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
@@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
ipi_data->ipi_type = 0x00;
/* Clear any pending IPIs for the current hart */
- sbi_ipi_raw_clear();
+ sbi_ipi_raw_clear(true);
/* Enable software interrupts */
csr_set(CSR_MIE, MIP_MSIP);
diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
index 9e55078a..23510551 100644
--- a/lib/utils/ipi/aclint_mswi.c
+++ b/lib/utils/ipi/aclint_mswi.c
@@ -62,6 +62,7 @@ static void mswi_ipi_clear(void)
static struct sbi_ipi_device aclint_mswi = {
.name = "aclint-mswi",
+ .rating = 100,
.ipi_send = mswi_ipi_send,
.ipi_clear = mswi_ipi_clear
};
diff --git a/lib/utils/ipi/andes_plicsw.c b/lib/utils/ipi/andes_plicsw.c
index 5d085d85..000fe9fd 100644
--- a/lib/utils/ipi/andes_plicsw.c
+++ b/lib/utils/ipi/andes_plicsw.c
@@ -61,6 +61,7 @@ static void plicsw_ipi_clear(void)
static struct sbi_ipi_device plicsw_ipi = {
.name = "andes_plicsw",
+ .rating = 200,
.ipi_send = plicsw_ipi_send,
.ipi_clear = plicsw_ipi_clear
};
diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
index 057b9fa7..3c150c37 100644
--- a/lib/utils/irqchip/imsic.c
+++ b/lib/utils/irqchip/imsic.c
@@ -199,6 +199,7 @@ static void imsic_ipi_send(u32 hart_index)
static struct sbi_ipi_device imsic_ipi_device = {
.name = "aia-imsic",
+ .rating = 300,
.ipi_send = imsic_ipi_send
};
--
2.43.0
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] include: sbi: Remove platform specific IPI init
2025-09-02 12:17 [PATCH 0/3] OpenSBI IPI device rating Anup Patel
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
@ 2025-09-02 12:17 ` Anup Patel
2025-09-02 23:44 ` Samuel Holland
2025-09-02 12:17 ` [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers Anup Patel
2 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2025-09-02 12:17 UTC (permalink / raw)
To: Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi, Anup Patel
The platform specfic IPI init is not need anymore because using
IPI device rating multiple IPI devices can be registered in any
order as part of the platform specific early init.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_platform.h | 17 -----------------
lib/sbi/sbi_ipi.c | 5 -----
platform/fpga/ariane/platform.c | 15 ++++++---------
platform/generic/openhwgroup/openpiton.c | 13 ++++---------
platform/generic/platform.c | 5 ++++-
platform/kendryte/k210/platform.c | 13 ++++++-------
platform/nuclei/ux600/platform.c | 11 +++++------
platform/template/platform.c | 16 ++++++----------
8 files changed, 31 insertions(+), 64 deletions(-)
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index c6d30080..d75c12de 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -116,9 +116,6 @@ struct sbi_platform_operations {
/** Initialize the platform interrupt controller during cold boot */
int (*irqchip_init)(void);
- /** Initialize IPI during cold boot */
- int (*ipi_init)(void);
-
/** Get tlb flush limit value **/
u64 (*get_tlbr_flush_limit)(void);
@@ -528,20 +525,6 @@ static inline int sbi_platform_irqchip_init(const struct sbi_platform *plat)
return 0;
}
-/**
- * Initialize the platform IPI support during cold boot
- *
- * @param plat pointer to struct sbi_platform
- *
- * @return 0 on success and negative error code on failure
- */
-static inline int sbi_platform_ipi_init(const struct sbi_platform *plat)
-{
- if (plat && sbi_platform_ops(plat)->ipi_init)
- return sbi_platform_ops(plat)->ipi_init();
- return 0;
-}
-
/**
* Initialize the platform timer during cold boot
*
diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
index b57ec652..c9f5d1d1 100644
--- a/lib/sbi/sbi_ipi.c
+++ b/lib/sbi/sbi_ipi.c
@@ -342,11 +342,6 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
if (ret < 0)
return ret;
ipi_halt_event = ret;
-
- /* Initialize platform IPI support */
- ret = sbi_platform_ipi_init(sbi_platform_ptr(scratch));
- if (ret)
- return ret;
} else {
if (!ipi_data_off)
return SBI_ENOMEM;
diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
index 4bc1c5be..4ca9ca94 100644
--- a/platform/fpga/ariane/platform.c
+++ b/platform/fpga/ariane/platform.c
@@ -71,9 +71,15 @@ static struct aclint_mtimer_data mtimer = {
*/
static int ariane_early_init(bool cold_boot)
{
+ int rc;
+
if (!cold_boot)
return 0;
+ rc = aclint_mswi_cold_init(&mswi);
+ if (rc)
+ return rc;
+
return uart8250_init(ARIANE_UART_ADDR,
ARIANE_UART_FREQ,
ARIANE_UART_BAUDRATE,
@@ -107,14 +113,6 @@ static int ariane_irqchip_init(void)
return plic_cold_irqchip_init(&plic);
}
-/*
- * Initialize IPI during cold boot.
- */
-static int ariane_ipi_init(void)
-{
- return aclint_mswi_cold_init(&mswi);
-}
-
/*
* Initialize ariane timer during cold boot.
*/
@@ -130,7 +128,6 @@ const struct sbi_platform_operations platform_ops = {
.early_init = ariane_early_init,
.final_init = ariane_final_init,
.irqchip_init = ariane_irqchip_init,
- .ipi_init = ariane_ipi_init,
.timer_init = ariane_timer_init,
};
diff --git a/platform/generic/openhwgroup/openpiton.c b/platform/generic/openhwgroup/openpiton.c
index 9be7f9a4..e4d7a8b0 100644
--- a/platform/generic/openhwgroup/openpiton.c
+++ b/platform/generic/openhwgroup/openpiton.c
@@ -104,6 +104,10 @@ static int openpiton_early_init(bool cold_boot)
ACLINT_DEFAULT_MTIMECMP_OFFSET;
}
+ rc = aclint_mswi_cold_init(&mswi);
+ if (rc)
+ return rc;
+
return uart8250_init(uart.addr, uart.freq, uart.baud,
OPENPITON_DEFAULT_UART_REG_SHIFT,
OPENPITON_DEFAULT_UART_REG_WIDTH,
@@ -135,14 +139,6 @@ static int openpiton_irqchip_init(void)
return plic_cold_irqchip_init(&plic);
}
-/*
- * Initialize IPI during cold boot.
- */
-static int openpiton_ipi_init(void)
-{
- return aclint_mswi_cold_init(&mswi);
-}
-
/*
* Initialize openpiton timer during cold boot.
*/
@@ -155,7 +151,6 @@ static int openhwgroup_openpiton_platform_init(const void *fdt, int nodeoff, con
{
generic_platform_ops.early_init = openpiton_early_init;
generic_platform_ops.timer_init = openpiton_timer_init;
- generic_platform_ops.ipi_init = openpiton_ipi_init;
generic_platform_ops.irqchip_init = openpiton_irqchip_init;
generic_platform_ops.final_init = openpiton_final_init;
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index 889d6905..8ba6bc11 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -229,6 +229,10 @@ int generic_early_init(bool cold_boot)
return rc;
fdt_driver_init_all(fdt, fdt_early_drivers);
+
+ rc = fdt_ipi_init();
+ if (rc)
+ return rc;
}
return 0;
@@ -337,7 +341,6 @@ struct sbi_platform_operations generic_platform_ops = {
.extensions_init = generic_extensions_init,
.domains_init = generic_domains_init,
.irqchip_init = fdt_irqchip_init,
- .ipi_init = fdt_ipi_init,
.pmu_init = generic_pmu_init,
.pmu_xlate_to_mhpmevent = generic_pmu_xlate_to_mhpmevent,
.get_tlbr_flush_limit = generic_tlbr_flush_limit,
diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
index aff133c6..cd71c795 100644
--- a/platform/kendryte/k210/platform.c
+++ b/platform/kendryte/k210/platform.c
@@ -112,9 +112,15 @@ static struct sbi_system_reset_device k210_reset = {
static int k210_early_init(bool cold_boot)
{
+ int rc;
+
if (!cold_boot)
return 0;
+ rc = aclint_mswi_cold_init(&mswi);
+ if (rc)
+ return rc;
+
sbi_system_reset_add_device(&k210_reset);
return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
@@ -141,11 +147,6 @@ static int k210_irqchip_init(void)
return plic_cold_irqchip_init(&plic);
}
-static int k210_ipi_init(void)
-{
- return aclint_mswi_cold_init(&mswi);
-}
-
static int k210_timer_init(void)
{
return aclint_mtimer_cold_init(&mtimer, NULL);
@@ -158,8 +159,6 @@ const struct sbi_platform_operations platform_ops = {
.irqchip_init = k210_irqchip_init,
- .ipi_init = k210_ipi_init,
-
.timer_init = k210_timer_init,
};
diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
index 1b67b644..ccaf87a4 100644
--- a/platform/nuclei/ux600/platform.c
+++ b/platform/nuclei/ux600/platform.c
@@ -151,10 +151,15 @@ static struct sbi_system_reset_device ux600_reset = {
static int ux600_early_init(bool cold_boot)
{
u32 regval;
+ int rc;
if (!cold_boot)
return 0;
+ rc = aclint_mswi_cold_init(&mswi);
+ if (rc)
+ return rc;
+
sbi_system_reset_add_device(&ux600_reset);
/* Measure CPU Frequency using Timer */
@@ -195,11 +200,6 @@ static int ux600_irqchip_init(void)
return plic_cold_irqchip_init(&plic);
}
-static int ux600_ipi_init(void)
-{
- return aclint_mswi_cold_init(&mswi);
-}
-
static int ux600_timer_init(void)
{
return aclint_mtimer_cold_init(&mtimer, NULL);
@@ -209,7 +209,6 @@ const struct sbi_platform_operations platform_ops = {
.early_init = ux600_early_init,
.final_init = ux600_final_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 292889d2..4ba3e59a 100644
--- a/platform/template/platform.c
+++ b/platform/template/platform.c
@@ -70,9 +70,15 @@ static struct aclint_mtimer_data mtimer = {
*/
static int platform_early_init(bool cold_boot)
{
+ int rc;
+
if (!cold_boot)
return 0;
+ rc = aclint_mswi_cold_init(&mswi);
+ if (rc)
+ return rc;
+
/* Example if the generic UART8250 driver is used */
return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
PLATFORM_UART_BAUDRATE, 0, 1, 0, 0);
@@ -95,15 +101,6 @@ static int platform_irqchip_init(void)
return plic_cold_irqchip_init(&plic);
}
-/*
- * Initialize IPI during cold boot.
- */
-static int platform_ipi_init(void)
-{
- /* Example if the generic ACLINT driver is used */
- return aclint_mswi_cold_init(&mswi);
-}
-
/*
* Initialize platform timer during cold boot.
*/
@@ -120,7 +117,6 @@ const struct sbi_platform_operations platform_ops = {
.early_init = platform_early_init,
.final_init = platform_final_init,
.irqchip_init = platform_irqchip_init,
- .ipi_init = platform_ipi_init,
.timer_init = platform_timer_init
};
const struct sbi_platform platform = {
--
2.43.0
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers
2025-09-02 12:17 [PATCH 0/3] OpenSBI IPI device rating Anup Patel
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
2025-09-02 12:17 ` [PATCH 2/3] include: sbi: Remove platform specific IPI init Anup Patel
@ 2025-09-02 12:17 ` Anup Patel
2025-09-02 23:53 ` Samuel Holland
2 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2025-09-02 12:17 UTC (permalink / raw)
To: Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi, Anup Patel
The fdt_ipi_init() is already called from generic_early_init() so
let's convert IPI drivers as early drivers.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi_utils/ipi/fdt_ipi.h | 26 --------------------------
lib/utils/ipi/fdt_ipi.c | 22 ----------------------
lib/utils/ipi/fdt_ipi_drivers.carray | 3 ---
lib/utils/ipi/fdt_ipi_mswi.c | 2 +-
lib/utils/ipi/fdt_ipi_plicsw.c | 2 +-
lib/utils/ipi/objects.mk | 7 ++-----
platform/generic/platform.c | 5 -----
7 files changed, 4 insertions(+), 63 deletions(-)
delete mode 100644 include/sbi_utils/ipi/fdt_ipi.h
delete mode 100644 lib/utils/ipi/fdt_ipi.c
delete mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
diff --git a/include/sbi_utils/ipi/fdt_ipi.h b/include/sbi_utils/ipi/fdt_ipi.h
deleted file mode 100644
index 9b014470..00000000
--- a/include/sbi_utils/ipi/fdt_ipi.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2020 Western Digital Corporation or its affiliates.
- *
- * Authors:
- * Anup Patel <anup.patel@wdc.com>
- */
-
-#ifndef __FDT_IPI_H__
-#define __FDT_IPI_H__
-
-#include <sbi/sbi_types.h>
-#include <sbi_utils/fdt/fdt_driver.h>
-
-#ifdef CONFIG_FDT_IPI
-
-int fdt_ipi_init(void);
-
-#else
-
-static inline int fdt_ipi_init(void) { return 0; }
-
-#endif
-
-#endif
diff --git a/lib/utils/ipi/fdt_ipi.c b/lib/utils/ipi/fdt_ipi.c
deleted file mode 100644
index 644c9c1c..00000000
--- a/lib/utils/ipi/fdt_ipi.c
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2020 Western Digital Corporation or its affiliates.
- *
- * Authors:
- * Anup Patel <anup.patel@wdc.com>
- */
-
-#include <sbi_utils/ipi/fdt_ipi.h>
-
-/* List of FDT ipi drivers generated at compile time */
-extern const struct fdt_driver *const fdt_ipi_drivers[];
-
-int fdt_ipi_init(void)
-{
- /*
- * On some single-hart system there is no need for IPIs,
- * so do not return a failure if no device is found.
- */
- return fdt_driver_init_all(fdt_get_address(), fdt_ipi_drivers);
-}
diff --git a/lib/utils/ipi/fdt_ipi_drivers.carray b/lib/utils/ipi/fdt_ipi_drivers.carray
deleted file mode 100644
index 006b0fd9..00000000
--- a/lib/utils/ipi/fdt_ipi_drivers.carray
+++ /dev/null
@@ -1,3 +0,0 @@
-HEADER: sbi_utils/ipi/fdt_ipi.h
-TYPE: const struct fdt_driver
-NAME: fdt_ipi_drivers
diff --git a/lib/utils/ipi/fdt_ipi_mswi.c b/lib/utils/ipi/fdt_ipi_mswi.c
index aa37d0d0..20f6fbcc 100644
--- a/lib/utils/ipi/fdt_ipi_mswi.c
+++ b/lib/utils/ipi/fdt_ipi_mswi.c
@@ -9,8 +9,8 @@
#include <sbi/sbi_error.h>
#include <sbi/sbi_heap.h>
+#include <sbi_utils/fdt/fdt_driver.h>
#include <sbi_utils/fdt/fdt_helper.h>
-#include <sbi_utils/ipi/fdt_ipi.h>
#include <sbi_utils/ipi/aclint_mswi.h>
static int ipi_mswi_cold_init(const void *fdt, int nodeoff,
diff --git a/lib/utils/ipi/fdt_ipi_plicsw.c b/lib/utils/ipi/fdt_ipi_plicsw.c
index be669980..e15a6b76 100644
--- a/lib/utils/ipi/fdt_ipi_plicsw.c
+++ b/lib/utils/ipi/fdt_ipi_plicsw.c
@@ -11,8 +11,8 @@
*/
#include <sbi/riscv_io.h>
+#include <sbi_utils/fdt/fdt_driver.h>
#include <sbi_utils/fdt/fdt_helper.h>
-#include <sbi_utils/ipi/fdt_ipi.h>
#include <sbi_utils/ipi/andes_plicsw.h>
extern struct plicsw_data plicsw;
diff --git a/lib/utils/ipi/objects.mk b/lib/utils/ipi/objects.mk
index d1c94af2..9ba8affb 100644
--- a/lib/utils/ipi/objects.mk
+++ b/lib/utils/ipi/objects.mk
@@ -10,11 +10,8 @@
libsbiutils-objs-$(CONFIG_IPI_MSWI) += ipi/aclint_mswi.o
libsbiutils-objs-$(CONFIG_IPI_PLICSW) += ipi/andes_plicsw.o
-libsbiutils-objs-$(CONFIG_FDT_IPI) += ipi/fdt_ipi.o
-libsbiutils-objs-$(CONFIG_FDT_IPI) += ipi/fdt_ipi_drivers.carray.o
-
-carray-fdt_ipi_drivers-$(CONFIG_FDT_IPI_MSWI) += fdt_ipi_mswi
+carray-fdt_early_drivers-$(CONFIG_FDT_IPI_MSWI) += fdt_ipi_mswi
libsbiutils-objs-$(CONFIG_FDT_IPI_MSWI) += ipi/fdt_ipi_mswi.o
-carray-fdt_ipi_drivers-$(CONFIG_FDT_IPI_PLICSW) += fdt_ipi_plicsw
+carray-fdt_early_drivers-$(CONFIG_FDT_IPI_PLICSW) += fdt_ipi_plicsw
libsbiutils-objs-$(CONFIG_FDT_IPI_PLICSW) += ipi/fdt_ipi_plicsw.o
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index 8ba6bc11..91140958 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -22,7 +22,6 @@
#include <sbi_utils/fdt/fdt_fixup.h>
#include <sbi_utils/fdt/fdt_helper.h>
#include <sbi_utils/fdt/fdt_pmu.h>
-#include <sbi_utils/ipi/fdt_ipi.h>
#include <sbi_utils/irqchip/fdt_irqchip.h>
#include <sbi_utils/irqchip/imsic.h>
#include <sbi_utils/mpxy/fdt_mpxy.h>
@@ -229,10 +228,6 @@ int generic_early_init(bool cold_boot)
return rc;
fdt_driver_init_all(fdt, fdt_early_drivers);
-
- rc = fdt_ipi_init();
- if (rc)
- return rc;
}
return 0;
--
2.43.0
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
@ 2025-09-02 23:41 ` Samuel Holland
2025-09-03 6:49 ` Anup Patel
2025-09-03 7:00 ` Nick Hu
1 sibling, 1 reply; 14+ messages in thread
From: Samuel Holland @ 2025-09-02 23:41 UTC (permalink / raw)
To: Anup Patel, Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi
Hi Anup,
On 2025-09-02 7:17 AM, Anup Patel wrote:
> A platform can have multiple IPI devices (such as ACLINT MSWI,
> AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> the sbi_ipi_set_device() function in correct order and prefer
> the first avaiable IPI device which is fragile.
>
> Instead of the above, introdcue IPI device rating and prefer
> the highest rated IPI device. This further allows extending
> the sbi_ipi_raw_clear() to clear all available IPI devices.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_ipi.h | 6 +++++-
> lib/sbi/sbi_init.c | 2 +-
> lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> lib/utils/ipi/aclint_mswi.c | 1 +
> lib/utils/ipi/andes_plicsw.c | 1 +
> lib/utils/irqchip/imsic.c | 1 +
> 6 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> index 62d61304..0a9ae03d 100644
> --- a/include/sbi/sbi_ipi.h
> +++ b/include/sbi/sbi_ipi.h
> @@ -14,6 +14,7 @@
>
> /* clang-format off */
>
> +#define SBI_IPI_DEVICE_MAX (4)
> #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
>
> /* clang-format on */
> @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> /** Name of the IPI device */
> char name[32];
>
> + /** Ratings of the IPI device (higher is better) */
> + unsigned long rating;
> +
> /** Send IPI to a target HART index */
> void (*ipi_send)(u32 hart_index);
>
> @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
>
> int sbi_ipi_raw_send(u32 hartindex);
>
> -void sbi_ipi_raw_clear(void);
> +void sbi_ipi_raw_clear(bool all_devices);
>
> const struct sbi_ipi_device *sbi_ipi_get_device(void);
>
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 84a63748..663b486b 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> if (hstate == SBI_HSM_STATE_SUSPENDED) {
> init_warm_resume(scratch, hartid);
> } else {
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(true);
> init_warm_startup(scratch, hartid);
> }
> }
> diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> index 2de459b0..b57ec652 100644
> --- a/lib/sbi/sbi_ipi.c
> +++ b/lib/sbi/sbi_ipi.c
> @@ -32,8 +32,9 @@ _Static_assert(
> "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> );
>
> -static unsigned long ipi_data_off;
> +static unsigned long ipi_data_off, ipi_dev_count;
> static const struct sbi_ipi_device *ipi_dev = NULL;
> +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
Since the only things we do with the array are append to it and iterate over it,
can we use a linked list here so there is no arbitrary limit?
Regards,
Samuel
> static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
>
> static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> sbi_scratch_offset_ptr(scratch, ipi_data_off);
>
> sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(false);
>
> ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> ipi_event = 0;
> @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> return 0;
> }
>
> -void sbi_ipi_raw_clear(void)
> +void sbi_ipi_raw_clear(bool all_devices)
> {
> - if (ipi_dev && ipi_dev->ipi_clear)
> - ipi_dev->ipi_clear();
> + int i;
> +
> + if (all_devices) {
> + for (i = 0; i < ipi_dev_count; i++) {
> + if (ipi_dev_array[i]->ipi_clear)
> + ipi_dev_array[i]->ipi_clear();
> + }
> + } else {
> + if (ipi_dev && ipi_dev->ipi_clear)
> + ipi_dev->ipi_clear();
> + }
>
> /*
> * Ensure that memory or MMIO writes after this
> @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
>
> void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> {
> - if (!dev || ipi_dev)
> + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> return;
> + ipi_dev_array[ipi_dev_count++] = dev;
>
> - ipi_dev = dev;
> + if (!ipi_dev || ipi_dev->rating < dev->rating)
> + ipi_dev = dev;
> }
>
> int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> ipi_data->ipi_type = 0x00;
>
> /* Clear any pending IPIs for the current hart */
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(true);
>
> /* Enable software interrupts */
> csr_set(CSR_MIE, MIP_MSIP);
> diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
> index 9e55078a..23510551 100644
> --- a/lib/utils/ipi/aclint_mswi.c
> +++ b/lib/utils/ipi/aclint_mswi.c
> @@ -62,6 +62,7 @@ static void mswi_ipi_clear(void)
>
> static struct sbi_ipi_device aclint_mswi = {
> .name = "aclint-mswi",
> + .rating = 100,
> .ipi_send = mswi_ipi_send,
> .ipi_clear = mswi_ipi_clear
> };
> diff --git a/lib/utils/ipi/andes_plicsw.c b/lib/utils/ipi/andes_plicsw.c
> index 5d085d85..000fe9fd 100644
> --- a/lib/utils/ipi/andes_plicsw.c
> +++ b/lib/utils/ipi/andes_plicsw.c
> @@ -61,6 +61,7 @@ static void plicsw_ipi_clear(void)
>
> static struct sbi_ipi_device plicsw_ipi = {
> .name = "andes_plicsw",
> + .rating = 200,
> .ipi_send = plicsw_ipi_send,
> .ipi_clear = plicsw_ipi_clear
> };
> diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
> index 057b9fa7..3c150c37 100644
> --- a/lib/utils/irqchip/imsic.c
> +++ b/lib/utils/irqchip/imsic.c
> @@ -199,6 +199,7 @@ static void imsic_ipi_send(u32 hart_index)
>
> static struct sbi_ipi_device imsic_ipi_device = {
> .name = "aia-imsic",
> + .rating = 300,
> .ipi_send = imsic_ipi_send
> };
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] include: sbi: Remove platform specific IPI init
2025-09-02 12:17 ` [PATCH 2/3] include: sbi: Remove platform specific IPI init Anup Patel
@ 2025-09-02 23:44 ` Samuel Holland
2025-09-03 6:50 ` Anup Patel
0 siblings, 1 reply; 14+ messages in thread
From: Samuel Holland @ 2025-09-02 23:44 UTC (permalink / raw)
To: Anup Patel, Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi
Hi Anup,
On 2025-09-02 7:17 AM, Anup Patel wrote:
> The platform specfic IPI init is not need anymore because using
> IPI device rating multiple IPI devices can be registered in any
> order as part of the platform specific early init.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_platform.h | 17 -----------------
> lib/sbi/sbi_ipi.c | 5 -----
> platform/fpga/ariane/platform.c | 15 ++++++---------
> platform/generic/openhwgroup/openpiton.c | 13 ++++---------
> platform/generic/platform.c | 5 ++++-
> platform/kendryte/k210/platform.c | 13 ++++++-------
> platform/nuclei/ux600/platform.c | 11 +++++------
> platform/template/platform.c | 16 ++++++----------
> 8 files changed, 31 insertions(+), 64 deletions(-)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index c6d30080..d75c12de 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -116,9 +116,6 @@ struct sbi_platform_operations {
> /** Initialize the platform interrupt controller during cold boot */
> int (*irqchip_init)(void);
>
> - /** Initialize IPI during cold boot */
> - int (*ipi_init)(void);
> -
> /** Get tlb flush limit value **/
> u64 (*get_tlbr_flush_limit)(void);
>
> @@ -528,20 +525,6 @@ static inline int sbi_platform_irqchip_init(const struct sbi_platform *plat)
> return 0;
> }
>
> -/**
> - * Initialize the platform IPI support during cold boot
> - *
> - * @param plat pointer to struct sbi_platform
> - *
> - * @return 0 on success and negative error code on failure
> - */
> -static inline int sbi_platform_ipi_init(const struct sbi_platform *plat)
> -{
> - if (plat && sbi_platform_ops(plat)->ipi_init)
> - return sbi_platform_ops(plat)->ipi_init();
> - return 0;
> -}
> -
> /**
> * Initialize the platform timer during cold boot
> *
> diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> index b57ec652..c9f5d1d1 100644
> --- a/lib/sbi/sbi_ipi.c
> +++ b/lib/sbi/sbi_ipi.c
> @@ -342,11 +342,6 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> if (ret < 0)
> return ret;
> ipi_halt_event = ret;
> -
> - /* Initialize platform IPI support */
> - ret = sbi_platform_ipi_init(sbi_platform_ptr(scratch));
> - if (ret)
> - return ret;
> } else {
> if (!ipi_data_off)
> return SBI_ENOMEM;
> diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
> index 4bc1c5be..4ca9ca94 100644
> --- a/platform/fpga/ariane/platform.c
> +++ b/platform/fpga/ariane/platform.c
> @@ -71,9 +71,15 @@ static struct aclint_mtimer_data mtimer = {
> */
> static int ariane_early_init(bool cold_boot)
> {
> + int rc;
> +
> if (!cold_boot)
> return 0;
>
> + rc = aclint_mswi_cold_init(&mswi);
> + if (rc)
> + return rc;
> +
> return uart8250_init(ARIANE_UART_ADDR,
> ARIANE_UART_FREQ,
> ARIANE_UART_BAUDRATE,
I would recommend to keep the serial device initialization first in these
functions to help with any future debugging.
Regards,
Samuel
> @@ -107,14 +113,6 @@ static int ariane_irqchip_init(void)
> return plic_cold_irqchip_init(&plic);
> }
>
> -/*
> - * Initialize IPI during cold boot.
> - */
> -static int ariane_ipi_init(void)
> -{
> - return aclint_mswi_cold_init(&mswi);
> -}
> -
> /*
> * Initialize ariane timer during cold boot.
> */
> @@ -130,7 +128,6 @@ const struct sbi_platform_operations platform_ops = {
> .early_init = ariane_early_init,
> .final_init = ariane_final_init,
> .irqchip_init = ariane_irqchip_init,
> - .ipi_init = ariane_ipi_init,
> .timer_init = ariane_timer_init,
> };
>
> diff --git a/platform/generic/openhwgroup/openpiton.c b/platform/generic/openhwgroup/openpiton.c
> index 9be7f9a4..e4d7a8b0 100644
> --- a/platform/generic/openhwgroup/openpiton.c
> +++ b/platform/generic/openhwgroup/openpiton.c
> @@ -104,6 +104,10 @@ static int openpiton_early_init(bool cold_boot)
> ACLINT_DEFAULT_MTIMECMP_OFFSET;
> }
>
> + rc = aclint_mswi_cold_init(&mswi);
> + if (rc)
> + return rc;
> +
> return uart8250_init(uart.addr, uart.freq, uart.baud,
> OPENPITON_DEFAULT_UART_REG_SHIFT,
> OPENPITON_DEFAULT_UART_REG_WIDTH,
> @@ -135,14 +139,6 @@ static int openpiton_irqchip_init(void)
> return plic_cold_irqchip_init(&plic);
> }
>
> -/*
> - * Initialize IPI during cold boot.
> - */
> -static int openpiton_ipi_init(void)
> -{
> - return aclint_mswi_cold_init(&mswi);
> -}
> -
> /*
> * Initialize openpiton timer during cold boot.
> */
> @@ -155,7 +151,6 @@ static int openhwgroup_openpiton_platform_init(const void *fdt, int nodeoff, con
> {
> generic_platform_ops.early_init = openpiton_early_init;
> generic_platform_ops.timer_init = openpiton_timer_init;
> - generic_platform_ops.ipi_init = openpiton_ipi_init;
> generic_platform_ops.irqchip_init = openpiton_irqchip_init;
> generic_platform_ops.final_init = openpiton_final_init;
>
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index 889d6905..8ba6bc11 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -229,6 +229,10 @@ int generic_early_init(bool cold_boot)
> return rc;
>
> fdt_driver_init_all(fdt, fdt_early_drivers);
> +
> + rc = fdt_ipi_init();
> + if (rc)
> + return rc;
> }
>
> return 0;
> @@ -337,7 +341,6 @@ struct sbi_platform_operations generic_platform_ops = {
> .extensions_init = generic_extensions_init,
> .domains_init = generic_domains_init,
> .irqchip_init = fdt_irqchip_init,
> - .ipi_init = fdt_ipi_init,
> .pmu_init = generic_pmu_init,
> .pmu_xlate_to_mhpmevent = generic_pmu_xlate_to_mhpmevent,
> .get_tlbr_flush_limit = generic_tlbr_flush_limit,
> diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
> index aff133c6..cd71c795 100644
> --- a/platform/kendryte/k210/platform.c
> +++ b/platform/kendryte/k210/platform.c
> @@ -112,9 +112,15 @@ static struct sbi_system_reset_device k210_reset = {
>
> static int k210_early_init(bool cold_boot)
> {
> + int rc;
> +
> if (!cold_boot)
> return 0;
>
> + rc = aclint_mswi_cold_init(&mswi);
> + if (rc)
> + return rc;
> +
> sbi_system_reset_add_device(&k210_reset);
>
> return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
> @@ -141,11 +147,6 @@ static int k210_irqchip_init(void)
> return plic_cold_irqchip_init(&plic);
> }
>
> -static int k210_ipi_init(void)
> -{
> - return aclint_mswi_cold_init(&mswi);
> -}
> -
> static int k210_timer_init(void)
> {
> return aclint_mtimer_cold_init(&mtimer, NULL);
> @@ -158,8 +159,6 @@ const struct sbi_platform_operations platform_ops = {
>
> .irqchip_init = k210_irqchip_init,
>
> - .ipi_init = k210_ipi_init,
> -
> .timer_init = k210_timer_init,
> };
>
> diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
> index 1b67b644..ccaf87a4 100644
> --- a/platform/nuclei/ux600/platform.c
> +++ b/platform/nuclei/ux600/platform.c
> @@ -151,10 +151,15 @@ static struct sbi_system_reset_device ux600_reset = {
> static int ux600_early_init(bool cold_boot)
> {
> u32 regval;
> + int rc;
>
> if (!cold_boot)
> return 0;
>
> + rc = aclint_mswi_cold_init(&mswi);
> + if (rc)
> + return rc;
> +
> sbi_system_reset_add_device(&ux600_reset);
>
> /* Measure CPU Frequency using Timer */
> @@ -195,11 +200,6 @@ static int ux600_irqchip_init(void)
> return plic_cold_irqchip_init(&plic);
> }
>
> -static int ux600_ipi_init(void)
> -{
> - return aclint_mswi_cold_init(&mswi);
> -}
> -
> static int ux600_timer_init(void)
> {
> return aclint_mtimer_cold_init(&mtimer, NULL);
> @@ -209,7 +209,6 @@ const struct sbi_platform_operations platform_ops = {
> .early_init = ux600_early_init,
> .final_init = ux600_final_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 292889d2..4ba3e59a 100644
> --- a/platform/template/platform.c
> +++ b/platform/template/platform.c
> @@ -70,9 +70,15 @@ static struct aclint_mtimer_data mtimer = {
> */
> static int platform_early_init(bool cold_boot)
> {
> + int rc;
> +
> if (!cold_boot)
> return 0;
>
> + rc = aclint_mswi_cold_init(&mswi);
> + if (rc)
> + return rc;
> +
> /* Example if the generic UART8250 driver is used */
> return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
> PLATFORM_UART_BAUDRATE, 0, 1, 0, 0);
> @@ -95,15 +101,6 @@ static int platform_irqchip_init(void)
> return plic_cold_irqchip_init(&plic);
> }
>
> -/*
> - * Initialize IPI during cold boot.
> - */
> -static int platform_ipi_init(void)
> -{
> - /* Example if the generic ACLINT driver is used */
> - return aclint_mswi_cold_init(&mswi);
> -}
> -
> /*
> * Initialize platform timer during cold boot.
> */
> @@ -120,7 +117,6 @@ const struct sbi_platform_operations platform_ops = {
> .early_init = platform_early_init,
> .final_init = platform_final_init,
> .irqchip_init = platform_irqchip_init,
> - .ipi_init = platform_ipi_init,
> .timer_init = platform_timer_init
> };
> const struct sbi_platform platform = {
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers
2025-09-02 12:17 ` [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers Anup Patel
@ 2025-09-02 23:53 ` Samuel Holland
0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2025-09-02 23:53 UTC (permalink / raw)
To: Anup Patel, Atish Patra; +Cc: Andrew Jones, Anup Patel, opensbi
On 2025-09-02 7:17 AM, Anup Patel wrote:
> The fdt_ipi_init() is already called from generic_early_init() so
> let's convert IPI drivers as early drivers.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi_utils/ipi/fdt_ipi.h | 26 --------------------------
> lib/utils/ipi/fdt_ipi.c | 22 ----------------------
> lib/utils/ipi/fdt_ipi_drivers.carray | 3 ---
> lib/utils/ipi/fdt_ipi_mswi.c | 2 +-
> lib/utils/ipi/fdt_ipi_plicsw.c | 2 +-
> lib/utils/ipi/objects.mk | 7 ++-----
> platform/generic/platform.c | 5 -----
> 7 files changed, 4 insertions(+), 63 deletions(-)
> delete mode 100644 include/sbi_utils/ipi/fdt_ipi.h
> delete mode 100644 lib/utils/ipi/fdt_ipi.c
> delete mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-02 23:41 ` Samuel Holland
@ 2025-09-03 6:49 ` Anup Patel
0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2025-09-03 6:49 UTC (permalink / raw)
To: Samuel Holland; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 5:11 AM Samuel Holland <samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-09-02 7:17 AM, Anup Patel wrote:
> > A platform can have multiple IPI devices (such as ACLINT MSWI,
> > AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> > the sbi_ipi_set_device() function in correct order and prefer
> > the first avaiable IPI device which is fragile.
> >
> > Instead of the above, introdcue IPI device rating and prefer
> > the highest rated IPI device. This further allows extending
> > the sbi_ipi_raw_clear() to clear all available IPI devices.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_ipi.h | 6 +++++-
> > lib/sbi/sbi_init.c | 2 +-
> > lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> > lib/utils/ipi/aclint_mswi.c | 1 +
> > lib/utils/ipi/andes_plicsw.c | 1 +
> > lib/utils/irqchip/imsic.c | 1 +
> > 6 files changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> > index 62d61304..0a9ae03d 100644
> > --- a/include/sbi/sbi_ipi.h
> > +++ b/include/sbi/sbi_ipi.h
> > @@ -14,6 +14,7 @@
> >
> > /* clang-format off */
> >
> > +#define SBI_IPI_DEVICE_MAX (4)
> > #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
> >
> > /* clang-format on */
> > @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> > /** Name of the IPI device */
> > char name[32];
> >
> > + /** Ratings of the IPI device (higher is better) */
> > + unsigned long rating;
> > +
> > /** Send IPI to a target HART index */
> > void (*ipi_send)(u32 hart_index);
> >
> > @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
> >
> > int sbi_ipi_raw_send(u32 hartindex);
> >
> > -void sbi_ipi_raw_clear(void);
> > +void sbi_ipi_raw_clear(bool all_devices);
> >
> > const struct sbi_ipi_device *sbi_ipi_get_device(void);
> >
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 84a63748..663b486b 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> > if (hstate == SBI_HSM_STATE_SUSPENDED) {
> > init_warm_resume(scratch, hartid);
> > } else {
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(true);
> > init_warm_startup(scratch, hartid);
> > }
> > }
> > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > index 2de459b0..b57ec652 100644
> > --- a/lib/sbi/sbi_ipi.c
> > +++ b/lib/sbi/sbi_ipi.c
> > @@ -32,8 +32,9 @@ _Static_assert(
> > "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> > );
> >
> > -static unsigned long ipi_data_off;
> > +static unsigned long ipi_data_off, ipi_dev_count;
> > static const struct sbi_ipi_device *ipi_dev = NULL;
> > +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
>
> Since the only things we do with the array are append to it and iterate over it,
> can we use a linked list here so there is no arbitrary limit?
Sure, I will use linked list in v2.
Regards,
Anup
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] include: sbi: Remove platform specific IPI init
2025-09-02 23:44 ` Samuel Holland
@ 2025-09-03 6:50 ` Anup Patel
0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2025-09-03 6:50 UTC (permalink / raw)
To: Samuel Holland; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 5:14 AM Samuel Holland <samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-09-02 7:17 AM, Anup Patel wrote:
> > The platform specfic IPI init is not need anymore because using
> > IPI device rating multiple IPI devices can be registered in any
> > order as part of the platform specific early init.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_platform.h | 17 -----------------
> > lib/sbi/sbi_ipi.c | 5 -----
> > platform/fpga/ariane/platform.c | 15 ++++++---------
> > platform/generic/openhwgroup/openpiton.c | 13 ++++---------
> > platform/generic/platform.c | 5 ++++-
> > platform/kendryte/k210/platform.c | 13 ++++++-------
> > platform/nuclei/ux600/platform.c | 11 +++++------
> > platform/template/platform.c | 16 ++++++----------
> > 8 files changed, 31 insertions(+), 64 deletions(-)
> >
> > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > index c6d30080..d75c12de 100644
> > --- a/include/sbi/sbi_platform.h
> > +++ b/include/sbi/sbi_platform.h
> > @@ -116,9 +116,6 @@ struct sbi_platform_operations {
> > /** Initialize the platform interrupt controller during cold boot */
> > int (*irqchip_init)(void);
> >
> > - /** Initialize IPI during cold boot */
> > - int (*ipi_init)(void);
> > -
> > /** Get tlb flush limit value **/
> > u64 (*get_tlbr_flush_limit)(void);
> >
> > @@ -528,20 +525,6 @@ static inline int sbi_platform_irqchip_init(const struct sbi_platform *plat)
> > return 0;
> > }
> >
> > -/**
> > - * Initialize the platform IPI support during cold boot
> > - *
> > - * @param plat pointer to struct sbi_platform
> > - *
> > - * @return 0 on success and negative error code on failure
> > - */
> > -static inline int sbi_platform_ipi_init(const struct sbi_platform *plat)
> > -{
> > - if (plat && sbi_platform_ops(plat)->ipi_init)
> > - return sbi_platform_ops(plat)->ipi_init();
> > - return 0;
> > -}
> > -
> > /**
> > * Initialize the platform timer during cold boot
> > *
> > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > index b57ec652..c9f5d1d1 100644
> > --- a/lib/sbi/sbi_ipi.c
> > +++ b/lib/sbi/sbi_ipi.c
> > @@ -342,11 +342,6 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > if (ret < 0)
> > return ret;
> > ipi_halt_event = ret;
> > -
> > - /* Initialize platform IPI support */
> > - ret = sbi_platform_ipi_init(sbi_platform_ptr(scratch));
> > - if (ret)
> > - return ret;
> > } else {
> > if (!ipi_data_off)
> > return SBI_ENOMEM;
> > diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
> > index 4bc1c5be..4ca9ca94 100644
> > --- a/platform/fpga/ariane/platform.c
> > +++ b/platform/fpga/ariane/platform.c
> > @@ -71,9 +71,15 @@ static struct aclint_mtimer_data mtimer = {
> > */
> > static int ariane_early_init(bool cold_boot)
> > {
> > + int rc;
> > +
> > if (!cold_boot)
> > return 0;
> >
> > + rc = aclint_mswi_cold_init(&mswi);
> > + if (rc)
> > + return rc;
> > +
> > return uart8250_init(ARIANE_UART_ADDR,
> > ARIANE_UART_FREQ,
> > ARIANE_UART_BAUDRATE,
>
> I would recommend to keep the serial device initialization first in these
> functions to help with any future debugging.
>
Sure, I will update in v2.
Regards,
Anup
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
2025-09-02 23:41 ` Samuel Holland
@ 2025-09-03 7:00 ` Nick Hu
2025-09-03 7:05 ` Anup Patel
2025-09-03 7:13 ` Anup Patel
1 sibling, 2 replies; 14+ messages in thread
From: Nick Hu @ 2025-09-03 7:00 UTC (permalink / raw)
To: Anup Patel; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> A platform can have multiple IPI devices (such as ACLINT MSWI,
> AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> the sbi_ipi_set_device() function in correct order and prefer
> the first avaiable IPI device which is fragile.
>
> Instead of the above, introdcue IPI device rating and prefer
> the highest rated IPI device. This further allows extending
> the sbi_ipi_raw_clear() to clear all available IPI devices.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_ipi.h | 6 +++++-
> lib/sbi/sbi_init.c | 2 +-
> lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> lib/utils/ipi/aclint_mswi.c | 1 +
> lib/utils/ipi/andes_plicsw.c | 1 +
> lib/utils/irqchip/imsic.c | 1 +
> 6 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> index 62d61304..0a9ae03d 100644
> --- a/include/sbi/sbi_ipi.h
> +++ b/include/sbi/sbi_ipi.h
> @@ -14,6 +14,7 @@
>
> /* clang-format off */
>
> +#define SBI_IPI_DEVICE_MAX (4)
> #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
>
> /* clang-format on */
> @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> /** Name of the IPI device */
> char name[32];
>
> + /** Ratings of the IPI device (higher is better) */
> + unsigned long rating;
> +
> /** Send IPI to a target HART index */
> void (*ipi_send)(u32 hart_index);
>
> @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
>
> int sbi_ipi_raw_send(u32 hartindex);
>
> -void sbi_ipi_raw_clear(void);
> +void sbi_ipi_raw_clear(bool all_devices);
>
> const struct sbi_ipi_device *sbi_ipi_get_device(void);
>
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 84a63748..663b486b 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> if (hstate == SBI_HSM_STATE_SUSPENDED) {
> init_warm_resume(scratch, hartid);
> } else {
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(true);
> init_warm_startup(scratch, hartid);
> }
> }
> diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> index 2de459b0..b57ec652 100644
> --- a/lib/sbi/sbi_ipi.c
> +++ b/lib/sbi/sbi_ipi.c
> @@ -32,8 +32,9 @@ _Static_assert(
> "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> );
>
> -static unsigned long ipi_data_off;
> +static unsigned long ipi_data_off, ipi_dev_count;
> static const struct sbi_ipi_device *ipi_dev = NULL;
> +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
> static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
>
> static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> sbi_scratch_offset_ptr(scratch, ipi_data_off);
>
> sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(false);
>
> ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> ipi_event = 0;
> @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> return 0;
> }
>
> -void sbi_ipi_raw_clear(void)
> +void sbi_ipi_raw_clear(bool all_devices)
> {
> - if (ipi_dev && ipi_dev->ipi_clear)
> - ipi_dev->ipi_clear();
> + int i;
> +
> + if (all_devices) {
> + for (i = 0; i < ipi_dev_count; i++) {
> + if (ipi_dev_array[i]->ipi_clear)
> + ipi_dev_array[i]->ipi_clear();
> + }
> + } else {
> + if (ipi_dev && ipi_dev->ipi_clear)
> + ipi_dev->ipi_clear();
> + }
>
> /*
> * Ensure that memory or MMIO writes after this
> @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
>
> void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> {
> - if (!dev || ipi_dev)
> + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> return;
> + ipi_dev_array[ipi_dev_count++] = dev;
>
> - ipi_dev = dev;
> + if (!ipi_dev || ipi_dev->rating < dev->rating)
> + ipi_dev = dev;
> }
>
> int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> ipi_data->ipi_type = 0x00;
>
> /* Clear any pending IPIs for the current hart */
> - sbi_ipi_raw_clear();
> + sbi_ipi_raw_clear(true);
Can we put it to the cold boot path since the init_warmboot() already
cleared the IPI?
>
> /* Enable software interrupts */
> csr_set(CSR_MIE, MIP_MSIP);
> diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
> index 9e55078a..23510551 100644
> --- a/lib/utils/ipi/aclint_mswi.c
> +++ b/lib/utils/ipi/aclint_mswi.c
> @@ -62,6 +62,7 @@ static void mswi_ipi_clear(void)
>
> static struct sbi_ipi_device aclint_mswi = {
> .name = "aclint-mswi",
> + .rating = 100,
> .ipi_send = mswi_ipi_send,
> .ipi_clear = mswi_ipi_clear
> };
> diff --git a/lib/utils/ipi/andes_plicsw.c b/lib/utils/ipi/andes_plicsw.c
> index 5d085d85..000fe9fd 100644
> --- a/lib/utils/ipi/andes_plicsw.c
> +++ b/lib/utils/ipi/andes_plicsw.c
> @@ -61,6 +61,7 @@ static void plicsw_ipi_clear(void)
>
> static struct sbi_ipi_device plicsw_ipi = {
> .name = "andes_plicsw",
> + .rating = 200,
> .ipi_send = plicsw_ipi_send,
> .ipi_clear = plicsw_ipi_clear
> };
> diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
> index 057b9fa7..3c150c37 100644
> --- a/lib/utils/irqchip/imsic.c
> +++ b/lib/utils/irqchip/imsic.c
> @@ -199,6 +199,7 @@ static void imsic_ipi_send(u32 hart_index)
>
> static struct sbi_ipi_device imsic_ipi_device = {
> .name = "aia-imsic",
> + .rating = 300,
> .ipi_send = imsic_ipi_send
> };
>
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-03 7:00 ` Nick Hu
@ 2025-09-03 7:05 ` Anup Patel
2025-09-03 7:13 ` Anup Patel
1 sibling, 0 replies; 14+ messages in thread
From: Anup Patel @ 2025-09-03 7:05 UTC (permalink / raw)
To: Nick Hu; +Cc: Anup Patel, Atish Patra, Andrew Jones, opensbi
On Wed, Sep 3, 2025 at 12:30 PM Nick Hu <nick.hu@sifive.com> wrote:
>
> On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > A platform can have multiple IPI devices (such as ACLINT MSWI,
> > AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> > the sbi_ipi_set_device() function in correct order and prefer
> > the first avaiable IPI device which is fragile.
> >
> > Instead of the above, introdcue IPI device rating and prefer
> > the highest rated IPI device. This further allows extending
> > the sbi_ipi_raw_clear() to clear all available IPI devices.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_ipi.h | 6 +++++-
> > lib/sbi/sbi_init.c | 2 +-
> > lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> > lib/utils/ipi/aclint_mswi.c | 1 +
> > lib/utils/ipi/andes_plicsw.c | 1 +
> > lib/utils/irqchip/imsic.c | 1 +
> > 6 files changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> > index 62d61304..0a9ae03d 100644
> > --- a/include/sbi/sbi_ipi.h
> > +++ b/include/sbi/sbi_ipi.h
> > @@ -14,6 +14,7 @@
> >
> > /* clang-format off */
> >
> > +#define SBI_IPI_DEVICE_MAX (4)
> > #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
> >
> > /* clang-format on */
> > @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> > /** Name of the IPI device */
> > char name[32];
> >
> > + /** Ratings of the IPI device (higher is better) */
> > + unsigned long rating;
> > +
> > /** Send IPI to a target HART index */
> > void (*ipi_send)(u32 hart_index);
> >
> > @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
> >
> > int sbi_ipi_raw_send(u32 hartindex);
> >
> > -void sbi_ipi_raw_clear(void);
> > +void sbi_ipi_raw_clear(bool all_devices);
> >
> > const struct sbi_ipi_device *sbi_ipi_get_device(void);
> >
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 84a63748..663b486b 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> > if (hstate == SBI_HSM_STATE_SUSPENDED) {
> > init_warm_resume(scratch, hartid);
> > } else {
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(true);
> > init_warm_startup(scratch, hartid);
> > }
> > }
> > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > index 2de459b0..b57ec652 100644
> > --- a/lib/sbi/sbi_ipi.c
> > +++ b/lib/sbi/sbi_ipi.c
> > @@ -32,8 +32,9 @@ _Static_assert(
> > "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> > );
> >
> > -static unsigned long ipi_data_off;
> > +static unsigned long ipi_data_off, ipi_dev_count;
> > static const struct sbi_ipi_device *ipi_dev = NULL;
> > +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
> > static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
> >
> > static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> > @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> > sbi_scratch_offset_ptr(scratch, ipi_data_off);
> >
> > sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(false);
> >
> > ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> > ipi_event = 0;
> > @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> > return 0;
> > }
> >
> > -void sbi_ipi_raw_clear(void)
> > +void sbi_ipi_raw_clear(bool all_devices)
> > {
> > - if (ipi_dev && ipi_dev->ipi_clear)
> > - ipi_dev->ipi_clear();
> > + int i;
> > +
> > + if (all_devices) {
> > + for (i = 0; i < ipi_dev_count; i++) {
> > + if (ipi_dev_array[i]->ipi_clear)
> > + ipi_dev_array[i]->ipi_clear();
> > + }
> > + } else {
> > + if (ipi_dev && ipi_dev->ipi_clear)
> > + ipi_dev->ipi_clear();
> > + }
> >
> > /*
> > * Ensure that memory or MMIO writes after this
> > @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
> >
> > void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> > {
> > - if (!dev || ipi_dev)
> > + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> > return;
> > + ipi_dev_array[ipi_dev_count++] = dev;
> >
> > - ipi_dev = dev;
> > + if (!ipi_dev || ipi_dev->rating < dev->rating)
> > + ipi_dev = dev;
> > }
> >
> > int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > ipi_data->ipi_type = 0x00;
> >
> > /* Clear any pending IPIs for the current hart */
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(true);
> Can we put it to the cold boot path since the init_warmboot() already
> cleared the IPI?
sbi_ipi_raw_clear() only clears IPI for the current CPU hence must
be done in init_warmboot(). This also helps when CPU is powered-off
and powered-on.
Regards,
Anup
>
> >
> > /* Enable software interrupts */
> > csr_set(CSR_MIE, MIP_MSIP);
> > diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
> > index 9e55078a..23510551 100644
> > --- a/lib/utils/ipi/aclint_mswi.c
> > +++ b/lib/utils/ipi/aclint_mswi.c
> > @@ -62,6 +62,7 @@ static void mswi_ipi_clear(void)
> >
> > static struct sbi_ipi_device aclint_mswi = {
> > .name = "aclint-mswi",
> > + .rating = 100,
> > .ipi_send = mswi_ipi_send,
> > .ipi_clear = mswi_ipi_clear
> > };
> > diff --git a/lib/utils/ipi/andes_plicsw.c b/lib/utils/ipi/andes_plicsw.c
> > index 5d085d85..000fe9fd 100644
> > --- a/lib/utils/ipi/andes_plicsw.c
> > +++ b/lib/utils/ipi/andes_plicsw.c
> > @@ -61,6 +61,7 @@ static void plicsw_ipi_clear(void)
> >
> > static struct sbi_ipi_device plicsw_ipi = {
> > .name = "andes_plicsw",
> > + .rating = 200,
> > .ipi_send = plicsw_ipi_send,
> > .ipi_clear = plicsw_ipi_clear
> > };
> > diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
> > index 057b9fa7..3c150c37 100644
> > --- a/lib/utils/irqchip/imsic.c
> > +++ b/lib/utils/irqchip/imsic.c
> > @@ -199,6 +199,7 @@ static void imsic_ipi_send(u32 hart_index)
> >
> > static struct sbi_ipi_device imsic_ipi_device = {
> > .name = "aia-imsic",
> > + .rating = 300,
> > .ipi_send = imsic_ipi_send
> > };
> >
> > --
> > 2.43.0
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-03 7:00 ` Nick Hu
2025-09-03 7:05 ` Anup Patel
@ 2025-09-03 7:13 ` Anup Patel
2025-09-03 7:23 ` Anup Patel
1 sibling, 1 reply; 14+ messages in thread
From: Anup Patel @ 2025-09-03 7:13 UTC (permalink / raw)
To: Nick Hu; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 12:30 PM Nick Hu <nick.hu@sifive.com> wrote:
>
> On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > A platform can have multiple IPI devices (such as ACLINT MSWI,
> > AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> > the sbi_ipi_set_device() function in correct order and prefer
> > the first avaiable IPI device which is fragile.
> >
> > Instead of the above, introdcue IPI device rating and prefer
> > the highest rated IPI device. This further allows extending
> > the sbi_ipi_raw_clear() to clear all available IPI devices.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_ipi.h | 6 +++++-
> > lib/sbi/sbi_init.c | 2 +-
> > lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> > lib/utils/ipi/aclint_mswi.c | 1 +
> > lib/utils/ipi/andes_plicsw.c | 1 +
> > lib/utils/irqchip/imsic.c | 1 +
> > 6 files changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> > index 62d61304..0a9ae03d 100644
> > --- a/include/sbi/sbi_ipi.h
> > +++ b/include/sbi/sbi_ipi.h
> > @@ -14,6 +14,7 @@
> >
> > /* clang-format off */
> >
> > +#define SBI_IPI_DEVICE_MAX (4)
> > #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
> >
> > /* clang-format on */
> > @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> > /** Name of the IPI device */
> > char name[32];
> >
> > + /** Ratings of the IPI device (higher is better) */
> > + unsigned long rating;
> > +
> > /** Send IPI to a target HART index */
> > void (*ipi_send)(u32 hart_index);
> >
> > @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
> >
> > int sbi_ipi_raw_send(u32 hartindex);
> >
> > -void sbi_ipi_raw_clear(void);
> > +void sbi_ipi_raw_clear(bool all_devices);
> >
> > const struct sbi_ipi_device *sbi_ipi_get_device(void);
> >
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 84a63748..663b486b 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> > if (hstate == SBI_HSM_STATE_SUSPENDED) {
> > init_warm_resume(scratch, hartid);
> > } else {
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(true);
> > init_warm_startup(scratch, hartid);
> > }
> > }
> > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > index 2de459b0..b57ec652 100644
> > --- a/lib/sbi/sbi_ipi.c
> > +++ b/lib/sbi/sbi_ipi.c
> > @@ -32,8 +32,9 @@ _Static_assert(
> > "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> > );
> >
> > -static unsigned long ipi_data_off;
> > +static unsigned long ipi_data_off, ipi_dev_count;
> > static const struct sbi_ipi_device *ipi_dev = NULL;
> > +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
> > static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
> >
> > static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> > @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> > sbi_scratch_offset_ptr(scratch, ipi_data_off);
> >
> > sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(false);
> >
> > ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> > ipi_event = 0;
> > @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> > return 0;
> > }
> >
> > -void sbi_ipi_raw_clear(void)
> > +void sbi_ipi_raw_clear(bool all_devices)
> > {
> > - if (ipi_dev && ipi_dev->ipi_clear)
> > - ipi_dev->ipi_clear();
> > + int i;
> > +
> > + if (all_devices) {
> > + for (i = 0; i < ipi_dev_count; i++) {
> > + if (ipi_dev_array[i]->ipi_clear)
> > + ipi_dev_array[i]->ipi_clear();
> > + }
> > + } else {
> > + if (ipi_dev && ipi_dev->ipi_clear)
> > + ipi_dev->ipi_clear();
> > + }
> >
> > /*
> > * Ensure that memory or MMIO writes after this
> > @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
> >
> > void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> > {
> > - if (!dev || ipi_dev)
> > + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> > return;
> > + ipi_dev_array[ipi_dev_count++] = dev;
> >
> > - ipi_dev = dev;
> > + if (!ipi_dev || ipi_dev->rating < dev->rating)
> > + ipi_dev = dev;
> > }
> >
> > int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > ipi_data->ipi_type = 0x00;
> >
> > /* Clear any pending IPIs for the current hart */
> > - sbi_ipi_raw_clear();
> > + sbi_ipi_raw_clear(true);
> Can we put it to the cold boot path since the init_warmboot() already
> cleared the IPI?
I think you are pointing at the sbi_ipi_raw_clear() just before
init_warmboot() in sbi_init.c.
Let me recall why it was added there. Anyway it's a separate
change so should be a separate patch.
Regards,
Anup
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-03 7:13 ` Anup Patel
@ 2025-09-03 7:23 ` Anup Patel
2025-09-03 10:36 ` Nick Hu
0 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2025-09-03 7:23 UTC (permalink / raw)
To: Nick Hu; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 12:43 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Wed, Sep 3, 2025 at 12:30 PM Nick Hu <nick.hu@sifive.com> wrote:
> >
> > On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > A platform can have multiple IPI devices (such as ACLINT MSWI,
> > > AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> > > the sbi_ipi_set_device() function in correct order and prefer
> > > the first avaiable IPI device which is fragile.
> > >
> > > Instead of the above, introdcue IPI device rating and prefer
> > > the highest rated IPI device. This further allows extending
> > > the sbi_ipi_raw_clear() to clear all available IPI devices.
> > >
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > > include/sbi/sbi_ipi.h | 6 +++++-
> > > lib/sbi/sbi_init.c | 2 +-
> > > lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> > > lib/utils/ipi/aclint_mswi.c | 1 +
> > > lib/utils/ipi/andes_plicsw.c | 1 +
> > > lib/utils/irqchip/imsic.c | 1 +
> > > 6 files changed, 29 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> > > index 62d61304..0a9ae03d 100644
> > > --- a/include/sbi/sbi_ipi.h
> > > +++ b/include/sbi/sbi_ipi.h
> > > @@ -14,6 +14,7 @@
> > >
> > > /* clang-format off */
> > >
> > > +#define SBI_IPI_DEVICE_MAX (4)
> > > #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
> > >
> > > /* clang-format on */
> > > @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> > > /** Name of the IPI device */
> > > char name[32];
> > >
> > > + /** Ratings of the IPI device (higher is better) */
> > > + unsigned long rating;
> > > +
> > > /** Send IPI to a target HART index */
> > > void (*ipi_send)(u32 hart_index);
> > >
> > > @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
> > >
> > > int sbi_ipi_raw_send(u32 hartindex);
> > >
> > > -void sbi_ipi_raw_clear(void);
> > > +void sbi_ipi_raw_clear(bool all_devices);
> > >
> > > const struct sbi_ipi_device *sbi_ipi_get_device(void);
> > >
> > > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > > index 84a63748..663b486b 100644
> > > --- a/lib/sbi/sbi_init.c
> > > +++ b/lib/sbi/sbi_init.c
> > > @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> > > if (hstate == SBI_HSM_STATE_SUSPENDED) {
> > > init_warm_resume(scratch, hartid);
> > > } else {
> > > - sbi_ipi_raw_clear();
> > > + sbi_ipi_raw_clear(true);
> > > init_warm_startup(scratch, hartid);
> > > }
> > > }
> > > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > > index 2de459b0..b57ec652 100644
> > > --- a/lib/sbi/sbi_ipi.c
> > > +++ b/lib/sbi/sbi_ipi.c
> > > @@ -32,8 +32,9 @@ _Static_assert(
> > > "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> > > );
> > >
> > > -static unsigned long ipi_data_off;
> > > +static unsigned long ipi_data_off, ipi_dev_count;
> > > static const struct sbi_ipi_device *ipi_dev = NULL;
> > > +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
> > > static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
> > >
> > > static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> > > @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> > > sbi_scratch_offset_ptr(scratch, ipi_data_off);
> > >
> > > sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> > > - sbi_ipi_raw_clear();
> > > + sbi_ipi_raw_clear(false);
> > >
> > > ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> > > ipi_event = 0;
> > > @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> > > return 0;
> > > }
> > >
> > > -void sbi_ipi_raw_clear(void)
> > > +void sbi_ipi_raw_clear(bool all_devices)
> > > {
> > > - if (ipi_dev && ipi_dev->ipi_clear)
> > > - ipi_dev->ipi_clear();
> > > + int i;
> > > +
> > > + if (all_devices) {
> > > + for (i = 0; i < ipi_dev_count; i++) {
> > > + if (ipi_dev_array[i]->ipi_clear)
> > > + ipi_dev_array[i]->ipi_clear();
> > > + }
> > > + } else {
> > > + if (ipi_dev && ipi_dev->ipi_clear)
> > > + ipi_dev->ipi_clear();
> > > + }
> > >
> > > /*
> > > * Ensure that memory or MMIO writes after this
> > > @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
> > >
> > > void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> > > {
> > > - if (!dev || ipi_dev)
> > > + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> > > return;
> > > + ipi_dev_array[ipi_dev_count++] = dev;
> > >
> > > - ipi_dev = dev;
> > > + if (!ipi_dev || ipi_dev->rating < dev->rating)
> > > + ipi_dev = dev;
> > > }
> > >
> > > int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > > @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > > ipi_data->ipi_type = 0x00;
> > >
> > > /* Clear any pending IPIs for the current hart */
> > > - sbi_ipi_raw_clear();
> > > + sbi_ipi_raw_clear(true);
> > Can we put it to the cold boot path since the init_warmboot() already
> > cleared the IPI?
>
> I think you are pointing at the sbi_ipi_raw_clear() just before
> init_warmboot() in sbi_init.c.
>
> Let me recall why it was added there. Anyway it's a separate
> change so should be a separate patch.
>
I recall now. The sbi_hsm_hart_wait() does WFI so we have to:
1) Clear IPI before entering the wait loop in sbi_hsm_hart_wait()
because there should not be any stale IPI from the previous booting
stage when HART enters OpenSBI in the warm-boot path. This is
why we have sbi_ipi_raw_clear() before init_warm_startup().
2) Clear IPI after coming out of wait loop in sbi_hsm_hart_wait()
because some other HART might have started current HART using
M-mode IPI which is why we have sbi_ipi_raw_clear() in sbi_ipi_init()
in the warm-boot path.
Regards,
Anup
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: sbi: Introduce IPI device rating
2025-09-03 7:23 ` Anup Patel
@ 2025-09-03 10:36 ` Nick Hu
0 siblings, 0 replies; 14+ messages in thread
From: Nick Hu @ 2025-09-03 10:36 UTC (permalink / raw)
To: Anup Patel; +Cc: Atish Patra, Andrew Jones, Anup Patel, opensbi
On Wed, Sep 3, 2025 at 3:24 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Wed, Sep 3, 2025 at 12:43 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Wed, Sep 3, 2025 at 12:30 PM Nick Hu <nick.hu@sifive.com> wrote:
> > >
> > > On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > A platform can have multiple IPI devices (such as ACLINT MSWI,
> > > > AIA IMSIC, etc). Currently, OpenSBI rely on platform calling
> > > > the sbi_ipi_set_device() function in correct order and prefer
> > > > the first avaiable IPI device which is fragile.
> > > >
> > > > Instead of the above, introdcue IPI device rating and prefer
> > > > the highest rated IPI device. This further allows extending
> > > > the sbi_ipi_raw_clear() to clear all available IPI devices.
> > > >
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > ---
> > > > include/sbi/sbi_ipi.h | 6 +++++-
> > > > lib/sbi/sbi_init.c | 2 +-
> > > > lib/sbi/sbi_ipi.c | 28 ++++++++++++++++++++--------
> > > > lib/utils/ipi/aclint_mswi.c | 1 +
> > > > lib/utils/ipi/andes_plicsw.c | 1 +
> > > > lib/utils/irqchip/imsic.c | 1 +
> > > > 6 files changed, 29 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> > > > index 62d61304..0a9ae03d 100644
> > > > --- a/include/sbi/sbi_ipi.h
> > > > +++ b/include/sbi/sbi_ipi.h
> > > > @@ -14,6 +14,7 @@
> > > >
> > > > /* clang-format off */
> > > >
> > > > +#define SBI_IPI_DEVICE_MAX (4)
> > > > #define SBI_IPI_EVENT_MAX (8 * __SIZEOF_LONG__)
> > > >
> > > > /* clang-format on */
> > > > @@ -23,6 +24,9 @@ struct sbi_ipi_device {
> > > > /** Name of the IPI device */
> > > > char name[32];
> > > >
> > > > + /** Ratings of the IPI device (higher is better) */
> > > > + unsigned long rating;
> > > > +
> > > > /** Send IPI to a target HART index */
> > > > void (*ipi_send)(u32 hart_index);
> > > >
> > > > @@ -87,7 +91,7 @@ void sbi_ipi_process(void);
> > > >
> > > > int sbi_ipi_raw_send(u32 hartindex);
> > > >
> > > > -void sbi_ipi_raw_clear(void);
> > > > +void sbi_ipi_raw_clear(bool all_devices);
> > > >
> > > > const struct sbi_ipi_device *sbi_ipi_get_device(void);
> > > >
> > > > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > > > index 84a63748..663b486b 100644
> > > > --- a/lib/sbi/sbi_init.c
> > > > +++ b/lib/sbi/sbi_init.c
> > > > @@ -507,7 +507,7 @@ static void __noreturn init_warmboot(struct sbi_scratch *scratch, u32 hartid)
> > > > if (hstate == SBI_HSM_STATE_SUSPENDED) {
> > > > init_warm_resume(scratch, hartid);
> > > > } else {
> > > > - sbi_ipi_raw_clear();
> > > > + sbi_ipi_raw_clear(true);
> > > > init_warm_startup(scratch, hartid);
> > > > }
> > > > }
> > > > diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> > > > index 2de459b0..b57ec652 100644
> > > > --- a/lib/sbi/sbi_ipi.c
> > > > +++ b/lib/sbi/sbi_ipi.c
> > > > @@ -32,8 +32,9 @@ _Static_assert(
> > > > "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> > > > );
> > > >
> > > > -static unsigned long ipi_data_off;
> > > > +static unsigned long ipi_data_off, ipi_dev_count;
> > > > static const struct sbi_ipi_device *ipi_dev = NULL;
> > > > +static const struct sbi_ipi_device *ipi_dev_array[SBI_IPI_DEVICE_MAX];
> > > > static const struct sbi_ipi_event_ops *ipi_ops_array[SBI_IPI_EVENT_MAX];
> > > >
> > > > static int sbi_ipi_send(struct sbi_scratch *scratch, u32 remote_hartindex,
> > > > @@ -248,7 +249,7 @@ void sbi_ipi_process(void)
> > > > sbi_scratch_offset_ptr(scratch, ipi_data_off);
> > > >
> > > > sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
> > > > - sbi_ipi_raw_clear();
> > > > + sbi_ipi_raw_clear(false);
> > > >
> > > > ipi_type = atomic_raw_xchg_ulong(&ipi_data->ipi_type, 0);
> > > > ipi_event = 0;
> > > > @@ -283,10 +284,19 @@ int sbi_ipi_raw_send(u32 hartindex)
> > > > return 0;
> > > > }
> > > >
> > > > -void sbi_ipi_raw_clear(void)
> > > > +void sbi_ipi_raw_clear(bool all_devices)
> > > > {
> > > > - if (ipi_dev && ipi_dev->ipi_clear)
> > > > - ipi_dev->ipi_clear();
> > > > + int i;
> > > > +
> > > > + if (all_devices) {
> > > > + for (i = 0; i < ipi_dev_count; i++) {
> > > > + if (ipi_dev_array[i]->ipi_clear)
> > > > + ipi_dev_array[i]->ipi_clear();
> > > > + }
> > > > + } else {
> > > > + if (ipi_dev && ipi_dev->ipi_clear)
> > > > + ipi_dev->ipi_clear();
> > > > + }
> > > >
> > > > /*
> > > > * Ensure that memory or MMIO writes after this
> > > > @@ -307,10 +317,12 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
> > > >
> > > > void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
> > > > {
> > > > - if (!dev || ipi_dev)
> > > > + if (!dev || ipi_dev_count >= SBI_IPI_DEVICE_MAX)
> > > > return;
> > > > + ipi_dev_array[ipi_dev_count++] = dev;
> > > >
> > > > - ipi_dev = dev;
> > > > + if (!ipi_dev || ipi_dev->rating < dev->rating)
> > > > + ipi_dev = dev;
> > > > }
> > > >
> > > > int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > > > @@ -347,7 +359,7 @@ int sbi_ipi_init(struct sbi_scratch *scratch, bool cold_boot)
> > > > ipi_data->ipi_type = 0x00;
> > > >
> > > > /* Clear any pending IPIs for the current hart */
> > > > - sbi_ipi_raw_clear();
> > > > + sbi_ipi_raw_clear(true);
> > > Can we put it to the cold boot path since the init_warmboot() already
> > > cleared the IPI?
> >
> > I think you are pointing at the sbi_ipi_raw_clear() just before
> > init_warmboot() in sbi_init.c.
> >
> > Let me recall why it was added there. Anyway it's a separate
> > change so should be a separate patch.
> >
>
> I recall now. The sbi_hsm_hart_wait() does WFI so we have to:
>
> 1) Clear IPI before entering the wait loop in sbi_hsm_hart_wait()
> because there should not be any stale IPI from the previous booting
> stage when HART enters OpenSBI in the warm-boot path. This is
> why we have sbi_ipi_raw_clear() before init_warm_startup().
>
> 2) Clear IPI after coming out of wait loop in sbi_hsm_hart_wait()
> because some other HART might have started current HART using
> M-mode IPI which is why we have sbi_ipi_raw_clear() in sbi_ipi_init()
> in the warm-boot path.
>
That makes sense. Thank you for the clarification.
> Regards,
> Anup
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-09-03 11:45 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-02 12:17 [PATCH 0/3] OpenSBI IPI device rating Anup Patel
2025-09-02 12:17 ` [PATCH 1/3] lib: sbi: Introduce " Anup Patel
2025-09-02 23:41 ` Samuel Holland
2025-09-03 6:49 ` Anup Patel
2025-09-03 7:00 ` Nick Hu
2025-09-03 7:05 ` Anup Patel
2025-09-03 7:13 ` Anup Patel
2025-09-03 7:23 ` Anup Patel
2025-09-03 10:36 ` Nick Hu
2025-09-02 12:17 ` [PATCH 2/3] include: sbi: Remove platform specific IPI init Anup Patel
2025-09-02 23:44 ` Samuel Holland
2025-09-03 6:50 ` Anup Patel
2025-09-02 12:17 ` [PATCH 3/3] lib: utils/ipi: Convert IPI drivers as early drivers Anup Patel
2025-09-02 23:53 ` Samuel Holland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox