[PATCH 1/3] lib: sbi: Introduce IPI device rating
Anup Patel
apatel at ventanamicro.com
Wed Sep 3 00:23:49 PDT 2025
On Wed, Sep 3, 2025 at 12:43 PM Anup Patel <apatel at ventanamicro.com> wrote:
>
> On Wed, Sep 3, 2025 at 12:30 PM Nick Hu <nick.hu at sifive.com> wrote:
> >
> > On Wed, Sep 3, 2025 at 12:08 AM Anup Patel <apatel at 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 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];
> > > 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
More information about the opensbi
mailing list