[PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[caina]

This reverts commit 6be6cba9c4371d27f78d900ccfe34bb880d9ee20.

Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
appears to cause a regression on Hi1616.

On-board hns NIC has two ports, enahisic2i0 and enahisic2i1, both
behind mbigen-v2.  Port 0 works; port 1 cannot pass any traffic.

Their interrupt pins fall on different mbigen nodes:

  enahisic2i0: pins 1152-1198 -> all in node 9
  enahisic2i1: pins 1200-1246 -> node 9 (1200-1215) + node 10 (1216-1246)

  (nid = (hwirq - 64) / 128 + 1; pin 1215 = node 9, pin 1216 = node 10)

/proc/interrupts shows the break happens exactly at the node boundary:

  enahisic2i1-rx0  pin 1200  count 102   <- node 9
  enahisic2i1-rx5  pin 1215  count   1   <- node 9, last pin
  enahisic2i1-tx5  pin 1216  count   0   <- node 10, first pin
  enahisic2i1-rx6  pin 1218  count   0   <- node 10
  ...all node 10 pins stay at zero.

Port 0 (entirely node 9) is unaffected.  Reverting the commit restores
normal operation.

The commit assumes CLEAR occupies a full 4 KB page at [0xa000, 0xb000)
and collides with node 10, so node 10+ gets shifted by 0x1000.

But get_mbigen_clear_reg() uses flat, chip-wide addressing -- it never
multiplies by the node ID:

    *addr = (hwirq / 32) * 4 + REG_MBIGEN_CLEAR_OFFSET;  /* 0xa000 */

Over the valid hwirq range [64, 1407], CLEAR only spans 0xa008-0xa0af
(168 bytes).  Node 10's registers are:

    TYPE: 0xa000-0xa00f  (16 B)   overlaps CLEAR by 8 B (0xa008-0xa00f)
    VEC:  0xa200-0xa3ff  (512 B)  no overlap with CLEAR

Shifting the whole page moves VEC from 0xa200 to 0xb200.  The hardware
reads the event ID from the fixed silicon address 0xa200 on interrupt
firing, but software wrote it to 0xb200 -- so the hardware gets an
uninitialised value and the interrupt is lost.

The only real overlap is 8 bytes of TYPE.  It can only trigger when a
single mbigen instance has devices on both node 1 (CLEAR 0xa008) and
node 10 (TYPE 0xa008).  On Hi1616 those nodes are on separate mbigen
instances, so it never triggers.

Suggested-by: Marc Zyngier <maz@kernel.org>
Fixes: 6be6cba9c4371d27f78d900ccfe34bb880d9ee20 ("irqchip/mbigen: Fix mbigen node address layout")
Cc: stable@vger.kernel.org
Signed-off-by: caina <caina@uniontech.com>
---
 drivers/irqchip/irq-mbigen.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 6f69f4e5dbac..12919836dadb 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -64,20 +64,6 @@ struct mbigen_device {
 	void __iomem		*base;
 };
 
-static inline unsigned int get_mbigen_node_offset(unsigned int nid)
-{
-	unsigned int offset = nid * MBIGEN_NODE_OFFSET;
-
-	/*
-	 * To avoid touched clear register in unexpected way, we need to directly
-	 * skip clear register when access to more than 10 mbigen nodes.
-	 */
-	if (nid >= (REG_MBIGEN_CLEAR_OFFSET / MBIGEN_NODE_OFFSET))
-		offset += MBIGEN_NODE_OFFSET;
-
-	return offset;
-}
-
 static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
 {
 	unsigned int nid, pin;
@@ -86,7 +72,8 @@ static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
 	nid = hwirq / IRQS_PER_MBIGEN_NODE + 1;
 	pin = hwirq % IRQS_PER_MBIGEN_NODE;
 
-	return pin * 4 + get_mbigen_node_offset(nid) + REG_MBIGEN_VEC_OFFSET;
+	return pin * 4 + nid * MBIGEN_NODE_OFFSET
+			+ REG_MBIGEN_VEC_OFFSET;
 }
 
 static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
@@ -101,7 +88,8 @@ static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
 	*mask = 1 << (irq_ofst % 32);
 	ofst = irq_ofst / 32 * 4;
 
-	*addr = ofst + get_mbigen_node_offset(nid) + REG_MBIGEN_TYPE_OFFSET;
+	*addr = ofst + nid * MBIGEN_NODE_OFFSET
+		+ REG_MBIGEN_TYPE_OFFSET;
 }
 
 static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,
-- 
2.20.1

Re: [PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[Hanjun Guo]

Hi caina, Marc,

On 2026/8/21 17:17, caina wrote:
> This reverts commit 6be6cba9c4371d27f78d900ccfe34bb880d9ee20.
> 
> Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
> appears to cause a regression on Hi1616.

This will break the new hardware :(

Sorry for the incompatible issue, but please wait for a while, Yipeng is
working on this now.

Thanks
Hanjun

Re: [PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[Yipeng Zou]

  Hi all,

     I’ll send a patch for this soon. I have a fix ready, but I need a 
bit more time for testing.

Thanks.

在 2026/8/22 17:55, Hanjun Guo 写道:
> Hi caina, Marc,
>
> On 2026/8/21 17:17, caina wrote:
>> This reverts commit 6be6cba9c4371d27f78d900ccfe34bb880d9ee20.
>>
>> Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
>> appears to cause a regression on Hi1616.
>
> This will break the new hardware :(
>
> Sorry for the incompatible issue, but please wait for a while, Yipeng is
> working on this now.
>
> Thanks
> Hanjun

-- 
Regards,
Yipeng Zou

Re: [PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[Marc Zyngier]

On Mon, 24 Aug 2026 05:00:42 +0100,
Yipeng Zou <zouyipeng@huawei.com> wrote:
> 
>  Hi all,
> 
>     I’ll send a patch for this soon. I have a fix ready, but I need a
> bit more time for testing.

I think the best option is to revert the breaking patch to get current
HW back into a working shape, and you can then provide properly tested
patches for the "new hardware" once you're ready, especially as this
needs to go into stable trees.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[Yipeng Zou]

Hi1616 is special hardware whose register layout differs from the other 
mbigen platforms: it does not have the clear register range fixed at 
[0xA000, 0xAFFF], on which the fix is based. As a result, skipping an 
extra node offset for node IDs greater than or equal to ten, as the fix 
does, makes the driver access wrong registers on Hi1616. The fix was 
originally introduced to address a real problem on the other platforms: 
each mbigen chip has its own independent set of clear registers, whose 
offset is fixed within the range [0xA000, 0xAFFF]. Meanwhile, mbigen 
allocates a consecutive 4-byte register to every hwirq for configuring 
the interrupt type, EOI and so on, so each 4KB node region accommodates 
128 hwirqs. Before the fix, the driver calculated the register address 
of each interrupt from its hwirq. This works without any problem as long 
as the number of interrupts is below 1280, which means no more than nine 
mbigen nodes. Once the maximum number of interrupts of a mbigen chip 
exceeds 1280, however, the node register addresses reach offset 0xA000 
and start to clobber the fixed clear-register range. The final solution 
will be worked out in cooperation with the BIOS: the firmware will pass 
a chip version to the driver so that it can take care of all platforms. 
Since this depends on the corresponding BIOS versions being adapted 
first, it cannot land immediately. In the meantime it is safe to revert 
the fix, because nobody has run into this problem for now: it can only 
be triggered on "new hardware" platforms, while all the platforms 
available today, including Hi1616, work correctly. So, let's revert this 
patch first. Acked-by: Yipeng Zou <zouyipeng@huawei.com>

-- 
Regards,
Yipeng Zou

Re: [PATCH] Revert "irqchip/mbigen: Fix mbigen node address layout"

[Yipeng Zou]

Oh Sorry, the format of the last email was messed up.

Hi1616 is special hardware whose register layout differs
from the other mbigen platforms: it does not have the clear register
range fixed at [0xA000, 0xAFFF], on which the fix is based.  As a
result, skipping an extra node offset for node IDs greater than or
equal to ten, as the fix does, makes the driver access wrong
registers on Hi1616.

The fix was originally introduced to address a real problem on the
other platforms: each mbigen chip has its own independent set of
clear registers, whose offset is fixed within the range [0xA000,
0xAFFF].  Meanwhile, mbigen allocates a consecutive 4-byte register
to every hwirq for configuring the interrupt type, EOI and so on, so
each 4KB node region accommodates 128 hwirqs.  Before the fix, the
driver calculated the register address of each interrupt from its
hwirq.  This works without any problem as long as the number of
interrupts is below 1280, which means no more than nine mbigen nodes.
Once the maximum number of interrupts of a mbigen chip exceeds 1280,
however, the node register addresses reach offset 0xA000 and start to
clobber the fixed clear-register range.

The final solution will be worked out in cooperation with the BIOS:
the firmware will pass a chip version to the driver so that it can
take care of all platforms.  Since this depends on the corresponding
BIOS versions being adapted first, it cannot land immediately.

In the meantime it is safe to revert the fix, because nobody has run
into this problem for now: it can only be triggered on "new hardware"
platforms, while all the platforms available today, including
Hi1616, work correctly.

So, let's revert this patch first.

Acked-by: Yipeng Zou <zouyipeng@huawei.com>

在 2026/8/21 17:17, caina 写道:
> This reverts commit 6be6cba9c4371d27f78d900ccfe34bb880d9ee20.
>
> Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
> appears to cause a regression on Hi1616.
>
> On-board hns NIC has two ports, enahisic2i0 and enahisic2i1, both
> behind mbigen-v2.  Port 0 works; port 1 cannot pass any traffic.
>
> Their interrupt pins fall on different mbigen nodes:
>
>    enahisic2i0: pins 1152-1198 -> all in node 9
>    enahisic2i1: pins 1200-1246 -> node 9 (1200-1215) + node 10 (1216-1246)
>
>    (nid = (hwirq - 64) / 128 + 1; pin 1215 = node 9, pin 1216 = node 10)
>
> /proc/interrupts shows the break happens exactly at the node boundary:
>
>    enahisic2i1-rx0  pin 1200  count 102   <- node 9
>    enahisic2i1-rx5  pin 1215  count   1   <- node 9, last pin
>    enahisic2i1-tx5  pin 1216  count   0   <- node 10, first pin
>    enahisic2i1-rx6  pin 1218  count   0   <- node 10
>    ...all node 10 pins stay at zero.
>
> Port 0 (entirely node 9) is unaffected.  Reverting the commit restores
> normal operation.
>
> The commit assumes CLEAR occupies a full 4 KB page at [0xa000, 0xb000)
> and collides with node 10, so node 10+ gets shifted by 0x1000.
>
> But get_mbigen_clear_reg() uses flat, chip-wide addressing -- it never
> multiplies by the node ID:
>
>      *addr = (hwirq / 32) * 4 + REG_MBIGEN_CLEAR_OFFSET;  /* 0xa000 */
>
> Over the valid hwirq range [64, 1407], CLEAR only spans 0xa008-0xa0af
> (168 bytes).  Node 10's registers are:
>
>      TYPE: 0xa000-0xa00f  (16 B)   overlaps CLEAR by 8 B (0xa008-0xa00f)
>      VEC:  0xa200-0xa3ff  (512 B)  no overlap with CLEAR
>
> Shifting the whole page moves VEC from 0xa200 to 0xb200.  The hardware
> reads the event ID from the fixed silicon address 0xa200 on interrupt
> firing, but software wrote it to 0xb200 -- so the hardware gets an
> uninitialised value and the interrupt is lost.
>
> The only real overlap is 8 bytes of TYPE.  It can only trigger when a
> single mbigen instance has devices on both node 1 (CLEAR 0xa008) and
> node 10 (TYPE 0xa008).  On Hi1616 those nodes are on separate mbigen
> instances, so it never triggers.
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Fixes: 6be6cba9c4371d27f78d900ccfe34bb880d9ee20 ("irqchip/mbigen: Fix mbigen node address layout")
> Cc: stable@vger.kernel.org
> Signed-off-by: caina <caina@uniontech.com>
> ---
>   drivers/irqchip/irq-mbigen.c | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 6f69f4e5dbac..12919836dadb 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -64,20 +64,6 @@ struct mbigen_device {
>   	void __iomem		*base;
>   };
>   
> -static inline unsigned int get_mbigen_node_offset(unsigned int nid)
> -{
> -	unsigned int offset = nid * MBIGEN_NODE_OFFSET;
> -
> -	/*
> -	 * To avoid touched clear register in unexpected way, we need to directly
> -	 * skip clear register when access to more than 10 mbigen nodes.
> -	 */
> -	if (nid >= (REG_MBIGEN_CLEAR_OFFSET / MBIGEN_NODE_OFFSET))
> -		offset += MBIGEN_NODE_OFFSET;
> -
> -	return offset;
> -}
> -
>   static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
>   {
>   	unsigned int nid, pin;
> @@ -86,7 +72,8 @@ static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
>   	nid = hwirq / IRQS_PER_MBIGEN_NODE + 1;
>   	pin = hwirq % IRQS_PER_MBIGEN_NODE;
>   
> -	return pin * 4 + get_mbigen_node_offset(nid) + REG_MBIGEN_VEC_OFFSET;
> +	return pin * 4 + nid * MBIGEN_NODE_OFFSET
> +			+ REG_MBIGEN_VEC_OFFSET;
>   }
>   
>   static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
> @@ -101,7 +88,8 @@ static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
>   	*mask = 1 << (irq_ofst % 32);
>   	ofst = irq_ofst / 32 * 4;
>   
> -	*addr = ofst + get_mbigen_node_offset(nid) + REG_MBIGEN_TYPE_OFFSET;
> +	*addr = ofst + nid * MBIGEN_NODE_OFFSET
> +		+ REG_MBIGEN_TYPE_OFFSET;
>   }
>   
>   static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,

-- 
Regards,
Yipeng Zou

[tip: irq/urgent] Revert "irqchip/mbigen: Fix mbigen node address layout"

[tip-bot2 for caina]

The following commit has been merged into the irq/urgent branch of tip:

Commit-ID:     e67091609cf85962f64391c1b0f93d4cbfcd4e22
Gitweb:        https://git.kernel.org/tip/e67091609cf85962f64391c1b0f93d4cbfcd4e22
Author:        caina <caina@uniontech.com>
AuthorDate:    Fri, 21 Aug 2026 17:17:20 +08:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 04 Sep 2026 16:12:53 +02:00

Revert "irqchip/mbigen: Fix mbigen node address layout"

This reverts commit 6be6cba9c4371d27f78d900ccfe34bb880d9ee20.

Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
appears to cause a regression on Hi1616.

On-board hns NIC has two ports, enahisic2i0 and enahisic2i1, both
behind mbigen-v2.  Port 0 works; port 1 cannot pass any traffic.

Their interrupt pins fall on different mbigen nodes:

  enahisic2i0: pins 1152-1198 -> all in node 9
  enahisic2i1: pins 1200-1246 -> node 9 (1200-1215) + node 10 (1216-1246)

  (nid = (hwirq - 64) / 128 + 1; pin 1215 = node 9, pin 1216 = node 10)

/proc/interrupts shows the break happens exactly at the node boundary:

  enahisic2i1-rx0  pin 1200  count 102   <- node 9
  enahisic2i1-rx5  pin 1215  count   1   <- node 9, last pin
  enahisic2i1-tx5  pin 1216  count   0   <- node 10, first pin
  enahisic2i1-rx6  pin 1218  count   0   <- node 10
  ...all node 10 pins stay at zero.

Port 0 (entirely node 9) is unaffected.  Reverting the commit restores
normal operation.

The commit assumes CLEAR occupies a full 4 KB page at [0xa000, 0xb000)
and collides with node 10, so node 10+ gets shifted by 0x1000.

But get_mbigen_clear_reg() uses flat, chip-wide addressing -- it never
multiplies by the node ID:

    *addr = (hwirq / 32) * 4 + REG_MBIGEN_CLEAR_OFFSET;  /* 0xa000 */

Over the valid hwirq range [64, 1407], CLEAR only spans 0xa008-0xa0af
(168 bytes).  Node 10's registers are:

    TYPE: 0xa000-0xa00f  (16 B)   overlaps CLEAR by 8 B (0xa008-0xa00f)
    VEC:  0xa200-0xa3ff  (512 B)  no overlap with CLEAR

Shifting the whole page moves VEC from 0xa200 to 0xb200.  The hardware
reads the event ID from the fixed silicon address 0xa200 on interrupt
firing, but software wrote it to 0xb200 -- so the hardware gets an
uninitialised value and the interrupt is lost.

The only real overlap is 8 bytes of TYPE.  It can only trigger when a
single mbigen instance has devices on both node 1 (CLEAR 0xa008) and
node 10 (TYPE 0xa008).  On Hi1616 those nodes are on separate mbigen
instances, so it never triggers.

Fixes: 6be6cba9c4371d27f78d900ccfe34bb880d9ee20 ("irqchip/mbigen: Fix mbigen node address layout")
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: caina <caina@uniontech.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Acked-by: Yipeng Zou <zouyipeng@huawei.com>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260821091720.16665-1-caina@uniontech.com
---
 drivers/irqchip/irq-mbigen.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 6f69f4e..1291983 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -64,20 +64,6 @@ struct mbigen_device {
 	void __iomem		*base;
 };
 
-static inline unsigned int get_mbigen_node_offset(unsigned int nid)
-{
-	unsigned int offset = nid * MBIGEN_NODE_OFFSET;
-
-	/*
-	 * To avoid touched clear register in unexpected way, we need to directly
-	 * skip clear register when access to more than 10 mbigen nodes.
-	 */
-	if (nid >= (REG_MBIGEN_CLEAR_OFFSET / MBIGEN_NODE_OFFSET))
-		offset += MBIGEN_NODE_OFFSET;
-
-	return offset;
-}
-
 static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
 {
 	unsigned int nid, pin;
@@ -86,7 +72,8 @@ static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
 	nid = hwirq / IRQS_PER_MBIGEN_NODE + 1;
 	pin = hwirq % IRQS_PER_MBIGEN_NODE;
 
-	return pin * 4 + get_mbigen_node_offset(nid) + REG_MBIGEN_VEC_OFFSET;
+	return pin * 4 + nid * MBIGEN_NODE_OFFSET
+			+ REG_MBIGEN_VEC_OFFSET;
 }
 
 static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
@@ -101,7 +88,8 @@ static inline void get_mbigen_type_reg(irq_hw_number_t hwirq,
 	*mask = 1 << (irq_ofst % 32);
 	ofst = irq_ofst / 32 * 4;
 
-	*addr = ofst + get_mbigen_node_offset(nid) + REG_MBIGEN_TYPE_OFFSET;
+	*addr = ofst + nid * MBIGEN_NODE_OFFSET
+		+ REG_MBIGEN_TYPE_OFFSET;
 }
 
 static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,