[PATCH 5/8] lib: sbi_irqchip: Support irqchip device targetting subset of harts
Raymond Mao
raymondmaoca at gmail.com
Wed Feb 11 09:49:59 PST 2026
Hi Anup,
On Wed, Feb 11, 2026 at 10:43 AM Anup Patel <anup at brainfault.org> wrote:
>
> On Wed, Feb 11, 2026 at 8:32 PM Raymond Mao <raymondmaoca at gmail.com> wrote:
> >
> > Hi Anup,
> >
> > On Tue, Feb 10, 2026 at 11:09 AM Anup Patel <anup at brainfault.org> wrote:
> > >
> > > On Tue, Feb 10, 2026 at 1:55 AM Raymond Mao <raymondmaoca at gmail.com> wrote:
> > > >
> > > > Hi Anup,
> > > >
> > > > On 2026-02-07 5:25 a.m., anup.patel at oss.qualcomm.com (Anup Patel) wrote:
> > > > > It is possible to have platform where an irqchip device targets
> > > > > a subset of harts and there are multiple irqchip devices to cover
> > > > > all harts.
> > > > >
> > > > > To support this scenario:
> > > > > 1) Add target_harts hartmask to struct sbi_irqchip_device which
> > > > > represents the set of harts targetted by the irqchip device
> > > > > 2) Call warm_init() and irq_handle() callbacks of an irqchip device
> > > > > on a hart only if irqchip device targets that particular hart
> > > > >
> > > > > Signed-off-by: Anup Patel <anup.patel at oss.qualcomm.com>
> > > > > ---
> > > > > include/sbi/sbi_irqchip.h | 8 +++++--
> > > > > lib/sbi/sbi_irqchip.c | 48 ++++++++++++++++++++++++++++-----------
> > > > > lib/utils/irqchip/aplic.c | 12 +++++++++-
> > > > > lib/utils/irqchip/imsic.c | 7 ++++--
> > > > > lib/utils/irqchip/plic.c | 5 ++--
> > > > > 5 files changed, 59 insertions(+), 21 deletions(-)
> > > > >
> > > > > diff --git a/include/sbi/sbi_irqchip.h b/include/sbi/sbi_irqchip.h
> > > > > index cda1e50f..c3ded271 100644
> > > > > --- a/include/sbi/sbi_irqchip.h
> > > > > +++ b/include/sbi/sbi_irqchip.h
> > > > > @@ -10,6 +10,7 @@
> > > > > #ifndef __SBI_IRQCHIP_H__
> > > > > #define __SBI_IRQCHIP_H__
> > > > >
> > > > > +#include <sbi/sbi_hartmask.h>
> > > > > #include <sbi/sbi_list.h>
> > > > > #include <sbi/sbi_types.h>
> > > > >
> > > > > @@ -20,11 +21,14 @@ struct sbi_irqchip_device {
> > > > > /** Node in the list of irqchip devices */
> > > > > struct sbi_dlist node;
> > > > >
> > > > > + /** Set of harts targetted by this irqchip */
> > > > > + struct sbi_hartmask target_harts;
> > > > > +
> > > > > /** Initialize per-hart state for the current hart */
> > > > > int (*warm_init)(struct sbi_irqchip_device *chip);
> > > > >
> > > > > /** Process hardware interrupts from this irqchip */
> > > > > - int (*process_hwirqs)(void);
> > > > > + int (*process_hwirqs)(struct sbi_irqchip_device *chip);
> > > > > };
> > > > >
> > > > > /**
> > > > > @@ -38,7 +42,7 @@ struct sbi_irqchip_device {
> > > > > int sbi_irqchip_process(void);
> > > > >
> > > > > /** Register an irqchip device to receive callbacks */
> > > > > -void sbi_irqchip_add_device(struct sbi_irqchip_device *chip);
> > > > > +int sbi_irqchip_add_device(struct sbi_irqchip_device *chip);
> > > > >
> > > > > /** Initialize interrupt controllers */
> > > > > int sbi_irqchip_init(struct sbi_scratch *scratch, bool cold_boot);
> > > > > diff --git a/lib/sbi/sbi_irqchip.c b/lib/sbi/sbi_irqchip.c
> > > > > index 3b970527..77ec05af 100644
> > > > > --- a/lib/sbi/sbi_irqchip.c
> > > > > +++ b/lib/sbi/sbi_irqchip.c
> > > > > @@ -13,24 +13,44 @@
> > > > >
> > > > > static SBI_LIST_HEAD(irqchip_list);
> > > > >
> > > > > -static int default_irqfn(void)
> > > > > +int sbi_irqchip_process(void)
> > > > > {
> > > > > - return SBI_ENODEV;
> > > > > -}
> > > > > + struct sbi_irqchip_device *chip;
> > > > > + int rc = SBI_ENODEV;
> > > > >
> > > > > -static int (*ext_irqfn)(void) = default_irqfn;
> > > > > + sbi_list_for_each_entry(chip, &irqchip_list, node) {
> > > > > + if (!chip->process_hwirqs)
> > > > > + continue;
> > > > > + if (!sbi_hartmask_test_hartindex(current_hartindex(), &chip->target_harts))
> > > > > + continue;
> > > > > + rc = chip->process_hwirqs(chip);
> > > > > + if (rc)
> > > > > + break;
> > > > > + }
> > > > >
> > > > > -int sbi_irqchip_process(void)
> > > > > -{
> > > > > - return ext_irqfn();
> > > > > + return rc;
> > > > > }
> > > > >
> > > > > -void sbi_irqchip_add_device(struct sbi_irqchip_device *chip)
> > > > > +int sbi_irqchip_add_device(struct sbi_irqchip_device *chip)
> > > > > {
> > > > > - sbi_list_add_tail(&chip->node, &irqchip_list);
> > > > > + struct sbi_irqchip_device *c;
> > > > > + struct sbi_hartmask hm;
> > > > > +
> > > > > + if (!chip || !sbi_hartmask_weight(&chip->target_harts))
> > > > > + return SBI_EINVAL;
> > > > > +
> > > > > + if (chip->process_hwirqs) {
> > > > > + sbi_list_for_each_entry(c, &irqchip_list, node) {
> > > > > + if (!c->process_hwirqs)
> > > > > + continue;
> > > > > + sbi_hartmask_and(&hm, &c->target_harts, &chip->target_harts);
> > > > > + if (sbi_hartmask_weight(&hm))
> > > > > + return SBI_EINVAL;
> > > >
> > > > I tested this by registering a 'process_hwirqs' hook, but
> > > > 'sbi_hartmask_weight' returns errors.
> > >
> > > This loop ensures that irqchips which directly connect
> > > to external interrupt line of harts (aka irqchips which
> > > implement process_hwirqs) don't have overlapping
> > > target_harts.
> > >
> >
> > If my understanding is correct, this means only single process_hwirqs
> > can be registered for a target hart - if IMSIC registered it then it
> > rejects when APLIC tries to register its own process_hwirqs.
>
> That's right.
>
> APLIC must register its own process_hwirqs() only when it is
> in DIRECT mode.
>
> On platforms with IMSIC+APLIC, the APLIC will be in MSI mode
> so in this case APLIC must not register process_hwirqs().
>
> > But according to our use case, each irqchip should be allowed to
> > register its own operation hooks, including process_hwirqs.
> > Then the IMSIC's process_hwirqs handles MSI, while the APLIC's
> > process_hwirqs handles wired ext interrupt, they don't overlap each
> > other.
> > I would suggest allowing multiple process_hwirqs hooks for one target hart.
>
> Your understanding is not correct. Please see the above comment.
>
> In your use-case, you have APLIC in DIRECT mode without any
> IMSIC.
>
In APLIC, I can add a condition:
```
if (!aplic->has_msicfg_mmode && !aplic->has_msicfg_smode)
aplic->irqchip.process_hwirqs = aplic_process_hwirqs,
```
to make sure process_hwirqs is registered when there are no IMSIC configs.
But in IMSIC side, we need to check the existence of an IMISC node
before registering process_hwirqs.
For example, when running QEMU with `-M virt,aia=aplic`, there is no
IMSIC node and it should skip registering process_hwirqs, but I think
this part is missing at the moment.
Regards,
Raymond
> Regards,
> Anup
More information about the opensbi
mailing list