> On 24 Sep 2026, at 12:24 PM, Eric Auger <[email protected]> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > > > On 9/21/26 2:32 PM, Khushit Shah wrote: >> kvm_arm_expose_idreg_properties() no longer depends on KVM-specific >> logic. Move it to cpu64.c as aarch64_add_idregs_properties() along with >> all SYSREG_ property infra, making it accelerator-agnostic. > Even before, the code had no real kvm adherence, I mean even if we keep > reg->writable_mask checks. This latter could be populated with another > accelerator. So I think I can move in this direction in the parent series. Makes sense.
Thanks, Khushit > > Eric >> >> Signed-off-by: Khushit Shah <[email protected]> >> --- >> target/arm/cpu64.c | 101 +++++++++++++++++++++++++++++++++++++++- >> target/arm/internals.h | 1 + >> target/arm/kvm.c | 100 --------------------------------------- >> target/arm/kvm_arm.h | 10 ---- >> target/arm/trace-events | 4 +- >> 5 files changed, 104 insertions(+), 112 deletions(-) >> >> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c >> index 3d522bf20d..7487b48cc9 100644 >> --- a/target/arm/cpu64.c >> +++ b/target/arm/cpu64.c >> @@ -38,6 +38,7 @@ >> #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) \ >> @@ -644,6 +645,104 @@ static const Property arm_cpu_pauth_qarma3_property = >> static Property arm_cpu_pauth_qarma5_property = >> DEFINE_PROP_BOOL("pauth-qarma5", ARMCPU, prop_pauth_qarma5, false); >> >> +#ifndef CONFIG_USER_ONLY >> +static void set_sysreg_prop(Object *obj, Visitor *v, >> + const char *name, void *opaque, >> + Error **errp) >> +{ >> + ARM64SysRegField *field = (ARM64SysRegField *)opaque; >> + ARMCPU *cpu = ARM_CPU(obj); >> + uint64_t *idregs = cpu->isar.idregs; >> + uint64_t old, value, mask; >> + int lower = field->shift; >> + int length = field->length; >> + int index = field->index; >> + >> + if (!visit_type_uint64(v, name, &value, errp)) { >> + return; >> + } >> + >> + if (length < 64 && value > ((1 << length) - 1)) { >> + error_setg(errp, >> + "idreg %s set value (0x%lx) exceeds length of field >> (%d)!", >> + name, value, length); >> + return; >> + } >> + >> + if (field->arch_vals) { >> + /* this field has some enum values */ >> + for (int i = 0; i < field->arch_vals_count; i++) { >> + if (value == field->arch_vals[i]) { >> + goto valid; >> + } >> + } >> + error_setg(errp, >> + "idreg %s set value (0x%lx) does not match any " >> + "arch valid enum value", name, value); >> + return; >> + } >> + >> +valid: >> + >> + mask = MAKE_64BIT_MASK(lower, length); >> + value = value << lower; >> + old = idregs[index]; >> + idregs[index] = old & ~mask; >> + idregs[index] |= value; >> + trace_set_sysreg_prop(name, old, mask, value, idregs[index]); >> +} >> + >> +static void get_sysreg_prop(Object *obj, Visitor *v, >> + const char *name, void *opaque, >> + Error **errp) >> +{ >> + ARM64SysRegField *field = (ARM64SysRegField *)opaque; >> + ARMCPU *cpu = ARM_CPU(obj); >> + uint64_t *idregs = cpu->isar.idregs; >> + uint64_t value, mask; >> + int lower = field->shift; >> + int length = field->length; >> + int index = field->index; >> + >> + mask = MAKE_64BIT_MASK(lower, length); >> + value = (idregs[index] & mask) >> lower; >> + visit_type_uint64(v, name, &value, errp); >> + trace_get_sysreg_prop(name, value); >> +} >> + >> +static void add_idregs_properties(Object *obj, ARM64SysReg *reg) >> +{ >> + int nb_sysreg_props = 0; >> + >> + for (int i = 0; i < reg->fields_count; i++) { >> + ARM64SysRegField *field = ®->fields[i]; >> + >> + /* skip reserved fields */ >> + if (g_str_equal(field->name, "RES0") || >> + g_str_equal(field->name, "RES1") || >> + g_str_equal(field->name, "RAZ")) { >> + continue; >> + } >> + >> + char *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); >> + g_free(prop_name); >> + nb_sysreg_props++; >> + } >> + >> + trace_nb_sysreg_props(reg->name, nb_sysreg_props); >> +} >> + >> +void aarch64_add_idregs_properties(Object *obj) >> +{ >> + for (int i = 0; i < NUM_ID_IDX; i++) { >> + add_idregs_properties(obj, &arm64_id_regs[i]); >> + } >> +} >> +#endif >> + >> void aarch64_add_pauth_properties(Object *obj) >> { >> ARMCPU *cpu = ARM_CPU(obj); >> @@ -810,7 +909,7 @@ void aarch64_host_initfn(Object *obj) >> aarch64_add_sve_properties(obj); >> >> /* generate SYSREG properties */ >> - kvm_arm_expose_idreg_properties(cpu, arm64_id_regs); >> + aarch64_add_idregs_properties(obj); >> >> #elif defined(CONFIG_HVF) >> hvf_arm_set_cpu_features_from_host(cpu); >> diff --git a/target/arm/internals.h b/target/arm/internals.h >> index f6b4e173a5..7c76ffea7d 100644 >> --- a/target/arm/internals.h >> +++ b/target/arm/internals.h >> @@ -1813,6 +1813,7 @@ void aarch64_max_tcg_initfn(Object *obj); >> void aarch64_add_pauth_properties(Object *obj); >> void aarch64_add_sve_properties(Object *obj); >> void aarch64_add_sme_properties(Object *obj); >> +void aarch64_add_idregs_properties(Object *obj); >> void aarch64_aa32_a57_init(ARMCPU *cpu, bool aa64_enabled); >> void aarch64_host_initfn(Object *obj); >> >> diff --git a/target/arm/kvm.c b/target/arm/kvm.c >> index 2f6c090e43..d5155a3706 100644 >> --- a/target/arm/kvm.c >> +++ b/target/arm/kvm.c >> @@ -416,106 +416,6 @@ bool kvm_idreg_write_scratch_vcpu(int cpufd, Visitor >> *v, const char *name, >> return true; >> } >> >> -static void set_sysreg_prop(Object *obj, Visitor *v, >> - const char *name, void *opaque, >> - Error **errp) >> -{ >> - ARM64SysRegField *field = (ARM64SysRegField *)opaque; >> - ARMCPU *cpu = ARM_CPU(obj); >> - uint64_t *idregs = cpu->isar.idregs; >> - uint64_t old, value, mask; >> - int lower = field->shift; >> - int length = field->length; >> - int index = field->index; >> - >> - if (!visit_type_uint64(v, name, &value, errp)) { >> - return; >> - } >> - >> - if (length < 64 && value > ((1 << length) - 1)) { >> - error_setg(errp, >> - "idreg %s set value (0x%lx) exceeds length of field >> (%d)!", >> - name, value, length); >> - return; >> - } >> - >> - if (field->arch_vals) { >> - /* this field has some enum values */ >> - for (int i = 0; i < field->arch_vals_count; i++) { >> - if (value == field->arch_vals[i]) { >> - goto valid; >> - } >> - } >> - error_setg(errp, >> - "idreg %s set value (0x%lx) does not match any " >> - "arch valid enum value", name, value); >> - return; >> - } >> - >> -valid: >> - >> - mask = MAKE_64BIT_MASK(lower, length); >> - value = value << lower; >> - old = idregs[index]; >> - idregs[index] = old & ~mask; >> - idregs[index] |= value; >> - trace_set_sysreg_prop(name, old, mask, value, idregs[index]); >> -} >> - >> -static void get_sysreg_prop(Object *obj, Visitor *v, >> - const char *name, void *opaque, >> - Error **errp) >> -{ >> - ARM64SysRegField *field = (ARM64SysRegField *)opaque; >> - ARMCPU *cpu = ARM_CPU(obj); >> - uint64_t *idregs = cpu->isar.idregs; >> - uint64_t value, mask; >> - int lower = field->shift; >> - int length = field->length; >> - int index = field->index; >> - >> - mask = MAKE_64BIT_MASK(lower, length); >> - value = (idregs[index] & mask) >> lower; >> - visit_type_uint64(v, name, &value, errp); >> - trace_get_sysreg_prop(name, value); >> -} >> - >> -static void >> -add_idreg_field_properties(Object *obj, ARM64SysReg *reg) >> -{ >> - int nb_sysreg_props = 0; >> - >> - for (int i = 0; i < reg->fields_count; i++) { >> - ARM64SysRegField *field = ®->fields[i]; >> - >> - /* skip reserved fields */ >> - if (g_str_equal(field->name, "RES0") || >> - g_str_equal(field->name, "RES1") || >> - g_str_equal(field->name, "RAZ")) { >> - continue; >> - } >> - >> - char *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); >> - g_free(prop_name); >> - nb_sysreg_props++; >> - } >> - >> - trace_nb_sysreg_props(reg->name, nb_sysreg_props); >> -} >> - >> -void kvm_arm_expose_idreg_properties(ARMCPU *cpu, ARM64SysReg *regs) >> -{ >> - Object *obj = OBJECT(cpu); >> - >> - for (int i = 0; i < NUM_ID_IDX; i++) { >> - ARM64SysReg *sysregdesc = ®s[i]; >> - add_idreg_field_properties(obj, sysregdesc); >> - } >> -} >> - >> static void >> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> { >> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h >> index aafb3cd232..997120966f 100644 >> --- a/target/arm/kvm_arm.h >> +++ b/target/arm/kvm_arm.h >> @@ -163,16 +163,6 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); >> */ >> void kvm_arm_add_vcpu_properties(ARMCPU *cpu); >> >> -typedef struct ARM64SysReg ARM64SysReg; >> -/** >> - * kvm_arm_expose_idreg_properties: >> - * @cpu: The CPU object to generate the properties for >> - * @reg: registers from the host >> - * >> - * analyze the writable mask and generate properties for writable fields >> - */ >> -void kvm_arm_expose_idreg_properties(ARMCPU *cpu, ARM64SysReg *regs); >> - >> /** >> * kvm_arm_steal_time_finalize: >> * @cpu: ARMCPU for which to finalize kvm-steal-time >> diff --git a/target/arm/trace-events b/target/arm/trace-events >> index 7696a8ed40..bbf2e5dfa2 100644 >> --- a/target/arm/trace-events >> +++ b/target/arm/trace-events >> @@ -15,10 +15,12 @@ arm_gt_update_irq(int timer, int irqstate) >> "gt_update_irq: timer %d irqstate %d" >> kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" >> is translated into 0x%"PRIx64 >> get_host_cpu_idregs(const char *name, uint64_t value) "scratch vcpu host >> value for %s is 0x%"PRIx64 >> kvm_arm_writable_idregs_to_cpreg_list(const char *name, uint64_t previous, >> uint64_t new) "%s overwrite default 0x%"PRIx64" with 0x%"PRIx64 >> +kvm_idreg_write_scratch_vcpu(const char *name, uint64_t oldfv, uint64_t >> newfv, uint64_t actualfv) "successful dry-run for %s old=0x%"PRIx64" >> new=0x%"PRIx64" actual=0x%"PRIx64 >> + >> +# cpu64.c >> 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" >> -kvm_idreg_write_scratch_vcpu(const char *name, uint64_t oldfv, uint64_t >> newfv, uint64_t actualfv) "successful dry-run for %s old=0x%"PRIx64" >> new=0x%"PRIx64" actual=0x%"PRIx64 >> >> # cpu.c >> arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64 >
