Re: [RFC 01/11] hw/arm/smmuv3: Introduce secure registers and commands

[Mostafa Saleh <smostafa@google.com>]

On Wed, Aug 06, 2025 at 11:11:24PM +0800, 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 <tangtao1634@phytium.com.cn>
> ---
>  hw/arm/smmuv3-internal.h | 57 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/smmuv3.h  | 23 ++++++++++++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index b6b7399347..483aaa915e 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -179,6 +179,63 @@ REG32(EVENTQ_IRQ_CFG2,     0xbc)
>  
>  #define A_IDREGS           0xfd0
>  
> +/* Secure registers */
> +#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)
> +
> +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)
> +
> +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)
> +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, SMMU_ENABLE);
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index d183a62766..72ad042514 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -63,6 +63,29 @@ struct SMMUv3State {
>      qemu_irq     irq[4];
>      QemuMutex mutex;
>      char *stage;
> +
> +    /* Secure state */
> +    uint32_t secure_idr[5];
> +    uint32_t secure_cr[3];
> +    uint32_t secure_cr0ack;
> +    uint32_t secure_init;
> +    uint32_t secure_gbpa;
> +    uint32_t secure_irq_ctrl;
> +    uint32_t secure_gerror;
> +    uint32_t secure_gerrorn;
> +    uint64_t secure_gerror_irq_cfg0;
> +    uint32_t secure_gerror_irq_cfg1;
> +    uint32_t secure_gerror_irq_cfg2;
> +    uint64_t secure_strtab_base;
> +    uint32_t secure_strtab_base_cfg;
> +    uint8_t  secure_sid_split;
> +    uint32_t secure_features;
> +
> +    uint64_t secure_eventq_irq_cfg0;
> +    uint32_t secure_eventq_irq_cfg1;
> +    uint32_t secure_eventq_irq_cfg2;
> +
> +    SMMUQueue secure_eventq, secure_cmdq;
>  };

As Philippe mentioned, this would be better the secure state is separated
in another instance of the struct, that seems it would reduce a lot of the
duplication later around the logic of MMIO and queues... in the next
patches.

Thanks,
Mostafa

>  
>  typedef enum {
> -- 
> 2.34.1
> 
>