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

[Anup Patel]

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.

Regards,
Anup