Re: [PATCH] irqchip/riscv-imsic: Use GENMASK for base address masks

[Anup Patel]

On Sat, Jul 18, 2026 at 10:04 AM Pengpeng Hou <pengpeng@xxxxxxxxxxx> wrote:
>
> The IMSIC DT binding allows riscv,guest-index-bits up to 7 and
> riscv,hart-index-bits up to 15. On RV32, guest-index-bits=7 and
> hart-index-bits=13, together with the 12-bit IMSIC page offset, is a
> binding-valid layout that consumes a 32-bit low-address mask.
>
> The parser accepts this equality case because it rejects only values
> larger than the remaining bit budget. The base address canonicalization
> then builds the low-address mask with BIT(sum) - 1. When sum is 32 on
> RV32, that expression shifts an unsigned long by its full width.
>
> Keep the legal equality layout and express the mask with GENMASK(sum - 1,
> 0) instead. The existing parser bounds guarantee that sum - 1 is below
> BITS_PER_LONG at both mask sites.
>
> Fixes: 21a8f8a0eb35 ("irqchip: Add RISC-V incoming MSI controller early driver")
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>

LGTM.

Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx>

Thanks,
Anup

> ---
>  drivers/irqchip/irq-riscv-imsic-state.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index b8d1bbbf42f73..9505ddbd9eec9 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -7,6 +7,7 @@
>  #define pr_fmt(fmt) "riscv-imsic: " fmt
>  #include <linux/acpi.h>
>  #include <linux/cpu.h>
> +#include <linux/bits.h>
>  #include <linux/bitmap.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> @@ -769,9 +770,9 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
>                 return -EINVAL;
>         }
>         global->base_addr = res.start;
> -       global->base_addr &= ~(BIT(global->guest_index_bits +
> -                                  global->hart_index_bits +
> -                                  IMSIC_MMIO_PAGE_SHIFT) - 1);
> +       global->base_addr &= ~GENMASK(global->guest_index_bits +
> +                                      global->hart_index_bits +
> +                                      IMSIC_MMIO_PAGE_SHIFT - 1, 0);
>         global->base_addr &= ~((BIT(global->group_index_bits) - 1) <<
>                                global->group_index_shift);
>
> @@ -850,9 +851,9 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
>                 }
>
>                 base_addr = mmios[i].start;
> -               base_addr &= ~(BIT(global->guest_index_bits +
> -                                  global->hart_index_bits +
> -                                  IMSIC_MMIO_PAGE_SHIFT) - 1);
> +               base_addr &= ~GENMASK(global->guest_index_bits +
> +                                     global->hart_index_bits +
> +                                     IMSIC_MMIO_PAGE_SHIFT - 1, 0);
>                 base_addr &= ~((BIT(global->group_index_bits) - 1) <<
>                                global->group_index_shift);
>                 if (base_addr != global->base_addr) {
> --
> 2.43.0
>