irqchip/riscv-imsic: Use GENMASK for base address masks

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

Posted by Pengpeng Hou 6 days, 23 hours ago

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@iscas.ac.cn>
---
 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

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

Posted by Anup Patel 5 days ago

On Sat, Jul 18, 2026 at 10:04 AM Pengpeng Hou <pengpeng@iscas.ac.cn> 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@iscas.ac.cn>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

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
>