[PATCH v2 1/3] lib: sbi: Introduce IPI device rating
Samuel Holland
samuel.holland at sifive.com
Wed Sep 3 07:56:10 PDT 2025
Hi Anup,
On 2025-09-03 1:52 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 at ventanamicro.com>
> ---
> include/sbi/sbi_ipi.h | 5 ++++-
> lib/sbi/sbi_init.c | 2 +-
> lib/sbi/sbi_ipi.c | 41 ++++++++++++++++++++++++++++++------
> lib/utils/ipi/aclint_mswi.c | 1 +
> lib/utils/ipi/andes_plicsw.c | 1 +
> lib/utils/irqchip/imsic.c | 1 +
> 6 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> index 62d61304..620a95cf 100644
> --- a/include/sbi/sbi_ipi.h
> +++ b/include/sbi/sbi_ipi.h
> @@ -23,6 +23,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 +90,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..d7ba1902 100644
> --- a/lib/sbi/sbi_ipi.c
> +++ b/lib/sbi/sbi_ipi.c
> @@ -15,9 +15,11 @@
> #include <sbi/sbi_domain.h>
> #include <sbi/sbi_error.h>
> #include <sbi/sbi_hart.h>
> +#include <sbi/sbi_heap.h>
> #include <sbi/sbi_hsm.h>
> #include <sbi/sbi_init.h>
> #include <sbi/sbi_ipi.h>
> +#include <sbi/sbi_list.h>
> #include <sbi/sbi_platform.h>
> #include <sbi/sbi_pmu.h>
> #include <sbi/sbi_string.h>
> @@ -32,8 +34,14 @@ _Static_assert(
> "type of sbi_ipi_data.ipi_type has changed, please redefine SBI_IPI_EVENT_MAX"
> );
>
> +struct sbi_ipi_device_node {
> + struct sbi_dlist head;
> + const struct sbi_ipi_device *dev;
> +};
> +
> static unsigned long ipi_data_off;
> static const struct sbi_ipi_device *ipi_dev = NULL;
> +static SBI_LIST_HEAD(ipi_dev_node_list);
> 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 +256,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 +291,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();
> + struct sbi_ipi_device_node *entry;
> +
> + if (all_devices) {
> + sbi_list_for_each_entry(entry, &ipi_dev_node_list, head) {
> + if (entry->dev->ipi_clear)
> + entry->dev->ipi_clear();
> + }
> + } else {
> + if (ipi_dev && ipi_dev->ipi_clear)
> + ipi_dev->ipi_clear();
> + }
>
> /*
> * Ensure that memory or MMIO writes after this
> @@ -307,10 +324,20 @@ const struct sbi_ipi_device *sbi_ipi_get_device(void)
>
> void sbi_ipi_set_device(const struct sbi_ipi_device *dev)
Maybe this should be renamed to sbi_ipi_add_device(), since we can now track
more than one device.
> {
> - if (!dev || ipi_dev)
> + struct sbi_ipi_device_node *entry;
> +
> + if (!dev)
> + return;
> +
> + entry = sbi_zalloc(sizeof(*entry));
> + if (!entry)
> return;
> + SBI_INIT_LIST_HEAD(&entry->head);
> + entry->dev = dev;
> + sbi_list_add_tail(&entry->head, &ipi_dev_node_list);
Why not put the list node directly in `struct sbi_ipi_device`, instead of using
a separate allocation? Or is there a reason why that would not work?
Regards,
Samuel
>
> - 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 +374,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
> };
>
More information about the opensbi
mailing list