> On 24 Sep 2026, at 7:05 PM, Eric Auger <[email protected]> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > Hi Khushit, > > On 9/21/26 2:32 PM, Khushit Shah wrote: >> Add scaffolding for named CPU models on Arm on top of the property >> layer. A model is a list of (property, value) overrides plus an >> optional parent; a child inherits its parent's property values and may >> override them. >> >> Instance init steps: >> - host feature discovery. >> - zero out isar.idregs[] and restore RES1 fields. >> - install SYSREG_ and sve* properties. >> - walk the parent chain root-first and apply each level's >> properties. > you should explain about prop types. I see they are either bool or uint. > Given the parent series bool usage is not obvious. Hi Eric,
Thinking about this more, the current approach will require some tweaking now as the underlying property infrastructure now sets the property values after the vCPU has been created. Some of the fields that the model defines (SVE/PAUTH) determines the vCPU init flags. Your suggestion of different setters is the way to go here :) >> This assumes that each property is idempotent. `pauth` is intentionally >> not installed because the model's ID register view is authoritative >> and can be configured through SYSREG_ properties. >> >> No models are defined yet. >> >> Signed-off-by: Khushit Shah <[email protected]> >> --- >> target/arm/cpu-models.c | 191 ++++++++++++++++++++++++++++++++++++++++ >> target/arm/cpu-models.h | 36 ++++++++ >> target/arm/meson.build | 1 + >> 3 files changed, 228 insertions(+) >> create mode 100644 target/arm/cpu-models.c >> create mode 100644 target/arm/cpu-models.h >> >> diff --git a/target/arm/cpu-models.c b/target/arm/cpu-models.c >> new file mode 100644 >> index 0000000000..fed3197186 >> --- /dev/null >> +++ b/target/arm/cpu-models.c >> @@ -0,0 +1,191 @@ >> +/* >> + * ARM named CPU models. >> + * >> + * Each model is defined as an array of MODEL_PROP macros, listing only >> + * the properties that DIFFER from the parent model. >> + * >> + * Initialization starts with a zeroed idregs[] array. >> + * >> + * During initialization, the parent chain is walked root-first and every >> + * level's properties are applied via QOM, so the leaf's values naturally >> + * override its ancestors. >> + * >> + * Assumptions: >> + * - All named CPU model properties are idempotent. >> + * >> + * SPDX-License-Identifier: GPL-2.0-or-later >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qapi/error.h" >> +#include "qemu/error-report.h" >> +#include "system/kvm.h" >> +#include "cpu.h" >> +#include "internals.h" >> +#include "kvm_arm.h" >> +#include "cpu-idregs.h" >> +#include "cpu-models.h" >> + >> +#define MODEL_PROP_FIELD_STR str >> +#define MODEL_PROP_FIELD_BOOL b >> +#define MODEL_PROP_FIELD_NUM num >> + >> +#define MODEL_PROP(_name, _type, _value) \ >> + { .name = (_name), .type = ARM_MODEL_PROP_##_type, \ >> + .u.MODEL_PROP_FIELD_##_type = (_value) } >> + >> +#define MODEL_PROP_END { .name = NULL } >> + >> +/* Maximum parent-chain depth for named CPU models. */ > inheritence depth? Yes, will fix. >> +#define ARM_NAMED_CPU_MODEL_MAX_DEPTH 16 >> + >> +static const ArmNamedCpuModel arm_cpu_models[] = { >> + {NULL, NULL, NULL} >> +}; >> + >> +const ArmNamedCpuModel *arm_find_model(const char *name) >> +{ >> + for (size_t i = 0; i < ARRAY_SIZE(arm_cpu_models); i++) { >> + if (!arm_cpu_models[i].name) { >> + break; >> + } >> + >> + if (g_str_equal(arm_cpu_models[i].name, name)) { >> + return &arm_cpu_models[i]; >> + } >> + } >> + return NULL; >> +} >> + >> +void arm_apply_model_props(Object *obj, const ArmModelPropValue *props, >> + Error **errp) >> +{ >> + const ArmModelPropValue *pv; >> + ERRP_GUARD(); >> + >> + for (pv = props; pv->name; pv++) { >> + switch (pv->type) { >> + case ARM_MODEL_PROP_BOOL: >> + object_property_set_bool(obj, pv->name, pv->u.b, errp); >> + break; >> + case ARM_MODEL_PROP_NUM: >> + object_property_set_uint(obj, pv->name, pv->u.num, errp); >> + break; >> + default: >> + g_assert_not_reached(); >> + } >> + >> + if (*errp) { >> + error_prepend(errp, "property '%s': ", pv->name); >> + return; >> + } >> + } >> +} >> + >> +void arm_realize_model_chain(Object *obj, const ArmNamedCpuModel *model, >> + Error **errp) >> +{ >> + const ArmNamedCpuModel *cur, *parent; >> + const ArmNamedCpuModel *chain[ARM_NAMED_CPU_MODEL_MAX_DEPTH]; >> + size_t depth = 0; >> + >> + for (cur = model; cur; ) { >> + if (depth >= ARRAY_SIZE(chain)) { >> + error_setg(errp, "model '%s': parent chain too deep", >> model->name); >> + return; >> + } >> + chain[depth++] = cur; > Please add a comment about the so-called chain and especially describing > that chain[0] is the child and high-level eindex lements are the parents > it derives from. Will do. >> + >> + if (!cur->parent) { >> + break; >> + } >> + parent = arm_find_model(cur->parent); > I guess you directly assert Okay. >> + if (!parent) { >> + error_setg(errp, "model '%s': unknown parent '%s'", cur->name, >> + cur->parent); >> + return; >> + } >> + cur = parent; >> + } >> + >> + while (depth--) { >> + arm_apply_model_props(obj, chain[depth]->props, errp); >> + if (*errp) { >> + return; >> + } >> + } >> +} >> + >> +static void reset_res1_fields(ARMCPU *cpu) >> +{ >> + for (size_t i = 0; i < NUM_ID_IDX; i++) { >> + for (size_t j = 0; j < arm64_id_regs[i].fields_count; j++) { >> + if (strcmp(arm64_id_regs[i].fields[j].name, "RES1") == 0) { >> + cpu->isar.idregs[i] = deposit64(cpu->isar.idregs[i], >> + arm64_id_regs[i].fields[j].shift, >> + arm64_id_regs[i].fields[j].length, >> + MAKE_64BIT_MASK(0, arm64_id_regs[i].fields[j].length)); >> + } >> + } >> + } >> +} >> + >> +static void arm_named_cpu_initfn(Object *obj) >> +{ >> + ARMCPU *cpu = ARM_CPU(obj); >> + ARMCPUClass *acc = ARM_CPU_GET_CLASS(obj); >> + const ArmNamedCpuModel *model = arm_find_model(acc->info->name); >> + >> + if (!model) { >> + error_report("'%s' CPU model entry not found", >> + acc->info->name); >> + return; >> + } >> + >> + if (!kvm_enabled()) { >> + error_report("'%s' CPU model requires KVM (-accel kvm)", >> + acc->info->name); >> + return; >> + } >> + >> + kvm_arm_set_cpu_features_from_host(cpu); >> + if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { >> + return; >> + } >> + >> + /* 1.a Reset ID regs to 0 */ >> + memset(cpu->isar.idregs, 0, sizeof(cpu->isar.idregs)); >> + >> + /* 1.b Properly set RES1 fields. */ >> + reset_res1_fields(cpu); >> + >> + /* 2. Add sveNNNN and SYSREG_ properties */ >> + aarch64_add_sve_properties(obj); >> + aarch64_add_idregs_properties(obj); > Can you give more explanations about why we don't have > aarch64_add_pauth_properties() > kvm_arm_set_cpreg_mig_tolerances() Haven’t looked into kvm_arm_set_cpreg_mig_tolerances(), will do. > > at least. > > By the way it is not clear what will become legacy compositive options > with named vcpu models. I see you keep sve but what about the others. As now your series handles the conflicts… I will take similar approach, user will not have two ways to set the same settings. >> + >> + /* 3. Realize the model chain. */ >> + arm_realize_model_chain(obj, model, &error_abort); >> +} >> + >> +void arm_register_named_cpu_models(void) >> +{ >> + static ARMCPUInfo *arm_named_cpu_infos; >> + size_t i, n = ARRAY_SIZE(arm_cpu_models); >> + >> + if (!n) { >> + return; >> + } >> + >> + arm_named_cpu_infos = g_new0(ARMCPUInfo, n); >> + for (i = 0; i < n; i++) { >> + if (!arm_cpu_models[i].name) { >> + break; >> + } >> + >> + arm_named_cpu_infos[i].name = arm_cpu_models[i].name; >> + arm_named_cpu_infos[i].initfn = arm_named_cpu_initfn; >> + arm_cpu_register(&arm_named_cpu_infos[i]); >> + } >> +} >> + >> +type_init(arm_register_named_cpu_models) >> diff --git a/target/arm/cpu-models.h b/target/arm/cpu-models.h >> new file mode 100644 >> index 0000000000..144778744e >> --- /dev/null >> +++ b/target/arm/cpu-models.h >> @@ -0,0 +1,36 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +#ifndef ARM_CPU_MODELS_H >> +#define ARM_CPU_MODELS_H >> + >> +#include "qapi/error.h" >> +#include "qom/object.h" >> + >> +typedef enum ArmModelPropType { >> + ARM_MODEL_PROP_BOOL, >> + ARM_MODEL_PROP_NUM, >> +} ArmModelPropType; >> + >> +typedef struct ArmModelPropValue { >> + const char *name; >> + ArmModelPropType type; >> + union { >> + bool b; >> + uint64_t num; >> + } u; >> +} ArmModelPropValue; >> + >> +typedef struct ArmNamedCpuModel { >> + const char *name; >> + const char *parent; > any reason why you don't point to the patent ArmNamedCpuModel pointer > instead? No specific reason, will change to ArmNamedCpuModel pointer. Thanks, Khushit >> + const ArmModelPropValue *props; >> +} ArmNamedCpuModel; >> + >> +void arm_register_named_cpu_models(void); >> + >> +const ArmNamedCpuModel *arm_find_model(const char *name); >> +void arm_apply_model_props(Object *obj, const ArmModelPropValue *props, >> + Error **errp); >> +void arm_realize_model_chain(Object *obj, const ArmNamedCpuModel *model, >> + Error **errp); >> + >> +#endif /* ARM_CPU_MODELS_H */ >> diff --git a/target/arm/meson.build b/target/arm/meson.build >> index 384d90c530..dd28146c52 100644 >> --- a/target/arm/meson.build >> +++ b/target/arm/meson.build >> @@ -32,6 +32,7 @@ arm_common_system_ss.add(files( >> 'arm-qmp-cmds.c', >> 'cpu-idregs.c', >> )) >> +arm_system_ss.add(when: 'TARGET_AARCH64', if_true: files('cpu-models.c')) >> arm_system_ss.add(when: 'CONFIG_KVM', if_true: files('hyp_gdbstub.c', >> 'kvm.c')) >> arm_system_ss.add(when: 'CONFIG_HVF', if_true: files('hyp_gdbstub.c')) >> > Eric
