Re: [RFC v5 03/28] hw/arm/smmuv3: Thread SEC_SID through helper APIs

[Mostafa Saleh <smostafa@google.com>]

On Fri, Aug 14, 2026 at 12:24:08AM +0800, Tao Tang wrote:
> Extend the register and queue helper routines to accept an explicit
> SEC_SID argument instead of hard-coding the non-secure bank.
> 
> All existing callers are updated to pass SMMU_SEC_SID_NS, so the
> behavior remains identical. This prepares the code for handling
> additional security state banks in the future. So Non-secure state
> is the only state bank supported for now.
> 
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> Link: https://lore.kernel.org/qemu-devel/3097d58e-3793-4434-8beb-2e4f4c52f772@redhat.com/
> ---
>  hw/arm/smmuv3-accel.c    |  5 +++--
>  hw/arm/smmuv3-internal.h | 21 +++++++++------------
>  hw/arm/smmuv3.c          | 15 ++++++++-------
>  3 files changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 3ff3da66acd..fa079c8acea 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -294,6 +294,7 @@ bool smmuv3_accel_install_ste(SMMUv3State *s, SMMUDevice *sdev, int sid,
>      SMMUS1Hwpt *s1_hwpt = NULL;
>      const char *type;
>      STE ste;
> +    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>  
>      if (!accel || !accel->viommu) {
>          return true;
> @@ -323,7 +324,7 @@ bool smmuv3_accel_install_ste(SMMUv3State *s, SMMUDevice *sdev, int sid,
>       * attach/alloc fails, since the Guest–Host SID mapping stays
>       * valid as long as the device is behind the accelerated SMMUv3.
>       */
> -    if (!smmu_enabled(s)) {
> +    if (!smmu_enabled(s, sec_sid)) {
>          hwpt_id = smmuv3_accel_gbpa_hwpt(s, accel);
>      } else {
>          config = STE_CONFIG(&ste);
> @@ -558,7 +559,7 @@ bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
>          return true;
>      }
>  
> -    if (!smmuv3_eventq_enabled(s)) {
> +    if (!smmuv3_eventq_enabled(s, sec_sid)) {
>          return true;
>      }
>  
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 10154ca161d..a88743ba9cd 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -39,9 +39,8 @@ typedef enum SMMUTranslationClass {
>      SMMU_CLASS_IN,
>  } SMMUTranslationClass;
>  
> -static inline int smmu_enabled(SMMUv3State *s)
> +static inline int smmu_enabled(SMMUv3State *s, SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      return FIELD_EX32(bank->cr[0], CR0, SMMUEN);
>  }
> @@ -69,16 +68,16 @@ static inline uint32_t smmuv3_idreg(int regoffset)
>      return smmuv3_ids[regoffset / 4];
>  }
>  
> -static inline bool smmuv3_eventq_irq_enabled(SMMUv3State *s)
> +static inline bool smmuv3_eventq_irq_enabled(SMMUv3State *s,
> +                                             SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      return FIELD_EX32(bank->irq_ctrl, IRQ_CTRL, EVENTQ_IRQEN);
>  }
>  
> -static inline bool smmuv3_gerror_irq_enabled(SMMUv3State *s)
> +static inline bool smmuv3_gerror_irq_enabled(SMMUv3State *s,
> +                                             SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      return FIELD_EX32(bank->irq_ctrl, IRQ_CTRL, GERROR_IRQEN);
>  }
> @@ -123,23 +122,21 @@ static inline void queue_cons_incr(SMMUQueue *q)
>      q->cons = deposit32(q->cons, 0, q->log2size + 1, q->cons + 1);
>  }
>  
> -static inline bool smmuv3_cmdq_enabled(SMMUv3State *s)
> +static inline bool smmuv3_cmdq_enabled(SMMUv3State *s, SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      return FIELD_EX32(bank->cr[0], CR0, CMDQEN);
>  }
>  
> -static inline bool smmuv3_eventq_enabled(SMMUv3State *s)
> +static inline bool smmuv3_eventq_enabled(SMMUv3State *s, SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      return FIELD_EX32(bank->cr[0], CR0, EVENTQEN);
>  }
>  
> -static inline void smmu_write_cmdq_err(SMMUv3State *s, uint32_t err_type)
> +static inline void smmu_write_cmdq_err(SMMUv3State *s, uint32_t err_type,
> +                                       SMMUSecSID sec_sid)
>  {
> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>      bank->cmdq.cons = FIELD_DP32(bank->cmdq.cons, CMDQ_CONS, ERR, err_type);
>  }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index d3baae8a32a..51e970d710a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -60,7 +60,7 @@ static void smmuv3_trigger_irq(SMMUv3State *s, SMMUIrq irq,
>  
>      switch (irq) {
>      case SMMU_IRQ_EVTQ:
> -        pulse = smmuv3_eventq_irq_enabled(s);
> +        pulse = smmuv3_eventq_irq_enabled(s, sec_sid);
>          break;
>      case SMMU_IRQ_PRIQ:
>          qemu_log_mask(LOG_UNIMP, "PRI not yet supported\n");
> @@ -80,7 +80,7 @@ static void smmuv3_trigger_irq(SMMUv3State *s, SMMUIrq irq,
>          bank->gerror ^= new_gerrors;
>          trace_smmuv3_write_gerror(new_gerrors, bank->gerror);
>  
> -        pulse = smmuv3_gerror_irq_enabled(s);
> +        pulse = smmuv3_gerror_irq_enabled(s, sec_sid);
>          break;
>      }
>      }
> @@ -156,7 +156,7 @@ static MemTxResult smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
>      SMMUQueue *q = &bank->eventq;
>      MemTxResult r;
>  
> -    if (!smmuv3_eventq_enabled(s)) {
> +    if (!smmuv3_eventq_enabled(s, sec_sid)) {
>          return MEMTX_ERROR;
>      }
>  
> @@ -191,8 +191,9 @@ void smmuv3_propagate_event(SMMUv3State *s, Evt *evt)
>  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
>  {
>      Evt evt = {};
> +    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>  
> -    if (!smmuv3_eventq_enabled(s)) {
> +    if (!smmuv3_eventq_enabled(s, sec_sid)) {
>          return;
>      }
>  
> @@ -1127,7 +1128,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>  
>      qemu_mutex_lock(&s->mutex);
>  
> -    if (!smmu_enabled(s)) {
> +    if (!smmu_enabled(s, sec_sid)) {
>          if (FIELD_EX32(bank->gbpa, GBPA, ABORT)) {
>              status = SMMU_TRANS_ABORT;
>          } else {
> @@ -1357,7 +1358,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp)
>      SMMUQueue *q = &bank->cmdq;
>      SMMUCommandType type = 0;
>  
> -    if (!smmuv3_cmdq_enabled(s)) {
> +    if (!smmuv3_cmdq_enabled(s, sec_sid)) {
>          return 0;
>      }
>      /*
> @@ -1617,7 +1618,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp)
>  
>      if (cmd_error) {
>          trace_smmuv3_cmdq_consume_error(smmu_cmd_string(type), cmd_error);
> -        smmu_write_cmdq_err(s, cmd_error);
> +        smmu_write_cmdq_err(s, cmd_error, sec_sid);
>          smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_CMDQ_ERR_MASK);
>      }
>  
> -- 
> 2.34.1
>