Hi Tao,

On 12/24/25 4:46 AM, Tao Tang wrote:
> Switch STE/CD bitfield definitions and accessors to the
> 'hw/registerfields.h' REG/FIELD API.
>
> Signed-off-by: Tao Tang <[email protected]>
> ---
>  include/hw/arm/smmuv3-common.h | 169 +++++++++++++++++++++++----------
>  1 file changed, 120 insertions(+), 49 deletions(-)
>
> diff --git a/include/hw/arm/smmuv3-common.h b/include/hw/arm/smmuv3-common.h
> index 9da817f41a..b6da2fd62c 100644
> --- a/include/hw/arm/smmuv3-common.h
> +++ b/include/hw/arm/smmuv3-common.h
> @@ -11,6 +11,8 @@
>  #ifndef HW_ARM_SMMUV3_COMMON_H
>  #define HW_ARM_SMMUV3_COMMON_H
>  
> +#include "hw/registerfields.h"
> +
>  /* Configuration Data */
>  
>  /* STE Level 1 Descriptor */
> @@ -35,63 +37,132 @@ typedef struct CD {
>  
>  /* STE fields */
>  
> -#define STE_VALID(x)   extract32((x)->word[0], 0, 1)
> +REG32(STE_0, 0)
> +    FIELD(STE_0, VALID, 0, 1)
> +    FIELD(STE_0, CONFIG, 1, 3)
> +    FIELD(STE_0, S1FMT, 4, 2)
> +    FIELD(STE_0, CTXPTR_LO, 6, 26)
> +REG32(STE_1, 4)
> +    FIELD(STE_1, CTXPTR_HI, 0, 16)
not related to your patch, but shouldn't it be 24 instead of 16
> +    FIELD(STE_1, S1CDMAX, 27, 5)
> +REG32(STE_2, 8)
> +    FIELD(STE_2, S1STALLD, 27, 1)
> +    FIELD(STE_2, EATS, 28, 2)
> +    FIELD(STE_2, STRW, 30, 2)
> +REG32(STE_4, 16)
> +    FIELD(STE_4, S2VMID, 0, 16)
> +REG32(STE_5, 20)
> +    FIELD(STE_5, S2T0SZ, 0, 6)
> +    FIELD(STE_5, S2SL0, 6, 2)
> +    FIELD(STE_5, S2TG, 14, 2)
> +    FIELD(STE_5, S2PS, 16, 3)
> +    FIELD(STE_5, S2AA64, 19, 1)
> +    FIELD(STE_5, S2ENDI, 20, 1)
> +    FIELD(STE_5, S2AFFD, 21, 1)
> +    FIELD(STE_5, S2HD, 23, 1)
> +    FIELD(STE_5, S2HA, 24, 1)
> +    FIELD(STE_5, S2S, 25, 1)
> +    FIELD(STE_5, S2R, 26, 1)
> +REG32(STE_6, 24)
> +    FIELD(STE_6, S2TTB_LO, 4, 28)
> +REG32(STE_7, 28)
> +    FIELD(STE_7, S2TTB_HI, 0, 16)
same here?
> +
> +/* Get STE fields */
> +#define STE_VALID(x)      FIELD_EX32((x)->word[0], STE_0, VALID)
> +#define STE_CONFIG(x)     FIELD_EX32((x)->word[0], STE_0, CONFIG)
> +#define STE_S1FMT(x)      FIELD_EX32((x)->word[0], STE_0, S1FMT)
> +#define STE_CTXPTR(x)                                               \
> +    (((uint64_t)FIELD_EX32((x)->word[0], STE_0, CTXPTR_LO) << 6) |  \
> +     ((uint64_t)FIELD_EX32((x)->word[1], STE_1, CTXPTR_HI) << 32))
> +#define STE_S1CDMAX(x)    FIELD_EX32((x)->word[1], STE_1, S1CDMAX)
> +#define STE_S1STALLD(x)   FIELD_EX32((x)->word[2], STE_2, S1STALLD)
> +#define STE_EATS(x)       FIELD_EX32((x)->word[2], STE_2, EATS)
> +#define STE_STRW(x)       FIELD_EX32((x)->word[2], STE_2, STRW)
> +#define STE_S2VMID(x)     FIELD_EX32((x)->word[4], STE_4, S2VMID)
> +#define STE_S2T0SZ(x)     FIELD_EX32((x)->word[5], STE_5, S2T0SZ)
> +#define STE_S2SL0(x)      FIELD_EX32((x)->word[5], STE_5, S2SL0)
> +#define STE_S2TG(x)       FIELD_EX32((x)->word[5], STE_5, S2TG)
> +#define STE_S2PS(x)       FIELD_EX32((x)->word[5], STE_5, S2PS)
> +#define STE_S2AA64(x)     FIELD_EX32((x)->word[5], STE_5, S2AA64)
> +#define STE_S2ENDI(x)     FIELD_EX32((x)->word[5], STE_5, S2ENDI)
> +#define STE_S2AFFD(x)     FIELD_EX32((x)->word[5], STE_5, S2AFFD)
> +#define STE_S2HD(x)       FIELD_EX32((x)->word[5], STE_5, S2HD)
> +#define STE_S2HA(x)       FIELD_EX32((x)->word[5], STE_5, S2HA)
> +#define STE_S2S(x)        FIELD_EX32((x)->word[5], STE_5, S2S)
> +#define STE_S2R(x)        FIELD_EX32((x)->word[5], STE_5, S2R)
> +#define STE_S2TTB(x)                                                \
> +    (((uint64_t)FIELD_EX32((x)->word[6], STE_6, S2TTB_LO) << 4) |   \
> +     ((uint64_t)FIELD_EX32((x)->word[7], STE_7, S2TTB_HI) << 32))
>  
> -#define STE_CONFIG(x)  extract32((x)->word[0], 1, 3)
>  #define STE_CFG_S1_ENABLED(config) (config & 0x1)
>  #define STE_CFG_S2_ENABLED(config) (config & 0x2)
>  #define STE_CFG_ABORT(config)      (!(config & 0x4))
>  #define STE_CFG_BYPASS(config)     (config == 0x4)
>  
> -#define STE_S1FMT(x)       extract32((x)->word[0], 4 , 2)
> -#define STE_S1CDMAX(x)     extract32((x)->word[1], 27, 5)
> -#define STE_S1STALLD(x)    extract32((x)->word[2], 27, 1)
> -#define STE_EATS(x)        extract32((x)->word[2], 28, 2)
> -#define STE_STRW(x)        extract32((x)->word[2], 30, 2)
> -#define STE_S2VMID(x)      extract32((x)->word[4], 0 , 16)
> -#define STE_S2T0SZ(x)      extract32((x)->word[5], 0 , 6)
> -#define STE_S2SL0(x)       extract32((x)->word[5], 6 , 2)
> -#define STE_S2TG(x)        extract32((x)->word[5], 14, 2)
> -#define STE_S2PS(x)        extract32((x)->word[5], 16, 3)
> -#define STE_S2AA64(x)      extract32((x)->word[5], 19, 1)
> -#define STE_S2ENDI(x)      extract32((x)->word[5], 20, 1)
> -#define STE_S2AFFD(x)      extract32((x)->word[5], 21, 1)
> -#define STE_S2HD(x)        extract32((x)->word[5], 23, 1)
> -#define STE_S2HA(x)        extract32((x)->word[5], 24, 1)
> -#define STE_S2S(x)         extract32((x)->word[5], 25, 1)
> -#define STE_S2R(x)         extract32((x)->word[5], 26, 1)
> -
> -#define STE_CTXPTR(x)                                   \
> -    ((extract64((x)->word[1], 0, 16) << 32) |           \
> -     ((x)->word[0] & 0xffffffc0))
> -
> -#define STE_S2TTB(x)                                    \
> -    ((extract64((x)->word[7], 0, 16) << 32) |           \
> -     ((x)->word[6] & 0xfffffff0))
> -
>  /* CD fields */
>  
> -#define CD_VALID(x)   extract32((x)->word[0], 31, 1)
> -#define CD_ASID(x)    extract32((x)->word[1], 16, 16)
> -#define CD_TTB(x, sel)                                          \
> -    ((extract64((x)->word[(sel) * 2 + 3], 0, 19) << 32) |       \
> -     ((x)->word[(sel) * 2 + 2] & ~0xfULL))
> -
> -#define CD_HAD(x, sel)   extract32((x)->word[(sel) * 2 + 2], 1, 1)
> -
> -#define CD_TSZ(x, sel)   extract32((x)->word[0], (16 * (sel)) + 0, 6)
> -#define CD_TG(x, sel)    extract32((x)->word[0], (16 * (sel)) + 6, 2)
> -#define CD_EPD(x, sel)   extract32((x)->word[0], (16 * (sel)) + 14, 1)
> -#define CD_ENDI(x)       extract32((x)->word[0], 15, 1)
> -#define CD_IPS(x)        extract32((x)->word[1], 0 , 3)
> -#define CD_AFFD(x)       extract32((x)->word[1], 3 , 1)
> -#define CD_TBI(x)        extract32((x)->word[1], 6 , 2)
> -#define CD_HD(x)         extract32((x)->word[1], 10 , 1)
> -#define CD_HA(x)         extract32((x)->word[1], 11 , 1)
> -#define CD_S(x)          extract32((x)->word[1], 12, 1)
> -#define CD_R(x)          extract32((x)->word[1], 13, 1)
> -#define CD_A(x)          extract32((x)->word[1], 14, 1)
> -#define CD_AARCH64(x)    extract32((x)->word[1], 9 , 1)
> +REG32(CD_0, 0)
> +    FIELD(CD_0, TSZ0, 0, 6)
> +    FIELD(CD_0, TG0, 6, 2)
> +    FIELD(CD_0, EPD0, 14, 1)
> +    FIELD(CD_0, ENDI, 15, 1)
> +    FIELD(CD_0, TSZ1, 16, 6)
> +    FIELD(CD_0, TG1, 22, 2)
> +    FIELD(CD_0, EPD1, 30, 1)
> +    FIELD(CD_0, VALID, 31, 1)
> +REG32(CD_1, 4)
> +    FIELD(CD_1, IPS, 0, 3)
> +    FIELD(CD_1, AFFD, 3, 1)
> +    FIELD(CD_1, TBI, 6, 2)
> +    FIELD(CD_1, AARCH64, 9, 1)
> +    FIELD(CD_1, HD, 10, 1)
> +    FIELD(CD_1, HA, 11, 1)
> +    FIELD(CD_1, S, 12, 1)
> +    FIELD(CD_1, R, 13, 1)
> +    FIELD(CD_1, A, 14, 1)
> +    FIELD(CD_1, ASID, 16, 16)
> +REG32(CD_2, 8)
> +    FIELD(CD_2, HAD0, 1, 1)
> +    FIELD(CD_2, TTB0_LO, 4, 28)
> +REG32(CD_3, 12)
> +    FIELD(CD_3, TTB0_HI, 0, 19)
I think it is 24 now
> +REG32(CD_4, 16)
> +    FIELD(CD_4, HAD1, 1, 1)
> +    FIELD(CD_4, TTB1_LO, 4, 28)
> +REG32(CD_5, 20)
> +    FIELD(CD_5, TTB1_HI, 0, 19)
same
> +
> +/* Get CD fields */
> +#define CD_TSZ(x, sel)   ((sel) ?          \
> +    FIELD_EX32((x)->word[0], CD_0, TSZ1) : \
> +    FIELD_EX32((x)->word[0], CD_0, TSZ0))
> +#define CD_TG(x, sel)    ((sel) ?          \
> +    FIELD_EX32((x)->word[0], CD_0, TG1) :  \
> +    FIELD_EX32((x)->word[0], CD_0, TG0))
> +#define CD_EPD(x, sel)   ((sel) ?          \
> +    FIELD_EX32((x)->word[0], CD_0, EPD1) : \
> +    FIELD_EX32((x)->word[0], CD_0, EPD0))
> +#define CD_ENDI(x)       FIELD_EX32((x)->word[0], CD_0, ENDI)
> +#define CD_VALID(x)      FIELD_EX32((x)->word[0], CD_0, VALID)
> +#define CD_IPS(x)        FIELD_EX32((x)->word[1], CD_1, IPS)
> +#define CD_AFFD(x)       FIELD_EX32((x)->word[1], CD_1, AFFD)
> +#define CD_TBI(x)        FIELD_EX32((x)->word[1], CD_1, TBI)
> +#define CD_AARCH64(x)    FIELD_EX32((x)->word[1], CD_1, AARCH64)
> +#define CD_HD(x)         FIELD_EX32((x)->word[1], CD_1, HD)
> +#define CD_HA(x)         FIELD_EX32((x)->word[1], CD_1, HA)
> +#define CD_S(x)          FIELD_EX32((x)->word[1], CD_1, S)
> +#define CD_R(x)          FIELD_EX32((x)->word[1], CD_1, R)
> +#define CD_A(x)          FIELD_EX32((x)->word[1], CD_1, A)
> +#define CD_ASID(x)       FIELD_EX32((x)->word[1], CD_1, ASID)
> +#define CD_HAD(x, sel)   ((sel) ?                                         \
> +    FIELD_EX32((x)->word[4], CD_4, HAD1) :                                \
> +    FIELD_EX32((x)->word[2], CD_2, HAD0))
> +#define CD_TTB(x, sel)                                                    \
> +    ((sel) ? (((uint64_t)FIELD_EX32((x)->word[5], CD_5, TTB1_HI) << 32) | \
> +              ((uint64_t)FIELD_EX32((x)->word[4], CD_4, TTB1_LO) << 4)) : \
> +             (((uint64_t)FIELD_EX32((x)->word[3], CD_3, TTB0_HI) << 32) | \
> +              ((uint64_t)FIELD_EX32((x)->word[2], CD_2, TTB0_LO) << 4)))
>  
>  /* MMIO Registers */
>  
Otherwise looks good to me

Reviewed-by: Eric Auger <[email protected]>

Eric


Reply via email to