> On 10 Oct 2026, at 7:06 PM, Eric Auger <[email protected]> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > For KVM vcpu models, we add properties for all writable > ID reg fields. This relies on the idreg writable mask retrieved > from KVM, which is matched aginst the register description > found in target/arm/cpu-idregs.h.inc. > > The code is prepared to extend this capability to all ID > register fields independently on their writability. This will > be needed for named vcpu models which may want to relax some > values to the host supported one. > > REVIDR_EL1 and AIDR_EL1 are writable but they do not expose any > field. So they will need to be handled separately. > > The setter and getter are not yet implemented and currently > return an error. > > Note in the future, other acceleration types may benefit from > those properties. > > Signed-off-by: Eric Auger <[email protected]> > > --- > v9 -> v10 > - instead of parsing the writable mask, scroll through the fields > but still do some sanity checks. Add the writable param > - move the code to cpu64.c Do not pass any ARM64SysReg handle > anymore. Rename the functions > > v7 -> v8 > - scratch the '!' in the error_setg > - addapt to the removal of struct ArmIdRegArchVal > > v5 -> v6 > - for enum values, check against value value when setting the prop > - do not check writable_map flag anymore > > v4 -> v5: > - free prop_name > - check cpu->writable_map as a preamble in kvm_arm_expose_idreg_properties > --- > target/arm/cpu64.c | 113 ++++++++++++++++++++++++++++++++++++++++ > target/arm/trace-events | 6 +++ > 2 files changed, 119 insertions(+) > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 64103df0a7..b1738c9be3 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -37,6 +37,8 @@ > #include "hw/core/qdev-properties.h" > #include "internals.h" > #include "cpu-features.h" > +#include "cpu-idregs.h" > +#include "trace.h" > > /* convert between <register>_IDX and SYS_<register> */ > #define DEF(NAME, OP0, OP1, CRN, CRM, OP2) \ > @@ -798,6 +800,116 @@ static void kvm_arm_set_cpreg_mig_tolerances(ARMCPU > *cpu) > arm_register_cpreg_mig_tolerance(cpu, KVM_REG_ARM_FW_FEAT_BMAP_REG(3), > UINT64_MAX, 0, > ToleranceOnlySrcTestValue); > } > + > +static void set_sysreg_prop(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + error_setg(errp, "%s setter not yet implemented", name); > +} > + > +static void get_sysreg_prop(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + error_setg(errp, "%s getter not yet implemented", name); > +} > + > +/** > + * aarch64_add_idreg_properties: Add SYSREG_<REG>_<FIELD> properties > + * for named fields of an ID register > + * > + * @obj: CPU object > + * @reg: Description of the sysreg > + * @writable_only: if set, add a property only for writable fields, > + * otherwise, add properties unconditionnally > + * > + * Independently on @writable_only, always check the consistency between > + * writable_mask and register fields description: if a field is writable > + * make sure all its bits are writable. Also make sure only named fields > + * are writable. > + */ > +static int > +aarch64_add_one_idreg_properties(Object *obj, ARM64SysReg *reg, > + bool writable_only) > +{ > + uint64_t map = reg->writable_mask; > + int nb_sysreg_props = 0; > + > + for (int i = 0; i < reg->fields_count; i++) { > + ARM64SysRegField *field = ®->fields[i]; > + int lower = field->shift; > + int length = field->length; > + int upper = lower + length - 1; > + uint64_t field_writable_mask = extract64(map, lower, length); > + uint64_t field_mask = MAKE_64BIT_MASK(lower, length); > + char *prop_name; > + > + /* skip reserved fields, but still account them in writable_mask */ > + if (g_str_equal(field->name, "RES0") || > + g_str_equal(field->name, "RES1") || > + g_str_equal(field->name, "RAZ")) { > + map &= ~field_mask; > + continue; > + } > + > + if (field_writable_mask && > + field_writable_mask != MAKE_64BIT_MASK(0, length)) { > + /* only a part of the field is writable, this should never > happen */ > + error_report("field %s is not fully writable! (0x%"PRIx64")", > + field->name, field_writable_mask); > + } > + map = map & ~field_mask; > + > + if (writable_only && !field_writable_mask) { > + continue; > + } > + > + prop_name = g_strdup_printf("SYSREG_%s_%s", reg->name, field->name); > + object_property_add(obj, prop_name, "uint64", > + get_sysreg_prop, set_sysreg_prop, NULL, field); > + trace_add_idreg_property(prop_name, lower, upper, > field_writable_mask); > + g_free(prop_name); > + nb_sysreg_props++; > + } > + if (map) { > + error_report("%s has writable bits which do not match " > + "any named field! (0x%" PRIx64")", reg->name, map); > + } > + trace_nb_sysreg_props(reg->name, nb_sysreg_props); > + return 0; > +} > + > + > +/** > + * aarch64_add_idreg_properties: > + * > + * Add SYSREG_<REG>_<FIELD> properties for every IDReg fields > + * > + * @cpu: cpu the properties are attached to > + * @writable_only: if set, properties are added only if the field is > + * writable. Otherwise, added unconditionnally > + */ > +static void aarch64_add_idreg_properties(Object *obj, bool writable_only) > +{ > + for (int i = 0; i < NUM_ID_IDX; i++) { > + ARM64SysReg *sysregdesc = &arm64_id_regs[i]; > + > + if (writable_only && !sysregdesc->writable_mask) { > + continue; > + } > + /* > + * 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")) { > + continue; > + } > + aarch64_add_one_idreg_properties(obj, sysregdesc, true); Hi Eric,
I suppose you mean `writable_only` here? Using a local commit to fix this for now. Thanks, Khushit > + } > +} > #endif > > void aarch64_host_initfn(Object *obj) > @@ -815,6 +927,7 @@ void aarch64_host_initfn(Object *obj) > kvm_arm_set_cpreg_mig_tolerances(cpu); > kvm_arm_set_cpu_features_from_host(cpu); > aarch64_add_sve_properties(obj); > + aarch64_add_idreg_properties(obj, true); > #elif defined(CONFIG_HVF) > hvf_arm_set_cpu_features_from_host(cpu); > #elif defined(CONFIG_WHPX) > diff --git a/target/arm/trace-events b/target/arm/trace-events > index 8c7faf57c7..ddc79eb666 100644 > --- a/target/arm/trace-events > +++ b/target/arm/trace-events > @@ -19,6 +19,12 @@ get_host_cpu_idregs(const char *name, uint64_t value) > "scratch vcpu host value f > arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64 > arm_emulate_firmware_reset(uint64_t mp_aff, unsigned target_el) "cpu %" > PRIu64 " @EL%u" > > +# cpu64.c > +add_idreg_property(const char* name, int lower, int upper, bool writable) > "%s [%d:%d] writable=%d" > +get_sysreg_prop(const char *name, uint64_t value) "%s 0x%"PRIx64 > +set_sysreg_prop(const char *name, uint64_t old, uint64_t mask, uint64_t > field_value, uint64_t new) "%s old reg value=0x%"PRIx64" mask=0x%"PRIx64" new > field value=0x%"PRIx64" new reg value=0x%"PRIx64 > +nb_sysreg_props(const char *name, int count) "%s: %d SYSREG properties" > + > # arm-powerctl.c > arm_powerctl_set_cpu_on(uint64_t mp_aff, unsigned target_el, const char > *mode, uint64_t entry, uint64_t context_id) "cpu %" PRIu64 " (EL %u, %s) @ > 0x%" PRIx64 " with R0 = 0x%" PRIx64 > arm_powerctl_set_cpu_on_and_reset(uint64_t mp_aff) "cpu %" PRIu64 > -- > 2.53.0 >
