imsic_init() is introduced to parse device tree node, which has the following
bindings [2], and based on the parsed information update IMSIC configuration
which is stored in imsic_cfg.

The following helpers are introduces for imsic_init() usage:
  - imsic_parse_node() parses IMSIC node from DTS
  - imsic_get_parent_cpuid() returns the hart ( CPU ) ID of the given device
    tree node.

This patch is based on the code from [1].

Since Microchip originally developed imsic.{c,h}, an internal discussion with
them led to the decision to use the MIT license.

[1] https://gitlab.com/xen-project/people/olkur/xen/-/commit/0b1a94f2bc3bb1a81cd26bb75f0bf578f84cb4d4
[2] https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml

Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
 - Drop years in copyrights.
 - s/riscv_of_processor_hartid/dt_processor_cpuid.
 - s/imsic_get_parent_hartid/imsic_get_parent_cpuid.
   Rename argument hartid to cpuid.
   Make node argument const.
   Return res instead of -EINVAL for the failure case of dt_processor_cpuid().
   Drop local variable hart and use cpuid argument instead.
   Drop useless return res;
 - imsic_parse_node() changes:
   - Make node argument const.
   - Check the return value of dt_property_read_u32() directly instead of
     saving it to rc variable.
   - Update tmp usage, use short form "-=".
   - Update a check (imsic_cfg.nr_ids >= IMSIC_MAX_ID) to (imsic_cfg.nr_ids > IMSIC_MAX_ID)
     as IMSIC_MAX_ID is changed to maximum valid value, not just the firsr out-of-range.
   - Use `rc` to return value instead of explicitly use -EINVAL.
   - Use do {} while() to find number of MMIO register sets.
 - Set IMSIC_MAX_ID to 2047 (maximum possible IRQ number).
 - imsic_init() changes:
   - Use unsigned int in for's expression1.
   - s/xfree/XFEE.
   - Allocate msi and cpus array dynamically.
 - Drop forward declaration before declaration of imsic_get_config() in asm/imsic.h
   as it is not used as parameter type.
 - Align declaration of imisic_init with defintion.
 - s/harts/cpus in imisic_mmios.
   Also, change type from bool harts[NR_CPUS] to unsigned long *cpus.
 - Allocate msi member of imsic_config dynamically to save some memory.
 - Code style fixes.
 - Update the commit message.
---
 xen/arch/riscv/Makefile            |   1 +
 xen/arch/riscv/imsic.c             | 286 +++++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/imsic.h |  65 +++++++
 3 files changed, 352 insertions(+)
 create mode 100644 xen/arch/riscv/imsic.c
 create mode 100644 xen/arch/riscv/include/asm/imsic.h

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index a1c145c506..e2b8aa42c8 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -2,6 +2,7 @@ obj-y += aplic.o
 obj-y += cpufeature.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
+obj-y += imsic.o
 obj-y += intc.o
 obj-y += irq.o
 obj-y += mm.o
diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
new file mode 100644
index 0000000000..43d0c92cbd
--- /dev/null
+++ b/xen/arch/riscv/imsic.c
@@ -0,0 +1,286 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * xen/arch/riscv/imsic.c
+ *
+ * RISC-V Incoming MSI Controller support
+ *
+ * (c) Microchip Technology Inc.
+ * (c) Vates
+ */
+
+#include <xen/const.h>
+#include <xen/device_tree.h>
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/macros.h>
+#include <xen/xmalloc.h>
+
+#include <asm/imsic.h>
+
+static struct imsic_config imsic_cfg;
+
+/* Callers aren't expected to changed imsic_cfg so return const. */
+const struct imsic_config *imsic_get_config(void)
+{
+    return &imsic_cfg;
+}
+
+static int __init imsic_get_parent_cpuid(const struct dt_device_node *node,
+                                         unsigned int index,
+                                         unsigned long *cpuid)
+{
+    int res;
+    struct dt_phandle_args args;
+
+    res = dt_parse_phandle_with_args(node, "interrupts-extended",
+                                     "#interrupt-cells", index, &args);
+    if ( !res )
+        res = dt_processor_cpuid(args.np->parent, cpuid);
+
+    return res;
+}
+
+static int imsic_parse_node(const struct dt_device_node *node,
+                            unsigned int *nr_parent_irqs)
+{
+    int rc;
+    unsigned int tmp;
+    paddr_t base_addr;
+
+    /* Find number of parent interrupts */
+    *nr_parent_irqs = dt_number_of_irq(node);
+    if ( !*nr_parent_irqs )
+    {
+        printk(XENLOG_ERR "%s: no parent irqs available\n", node->name);
+        return -ENOENT;
+    }
+
+    if ( !dt_property_read_u32(node, "riscv,guest-index-bits",
+                               &imsic_cfg.guest_index_bits) )
+        imsic_cfg.guest_index_bits = 0;
+    tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
+    if ( tmp < imsic_cfg.guest_index_bits )
+    {
+        printk(XENLOG_ERR "%s: guest index bits too big\n", node->name);
+        return -ENOENT;
+    }
+
+    /* Find number of HART index bits */
+    if ( !dt_property_read_u32(node, "riscv,hart-index-bits",
+                               &imsic_cfg.hart_index_bits) )
+    {
+        /* Assume default value */
+        imsic_cfg.hart_index_bits = fls(*nr_parent_irqs);
+        if ( BIT(imsic_cfg.hart_index_bits, UL) < *nr_parent_irqs )
+            imsic_cfg.hart_index_bits++;
+    }
+    tmp -= imsic_cfg.guest_index_bits;
+    if ( tmp < imsic_cfg.hart_index_bits )
+    {
+        printk(XENLOG_ERR "%s: HART index bits too big\n", node->name);
+        return -ENOENT;
+    }
+
+    /* Find number of group index bits */
+    if ( !dt_property_read_u32(node, "riscv,group-index-bits",
+                               &imsic_cfg.group_index_bits) )
+        imsic_cfg.group_index_bits = 0;
+    tmp -= imsic_cfg.hart_index_bits;
+    if ( tmp < imsic_cfg.group_index_bits )
+    {
+        printk(XENLOG_ERR "%s: group index bits too big\n", node->name);
+        return -ENOENT;
+    }
+
+    /* Find first bit position of group index */
+    tmp = IMSIC_MMIO_PAGE_SHIFT * 2;
+    if ( !dt_property_read_u32(node, "riscv,group-index-shift",
+                               &imsic_cfg.group_index_shift) )
+        imsic_cfg.group_index_shift = tmp;
+    if ( imsic_cfg.group_index_shift < tmp )
+    {
+        printk(XENLOG_ERR "%s: group index shift too small\n", node->name);
+        return -ENOENT;
+    }
+    tmp = imsic_cfg.group_index_bits + imsic_cfg.group_index_shift - 1;
+    if ( tmp >= BITS_PER_LONG )
+    {
+        printk(XENLOG_ERR "%s: group index shift too big\n", node->name);
+        return -EINVAL;
+    }
+
+    /* Find number of interrupt identities */
+    if ( !dt_property_read_u32(node, "riscv,num-ids", &imsic_cfg.nr_ids) )
+    {
+        printk(XENLOG_ERR "%s: number of interrupt identities not found\n",
+               node->name);
+        return -ENOENT;
+    }
+
+    if ( (imsic_cfg.nr_ids < IMSIC_MIN_ID) ||
+         (imsic_cfg.nr_ids > IMSIC_MAX_ID) ||
+         ((imsic_cfg.nr_ids & IMSIC_MIN_ID) != IMSIC_MIN_ID) )
+    {
+        printk(XENLOG_ERR "%s: invalid number of interrupt identities\n",
+               node->name);
+        return -EINVAL;
+    }
+
+    /* Compute base address */
+    imsic_cfg.nr_mmios = 0;
+    rc = dt_device_get_address(node, imsic_cfg.nr_mmios, &base_addr, NULL);
+    if ( rc )
+    {
+        printk(XENLOG_ERR "%s: first MMIO resource not found\n", node->name);
+        return rc;
+    }
+
+    imsic_cfg.base_addr = base_addr;
+    imsic_cfg.base_addr &= ~(BIT(imsic_cfg.guest_index_bits +
+                           imsic_cfg.hart_index_bits +
+                           IMSIC_MMIO_PAGE_SHIFT, UL) - 1);
+    imsic_cfg.base_addr &= ~((BIT(imsic_cfg.group_index_bits, UL) - 1) <<
+                           imsic_cfg.group_index_shift);
+
+    /* Find number of MMIO register sets */
+    do {
+        imsic_cfg.nr_mmios++;
+    } while ( !dt_device_get_address(node, imsic_cfg.nr_mmios, &base_addr, NULL) );
+
+    return 0;
+}
+
+int __init imsic_init(const struct dt_device_node *node)
+{
+    int rc;
+    unsigned long reloff, cpuid;
+    uint32_t nr_parent_irqs, index, nr_handlers = 0;
+    paddr_t base_addr;
+    unsigned int nr_mmios;
+
+    /* Parse IMSIC node */
+    rc = imsic_parse_node(node, &nr_parent_irqs);
+    if ( rc )
+        return rc;
+
+    nr_mmios = imsic_cfg.nr_mmios;
+
+    /* Allocate MMIO resource array */
+    imsic_cfg.mmios = xzalloc_array(struct imsic_mmios, nr_mmios);
+    if ( !imsic_cfg.mmios )
+        return -ENOMEM;
+
+    imsic_cfg.msi = xzalloc_array(struct imsic_msi, nr_parent_irqs);
+    if ( !imsic_cfg.msi )
+        return -ENOMEM;
+
+    /* Check MMIO register sets */
+    for ( unsigned int i = 0; i < nr_mmios; i++ )
+    {
+        imsic_cfg.mmios[i].cpus = xzalloc_array(unsigned long,
+                                                BITS_TO_LONGS(nr_parent_irqs));
+        if ( !imsic_cfg.mmios[i].cpus )
+            return -ENOMEM;
+
+        rc = dt_device_get_address(node, i, &imsic_cfg.mmios[i].base_addr,
+                                   &imsic_cfg.mmios[i].size);
+        if ( rc )
+        {
+            printk(XENLOG_ERR "%s:  unable to parse MMIO regset %d\n",
+                   node->name, i);
+            goto imsic_init_err;
+        }
+
+        base_addr = imsic_cfg.mmios[i].base_addr;
+        base_addr &= ~(BIT(imsic_cfg.guest_index_bits +
+                     imsic_cfg.hart_index_bits +
+                     IMSIC_MMIO_PAGE_SHIFT, UL) - 1);
+        base_addr &= ~((BIT(imsic_cfg.group_index_bits, UL) - 1) <<
+                     imsic_cfg.group_index_shift);
+        if ( base_addr != imsic_cfg.base_addr )
+        {
+            rc = -EINVAL;
+            printk(XENLOG_ERR "%s: address mismatch for regset %d\n",
+                   node->name, i);
+            goto imsic_init_err;
+        }
+    }
+
+    /* Configure handlers for target CPUs */
+    for ( unsigned int i = 0; i < nr_parent_irqs; i++ )
+    {
+        rc = imsic_get_parent_cpuid(node, i, &cpuid);
+        if ( rc )
+        {
+            printk(XENLOG_WARNING "%s: cpu ID for parent irq%d not found\n",
+                   node->name, i);
+            continue;
+        }
+
+        if ( cpuid >= num_possible_cpus() )
+        {
+            printk(XENLOG_WARNING "%s: unsupported cpu ID=%lu for parent irq%d\n",
+                   node->name, cpuid, i);
+            continue;
+        }
+
+        /* Find MMIO location of MSI page */
+        index = nr_mmios;
+        reloff = i * BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ;
+        for ( unsigned int j = 0; nr_mmios; j++ )
+        {
+            if ( reloff < imsic_cfg.mmios[j].size )
+            {
+                index = j;
+                break;
+            }
+
+            /*
+             * MMIO region size may not be aligned to
+             * BIT(global->guest_index_bits) * IMSIC_MMIO_PAGE_SZ
+             * if holes are present.
+             */
+            reloff -= ROUNDUP(imsic_cfg.mmios[j].size,
+                BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ);
+        }
+
+        if ( index >= nr_mmios )
+        {
+            printk(XENLOG_WARNING "%s: MMIO not found for parent irq%d\n",
+                   node->name, i);
+            continue;
+        }
+
+        if ( !IS_ALIGNED(imsic_cfg.msi[cpuid].base_addr + reloff, PAGE_SIZE) )
+        {
+            printk(XENLOG_WARNING "%s: MMIO address 0x%lx is not aligned on a page\n",
+                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
+            imsic_cfg.msi[cpuid].offset = 0;
+            imsic_cfg.msi[cpuid].base_addr = 0;
+            continue;
+        }
+
+        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);
+        imsic_cfg.msi[cpuid].base_addr = imsic_cfg.mmios[index].base_addr;
+        imsic_cfg.msi[cpuid].offset = reloff;
+        nr_handlers++;
+    }
+
+    if ( !nr_handlers )
+    {
+        printk(XENLOG_ERR "%s: No CPU handlers found\n", node->name);
+        rc = -ENODEV;
+        goto imsic_init_err;
+    }
+
+    return 0;
+
+ imsic_init_err:
+    for ( unsigned int i = 0; i < nr_mmios; i++ )
+        XFREE(imsic_cfg.mmios[i].cpus);
+    XFREE(imsic_cfg.mmios);
+    XFREE(imsic_cfg.msi);
+
+    return rc;
+}
diff --git a/xen/arch/riscv/include/asm/imsic.h b/xen/arch/riscv/include/asm/imsic.h
new file mode 100644
index 0000000000..ed51cac780
--- /dev/null
+++ b/xen/arch/riscv/include/asm/imsic.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * xen/arch/riscv/imsic.h
+ *
+ * RISC-V Incoming MSI Controller support
+ *
+ * (c) 2023 Microchip Technology Inc.
+ */
+
+#ifndef ASM__RISCV__IMSIC_H
+#define ASM__RISCV__IMSIC_H
+
+#include <xen/types.h>
+
+#define IMSIC_MMIO_PAGE_SHIFT   12
+#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
+
+#define IMSIC_MIN_ID            63
+#define IMSIC_MAX_ID            2047
+
+struct imsic_msi {
+    paddr_t base_addr;
+    unsigned long offset;
+};
+
+struct imsic_mmios {
+    paddr_t base_addr;
+    unsigned long size;
+    unsigned long *cpus;
+};
+
+struct imsic_config {
+    /* base address */
+    paddr_t base_addr;
+
+    /* Bits representing Guest index, HART index, and Group index */
+    unsigned int guest_index_bits;
+    unsigned int hart_index_bits;
+    unsigned int group_index_bits;
+    unsigned int group_index_shift;
+
+    /* imsic phandle */
+    unsigned int phandle;
+
+    /* number of parent irq */
+    unsigned int nr_parent_irqs;
+
+    /* number off interrupt identities */
+    unsigned int nr_ids;
+
+    /* mmios */
+    unsigned int nr_mmios;
+    struct imsic_mmios *mmios;
+
+    /* MSI */
+    struct imsic_msi *msi;
+};
+
+struct dt_device_node;
+int imsic_init(const struct dt_device_node *node);
+
+const struct imsic_config *imsic_get_config(void);
+
+#endif /* ASM__RISCV__IMSIC_H */
-- 
2.49.0

On 06.05.2025 18:51, Oleksii Kurochko wrote:
> imsic_init() is introduced to parse device tree node, which has the following
> bindings [2], and based on the parsed information update IMSIC configuration
> which is stored in imsic_cfg.
> 
> The following helpers are introduces for imsic_init() usage:
>   - imsic_parse_node() parses IMSIC node from DTS
>   - imsic_get_parent_cpuid() returns the hart ( CPU ) ID of the given device
>     tree node.
> 
> This patch is based on the code from [1].
> 
> Since Microchip originally developed imsic.{c,h}, an internal discussion with
> them led to the decision to use the MIT license.
> 
> [1] https://gitlab.com/xen-project/people/olkur/xen/-/commit/0b1a94f2bc3bb1a81cd26bb75f0bf578f84cb4d4
> [2] https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
> 
> Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>  - Drop years in copyrights.

Did you, really?

>  - s/riscv_of_processor_hartid/dt_processor_cpuid.
>  - s/imsic_get_parent_hartid/imsic_get_parent_cpuid.
>    Rename argument hartid to cpuid.
>    Make node argument const.
>    Return res instead of -EINVAL for the failure case of dt_processor_cpuid().
>    Drop local variable hart and use cpuid argument instead.
>    Drop useless return res;
>  - imsic_parse_node() changes:
>    - Make node argument const.
>    - Check the return value of dt_property_read_u32() directly instead of
>      saving it to rc variable.
>    - Update tmp usage, use short form "-=".
>    - Update a check (imsic_cfg.nr_ids >= IMSIC_MAX_ID) to (imsic_cfg.nr_ids > IMSIC_MAX_ID)
>      as IMSIC_MAX_ID is changed to maximum valid value, not just the firsr out-of-range.
>    - Use `rc` to return value instead of explicitly use -EINVAL.
>    - Use do {} while() to find number of MMIO register sets.
>  - Set IMSIC_MAX_ID to 2047 (maximum possible IRQ number).
>  - imsic_init() changes:
>    - Use unsigned int in for's expression1.
>    - s/xfree/XFEE.
>    - Allocate msi and cpus array dynamically.
>  - Drop forward declaration before declaration of imsic_get_config() in asm/imsic.h
>    as it is not used as parameter type.
>  - Align declaration of imisic_init with defintion.
>  - s/harts/cpus in imisic_mmios.
>    Also, change type from bool harts[NR_CPUS] to unsigned long *cpus.
>  - Allocate msi member of imsic_config dynamically to save some memory.
>  - Code style fixes.
>  - Update the commit message.
> ---
>  xen/arch/riscv/Makefile            |   1 +
>  xen/arch/riscv/imsic.c             | 286 +++++++++++++++++++++++++++++
>  xen/arch/riscv/include/asm/imsic.h |  65 +++++++
>  3 files changed, 352 insertions(+)
>  create mode 100644 xen/arch/riscv/imsic.c
>  create mode 100644 xen/arch/riscv/include/asm/imsic.h
> 
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index a1c145c506..e2b8aa42c8 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -2,6 +2,7 @@ obj-y += aplic.o
>  obj-y += cpufeature.o
>  obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>  obj-y += entry.o
> +obj-y += imsic.o
>  obj-y += intc.o
>  obj-y += irq.o
>  obj-y += mm.o
> diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
> new file mode 100644
> index 0000000000..43d0c92cbd
> --- /dev/null
> +++ b/xen/arch/riscv/imsic.c
> @@ -0,0 +1,286 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * xen/arch/riscv/imsic.c
> + *
> + * RISC-V Incoming MSI Controller support
> + *
> + * (c) Microchip Technology Inc.
> + * (c) Vates
> + */
> +
> +#include <xen/const.h>
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
> +#include <xen/init.h>
> +#include <xen/macros.h>
> +#include <xen/xmalloc.h>
> +
> +#include <asm/imsic.h>
> +
> +static struct imsic_config imsic_cfg;
> +
> +/* Callers aren't expected to changed imsic_cfg so return const. */
> +const struct imsic_config *imsic_get_config(void)
> +{
> +    return &imsic_cfg;
> +}
> +
> +static int __init imsic_get_parent_cpuid(const struct dt_device_node *node,
> +                                         unsigned int index,
> +                                         unsigned long *cpuid)
> +{
> +    int res;
> +    struct dt_phandle_args args;
> +
> +    res = dt_parse_phandle_with_args(node, "interrupts-extended",
> +                                     "#interrupt-cells", index, &args);
> +    if ( !res )
> +        res = dt_processor_cpuid(args.np->parent, cpuid);
> +
> +    return res;
> +}
> +
> +static int imsic_parse_node(const struct dt_device_node *node,
> +                            unsigned int *nr_parent_irqs)
> +{
> +    int rc;
> +    unsigned int tmp;
> +    paddr_t base_addr;
> +
> +    /* Find number of parent interrupts */
> +    *nr_parent_irqs = dt_number_of_irq(node);
> +    if ( !*nr_parent_irqs )
> +    {
> +        printk(XENLOG_ERR "%s: no parent irqs available\n", node->name);
> +        return -ENOENT;
> +    }
> +
> +    if ( !dt_property_read_u32(node, "riscv,guest-index-bits",
> +                               &imsic_cfg.guest_index_bits) )
> +        imsic_cfg.guest_index_bits = 0;
> +    tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
> +    if ( tmp < imsic_cfg.guest_index_bits )
> +    {
> +        printk(XENLOG_ERR "%s: guest index bits too big\n", node->name);
> +        return -ENOENT;
> +    }
> +
> +    /* Find number of HART index bits */
> +    if ( !dt_property_read_u32(node, "riscv,hart-index-bits",
> +                               &imsic_cfg.hart_index_bits) )
> +    {
> +        /* Assume default value */
> +        imsic_cfg.hart_index_bits = fls(*nr_parent_irqs);
> +        if ( BIT(imsic_cfg.hart_index_bits, UL) < *nr_parent_irqs )
> +            imsic_cfg.hart_index_bits++;
> +    }
> +    tmp -= imsic_cfg.guest_index_bits;
> +    if ( tmp < imsic_cfg.hart_index_bits )
> +    {
> +        printk(XENLOG_ERR "%s: HART index bits too big\n", node->name);
> +        return -ENOENT;
> +    }
> +
> +    /* Find number of group index bits */
> +    if ( !dt_property_read_u32(node, "riscv,group-index-bits",
> +                               &imsic_cfg.group_index_bits) )
> +        imsic_cfg.group_index_bits = 0;
> +    tmp -= imsic_cfg.hart_index_bits;
> +    if ( tmp < imsic_cfg.group_index_bits )
> +    {
> +        printk(XENLOG_ERR "%s: group index bits too big\n", node->name);
> +        return -ENOENT;
> +    }
> +
> +    /* Find first bit position of group index */
> +    tmp = IMSIC_MMIO_PAGE_SHIFT * 2;
> +    if ( !dt_property_read_u32(node, "riscv,group-index-shift",
> +                               &imsic_cfg.group_index_shift) )
> +        imsic_cfg.group_index_shift = tmp;
> +    if ( imsic_cfg.group_index_shift < tmp )
> +    {
> +        printk(XENLOG_ERR "%s: group index shift too small\n", node->name);
> +        return -ENOENT;
> +    }
> +    tmp = imsic_cfg.group_index_bits + imsic_cfg.group_index_shift - 1;
> +    if ( tmp >= BITS_PER_LONG )
> +    {
> +        printk(XENLOG_ERR "%s: group index shift too big\n", node->name);
> +        return -EINVAL;
> +    }
> +
> +    /* Find number of interrupt identities */
> +    if ( !dt_property_read_u32(node, "riscv,num-ids", &imsic_cfg.nr_ids) )
> +    {
> +        printk(XENLOG_ERR "%s: number of interrupt identities not found\n",
> +               node->name);
> +        return -ENOENT;
> +    }
> +
> +    if ( (imsic_cfg.nr_ids < IMSIC_MIN_ID) ||
> +         (imsic_cfg.nr_ids > IMSIC_MAX_ID) ||
> +         ((imsic_cfg.nr_ids & IMSIC_MIN_ID) != IMSIC_MIN_ID) )
> +    {
> +        printk(XENLOG_ERR "%s: invalid number of interrupt identities\n",
> +               node->name);
> +        return -EINVAL;
> +    }
> +
> +    /* Compute base address */
> +    imsic_cfg.nr_mmios = 0;
> +    rc = dt_device_get_address(node, imsic_cfg.nr_mmios, &base_addr, NULL);
> +    if ( rc )
> +    {
> +        printk(XENLOG_ERR "%s: first MMIO resource not found\n", node->name);
> +        return rc;
> +    }
> +
> +    imsic_cfg.base_addr = base_addr;
> +    imsic_cfg.base_addr &= ~(BIT(imsic_cfg.guest_index_bits +
> +                           imsic_cfg.hart_index_bits +
> +                           IMSIC_MMIO_PAGE_SHIFT, UL) - 1);

Nit: indentation, similarly ...

> +    imsic_cfg.base_addr &= ~((BIT(imsic_cfg.group_index_bits, UL) - 1) <<
> +                           imsic_cfg.group_index_shift);

... here.

> +    /* Find number of MMIO register sets */
> +    do {
> +        imsic_cfg.nr_mmios++;
> +    } while ( !dt_device_get_address(node, imsic_cfg.nr_mmios, &base_addr, NULL) );
> +
> +    return 0;
> +}
> +
> +int __init imsic_init(const struct dt_device_node *node)
> +{
> +    int rc;
> +    unsigned long reloff, cpuid;
> +    uint32_t nr_parent_irqs, index, nr_handlers = 0;

I can't spot why unsigned int wouldn't be suitable here. In fact e.g. ...

> +    paddr_t base_addr;
> +    unsigned int nr_mmios;
> +
> +    /* Parse IMSIC node */
> +    rc = imsic_parse_node(node, &nr_parent_irqs);

... this one wants to yield unsigned int * according to the function parameter's
type.

> +    if ( rc )
> +        return rc;
> +
> +    nr_mmios = imsic_cfg.nr_mmios;
> +
> +    /* Allocate MMIO resource array */
> +    imsic_cfg.mmios = xzalloc_array(struct imsic_mmios, nr_mmios);
> +    if ( !imsic_cfg.mmios )
> +        return -ENOMEM;
> +
> +    imsic_cfg.msi = xzalloc_array(struct imsic_msi, nr_parent_irqs);
> +    if ( !imsic_cfg.msi )
> +        return -ENOMEM;

Leaking the earlier successful allocation?

> +    /* Check MMIO register sets */
> +    for ( unsigned int i = 0; i < nr_mmios; i++ )
> +    {
> +        imsic_cfg.mmios[i].cpus = xzalloc_array(unsigned long,
> +                                                BITS_TO_LONGS(nr_parent_irqs));
> +        if ( !imsic_cfg.mmios[i].cpus )
> +            return -ENOMEM;

Leaking all earlier successful allocations?

> +        rc = dt_device_get_address(node, i, &imsic_cfg.mmios[i].base_addr,
> +                                   &imsic_cfg.mmios[i].size);
> +        if ( rc )
> +        {
> +            printk(XENLOG_ERR "%s:  unable to parse MMIO regset %d\n",

Nit: Excess blank.

> +                   node->name, i);
> +            goto imsic_init_err;
> +        }
> +
> +        base_addr = imsic_cfg.mmios[i].base_addr;
> +        base_addr &= ~(BIT(imsic_cfg.guest_index_bits +
> +                     imsic_cfg.hart_index_bits +
> +                     IMSIC_MMIO_PAGE_SHIFT, UL) - 1);
> +        base_addr &= ~((BIT(imsic_cfg.group_index_bits, UL) - 1) <<
> +                     imsic_cfg.group_index_shift);

As above, indentation again.

> +        if ( base_addr != imsic_cfg.base_addr )
> +        {
> +            rc = -EINVAL;
> +            printk(XENLOG_ERR "%s: address mismatch for regset %d\n",
> +                   node->name, i);
> +            goto imsic_init_err;
> +        }
> +    }
> +
> +    /* Configure handlers for target CPUs */
> +    for ( unsigned int i = 0; i < nr_parent_irqs; i++ )
> +    {
> +        rc = imsic_get_parent_cpuid(node, i, &cpuid);

Coming back to a comment I gave on the respective earlier patch: What you get back
here is a DT value, aiui. There's no translation to Xen CPU numbering space, as
would be required for e.g. ...

> +        if ( rc )
> +        {
> +            printk(XENLOG_WARNING "%s: cpu ID for parent irq%d not found\n",
> +                   node->name, i);
> +            continue;
> +        }
> +
> +        if ( cpuid >= num_possible_cpus() )

... this. Are you using DT numbering without any translation, no matter that it
(I suppose) could be very sparse?

> +        {
> +            printk(XENLOG_WARNING "%s: unsupported cpu ID=%lu for parent irq%d\n",
> +                   node->name, cpuid, i);

Nit: i is unsigned int, so wants formatting with %u (also applicable elsewhere).

> +            continue;
> +        }
> +
> +        /* Find MMIO location of MSI page */
> +        index = nr_mmios;
> +        reloff = i * BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ;
> +        for ( unsigned int j = 0; nr_mmios; j++ )

DYM "j < nr_mmios"?

> +        {
> +            if ( reloff < imsic_cfg.mmios[j].size )
> +            {
> +                index = j;
> +                break;
> +            }
> +
> +            /*
> +             * MMIO region size may not be aligned to
> +             * BIT(global->guest_index_bits) * IMSIC_MMIO_PAGE_SZ
> +             * if holes are present.
> +             */
> +            reloff -= ROUNDUP(imsic_cfg.mmios[j].size,
> +                BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ);
> +        }
> +
> +        if ( index >= nr_mmios )

Why is it that you need both "index" and "j"?

> +        {
> +            printk(XENLOG_WARNING "%s: MMIO not found for parent irq%d\n",
> +                   node->name, i);
> +            continue;
> +        }
> +
> +        if ( !IS_ALIGNED(imsic_cfg.msi[cpuid].base_addr + reloff, PAGE_SIZE) )
> +        {
> +            printk(XENLOG_WARNING "%s: MMIO address 0x%lx is not aligned on a page\n",

Please prefer to use %#lx, as we do elsewhere.

> +                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
> +            imsic_cfg.msi[cpuid].offset = 0;
> +            imsic_cfg.msi[cpuid].base_addr = 0;
> +            continue;
> +        }
> +
> +        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);

Depending on clarification on the number space used, this may want to be
cpumask_set_cpu() instead. Else I think this is simply __set_bit().

> +        imsic_cfg.msi[cpuid].base_addr = imsic_cfg.mmios[index].base_addr;
> +        imsic_cfg.msi[cpuid].offset = reloff;

How come it's cpuid that's used as array index here? Shouldn't this be i,
seeing that the array allocation is using nr_parent_irqs?

> +        nr_handlers++;
> +    }
> +
> +    if ( !nr_handlers )
> +    {
> +        printk(XENLOG_ERR "%s: No CPU handlers found\n", node->name);
> +        rc = -ENODEV;
> +        goto imsic_init_err;
> +    }
> +
> +    return 0;
> +
> + imsic_init_err:
> +    for ( unsigned int i = 0; i < nr_mmios; i++ )
> +        XFREE(imsic_cfg.mmios[i].cpus);

This can be just xfree(), as the array itself ...

> +    XFREE(imsic_cfg.mmios);

... is then also freed.

> +    XFREE(imsic_cfg.msi);
> +
> +    return rc;
> +}
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/imsic.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * xen/arch/riscv/imsic.h
> + *
> + * RISC-V Incoming MSI Controller support
> + *
> + * (c) 2023 Microchip Technology Inc.
> + */
> +
> +#ifndef ASM__RISCV__IMSIC_H
> +#define ASM__RISCV__IMSIC_H
> +
> +#include <xen/types.h>
> +
> +#define IMSIC_MMIO_PAGE_SHIFT   12
> +#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
> +
> +#define IMSIC_MIN_ID            63

This isn't the "minimum ID", but the "minimum permissible number of IDs" as per
its first use in imsic_parse_node(). Further uses there consider it a mask,
which makes me wonder whether the imsic_cfg.nr_ids field name is actually
correct: Isn't this the maximum valid ID rather than their count/number?

Jan

On 5/15/25 10:42 AM, Jan Beulich wrote:
>> +                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
>> +            imsic_cfg.msi[cpuid].offset = 0;
>> +            imsic_cfg.msi[cpuid].base_addr = 0;
>> +            continue;
>> +        }
>> +
>> +        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);
> Depending on clarification on the number space used, this may want to be
> cpumask_set_cpu() instead. Else I think this is simply __set_bit().

cpumask_set_cpu() requires cpumask_t which uses static definition but we are
using dynamic allocation.
So it seems like bitmap_set() is the  best one option or reworking to static
allocation will require.
Am I missing something?

~ Oleksii

On 19.05.2025 17:26, Oleksii Kurochko wrote:
> On 5/15/25 10:42 AM, Jan Beulich wrote:
>>> +                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
>>> +            imsic_cfg.msi[cpuid].offset = 0;
>>> +            imsic_cfg.msi[cpuid].base_addr = 0;
>>> +            continue;
>>> +        }
>>> +
>>> +        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);
>> Depending on clarification on the number space used, this may want to be
>> cpumask_set_cpu() instead. Else I think this is simply __set_bit().
> 
> cpumask_set_cpu() requires cpumask_t which uses static definition but we are
> using dynamic allocation.

But you're aware of cpumask_var_t (and respective allocation functions)?

Jan

> So it seems like bitmap_set() is the  best one option or reworking to static
> allocation will require.
> Am I missing something?
> 
> ~ Oleksii
> 
>

On 5/19/25 8:33 PM, Jan Beulich wrote:
> On 19.05.2025 17:26, Oleksii Kurochko wrote:
>> On 5/15/25 10:42 AM, Jan Beulich wrote:
>>>> +                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
>>>> +            imsic_cfg.msi[cpuid].offset = 0;
>>>> +            imsic_cfg.msi[cpuid].base_addr = 0;
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);
>>> Depending on clarification on the number space used, this may want to be
>>> cpumask_set_cpu() instead. Else I think this is simply __set_bit().
>> cpumask_set_cpu() requires cpumask_t which uses static definition but we are
>> using dynamic allocation.
> But you're aware of cpumask_var_t (and respective allocation functions)?

Now yes, thanks.

~ Oleksii

On 5/15/25 10:42 AM, Jan Beulich wrote:
> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>> imsic_init() is introduced to parse device tree node, which has the following
>> bindings [2], and based on the parsed information update IMSIC configuration
>> which is stored in imsic_cfg.
>>
>> The following helpers are introduces for imsic_init() usage:
>>    - imsic_parse_node() parses IMSIC node from DTS
>>    - imsic_get_parent_cpuid() returns the hart ( CPU ) ID of the given device
>>      tree node.
>>
>> This patch is based on the code from [1].
>>
>> Since Microchip originally developed imsic.{c,h}, an internal discussion with
>> them led to the decision to use the MIT license.
>>
>> [1]https://gitlab.com/xen-project/people/olkur/xen/-/commit/0b1a94f2bc3bb1a81cd26bb75f0bf578f84cb4d4
>> [2]https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
>>
>> Co-developed-by: Romain Caritey<Romain.Caritey@microchip.com>
>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>> ---
>> Changes in V2:
>>   - Drop years in copyrights.
> Did you, really?

Oops, missed to drop it in asm/imsic.h. Thanks.

>> +    if ( rc )
>> +        return rc;
>> +
>> +    nr_mmios = imsic_cfg.nr_mmios;
>> +
>> +    /* Allocate MMIO resource array */
>> +    imsic_cfg.mmios = xzalloc_array(struct imsic_mmios, nr_mmios);
>> +    if ( !imsic_cfg.mmios )
>> +        return -ENOMEM;
>> +
>> +    imsic_cfg.msi = xzalloc_array(struct imsic_msi, nr_parent_irqs);
>> +    if ( !imsic_cfg.msi )
>> +        return -ENOMEM;
> Leaking the earlier successful allocation?
>
>> +    /* Check MMIO register sets */
>> +    for ( unsigned int i = 0; i < nr_mmios; i++ )
>> +    {
>> +        imsic_cfg.mmios[i].cpus = xzalloc_array(unsigned long,
>> +                                                BITS_TO_LONGS(nr_parent_irqs));
>> +        if ( !imsic_cfg.mmios[i].cpus )
>> +            return -ENOMEM;
> Leaking all earlier successful allocations?

In this cases should be:
     {
         rc = -ENOMEM;
         goto imsic_init_err;

>> +        if ( base_addr != imsic_cfg.base_addr )
>> +        {
>> +            rc = -EINVAL;
>> +            printk(XENLOG_ERR "%s: address mismatch for regset %d\n",
>> +                   node->name, i);
>> +            goto imsic_init_err;
>> +        }
>> +    }
>> +
>> +    /* Configure handlers for target CPUs */
>> +    for ( unsigned int i = 0; i < nr_parent_irqs; i++ )
>> +    {
>> +        rc = imsic_get_parent_cpuid(node, i, &cpuid);
> Coming back to a comment I gave on the respective earlier patch: What you get back
> here is a DT value, aiui. There's no translation to Xen CPU numbering space, as
> would be required for e.g. ...
>
>> +        if ( rc )
>> +        {
>> +            printk(XENLOG_WARNING "%s: cpu ID for parent irq%d not found\n",
>> +                   node->name, i);
>> +            continue;
>> +        }
>> +
>> +        if ( cpuid >= num_possible_cpus() )
> ... this. Are you using DT numbering without any translation, no matter that it
> (I suppose) could be very sparse?

Agree, it should translation of cpuid to Xen CPU numbering space as cpuid could be
sparsed according to the RISC-V spec, which guarantees only that cpuid 0 will be present,
all other could be any number.

I thought about to update that with:
    xen_cpuid = hartid_to_cpuid(hartid);

    if ( xen_cpuid >= num_possible_cpus() )
    {
         printk(XENLOG_WARNING "%s: unsupported cpu ID=%lu for parent irq%u\n",
                node->name, hartid, i);
         continue;
    }
       ...

   /* defined in asm/smp.h */
   static inline unsigned long hartid_to_cpuid(unsigned long hartid)
   {
     for ( unsigned int cpuid = 0; cpuid < ARRAY_SIZE(pcpu_info); cpuid++ )
     {
         if ( hartid == cpuid_to_hartid(cpuid) )
             return cpuid;
     }

     return NR_CPUS;
   }
   
(the proper name of variable 'cpuid' I think we will agree in the discussion of one of the
earlier patches.)

But then it will be an issue that if hart_id (from DT) hasn't been assigned to Xen's cpuid,
then IMSIC's msi[] and mmio[] won't be configured properly.
Probably this is not an issue and this assignment of cpuid to hartid could be done before
imsic_init() in case of CONFIG_SMP=y.

>> +            continue;
>> +        }
>> +
>> +        /* Find MMIO location of MSI page */
>> +        index = nr_mmios;
>> +        reloff = i * BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ;
>> +        for ( unsigned int j = 0; nr_mmios; j++ )
> DYM "j < nr_mmios"?

Yes, the condition should be corrected. Interesting why I am not faced an issue with such
condition, nr_mmios shouldn't be zero (I'll double check) and Linux kernel has the same
condition:
   https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-riscv-imsic-state.c#L907C31-L908C1
It seems like LK wants to have a fix too.

>> +        {
>> +            if ( reloff < imsic_cfg.mmios[j].size )
>> +            {
>> +                index = j;
>> +                break;
>> +            }
>> +
>> +            /*
>> +             * MMIO region size may not be aligned to
>> +             * BIT(global->guest_index_bits) * IMSIC_MMIO_PAGE_SZ
>> +             * if holes are present.
>> +             */
>> +            reloff -= ROUNDUP(imsic_cfg.mmios[j].size,
>> +                BIT(imsic_cfg.guest_index_bits, UL) * IMSIC_MMIO_PAGE_SZ);
>> +        }
>> +
>> +        if ( index >= nr_mmios )
> Why is it that you need both "index" and "j"?

There is no need. I will drop 'j'.

>> +                   node->name, imsic_cfg.msi[cpuid].base_addr + reloff);
>> +            imsic_cfg.msi[cpuid].offset = 0;
>> +            imsic_cfg.msi[cpuid].base_addr = 0;
>> +            continue;
>> +        }
>> +
>> +        bitmap_set(imsic_cfg.mmios[index].cpus, cpuid, 1);
> Depending on clarification on the number space used, this may want to be
> cpumask_set_cpu() instead. Else I think this is simply __set_bit().

Considering that cpuid is taken from DT and the value in device tree is what we are
using as hartid and hartid could be according any number from 0 to sizeof(unsigned long)
then we can't use cpuid for msi[] as overflow could happen, it seems like we should use
an id from Xen space. (so basically xen_cpuid which I mentioned above)

>
>> +        imsic_cfg.msi[cpuid].base_addr = imsic_cfg.mmios[index].base_addr;
>> +        imsic_cfg.msi[cpuid].offset = reloff;
> How come it's cpuid that's used as array index here? Shouldn't this be i,
> seeing that the array allocation is using nr_parent_irqs?

Agree, something wrong is here. As I mentioned above, I think, Xen cpu space should be used here.

>> +    XFREE(imsic_cfg.msi);
>> +
>> +    return rc;
>> +}
>> --- /dev/null
>> +++ b/xen/arch/riscv/include/asm/imsic.h
>> @@ -0,0 +1,65 @@
>> +/* SPDX-License-Identifier: MIT */
>> +
>> +/*
>> + * xen/arch/riscv/imsic.h
>> + *
>> + * RISC-V Incoming MSI Controller support
>> + *
>> + * (c) 2023 Microchip Technology Inc.
>> + */
>> +
>> +#ifndef ASM__RISCV__IMSIC_H
>> +#define ASM__RISCV__IMSIC_H
>> +
>> +#include <xen/types.h>
>> +
>> +#define IMSIC_MMIO_PAGE_SHIFT   12
>> +#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
>> +
>> +#define IMSIC_MIN_ID            63
> This isn't the "minimum ID", but the "minimum permissible number of IDs" as per
> its first use in imsic_parse_node(). Further uses there consider it a mask,
> which makes me wonder whether the imsic_cfg.nr_ids field name is actually
> correct: Isn't this the maximum valid ID rather than their count/number?

imsic_cfg.nr_ids it is a value of interrupt identities supported by IMSIC interrupt file according to
DT binding:
   riscv,num-ids:
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 63
     maximum: 2047
     description:
       Number of interrupt identities supported by IMSIC interrupt file.

 From some point of view it could be called as maximum valid ID specifically for a platform, but the number
of ids looks better to me.

Thanks.

~ Oleksii

On 19.05.2025 17:19, Oleksii Kurochko wrote:
> On 5/15/25 10:42 AM, Jan Beulich wrote:
>> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/imsic.h
>>> @@ -0,0 +1,65 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +
>>> +/*
>>> + * xen/arch/riscv/imsic.h
>>> + *
>>> + * RISC-V Incoming MSI Controller support
>>> + *
>>> + * (c) 2023 Microchip Technology Inc.
>>> + */
>>> +
>>> +#ifndef ASM__RISCV__IMSIC_H
>>> +#define ASM__RISCV__IMSIC_H
>>> +
>>> +#include <xen/types.h>
>>> +
>>> +#define IMSIC_MMIO_PAGE_SHIFT   12
>>> +#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
>>> +
>>> +#define IMSIC_MIN_ID            63
>> This isn't the "minimum ID", but the "minimum permissible number of IDs" as per
>> its first use in imsic_parse_node(). Further uses there consider it a mask,
>> which makes me wonder whether the imsic_cfg.nr_ids field name is actually
>> correct: Isn't this the maximum valid ID rather than their count/number?
> 
> imsic_cfg.nr_ids it is a value of interrupt identities supported by IMSIC interrupt file according to
> DT binding:
>    riscv,num-ids:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      minimum: 63
>      maximum: 2047
>      description:
>        Number of interrupt identities supported by IMSIC interrupt file.

Unless this count accounts for 0 being invalid (and hence isn't counted), I'm
still confused: With the maximum value this would mean 2046 is still valid,
but 2047 isn't anymore. When normally such a boundary would be at an exact
power of 2, i.e. between 2047 and 2048 here.

Jan

On 5/19/25 8:32 PM, Jan Beulich wrote:
> On 19.05.2025 17:19, Oleksii Kurochko wrote:
>> On 5/15/25 10:42 AM, Jan Beulich wrote:
>>> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>>>> --- /dev/null
>>>> +++ b/xen/arch/riscv/include/asm/imsic.h
>>>> @@ -0,0 +1,65 @@
>>>> +/* SPDX-License-Identifier: MIT */
>>>> +
>>>> +/*
>>>> + * xen/arch/riscv/imsic.h
>>>> + *
>>>> + * RISC-V Incoming MSI Controller support
>>>> + *
>>>> + * (c) 2023 Microchip Technology Inc.
>>>> + */
>>>> +
>>>> +#ifndef ASM__RISCV__IMSIC_H
>>>> +#define ASM__RISCV__IMSIC_H
>>>> +
>>>> +#include <xen/types.h>
>>>> +
>>>> +#define IMSIC_MMIO_PAGE_SHIFT   12
>>>> +#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
>>>> +
>>>> +#define IMSIC_MIN_ID            63
>>> This isn't the "minimum ID", but the "minimum permissible number of IDs" as per
>>> its first use in imsic_parse_node(). Further uses there consider it a mask,
>>> which makes me wonder whether the imsic_cfg.nr_ids field name is actually
>>> correct: Isn't this the maximum valid ID rather than their count/number?
>> imsic_cfg.nr_ids it is a value of interrupt identities supported by IMSIC interrupt file according to
>> DT binding:
>>     riscv,num-ids:
>>       $ref: /schemas/types.yaml#/definitions/uint32
>>       minimum: 63
>>       maximum: 2047
>>       description:
>>         Number of interrupt identities supported by IMSIC interrupt file.
> Unless this count accounts for 0 being invalid (and hence isn't counted), I'm
> still confused: With the maximum value this would mean 2046 is still valid,
> but 2047 isn't anymore. When normally such a boundary would be at an exact
> power of 2, i.e. between 2047 and 2048 here.

2047 is inclusive according to the AIA spec:
   The number of interrupt identities supported by an interrupt file
   (and hence the number of active bits in each array) is one less than a multiple
   of 64, and may be a minimum of 63 and a maximum of 2047.
   ...
   When an interrupt file supports N distinct interrupt identities,
   valid identity numbers are between 1 and N inclusive.
   The identity numbers within this range are said to be implemented by the interrupt
   file; numbers outside this range are not implemented.
   The number zero is never a valid interrupt identity. Identity 0 is just ignored.

It is still  not a power of two but it was the AIA spec tells us.

Also, this maximum identity number of 2047 is consistent with related fields like
the EIID (External Interrupt Identity) field used in APLICs when forwarding MSIs,
which specifies the MSI data value that becomes the minor identity at the target
hart's interrupt file.
The EIID field is typically an 11-bit field, able to hold values from 0 through
2047.
Since identity 0 is invalid, the entire range of valid identity numbers (1-2047)
fits within the values representable by an 11-bit field.

~ Oleksii