[PATCH 1/3] lib: sbi: Introduce IPI device rating

[Samuel Holland]

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 at 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
>  };
>