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.
>
> 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?
> +#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.
> +
> + if (!cur->parent) {
> + break;
> + }
> + parent = arm_find_model(cur->parent);
I guess you directly assert
> + 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()
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.
> +
> + /* 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?
> + 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
