Re: [RFC 1/9] target/arm: convert isar regs to array

[Philippe Mathieu-Daudé]

On 8/13/20 12:26 PM, Peng Liang wrote:
> The isar in ARMCPU is a struct, each field of which represents an ID
> register.  It's not convenient for us to support CPU feature in AArch64.
> So let's change it to an array first and add an enum as the index of the
> array for convenience.  Since we will never access high 32-bits of ID
> registers in AArch32, it's harmless to change the ID registers in
> AArch32 to 64-bits.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> ---
>  hw/intc/armv7m_nvic.c  |  48 ++++----
>  target/arm/cpu.c       | 232 +++++++++++++++++++-------------------
>  target/arm/cpu.h       | 226 +++++++++++++++++++------------------
>  target/arm/cpu64.c     | 170 ++++++++++++++--------------
>  target/arm/cpu_tcg.c   | 250 +++++++++++++++++++++--------------------
>  target/arm/helper.c    |  54 ++++-----
>  target/arm/internals.h |  15 +--
>  target/arm/kvm64.c     |  72 ++++++------
>  8 files changed, 541 insertions(+), 526 deletions(-)

Please consider using scripts/git.orderfile.

[...]
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9e8ed423ea..5d8074d03b 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -67,6 +67,37 @@
>  #define ARMV7M_EXCP_PENDSV  14
>  #define ARMV7M_EXCP_SYSTICK 15
>  
> +typedef enum CPUIDReg {
> +    MIDR_EL1,
> +    ID_ISAR0,
> +    ID_ISAR1,
> +    ID_ISAR2,
> +    ID_ISAR3,
> +    ID_ISAR4,
> +    ID_ISAR5,
> +    ID_ISAR6,
> +    ID_MMFR0,
> +    ID_MMFR1,
> +    ID_MMFR2,
> +    ID_MMFR3,
> +    ID_MMFR4,
> +    ID_AA64ISAR0,
> +    ID_AA64ISAR1,
> +    ID_AA64PFR0,
> +    ID_AA64PFR1,
> +    ID_AA64MMFR0,
> +    ID_AA64MMFR1,
> +    ID_AA64MMFR2,
> +    ID_AA64DFR0,
> +    ID_AA64DFR1,
> +    ID_DFR0,
> +    MVFR0,
> +    MVFR1,
> +    MVFR2,
> +    DBGDIDR,
> +    ID_MAX,

(nitpicking, drop the last comma).

> +} CPUIDReg;
> +
>  /* For M profile, some registers are banked secure vs non-secure;
>   * these are represented as a 2-element array where the first element
>   * is the non-secure copy and the second is the secure copy.
> @@ -890,32 +921,7 @@ struct ARMCPU {
>       * field by reading the value from the KVM vCPU.
>       */
>      struct ARMISARegisters {
> -        uint32_t id_isar0;
> -        uint32_t id_isar1;
> -        uint32_t id_isar2;
> -        uint32_t id_isar3;
> -        uint32_t id_isar4;
> -        uint32_t id_isar5;
> -        uint32_t id_isar6;
> -        uint32_t id_mmfr0;
> -        uint32_t id_mmfr1;
> -        uint32_t id_mmfr2;
> -        uint32_t id_mmfr3;
> -        uint32_t id_mmfr4;
> -        uint32_t mvfr0;
> -        uint32_t mvfr1;
> -        uint32_t mvfr2;
> -        uint32_t id_dfr0;
> -        uint32_t dbgdidr;
> -        uint64_t id_aa64isar0;
> -        uint64_t id_aa64isar1;
> -        uint64_t id_aa64pfr0;
> -        uint64_t id_aa64pfr1;
> -        uint64_t id_aa64mmfr0;
> -        uint64_t id_aa64mmfr1;
> -        uint64_t id_aa64mmfr2;
> -        uint64_t id_aa64dfr0;
> -        uint64_t id_aa64dfr1;
> +        uint64_t regs[ID_MAX];
>      } isar;

Why not simply this?

       uint64_t isar[ID_MAX];

>      uint64_t midr;
>      uint32_t revidr;
[...]