APLIC: clear level-sensitive MSI pending bit when the rectified input is low

[dreamqin68]

Summary

This PR fixes a stale pending-bit case for level-sensitive APLIC sources in MSI
delivery mode.

For a valid active Level1/Level0 source with domaincfg.DM = 1, the current
ips.wBitUI helper gates every pending-bit write by intSrcsRectified(ui):

when (domaincfg.DM) {
  when (intSrcsRectified(ui)) { bits(ui):=bit }
}

That gate correctly prevents setting a level-sensitive pending bit while the
rectified input is low, but it also blocks clears. As a result, if a source
deasserts after its pending bit was already set, the pending bit can remain set.
If an MSI was already forwarded, the later ack clear can also be blocked by the
same gate.

Impact

Software can observe a stale pending bit when reading setip. A stale source
can also be forwarded again as a spurious MSI after the original level input has
already deasserted.

Spec Anchor

RISC-V AIA (riscv-aia commit
9507866427961d4ec2d2400ea875103b2a09520b, 20250312 revision),
src/AdvPLIC.adoc, "Precise effects on interrupt-pending bits" / Level1
and Level0 source behavior in MSI delivery mode, raw src/AdvPLIC.adoc lines
1013-1027 in that source revision, says:

• the pending bit is "cleared whenever the rectified input value is low"
• the pending bit is also cleared "when the interrupt is forwarded by MSI"
• a level source is "never set (= 1) when the rectified input value is low"

So low rectified input must clear pending, and forwarded-MSI ack/clear must not
be suppressed by the current low rectified input.

Minimal Reproducer Shape

Focused architectural state:

• source is active
• source mode is Level1 or Level0
• domaincfg.DM = 1 / MSI delivery mode
• pending bit is already set from a previous asserted level
• current rectified input is low

Expected by AIA:

• the pending bit clears

Observed before this patch in a focused generated-RTL replay:

• low_rectified_without_ack: pending remains set
• ack_after_forwarded_msi: pending remains set

Observed after this patch in the same replay:

• low_rectified_without_ack: pending clears
• ack_after_forwarded_msi: pending clears

The patch also adds a cocotb regression,
aplic_level_msi_pending_clear_on_low_test, that uses the existing
test/aplic register-access helpers to program domaincfg/sourcecfg, drive
the external intSrcs_N pins, and read setips for both Level1 and Level0.
That regression is still a focused APLIC test, not a full-system XiangShan
cosim.

Root Cause

The common root cause is that ips.wBitUI gates all Level1/Level0 MSI-delivery
pending-bit writes by intSrcsRectified(ui). That predicate is right for
set-to-one writes, but wrong for clear-to-zero writes.

This shows up in two places:

1. The low-rectified source-deassert case has no explicit clear path. The
   source-trigger block calls ips.wBitUI(i.U, true.B) on a trigger, but there
   is no matching ips.wBitUI(i.U, false.B) for Level1/Level0 MSI-delivery
   sources while the rectified input is low.
2. Existing clear callers, including the MSI-forwarding ack path
   (ips.wBitUI(topi, false.B)) and explicit clear-register paths, can be
   blocked by the same rectified-input gate.

Fix

This patch does two things:

1. Corrects the central ips.wBitUI helper so clear-to-zero writes reach
   ips.bits even when the current rectified input is low, while set-to-one
   writes remain blocked when low.
2. Adds the missing clear request for active Level1/Level0 MSI-delivery sources
   when the rectified input is low, so source deassertion clears stale pending
   state without relying on a later MSI ack.

The first change is not an ack-only special case: the new low-rectified clear
path also calls ips.wBitUI(i.U, false.B), so without the helper change it
would be blocked by the original gate. Keeping all clear writes in wBitUI
also preserves one central pending-bit write rule for explicit clrip/in_clrip
clears and MSI-ack clears.

There is also a real timing window for the forwarded-MSI case. When the MSI
request A-channel fires, Domain enters waiting_ack; the pending bit is not
cleared until the later D-channel ack. During that wait, the synchronized
rectified input can become low. In that state, both the low-rectified clear
request and the eventual ack clear are clear-to-zero writes, so they must not be
rejected by the old intSrcsRectified(ui) set-write gate.

Patch diff:

diff --git a/src/main/scala/APLIC.scala b/src/main/scala/APLIC.scala
index 76bdbc6..9ac812e 100644
--- a/src/main/scala/APLIC.scala
+++ b/src/main/scala/APLIC.scala
@@ -164,7 +164,11 @@ class APLIC(
       def wBitUI(ui:UInt, bit:Bool): Unit = when (sourcecfgs.actives(ui)) {
         when (sourcecfgs.regs(ui).SM===sourcecfgs.level1 || sourcecfgs.regs(ui).SM===sourcecfgs.level0) {
           when (domaincfg.DM) {
-            when (intSrcsRectified(ui)) { bits(ui):=bit }
+            // The !bit case is intentional: clear-to-zero writes must pass
+            // even when rectified is low. This covers low-rectified source
+            // clears, in_clrip/clripnum, and waiting_ack MSI D-channel ack
+            // clears after a forwarded MSI when rectified has since gone low.
+            when (!bit || intSrcsRectified(ui)) { bits(ui):=bit }
           }.otherwise {/* Currently not support domaincfg.DM===0 */}
         }.otherwise { bits(ui):=bit }
       }
@@ -257,7 +261,13 @@ class APLIC(
       trigger := rect && !RegNext(rect)
     }}
     intSrcsTriggered.zipWithIndex.map { case (trigger:Bool, i:Int) =>
-      when(trigger) {ips.wBitUI(i.U, true.B)}
+      val isLevel = sourcecfgs.regs(i).SM===sourcecfgs.level1 || sourcecfgs.regs(i).SM===sourcecfgs.level0
+      // Reuse wBitUI even though the outer guard repeats its predicates: this
+      // keeps the active/level/MSI pending-write rules centralized. The outer
+      // guard only selects the low-rectified clear case.
+      when (sourcecfgs.actives(i) && domaincfg.DM && isLevel && !intSrcsRectified(i)) {
+        ips.wBitUI(i.U, false.B)
+      }.elsewhen(trigger) {ips.wBitUI(i.U, true.B)}
     }
 
     // The ":+ true.B" trick explain:
diff --git a/test/aplic/main.py b/test/aplic/main.py
index f90cd52..5f629a6 100644
--- a/test/aplic/main.py
+++ b/test/aplic/main.py
@@ -18,6 +18,11 @@ from cocotb.clock import Clock
 from cocotb.triggers import RisingEdge, FallingEdge, Edge
 from common import *
 
+# domaincfg.r has the fixed high field at bit31, DM=1 at bit2, and IE=0 here.
+domaincfg_read_high = 0x80000000
+domaincfg_dm_msi = 0x4
+domaincfg_msi_mode_ie_disabled = domaincfg_read_high | domaincfg_dm_msi
+
 @cocotb.test()
 async def aplic_write_read_test(dut):
   # Start the clock
@@ -193,6 +198,57 @@ async def aplic_in_clrips_test(dut):
   await a_put_full32(dut, aplic_m_base_addr+offset_sourcecfg+6*4, 0)
   await FallingEdge(dut.clock)
 
+@cocotb.test()
+async def aplic_level_msi_pending_clear_on_low_test(dut):
+  # Start the clock
+  cocotb.start_soon(Clock(dut.clock, 1, units="ns").start())
+
+  dut.reset.value = 1
+  dut.toaia_0_d_ready.value = 1
+  dut.toaia_0_a_valid.value = 0
+  for i in range(64):
+    getattr(dut, f"intSrcs_{i}").value = 0
+  for _ in range(10):
+    await RisingEdge(dut.clock)
+  dut.reset.value = 0
+  await RisingEdge(dut.clock)
+
+  await a_put_full32(dut, aplic_m_base_addr+offset_domaincfg, domaincfg_msi_mode_ie_disabled)
+  # Keep domain IE and targets unconfigured here. The test intentionally
+  # observes only pending set/clear-on-low through setips; MSI forwarding is
+  # exercised separately by aplic_msi_test.
+
+  async def check_level_source(int_num, source_mode, asserted_value, deasserted_value):
+    intSrc = getattr(dut, f"intSrcs_{int_num}")
+    ip_addr = aplic_m_base_addr + offset_setips + (int_num // 32) * 4
+    ip_mask = 1 << (int_num % 32)
+
+    intSrc.value = deasserted_value
+    # APLIC source numbers are 1-based; sourcecfg register slots are 0-based.
+    await a_put_full32(dut, aplic_m_base_addr+offset_sourcecfg+(int_num-1)*4, source_mode)
+    for _ in range(6):
+      await FallingEdge(dut.clock)
+    assert (await a_get32(dut, ip_addr) & ip_mask) == 0
+
+    intSrc.value = asserted_value
+    for _ in range(6):
+      await FallingEdge(dut.clock)
+    assert (await a_get32(dut, ip_addr) & ip_mask) != 0
+
+    intSrc.value = deasserted_value
+    for _ in range(6):
+      await FallingEdge(dut.clock)
+    assert (await a_get32(dut, ip_addr) & ip_mask) == 0
+
+    await a_put_full32(dut, aplic_m_base_addr+offset_sourcecfg+(int_num-1)*4, 0)
+    intSrc.value = deasserted_value
+    for _ in range(2):
+      await FallingEdge(dut.clock)
+
+  await check_level_source(19, sourcecfg_sm_level1, 1, 0)
+  # Level0 is low-active: raw intSrc=0 is asserted, raw intSrc=1 is deasserted.
+  await check_level_source(20, sourcecfg_sm_level0, 0, 1)
+
 @cocotb.test()
 async def aplic_msi_test(dut):
   # Start the clock
@@ -258,4 +314,3 @@ async def aplic_msi_test(dut):
   await FallingEdge(dut.clock)
   dut.intSrcs_43.value = 0
   await expect_int_num(dut, eiid, imsic_sg_base_addr+0x1000*guest_id)
-

The patch is intentionally narrow: edge-triggered sources continue to use the
existing path, and set-to-one writes for Level1/Level0 MSI sources are still
blocked when the rectified input is low.

Write ordering follows the existing last-connect structure in Domain:
register-map writes are connected first, source-trigger updates are connected
next, and the MSI ack clear is connected last. The new low-rectified clear and
the trigger set are mutually exclusive in the same source-update block:
low-clear requires !intSrcsRectified(i), while trigger is
intSrcsRectified(i) && !RegNext(intSrcsRectified(i)). If a set-to-one write
collides with a low rectified input, the low-clear wins, which is the required
AIA behavior; if a later MSI ack also clears, it writes the same false value.
The explicit low-clear branch intentionally calls ips.wBitUI instead of
writing bits directly so the normal active/level/MSI pending-write semantics
remain centralized in one helper.

This patch follows the current AIA text for the level-sensitive MSI race
discussed in riscv/riscv-aia#58: when the rectified input becomes low, the
pending bit is cleared. If a level source asserts and then deasserts before an
MSI is forwarded, this implementation does not retain an extra latched
interrupt after deassertion; it reflects the spec rule that a Level1/Level0 MSI
pending bit is never set while the rectified input is low.

Validation

I checked the two load-bearing scenarios with a focused generated-RTL replay of
Domain.

Scenario	Old RTL	Patched RTL	Expected
low rectified input, no ack	pending remains set	pending clears	pending clears
ack after forwarded MSI	pending remains set	pending clears	pending clears

I also added aplic_level_msi_pending_clear_on_low_test to the existing
cocotb APLIC test. It covers the register-programmed Level1 and Level0
clear-on-low path through setips. It intentionally leaves domain interrupt
enable and MSI targets unconfigured, so MSI forwarding cannot consume the
pending bit while the test is observing pending set/clear behavior; forwarding
itself is already covered by the existing aplic_msi_test.

The load-bearing cocotb check is non-vacuous: the same testcase fails on
unpatched generated RTL and passes on patched generated RTL:

PYTHONPATH=$PWD/test make -C test/aplic \
  TESTCASE=aplic_level_msi_pending_clear_on_low_test \
  SIM=verilator EXTRA_ARGS=--trace

The run reports TESTS=1 PASS=1 FAIL=0 SKIP=0 at 71.5 ns of simulated time.
On unpatched generated RTL, the same testcase reports
TESTS=1 PASS=0 FAIL=1 SKIP=0 at 38 ns, failing after a Level1 source
deassertion because the setips pending bit remains set.

The ack_after_forwarded_msi subcase is covered by the focused Domain replay
above. The cocotb regression is intentionally the normal register-programmed
clear-on-low path; after that low-clear fix is present, a top-level APLIC+IMSIC
test no longer isolates the later ack clear without adding internal
response-stall/poke machinery.

Public Context

I did not find an existing ChiselAIA/XiangShan implementation report or fix for
this stale-pending clear case.

The RISC-V AIA specification discussions riscv/riscv-aia#58 and
riscv/riscv-aia#130 discuss Level-sensitive APLIC behavior in MSI mode and are
consistent with the clear-on-low / clear-on-forwarding reading above. They are
specification-context discussions, not ChiselAIA implementation reports for
this bug.

Scope

This PR fixes the focused APLIC pending-bit behavior. The validation above
does not claim full-system XiangShan cosim.