Re: [PATCH v4 04/17] arm/cpu: Add infra to handle generated ID register definitions

[Eric Auger <eric.auger@redhat.com>]

On 5/7/26 10:58 AM, Shameer Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: 03 May 2026 08:33
>> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
>> devel@nongnu.org; qemu-arm@nongnu.org; kvmarm@lists.linux.dev;
>> peter.maydell@linaro.org; richard.henderson@linaro.org;
>> cohuck@redhat.com; sebott@redhat.com; Shameer Kolothum Thodi
>> <skolothumtho@nvidia.com>; philmd@linaro.org
>> Cc: maz@kernel.org; oliver.upton@linux.dev; pbonzini@redhat.com;
>> armbru@redhat.com; berrange@redhat.com; abologna@redhat.com;
>> jdenemar@redhat.com
>> Subject: [PATCH v4 04/17] arm/cpu: Add infra to handle generated ID register
>> definitions
>>
>> External email: Use caution opening links or attachments
>>
>>
>> The known ID regs are populated in a new initialization function
>> named initialize_cpu_sysreg_properties(). That code will be
>> automatically generated from AARCHMRS Registers.json. For the
>> time being let's just describe a single id reg, CTR_EL0. In this
>> description we only care about non RES/RAZ fields, ie. named fields.
>>
>> The registers are populated in an array indexed by ARMIDRegisterIdx
>> and their fields are added in a sorted list.
>>
>> [CH: adapted to reworked register storage]
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  target/arm/cpu-idregs.h            | 59 ++++++++++++++++++++++++++++++
>>  target/arm/cpu-sysreg-properties.c | 30 +++++++++++++++
>>  target/arm/cpu64.c                 |  3 ++
>>  target/arm/meson.build             |  3 +-
>>  4 files changed, 94 insertions(+), 1 deletion(-)
>>  create mode 100644 target/arm/cpu-idregs.h
>>  create mode 100644 target/arm/cpu-sysreg-properties.c
>>
>> diff --git a/target/arm/cpu-idregs.h b/target/arm/cpu-idregs.h
>> new file mode 100644
>> index 0000000000..4a9034594d
>> --- /dev/null
>> +++ b/target/arm/cpu-idregs.h
>> @@ -0,0 +1,59 @@
>> +/*
>> + * handle ID registers and their fields
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#ifndef ARM_CPU_CUSTOM_H
>> +#define ARM_CPU_CUSTOM_H
> ARM_CPU_IDREGS_H ?
indeed: TARGET_ARM_CPU_IDREGS__H
>
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>> +#include "cpu.h"
>> +#include "cpu-sysregs.h"
>> +
>> +typedef struct ARM64SysRegField {
>> +    const char *name; /* name of the field, for instance CTR_EL0_IDC */
>> +    ARMIDRegisterIdx index; /* parent register, e.g. CTR_EL0_IDX */
>> +    int lower; /* lowest bit number of the field in the register */
>> +    int upper; /* highest bit number */
>> +} ARM64SysRegField;
>> +
>> +typedef struct ARM64SysReg {
>> +    const char *name;   /* name of the sysreg, for instance CTR_EL0 */
>> +    ARMSysRegs sysreg;
>> +    ARMIDRegisterIdx index; /* register index, e.g. CTR_EL0_IDX */
>> +    GList *fields; /* list of named fields, excluding RES* */
>> +} ARM64SysReg;
>> +
>> +void initialize_cpu_sysreg_properties(void);
>> +
>> +/*
>> + * List of exposed ID regs (automatically populated from AARCHMRS
>> Registers.json)
>> + */
>> +extern ARM64SysReg arm64_id_regs[NUM_ID_IDX];
>> +
>> +/* Allocate a new field and insert it at the head of the @reg list */
> It says insert at the head...
>
>> +static inline GList *arm64_sysreg_add_field(ARM64SysReg *reg, const char
>> *name,
>> +                                     uint8_t min, uint8_t max) {
>> +
>> +     ARM64SysRegField *field = g_new0(ARM64SysRegField, 1);
>> +
>> +     field->name = name;
>> +     field->lower = min;
>> +     field->upper = max;
>> +     field->index = reg->index;
>> +
>> +     reg->fields = g_list_append(reg->fields, field);
> But this is adding to the tail, right?
indeed use prepend instead
>
>> +     return reg->fields;
> Don't think the return is used anywhere.
now becoming static inline void 

arm64_sysreg_add_field()

Thanks

Eric

>
> Thanks,
> Shameer
>
>> +}
>> +
>> +static inline ARM64SysReg *arm64_sysreg_get(ARMIDRegisterIdx index)
>> +{
>> +        ARM64SysReg *reg = &arm64_id_regs[index];
>> +
>> +        reg->index = index;
>> +        reg->sysreg = id_register_sysreg[index];
>> +        return reg;
>> +}
>> +
>> +#endif
>> diff --git a/target/arm/cpu-sysreg-properties.c b/target/arm/cpu-sysreg-
>> properties.c
>> new file mode 100644
>> index 0000000000..5cc06c8f24
>> --- /dev/null
>> +++ b/target/arm/cpu-sysreg-properties.c
>> @@ -0,0 +1,30 @@
>> +/*
>> + * QEMU ARM CPU SYSREG PROPERTIES
>> + * will be automatically generated
>> + *
>> + * Copyright (c) Red Hat, Inc. 2026
>> + *
>> + */
>> +
>> + /* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#include "cpu-idregs.h"
>> +
>> +ARM64SysReg arm64_id_regs[NUM_ID_IDX];
>> +
>> +void initialize_cpu_sysreg_properties(void)
>> +{
>> +    memset(arm64_id_regs, 0, sizeof(ARM64SysReg) * NUM_ID_IDX);
>> +    /* CTR_EL0 */
>> +    ARM64SysReg *CTR_EL0 = arm64_sysreg_get(CTR_EL0_IDX);
>> +    CTR_EL0->name = "CTR_EL0";
>> +    arm64_sysreg_add_field(CTR_EL0, "TminLine", 32, 37);
>> +    arm64_sysreg_add_field(CTR_EL0, "DIC", 29, 29);
>> +    arm64_sysreg_add_field(CTR_EL0, "IDC", 28, 28);
>> +    arm64_sysreg_add_field(CTR_EL0, "CWG", 24, 27);
>> +    arm64_sysreg_add_field(CTR_EL0, "ERG", 20, 23);
>> +    arm64_sysreg_add_field(CTR_EL0, "DminLine", 16, 19);
>> +    arm64_sysreg_add_field(CTR_EL0, "L1Ip", 14, 15);
>> +    arm64_sysreg_add_field(CTR_EL0, "IminLine", 0, 3);
>> +}
>> +
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index a93ad2da5a..b940842d9e 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -37,6 +37,7 @@
>>  #include "hw/core/qdev-properties.h"
>>  #include "internals.h"
>>  #include "cpu-features.h"
>> +#include "cpu-idregs.h"
>>
>>  /* convert between <register>_IDX and SYS_<register> */
>>  #define DEF(NAME, OP0, OP1, CRN, CRM, OP2)      \
>> @@ -906,6 +907,8 @@ static void aarch64_cpu_register_types(void)
>>  {
>>      size_t i;
>>
>> +    initialize_cpu_sysreg_properties();
>> +
>>      for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
>>          arm_cpu_register(&aarch64_cpus[i]);
>>      }
>> diff --git a/target/arm/meson.build b/target/arm/meson.build
>> index 192ac7c31e..e2f740e48f 100644
>> --- a/target/arm/meson.build
>> +++ b/target/arm/meson.build
>> @@ -9,7 +9,8 @@ arm_user_ss.add(files('gdbstub.c'))
>>
>>  arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
>>    'cpu64.c',
>> -  'gdbstub64.c'
>> +  'gdbstub64.c',
>> +  'cpu-sysreg-properties.c',
>>  ))
>>
>>  arm_common_ss.add(files(
>> --
>> 2.53.0