Hi Khushit, On 9/21/26 2:32 PM, Khushit Shah wrote: > Add kvm_arm_field_get_supported_values() which, for a given ID register > field, builds the set of values KVM allows it to take on the given host. > > Non-writable fields are pinned to the host value. Writable fields follow > their per-field constraints. This function must be updated 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.
This effectively advocates for having a snapshot of all the IDREGS that are known to be writable and handled by the qemu code. Because otherwise we may return wrong safe values for a given prop. > signed-off-by: Khushit Shah <[email protected]> > > -------- > Note: > - We should have an authoritative static list of "writable" fields in > QEMU. > --- > target/arm/kvm-stub.c | 7 +++ > target/arm/kvm.c | 99 +++++++++++++++++++++++++++++++++++++++++++ > target/arm/kvm_arm.h | 28 ++++++++++++ > 3 files changed, 134 insertions(+) > > diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c > index d263adc921..c9cd47bf47 100644 > --- a/target/arm/kvm-stub.c > +++ b/target/arm/kvm-stub.c > @@ -144,3 +144,10 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray) > { > g_assert_not_reached(); > } > + > +void kvm_arm_field_get_supported_values(const ARM64SysRegField *field, > + const ARMISARegisters *host_isar, > + ArmFieldValueSet **value_set) > +{ > + g_assert_not_reached(); > +} > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index 6f704c0509..77b1bcdf55 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -1324,6 +1324,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"); > +} > + > +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 kvm_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; > + } > + > + 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); those are tagged with SAFE_EXACT policy in the kernel. Could you elaborate? > + } > + } 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); *ID_AA64FPFR0_EL1_F8* hals have* EXACT policy and I don't see them special cased here. > + } > + } 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]; > + 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 4e563fab96..9481e56412 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -175,6 +175,34 @@ int kvm_arm_read_sys_reg64(int fd, uint64_t *val, > */ > 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; > + > +/** > + * kvm_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 kvm_arm_field_get_supported_values(const ARM64SysRegField *field, > + const ARMISARegisters *host_isar, > + ArmFieldValueSet **value_set); > + > /** > * kvm_arm_steal_time_finalize: > * @cpu: ARMCPU for which to finalize kvm-steal-time Eric
