Re: [RFC PATCH v4 01/25] target/arm: expose SYSREG_ props for REVIDR_EL1 and AIDR_EL1

[Eric Auger]

Hi Khushit,

On 9/21/26 2:32 PM, Khushit Shah wrote:
> REVIDR_EL1 and AIDR_EL1 are implementation-defined and have no named
> fields in Registers.json.
>
> Emit dummy VAL fields covering the full register for these registers
> in scripts/update-aarch64-cpu-sysreg-properties.py, then remove the
> special case for these in kvm_arm_expose_idreg_properties(), exposing
> them as SYSREG_ props.
>
> Signed-off-by: Khushit Shah <khushit.s...@nutanix.com>
> ---
>  scripts/update-aarch64-cpu-sysreg-properties.py |  9 +++++++++
>  target/arm/cpu-idregs.h.inc                     |  2 ++
>  target/arm/kvm.c                                | 10 +---------
>  3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/update-aarch64-cpu-sysreg-properties.py 
> b/scripts/update-aarch64-cpu-sysreg-properties.py
> index 0b4e4d3e0c..2269063408 100644
> --- a/scripts/update-aarch64-cpu-sysreg-properties.py
> +++ b/scripts/update-aarch64-cpu-sysreg-properties.py
> @@ -246,6 +246,15 @@ def 
> generate_sysreg_properties_from_registers_json(id_reg_names, raw_json_path):
>          # Sort decreasing lsbs
>          sorted_fields = sorted(unique_fields.items(),
>                                 key=lambda x: x[1]['lsb'], reverse=True)
> +        if len(sorted_fields) == 0:
> +            # For cases like REVIDR_EL1, add a dummy field covering the
> +            # full register.
> +            sorted_fields.append(('VAL', {
though I think it is important the property name stays SYSREG_REVIDR_EL1
and not SYSREG_REVIDR_EL1_VAL so I guess we need either a property alias
or a special handling for those regs (better as alias were not really
meant for that purpose I think).

Eric
> +                'lsb': 0,
> +                'msb': 63,
> +                'width': 64,
> +                'enums': []
> +            }))
>  
>          for unique_key, bits in sorted_fields:
>              enums_list = bits.get('enums', [])
> diff --git a/target/arm/cpu-idregs.h.inc b/target/arm/cpu-idregs.h.inc
> index c7e93c48b7..bc52112430 100644
> --- a/target/arm/cpu-idregs.h.inc
> +++ b/target/arm/cpu-idregs.h.inc
> @@ -12,6 +12,7 @@
>  /* IDREG_END(REG) */
>  
>  IDREG_START(AIDR_EL1)
> +    IDREG_FIELD(AIDR_EL1, VAL, 0, 64)
>  IDREG_END(AIDR_EL1)
>  
>  IDREG_START(CLIDR_EL1)
> @@ -2438,6 +2439,7 @@ IDREG_START(MVFR2_EL1)
>  IDREG_END(MVFR2_EL1)
>  
>  IDREG_START(REVIDR_EL1)
> +    IDREG_FIELD(REVIDR_EL1, VAL, 0, 64)
>  IDREG_END(REVIDR_EL1)
>  
>  IDREG_START(SMIDR_EL1)
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index e3a0dc07de..8a823d0324 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -554,15 +554,7 @@ void kvm_arm_expose_idreg_properties(ARMCPU *cpu, 
> ARM64SysReg *regs)
>          ARM64SysReg *sysregdesc = &regs[i];
>  
>          if (sysregdesc->writable_mask) {
> -            /*
> -             * special case REVIDR_EL1 and AIDR_EL1 which are writable but
> -             * do not expose named fields. They will need to be handled
> -             * separately
> -             */
> -            if (strcmp(sysregdesc->name, "REVIDR_EL1") &&
> -                strcmp(sysregdesc->name, "AIDR_EL1")) {
> -                decode_idreg_writemap(obj, sysregdesc);
> -            }
> +            decode_idreg_writemap(obj, sysregdesc);
>          }
>      }
>  }