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.
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
