Hi Jim,
On 8/21/2026 5:52 PM, Jim MacArthur wrote:
On Fri, Aug 14, 2026 at 12:24:07AM +0800, Tao Tang wrote:
Rework the SMMUv3 state management by introducing a banked register
structure. This is a purely mechanical refactoring with no functional
changes.
To support multiple security states, a new enum, SMMUSecSID, is
introduced to identify each state, sticking to the spec terminology.
A new structure, SMMUv3RegBank, is then defined to hold the state
for a single security context. The main SMMUv3State now contains an
array of these banks, indexed by SMMUSecSID. This avoids the need for
separate fields for non-secure and future secure registers.
All existing code, which handles only the Non-secure state, is updated
to access its state via s->bank[SMMU_SEC_SID_NS]. A local bank helper
pointer is used where it improves readability.
Function signatures and logic remain untouched in this commit to
isolate the structural changes and simplify review. This is the
foundational step for building multi-security-state support.
static void smmuv3_reset(SMMUv3State *s)
{
- s->cmdq.base = deposit64(s->cmdq.base, 0, 5, SMMU_CMDQS);
- s->cmdq.prod = 0;
- s->cmdq.cons = 0;
- s->cmdq.entry_size = sizeof(struct Cmd);
- s->eventq.base = deposit64(s->eventq.base, 0, 5, SMMU_EVENTQS);
- s->eventq.prod = 0;
- s->eventq.cons = 0;
- s->eventq.entry_size = sizeof(struct Evt);
-
- s->features = 0;
- s->sid_split = 0;
- s->cr[0] = 0;
- s->cr0ack = 0;
- s->irq_ctrl = 0;
- s->gerror = 0;
- s->gerrorn = 0;
+ SMMUv3RegBank *bank = smmuv3_bank(s, SMMU_SEC_SID_NS);
+
+ bank->cmdq.base = deposit64(bank->cmdq.base, 0, 5, SMMU_CMDQS);
+ bank->cmdq.prod = 0;
+ bank->cmdq.cons = 0;
+ bank->cmdq.entry_size = sizeof(struct Cmd);
+ bank->eventq.base = deposit64(bank->eventq.base, 0, 5, SMMU_EVENTQS);
+ bank->eventq.prod = 0;
+ bank->eventq.cons = 0;
+ bank->eventq.entry_size = sizeof(struct Evt);
+
+ bank->features = 0;
+ bank->sid_split = 0;
+ bank->cr[0] = 0;
+ bank->cr0ack = 0;
+ bank->irq_ctrl = 0;
+ bank->gerror = 0;
+ bank->gerrorn = 0;
+ bank->gbpa = SMMU_GBPA_RESET_VAL;
+
+ s->aidr = 0x1;
s->statusr = 0;
- s->gbpa = SMMU_GBPA_RESET_VAL;
}
Why reset s->aidr here? It should be read-only and set in smmuv3_init_id_regs.
Good catch, this is redundant since AIDR is initialized in
smmuv3_init_id_regs(). I'll remove it in the next revision.
It won't do any harm and the reset of this patch looks correct, so:
Reviewed-by: Jim MacArthur <[email protected]>
Thanks,
Tao
