Hi Khushit, On 7/16/26 11:38 PM, Khushit Shah wrote: > Add arm_field_get_supported_values() which, for a given ID register > field, builds the set of values KVM allows it to take on the live host. > > Non-writable fields are pinned to the host value. Writable fields follow > their per field constraints. This function likely needs change when > KVM starts exposing a new writable field that is not lower-safe, hence, > the special handling of TGranX_2/SpecSEI/L1Ip/MIDR/REVIDR/AIDR. so this looks quite risky. > > Cross-field constraints are not modelled; for example ID_AA64ZFR0_EL1 > is gated by SVE. Reproducing every inter-field dependency in QEMU > would only duplicate KVM's logic and drift out of sync with it.
Isn't qmp_query_cpu_model_expansion sufficient? In general upper layers will try to apply raw named models. If ajustements are needed between source and destination, we know field candidate values (either the source or dest one) and this latter can be directly tried using qmp_query_cpu_model_expansion write. On top of that qmp_query_cpu_model_expansion can be directly checked against a scratch vcpu reusing the logic implemented at kernel level. > > Signed-off-by: Khushit Shah <[email protected]> > --- > target/arm/kvm.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ > target/arm/kvm_arm.h | 26 ++++++++++++ > 2 files changed, 125 insertions(+) > > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index c38b99cfce..8f452f9570 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -1262,6 +1262,105 @@ bool kvm_arm_cpu_post_load(ARMCPU *cpu) > return true; > } > > +static bool arm_field_is_signed(const ARM64SysRegField *field) > +{ > + return field_matches(field, ID_AA64MMFR0_EL1_IDX, "TGran4") || > + field_matches(field, ID_AA64MMFR0_EL1_IDX, "TGran64") || > + field_matches(field, ID_MMFR0_EL1_IDX, "InnerShr") || > + field_matches(field, ID_MMFR0_EL1_IDX, "OuterShr") || > + field_matches(field, ID_AA64DFR0_EL1_IDX, "DoubleLock") || > + field_matches(field, ID_AA64DFR0_EL1_IDX, "PMUVer") || > + field_matches(field, ID_DFR0_EL1_IDX, "PerfMon") || > + field_matches(field, ID_AA64PFR1_EL1_IDX, "MTE_frac") || > + field_matches(field, ID_AA64MMFR4_EL1_IDX, "E2H0") || > + field_matches(field, ID_DFR1_EL1_IDX, "MTPMU") || > + field_matches(field, ID_AA64PFR0_EL1_IDX, "FP") || > + field_matches(field, ID_AA64PFR0_EL1_IDX, "AdvSIMD"); Can't we extract this from Register.json instead? > +} > + > +static void ranges_add(GArray *ranges, uint64_t min, uint64_t max) > +{ > + ArmFieldRange r = { .min = min, .max = max }; > + g_array_append_val(ranges, r); > +} > + > +void arm_field_get_supported_values(const ARM64SysRegField *field, > + const ARMISARegisters *host_isar, > + ArmFieldValueSet **value_set) > +{ > + bool is_signed = arm_field_is_signed(field); > + uint64_t host = extract64(host_isar->idregs[field->index], > + field->shift, field->length); > + GArray *ranges = g_array_new(false, false, sizeof(ArmFieldRange)); > + > + /* A non-writable field can only ever hold the host value. */ > + if (!arm_field_is_writable(field)) { > + ranges_add(ranges, host, host); > + goto done; you already get this info from qmp_query_cpu_model_expansion > + } > + > + if (field_matches(field, ID_AA64MMFR0_EL1_IDX, "TGran4_2") || > + field_matches(field, ID_AA64MMFR0_EL1_IDX, "TGran16_2") || > + field_matches(field, ID_AA64MMFR0_EL1_IDX, "TGran64_2")) { > + /* Either support the host value or the "off" value */ > + ranges_add(ranges, host, host); > + if (host != 1) { /* 1 = "off" */ > + ranges_add(ranges, 1, 1); > + } > + } else if (field_matches(field, CTR_EL0_IDX, "L1Ip")) { > + /* Only safe to downgrade to VIPT, other values are reserved. */ > + ranges_add(ranges, host, host); > + if (host != 2) { /* 2 = "VIPT" */ > + ranges_add(ranges, 2, 2); > + } > + } else if (field_matches(field, ID_AA64MMFR1_EL1_IDX, "SpecSEI") || > + field_matches(field, ID_MMFR4_EL1_IDX, "SpecSEI")) { > + /* It is safe to upgrade SpecSEI to 1, other values are reserved. */ > + ranges_add(ranges, host, host); > + if (host != 1) { > + ranges_add(ranges, 1, 1); > + } > + } else if (field->index == MIDR_EL1_IDX || > + field->index == REVIDR_EL1_IDX || > + field->index == AIDR_EL1_IDX) { > + /* > + * No restriction on value that can be set for implementation ID > + * registers fields. > + */ > + uint64_t max = 0; > + if (field->length == 64) { > + max = ~0ULL; > + } else { > + max = (1ULL << field->length) - 1; > + } > + ranges_add(ranges, 0, max); > + } else { > + /* > + * After handling the special cases, other writable fields are > + * either lower-safe or signed lower-safe. > + */ > + if (field->arch_vals_count) { > + for (uint32_t i = 0; i < field->arch_vals_count; i++) { > + uint64_t av = field->arch_vals[i].value; > + int64_t v = is_signed ? > + sextract64(av, 0, field->length) : (int64_t)av; > + int64_t hv = is_signed ? > + sextract64(host, 0, field->length) : (int64_t)host; > + if (v <= hv) { > + ranges_add(ranges, av, av); > + } > + } > + } else { > + g_assert(!is_signed); /* No signed field with no arch vals */ > + ranges_add(ranges, 0, host); > + } > + } > +done: > + *value_set = g_new0(ArmFieldValueSet, 1); > + (*value_set)->n_ranges = ranges->len; > + (*value_set)->ranges = (ArmFieldRange *)g_array_free(ranges, false); > +} > + > static bool arm_field_skip_writeback_always(const ARM64SysRegField *field) > { > /* > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index 133a026036..9f15c91c4e 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -143,6 +143,32 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); > void kvm_arm_add_vcpu_properties(ARMCPU *cpu); > > typedef struct ARM64SysReg ARM64SysReg; > +typedef struct ARM64SysRegField ARM64SysRegField; > +typedef struct ARMISARegisters ARMISARegisters; > + > +typedef struct ArmFieldRange { > + uint64_t min; > + uint64_t max; > +} ArmFieldRange; > + > +typedef struct ArmFieldValueSet { > + ArmFieldRange *ranges; > + size_t n_ranges; > +} ArmFieldValueSet; > + > +/** > + * arm_field_get_supported_values: > + * @field: The field to get the supported values for > + * @host_isar: The host ISAR registers > + * @value_set: The set of supported values for the @field > + * > + * Will be allocated and filled in with the supported values for the @field > + * based on the host_isar and whether the field is writable or not. > + * The caller must free the value_set. > + */ > +void arm_field_get_supported_values(const ARM64SysRegField *field, > + const ARMISARegisters *host_isar, > + ArmFieldValueSet **value_set); > > /** > * kvm_arm_steal_time_finalize: Thanks Eric
