Hi Tao, On 9/25/25 6:26 PM, Tao Tang wrote: > The Arm SMMUv3 architecture defines a set of registers and commands for > managing secure transactions and context. > > This patch introduces the definitions for these secure registers and > commands within the SMMUv3 device model internal header. > > Signed-off-by: Tao Tang <[email protected]> > --- > hw/arm/smmuv3-internal.h | 72 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h > index 71a3c0c02c..3820157eaa 100644 > --- a/hw/arm/smmuv3-internal.h > +++ b/hw/arm/smmuv3-internal.h > @@ -38,7 +38,7 @@ typedef enum SMMUTranslationClass { > SMMU_CLASS_IN, > } SMMUTranslationClass; > > -/* MMIO Registers */ > +/* MMIO Registers. The offsets are shared by Non-secure/Realm/Root states. */ s/The offsets are shared/Shared ? > > REG32(IDR0, 0x0) > FIELD(IDR0, S2P, 0 , 1) > @@ -121,6 +121,7 @@ REG32(CR0, 0x20) > FIELD(CR0, CMDQEN, 3, 1) > > #define SMMU_CR0_RESERVED 0xFFFFFA20 > +#define SMMU_S_CR0_RESERVED 0xFFFFFC12 > > > REG32(CR0ACK, 0x24) > @@ -180,6 +181,75 @@ REG32(EVENTQ_IRQ_CFG2, 0xbc) > > #define A_IDREGS 0xfd0 > > +/* Secure registers. The offsets are begin with SMMU_SECURE_BASE_OFFSET */ Start of secure-only registers? At least it deserves some reworking. > +#define SMMU_SECURE_BASE_OFFSET 0x8000 > + > +REG32(S_IDR0, 0x8000) > +REG32(S_IDR1, 0x8004) > + FIELD(S_IDR1, S_SIDSIZE, 0 , 6) > + FIELD(S_IDR1, SEL2, 29, 1) > + FIELD(S_IDR1, SECURE_IMPL, 31, 1) > + > +REG32(S_IDR2, 0x8008) > +REG32(S_IDR3, 0x800c) > +REG32(S_IDR4, 0x8010) > + > +REG32(S_CR0, 0x8020) > + FIELD(S_CR0, SMMUEN, 0, 1) > + FIELD(S_CR0, EVENTQEN, 2, 1) > + FIELD(S_CR0, CMDQEN, 3, 1) > + > +REG32(S_CR0ACK, 0x8024) > +REG32(S_CR1, 0x8028) > +REG32(S_CR2, 0x802c) > + > +REG32(S_INIT, 0x803c) > + FIELD(S_INIT, INV_ALL, 0, 1) > +/* Alias for the S_INIT offset to match in the dispatcher switch */ what is the S_INIT_ALIAS purpose? At this stage of the reading I don't understand above comment. This it does not match any actual reg, I would move this defintion in the patch that actually uses it. > +#define A_S_INIT_ALIAS 0x3c > + > +REG32(S_GBPA, 0x8044) > + FIELD(S_GBPA, ABORT, 20, 1) > + FIELD(S_GBPA, UPDATE, 31, 1) > + > +REG32(S_IRQ_CTRL, 0x8050) > + FIELD(S_IRQ_CTRL, GERROR_IRQEN, 0, 1) > + FIELD(S_IRQ_CTRL, EVENTQ_IRQEN, 2, 1) > + > +REG32(S_IRQ_CTRLACK, 0x8054) > + > +REG32(S_GERROR, 0x8060) > + FIELD(S_GERROR, CMDQ_ERR, 0, 1) > + > +#define SMMU_GERROR_IRQ_CFG0_RESERVED 0x00FFFFFFFFFFFFFC > +#define SMMU_GERROR_IRQ_CFG2_RESERVED 0x000000000000003F > + > +#define SMMU_STRTAB_BASE_RESERVED 0x40FFFFFFFFFFFFC0 > +#define SMMU_QUEUE_BASE_RESERVED 0x40FFFFFFFFFFFFFF > +#define SMMU_EVENTQ_IRQ_CFG0_RESERVED 0x00FFFFFFFFFFFFFC > + > +REG32(S_GERRORN, 0x8064) > +REG64(S_GERROR_IRQ_CFG0, 0x8068) > +REG32(S_GERROR_IRQ_CFG1, 0x8070) > +REG32(S_GERROR_IRQ_CFG2, 0x8074) > +REG64(S_STRTAB_BASE, 0x8080) > +REG32(S_STRTAB_BASE_CFG, 0x8088) > + FIELD(S_STRTAB_BASE_CFG, LOG2SIZE, 0, 6) > + FIELD(S_STRTAB_BASE_CFG, SPLIT, 6, 5) > + FIELD(S_STRTAB_BASE_CFG, FMT, 16, 2) > + > +REG64(S_CMDQ_BASE, 0x8090) > +REG32(S_CMDQ_PROD, 0x8098) > +REG32(S_CMDQ_CONS, 0x809c) > + FIELD(S_CMDQ_CONS, ERR, 24, 7) > + > +REG64(S_EVENTQ_BASE, 0x80a0) > +REG32(S_EVENTQ_PROD, 0x80a8) > +REG32(S_EVENTQ_CONS, 0x80ac) > +REG64(S_EVENTQ_IRQ_CFG0, 0x80b0) > +REG32(S_EVENTQ_IRQ_CFG1, 0x80b8) > +REG32(S_EVENTQ_IRQ_CFG2, 0x80bc) > + > static inline int smmu_enabled(SMMUv3State *s) > { > return FIELD_EX32(s->cr[0], CR0, SMMUEN); Besides other definitions look good to me
Thanks Eric
