Re: [RFC v5 20/28] hw/arm/smmuv3: Route IRQ and GERROR handling by SEC_SID

[Tao Tang]

Hi Mostafa,

On 9/1/2026 10:33 PM, Mostafa Saleh wrote:
On Fri, Aug 14, 2026 at 12:25:58AM +0800, Tao Tang wrote:
Interrupt and global error handling currently always selects the
Non-secure register bank, even when the event or command originates
from the Secure programming interface. This splits CMDQ_CONS.ERR from
GERROR.CMDQ_ERR and prevents Secure software from acknowledging the
error through S_GERRORN.
Pass SEC_SID through the IRQ and GERROR helpers and all queue call
sites so interrupt enable checks, global error reporting, and
acknowledgement use the originating programming-interface bank.

Signed-off-by: Tao Tang <tangtao1...@phytium.com.cn>
---
  hw/arm/smmuv3.c | 20 +++++++++++---------
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index b36bc4a54e0..d69fc0898af 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -49,11 +49,11 @@
   *
   * @irq: irq type
   * @gerror_mask: mask of gerrors to toggle (relevant if @irq is GERROR)
+ * @sec_sid: security state of the programming interface
   */
  static void smmuv3_trigger_irq(SMMUv3State *s, SMMUIrq irq,
-                               uint32_t gerror_mask)
+                               uint32_t gerror_mask, SMMUSecSID sec_sid)
  {
-    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
  
      bool pulse = false;
@@ -90,9 +90,9 @@ static void smmuv3_trigger_irq(SMMUv3State *s, SMMUIrq irq,
      }
  }
  
-static void smmuv3_write_gerrorn(SMMUv3State *s, uint32_t new_gerrorn)
+static void smmuv3_write_gerrorn(SMMUv3State *s, uint32_t new_gerrorn,
+                                 SMMUSecSID sec_sid)
  {
-    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
      uint32_t pending = bank->gerror ^ bank->gerrorn;
      uint32_t toggled = bank->gerrorn ^ new_gerrorn;
@@ -173,7 +173,7 @@ static MemTxResult smmuv3_write_eventq(SMMUv3State *s, 
SMMUSecSID sec_sid,
      }
  
      if (!smmuv3_q_empty(q)) {
-        smmuv3_trigger_irq(s, SMMU_IRQ_EVTQ, 0);
+        smmuv3_trigger_irq(s, SMMU_IRQ_EVTQ, 0, sec_sid);
      }
      return MEMTX_OK;
  }
@@ -187,7 +187,8 @@ void smmuv3_propagate_event(SMMUv3State *s, Evt *evt, 
SMMUSecSID sec_sid)
      QEMU_LOCK_GUARD(&s->mutex);
      r = smmuv3_write_eventq(s, sec_sid, evt);
      if (r != MEMTX_OK) {
-        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_EVENTQ_ABT_ERR_MASK);
+        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR,
+                           R_GERROR_EVENTQ_ABT_ERR_MASK, sec_sid);
      }
  }
  
@@ -1645,7 +1646,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp, SMMUSecSID sec_sid)
          switch (type) {
          case SMMU_CMD_SYNC:
              if (CMD_SYNC_CS(&cmd) & CMD_SYNC_SIG_IRQ) {
-                smmuv3_trigger_irq(s, SMMU_IRQ_CMD_SYNC, 0);
+                smmuv3_trigger_irq(s, SMMU_IRQ_CMD_SYNC, 0, sec_sid);
              }
              break;
          case SMMU_CMD_PREFETCH_CONFIG:
@@ -1879,7 +1880,8 @@ static int smmuv3_cmdq_consume(SMMUv3State *s, Error 
**errp, SMMUSecSID sec_sid)
      if (cmd_error) {
          trace_smmuv3_cmdq_consume_error(smmu_cmd_string(type), cmd_error);
          smmu_write_cmdq_err(s, cmd_error, sec_sid);
-        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_CMDQ_ERR_MASK);
+        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR,
+                           R_GERROR_CMDQ_ERR_MASK, sec_sid);
Won't we need a new line for the secure smmu instead of sharing
the same one in this function "qemu_irq_pulse(s->irq[irq])"?
The spec says:
        The SMMU has the following interrupt sources. Depending on the
        implementation, each interrupt source asserts a wired interrupt
        output that is unique to the source, or sends an MSI, or both.
                - Event queue
                - Secure Event queue

The way I read it, they have to be different.

You're right. Secure EventQ, CMD_SYNC and GERROR need separate wired 
outputs. I'll add the corresponding SMMU_IRQ_S_* indices, expand irq[], 
and update IRQ registration and board wiring in v6.

Callers will still pass the notification type and source sec_sid:

smmuv3_trigger_irq(s, SMMU_IRQ_EVTQ, 0, sec_sid);

After the existing bank-specific checks, the helper will select 
SMMU_IRQ_S_EVTQ when sec_sid is Secure, and similarly for CMD_SYNC and 
GERROR.

Thanks,
Mostafa


Thanks,

Tao