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.
Eric > bool disabled; /* smmu is disabled */ > bool bypassed; /* translation is bypassed */ > bool aborted; /* translation is aborted */
