Re: [PATCH v4 4/6] lib: utils/irqchip/imsic:migrate to irqchip framework

On Tue, Jul 21, 2026 at 2:38 AM Pawandeep Oza
<[email protected]> wrote:
>
> Rewrite imsic_process_hwirqs() to use container_of() for instance
> lookup, validate ownership via imsic_current_hart_owner(), mask
> IMSIC_TOPEI_ID_MASK on the raw CSR value, and dispatch non-IPI EIIDs
> through sbi_irqchip_process_hwirq() instead of an open-coded switch.
>
> Rewrite imsic_warm_irqchip_init() to resolve the imsic_data instance
> via container_of() from the sbi_irqchip_device pointer, and skip
> initialization silently for harts not owned by this instance.
>
> Extend the sbi_irqchip callback set:
> - hwirq_setup: registers sbi_irqchip_raw_handler_default for the EIID
> - hwirq_cleanup: clears EIE and EIP bits for the EIID on the owner hart
> - hwirq_eoi: no-op as claim/ack is already performed by CSR_MTOPEI
>   read in imsic_process_hwirqs()
> - hwirq_set_affinity: reprograms the upstream MSI source via
>   imsic_program_msi() to retarget the interrupt to the new hart
> - hwirq_mask: clears the EIE bit for the EIID on the owner hart
> - hwirq_unmask: sets the EIE bit for the EIID on the owner hart,
>   skipping IPI (EIID 1) and non-owner harts
>
> Add imsic_compose_msi_msg() to compute the MMIO target address for a
> given hart and EIID by walking the imsic_regs array and applying file
> and guest index offsets.
>
> Add imsic_program_msi() that composes an MSI message
> and delivers it to the upstream interrupt source via
> sbi_irqchip_write_msi(), enabling APLIC TARGET register programming
> from the IMSIC driver.
>
> Migrate imsic_cold_irqchip_init() to the instance-based model:
> preserve any pre-configured target_harts mask set by platform or FDT
> code before applying the imsic_device template, falling back to
> sbi_hartmask_set_all() only when no mask was provided. Register the
> irqchip and reserved EIIDs on the embedded irqchip rather than the
> global singleton.
>
> Signed-off-by: Oza Pawandeep <[email protected]>

LGTM.

Reviewed-by: Anup Patel <[email protected]>

Thanks,
Anup

> ---
>  lib/utils/irqchip/imsic.c | 240 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 222 insertions(+), 18 deletions(-)
>
> diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
> index 2abdc8ef..40bffcc0 100644
> --- a/lib/utils/irqchip/imsic.c
> +++ b/lib/utils/irqchip/imsic.c
> @@ -151,26 +151,43 @@ static int imsic_process_hwirqs(struct sbi_irqchip_device *chip)
>  {
>         struct imsic_data *imsic;
>         ulong mirq;
> +       u32 hwirq;
> +       int rc, ret = 0;
>
>         imsic = container_of(chip, struct imsic_data, irqchip);
>         if (!imsic || !imsic->targets_mmode)
>                 return SBI_EINVAL;
>
> +       if (imsic_get_data(current_hartindex()) != imsic)
> +               return 0;
> +
>         while ((mirq = csr_swap(CSR_MTOPEI, 0))) {
> -               mirq = (mirq >> IMSIC_TOPEI_ID_SHIFT);
> +               hwirq = (mirq >> IMSIC_TOPEI_ID_SHIFT) &
> +                       IMSIC_TOPEI_ID_MASK;
> +
> +               if (hwirq > imsic->num_ids) {
> +                       sbi_printf("%s: invalid hwirq=%lu num_ids=%lu\n",
> +                                       __func__,
> +                                       (unsigned long)hwirq,
> +                                       (unsigned long)imsic->num_ids);
> +                       continue;
> +               }
>
> -               switch (mirq) {
> -               case IMSIC_IPI_ID:
> +               if (hwirq == IMSIC_IPI_ID) {
>                         sbi_ipi_process();
> -                       break;
> -               default:
> -                       sbi_printf("%s: unhandled IRQ%d\n",
> -                                  __func__, (u32)mirq);
> -                       break;
> +                       continue;
> +               }
> +
> +               rc = sbi_irqchip_process_hwirq(chip, hwirq);
> +               if (rc && rc != SBI_ENOENT) {
> +                       sbi_printf("%s: hwirq=%lu failed rc=%d\n",
> +                                       __func__,
> +                                       (unsigned long)hwirq, rc);
> +                       ret = rc;
>                 }
>         }
>
> -       return 0;
> +       return ret;
>  }
>
>  static void imsic_ipi_send(u32 hart_index)
> @@ -360,13 +377,193 @@ static int imsic_hwirq_setup(struct sbi_irqchip_device *chip, u32 hwirq, u32 hwi
>  {
>         if (hwirq_flags != SBI_HWIRQ_FLAGS_NONE)
>                 return SBI_ENOTSUPP;
> +
> +       sbi_irqchip_set_raw_handler(chip, hwirq,
> +                                   sbi_irqchip_raw_handler_default);
> +
> +       return 0;
> +}
> +
> +static void imsic_hwirq_cleanup(struct sbi_irqchip_device *chip, u32 hwirq)
> +{
> +       struct imsic_data *imsic;
> +
> +       if (!chip)
> +               return;
> +
> +       imsic = container_of(chip, struct imsic_data, irqchip);
> +       if (!imsic || !imsic->targets_mmode)
> +               return;
> +
> +       imsic_local_eix_update(hwirq, 1, false, false);
> +       imsic_local_eix_update(hwirq, 1, true, false);
> +}
> +
> +static void imsic_hwirq_eoi(struct sbi_irqchip_device *chip, u32 hwirq)
> +{
> +       /*
> +        * IMSIC interrupt claim/ack is already done by reading CSR_MTOPEI
> +        * in imsic_process_hwirqs(). No extra EOI operation is required.
> +        */
> +}
> +
> +static int imsic_compose_msi_msg(struct imsic_data *imsic,
> +                                u32 hart_index, u32 eiid,
> +                                struct sbi_irqchip_msi_msg *msg)
> +{
> +       struct imsic_regs *regs;
> +       unsigned long reloff;
> +       unsigned long msi_addr;
> +       int file;
> +
> +       if (!imsic || !msg)
> +               return SBI_EINVAL;
> +
> +       if (!eiid || eiid == IMSIC_IPI_ID)
> +               return 0;
> +
> +       file = imsic_get_target_file(hart_index);
> +       if (file < 0) {
> +               sbi_printf("%s: no file for hart=%lu rc=%d\n",
> +                               __func__,
> +                               (unsigned long)hart_index, file);
> +               return file;
> +       }
> +
> +       regs   = &imsic->regs[0];
> +       reloff = file * (1UL << imsic->guest_index_bits) * IMSIC_MMIO_PAGE_SZ;
> +
> +       while (regs->size && regs->size <= reloff) {
> +               reloff -= regs->size;
> +               regs++;
> +       }
> +
> +       if (!regs->size || regs->size <= reloff) {
> +               sbi_printf("%s: no regset for hart=%lu file=%d reloff=0x%lx\n",
> +                          __func__,
> +                          (unsigned long)hart_index, file, reloff);
> +               return SBI_ENODEV;
> +       }
> +
> +       msi_addr = regs->addr + reloff + IMSIC_MMIO_PAGE_LE;
> +       msg->address_lo = (u32)(msi_addr);
> +       msg->address_hi = (u32)(msi_addr >> 32);
> +       msg->data       = eiid;
> +
> +       sbi_printf("%s: hart=%lu file=%d eiid=%lu addr=0x%lx data=%lu\n",
> +                  __func__,
> +                  (unsigned long)hart_index, file,
> +                  (unsigned long)eiid, msi_addr,
> +                  (unsigned long)eiid);
> +
> +       return 0;
> +}
> +
> +static int imsic_program_msi(struct sbi_irqchip_device *chip,
> +                     u32 eiid, u32 hart_index)
> +{
> +       struct imsic_data *imsic;
> +       struct sbi_irqchip_msi_msg msg;
> +       int rc;
> +
> +       if (!chip)
> +               return SBI_EINVAL;
> +
> +       imsic = imsic_get_data(hart_index);
> +       if (!imsic || !imsic->targets_mmode)
> +               return SBI_ENODEV;
> +
> +       rc = imsic_compose_msi_msg(imsic, hart_index, eiid, &msg);
> +       if (rc) {
> +               sbi_printf("%s: compose failed eiid=%lu hart=%lu rc=%d\n",
> +                               __func__,
> +                               (unsigned long)eiid,
> +                               (unsigned long)hart_index, rc);
> +       }
> +
> +       rc = sbi_irqchip_write_msi(chip, eiid, &msg);
> +       if (rc) {
> +               sbi_printf("%s: write_msi failed eiid=%lu hart=%lu rc=%d\n",
> +                               __func__,
> +                               (unsigned long)eiid,
> +                               (unsigned long)hart_index, rc);
> +               return rc;
> +       }
> +
> +       return 0;
> +}
> +
> +static int imsic_hwirq_set_affinity(struct sbi_irqchip_device *chip,
> +                                   u32 hwirq, u32 hart_index)
> +{
> +       struct imsic_data *imsic;
> +       int rc;
> +
> +       if (!chip)
> +               return SBI_EINVAL;
> +
> +       imsic = container_of(chip, struct imsic_data, irqchip);
> +       if (!imsic || !imsic->targets_mmode)
> +               return SBI_EINVAL;
> +
> +       if (!hwirq || hwirq == IMSIC_IPI_ID)
> +               return 0;
> +
> +       if (!sbi_hartmask_test_hartindex(hart_index, &chip->target_harts))
> +               return SBI_EINVAL;
> +
> +       rc = imsic_program_msi(chip, hwirq, hart_index);
> +       if (rc) {
> +               sbi_printf("%s: failed hwirq=%lu hart=%lu rc=%d\n",
> +                               __func__,
> +                               (unsigned long)hwirq,
> +                               (unsigned long)hart_index, rc);
> +               return rc;
> +       }
> +
>         return 0;
>  }
>
> +static void imsic_hwirq_mask(struct sbi_irqchip_device *chip, u32 hwirq)
> +{
> +       struct imsic_data *imsic;
> +
> +       if (!chip)
> +               return;
> +
> +       imsic = container_of(chip, struct imsic_data, irqchip);
> +       if (!imsic || !imsic->targets_mmode)
> +               return;
> +
> +       imsic_local_eix_update(hwirq, 1, false, false);
> +}
> +
> +static void imsic_hwirq_unmask(struct sbi_irqchip_device *chip, u32 hwirq)
> +{
> +       struct imsic_data *imsic;
> +
> +       if (!chip)
> +               return;
> +
> +       imsic = container_of(chip, struct imsic_data, irqchip);
> +       if (!imsic || !imsic->targets_mmode)
> +               return;
> +
> +       if (!hwirq || hwirq == IMSIC_IPI_ID)
> +               return;
> +
> +       imsic_local_eix_update(hwirq, 1, false, true);
> +}
> +
>  static struct sbi_irqchip_device imsic_device = {
> -       .warm_init      = imsic_warm_irqchip_init,
> -       .process_hwirqs = imsic_process_hwirqs,
> -       .hwirq_setup    = imsic_hwirq_setup,
> +       .warm_init              = imsic_warm_irqchip_init,
> +       .process_hwirqs         = imsic_process_hwirqs,
> +       .hwirq_setup            = imsic_hwirq_setup,
> +       .hwirq_cleanup          = imsic_hwirq_cleanup,
> +       .hwirq_eoi              = imsic_hwirq_eoi,
> +       .hwirq_set_affinity     = imsic_hwirq_set_affinity,
> +       .hwirq_mask             = imsic_hwirq_mask,
> +       .hwirq_unmask           = imsic_hwirq_unmask,
>  };
>
>  int imsic_cold_irqchip_init(struct imsic_data *imsic)
> @@ -408,20 +605,27 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)
>                         return rc;
>         }
>
> -       /* Register irqchip device */
> -       imsic->irqchip = imsic_device;
> -       imsic->irqchip.id = imsic->unique_id;
> +       imsic->irqchip          = imsic_device;
> +       imsic->irqchip.id       = imsic->unique_id;
>         imsic_device.caps = SBI_IRQCHIP_CAPS_MSI;
>         imsic->irqchip.num_hwirq = imsic->num_ids + 1;
> +
>         sbi_hartmask_set_all(&imsic->irqchip.target_harts);
> +
> +       /* Register irqchip device */
>         rc = sbi_irqchip_add_device(&imsic->irqchip);
> -       if (rc)
> +       if (rc) {
> +               sbi_printf("%s: sbi_irqchip_add_device failed rc=%d\n",
> +                               __func__, rc);
>                 return rc;
> +       }
>
> -       /* Mark hwirq 0 and IPI hwirq as reserved */
>         rc = sbi_irqchip_register_reserved(&imsic->irqchip, 0, IMSIC_IPI_ID + 1);
> -       if (rc)
> +       if (rc) {
> +               sbi_printf("%s: register_reserved failed rc=%d\n",
> +                               __func__, rc);
>                 return rc;
> +       }
>
>         /* Register IPI device */
>         sbi_ipi_add_device(&imsic_ipi_device);
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi