smmuv3: Track SEC_SID in configs and events

Eric Auger via qemu development Mon, 02 Mar 2026 23:27:21 -0800

On 3/2/26 5:13 PM, Eric Auger wrote:
> Hi Tao,
> 
> On 2/21/26 11:02 AM, Tao Tang wrote:
>> Cache the SEC_SID inside SMMUTransCfg to keep configuration lookups
>> tied to the correct register bank.
>>
>> Plumb the SEC_SID through tracepoints and queue helpers so diagnostics
>> and event logs always show which security interface emitted the record.
>> To support this, the SEC_SID is placed in SMMUEventInfo so the bank is
>> identified as soon as an event record is built.
>>
>> Signed-off-by: Tao Tang <[email protected]>
>> Reviewed-by: Eric Auger <[email protected]>
>> ---
>>  hw/arm/smmuv3-internal.h     |  1 +
>>  hw/arm/smmuv3.c              | 20 +++++++++++++-------
>>  hw/arm/trace-events          |  2 +-
>>  include/hw/arm/smmu-common.h |  1 +
>>  4 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
>> index 866d62257e3..a1071f7b689 100644
>> --- a/hw/arm/smmuv3-internal.h
>> +++ b/hw/arm/smmuv3-internal.h
>> @@ -274,6 +274,7 @@ static inline const char 
>> *smmu_event_string(SMMUEventType type)
>>  
>>  /*  Encode an event record */
>>  typedef struct SMMUEventInfo {
>> +    SMMUSecSID sec_sid;
>>      SMMUEventType type;
>>      uint32_t sid;
>>      bool recorded;
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 2c107724e77..3438adcecd2 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -148,9 +148,9 @@ static MemTxResult queue_write(SMMUQueue *q, Evt *evt_in)
>>      return MEMTX_OK;
>>  }
>>  
>> -static MemTxResult smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
>> +static MemTxResult smmuv3_write_eventq(SMMUv3State *s, SMMUSecSID sec_sid,
>> +                                       Evt *evt)
>>  {
>> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>>      SMMUQueue *q = &bank->eventq;
>>      MemTxResult r;
>> @@ -178,7 +178,8 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo 
>> *info)
>>  {
>>      Evt evt = {};
>>      MemTxResult r;
>> -    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>> +    SMMUSecSID sec_sid = info->sec_sid;
>> +    g_assert(sec_sid < SMMU_SEC_SID_NUM);
>>  
>>      if (!smmuv3_eventq_enabled(s, sec_sid)) {
>>          return;
>> @@ -258,8 +259,9 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo 
>> *info)
>>          g_assert_not_reached();
>>      }
>>  
>> -    trace_smmuv3_record_event(smmu_event_string(info->type), info->sid);
>> -    r = smmuv3_write_eventq(s, &evt);
>> +    trace_smmuv3_record_event(sec_sid, smmu_event_string(info->type),
>> +                              info->sid);
>> +    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);
>>      }
>> @@ -917,6 +919,7 @@ static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, 
>> SMMUEventInfo *event)
>>                              100 * sdev->cfg_cache_hits /
>>                              (sdev->cfg_cache_hits + 
>> sdev->cfg_cache_misses));
>>          cfg = g_new0(SMMUTransCfg, 1);
>> +        cfg->sec_sid = SMMU_SEC_SID_NS;
>>  
>>          if (!smmuv3_decode_config(&sdev->iommu, cfg, event)) {
>>              g_hash_table_insert(bc->configs, sdev, cfg);
>> @@ -1074,7 +1077,8 @@ static IOMMUTLBEntry 
>> smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>      SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
>>      SMMUEventInfo event = {.type = SMMU_EVT_NONE,
>>                             .sid = sid,
>> -                           .inval_ste_allowed = false};
>> +                           .inval_ste_allowed = false,
>> +                           .sec_sid = sec_sid};
>>      SMMUTranslationStatus status;
>>      SMMUTransCfg *cfg = NULL;
>>      IOMMUTLBEntry entry = {
>> @@ -1176,7 +1180,9 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
>>                                 uint64_t num_pages, int stage)
>>  {
>>      SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
>> -    SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
>> +    SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
>> +    SMMUEventInfo eventinfo = {.sec_sid = sec_sid,
>> +                               .inval_ste_allowed = true};
>>      SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
>>      IOMMUTLBEvent event;
>>      uint8_t granule;
>> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
>> index 8135c0c7344..9c2cc131ab4 100644
>> --- a/hw/arm/trace-events
>> +++ b/hw/arm/trace-events
>> @@ -40,7 +40,7 @@ smmuv3_cmdq_opcode(const char *opcode) "<--- %s"
>>  smmuv3_cmdq_consume_out(uint32_t prod, uint32_t cons, uint8_t prod_wrap, 
>> uint8_t cons_wrap) "prod:%d, cons:%d, prod_wrap:%d, cons_wrap:%d "
>>  smmuv3_cmdq_consume_error(const char *cmd_name, uint8_t cmd_error) "Error 
>> on %s command execution: %d"
>>  smmuv3_write_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r) 
>> "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)"
>> -smmuv3_record_event(const char *type, uint32_t sid) "%s sid=0x%x"
>> +smmuv3_record_event(int sec_sid, const char *type, uint32_t sid) 
>> "sec_sid=%d %s sid=0x%x"
>>  smmuv3_find_ste(uint16_t sid, uint32_t features, uint16_t sid_split) 
>> "sid=0x%x features:0x%x, sid_split:0x%x"
>>  smmuv3_find_ste_2lvl(uint64_t strtab_base, uint64_t l1ptr, int 
>> l1_ste_offset, uint64_t l2ptr, int l2_ste_offset, int max_l2_ste) 
>> "strtab_base:0x%"PRIx64" l1ptr:0x%"PRIx64" l1_off:0x%x, l2ptr:0x%"PRIx64" 
>> l2_off:0x%x max_l2_ste:%d"
>>  smmuv3_get_ste(uint64_t addr) "STE addr: 0x%"PRIx64
>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index 6ea40f6b074..ae1489717fe 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -107,6 +107,7 @@ typedef struct SMMUS2Cfg {
>>  typedef struct SMMUTransCfg {
>>      /* Shared fields between stage-1 and stage-2. */
>>      SMMUStage stage;           /* translation stage */
>> +    SMMUSecSID sec_sid;        /* cached sec sid */
> I am now wondering if sec_sid shall be part of the TransCfg as this is
> never decoded from STE/CD, sin't it.

Having slept over it, caching sed_id here indicates whether the secure
stream table was used for decoding. So that can be considered as a
result of the config decode process.

a minor comment though, I think I would prefer having cfg->sec_sid
populated in smmuv3_decode_config as the other cfg fields. Meaning I
would rather add sed_sid to smmuv3_decode_config signature.

Thanks

Eric

> 
> Eric
>>      bool disabled;             /* smmu is disabled */
>>      bool bypassed;             /* translation is bypassed */
>>      bool aborted;              /* translation is aborted */
>
Re: [RFC v4 06/31] hw/arm/smmuv3: Track SEC_SID in configs and events

Reply via email to
