mbigen: node 10+ interrupts lost after "Fix mbigen node address layout"

mbigen: node 10+ interrupts lost after "Fix mbigen node address layout"

[caina]

Hi Thomas, Yipeng,

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

Symptom
-------

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.

Why the commit's premise seems wrong
------------------------------------

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.

I don't have mbigen hardware documentation, so before sending a fix
I'd like to understand whether the original commit was targeting a
specific newer mbigen revision where CLEAR occupies a full 4 KB page.
If so, the skip would need to be conditional on hardware revision
rather than unconditional.  If not, my inclination is to send a revert,
or a narrower fix that skips CLEAR only in the TYPE path and leaves VEC
untouched — the latter avoids the interrupt loss while still addressing
the 8-byte TYPE/CLEAR overlap.  Any insight into the target hardware
would help me avoid breaking a platform I can't test.

Thanks,
caina

mbigen: node 10+ interrupts lost after "Fix mbigen node address layout"

[caina]

Hi Thomas, Yipeng,

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

Symptom
-------

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.

Why the commit's premise seems wrong
------------------------------------

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.

I don't have mbigen hardware documentation, so before sending a fix
I'd like to understand whether the original commit was targeting a
specific newer mbigen revision where CLEAR occupies a full 4 KB page.
If so, the skip would need to be conditional on hardware revision
rather than unconditional.  If not, my inclination is to send a revert,
or a narrower fix that skips CLEAR only in the TYPE path and leaves VEC
untouched — the latter avoids the interrupt loss while still addressing
the 8-byte TYPE/CLEAR overlap.  Any insight into the target hardware
would help me avoid breaking a platform I can't test.

Thanks,
caina

Re: mbigen: node 10+ interrupts lost after "Fix mbigen node address layout"

[Marc Zyngier]

On Thu, 13 Aug 2026 08:27:35 +0100,
caina <caina@uniontech.com> wrote:
> 
> Hi Thomas, Yipeng,
> 
> Commit 6be6cba9c437 ("irqchip/mbigen: Fix mbigen node address layout")
> appears to cause a regression on Hi1616.

[...]

Please send a patch reverting this. It clearly wasn't thought out, and
in the absence of any other justification, it makes more sense to
simply go back to a working state and get the HiSi people to sort
things out.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[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