On 9/28/26 12:44 PM, Khushit Shah wrote:
>
>> On 16 Sep 2026, at 8:15 PM, Eric Auger <[email protected]> wrote:
>>
>> !-------------------------------------------------------------------|
>> CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> SYSREG prop settings were recorded in a hashtable but they
>> are not yet injected on guest. Iterate over all set fields and
>> apply the new value through KVM_SET_ONE_REG. The new field
>> value overrides the current observable vcpu register value.
>>
>> Signed-off-by: Eric Auger <[email protected]>
> With this, I suppose, what named models will do is directly write to
> ID register state, As it also defines state which affects vCPU init flags
> also.
>
>> ---
>> target/arm/kvm.c | 65 +++++++++++++++++++++++++++++++++++++++++
>> target/arm/trace-events | 1 +
>> 2 files changed, 66 insertions(+)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 3ec2618cc1..ddf320d0e6 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -343,6 +343,9 @@ static ARM64SysRegField *get_field(int i, ARM64SysReg
>> *reg)
>> #define MAKE_IDREG_KEY(reg_idx, field_shift) \
>> (((uint64_t)(reg_idx) << 8) | ((uint64_t)(field_shift) & 0xFF))
>>
>> +#define KEY_TO_REG_IDX(key) ((uint32_t)(((uint64_t)(key)) >> 8))
>> +#define KEY_TO_SHIFT(key) ((uint8_t)(((uint64_t)(key)) & 0xFF))
>> +
>> static void set_sysreg_prop(Object *obj, Visitor *v,
>> const char *name, void *opaque,
>> Error **errp)
>> @@ -2215,6 +2218,63 @@ int kvm_arch_pre_create_vcpu(CPUState *cpu, Error
>> **errp)
>> return 0;
>> }
>>
>> +static int kvm_arm_apply_sysreg_props(ARMCPU *cpu, Error **errp)
>> +{
>> + uint64_t *idregs = cpu->isar.idregs;
>> + gpointer key_ptr, value_ptr;
>> + CPUState *cs = CPU(cpu);
>> + GHashTableIter iter;
>> +
>> + g_hash_table_iter_init(&iter, cpu->sysreg_props);
>> +
>> + while (g_hash_table_iter_next(&iter, &key_ptr, &value_ptr)) {
>> + uint64_t key = (uint64_t)key_ptr;
>> + uint64_t value = (uint64_t)value_ptr;
>> + uint32_t reg_idx = KEY_TO_REG_IDX(key);
>> + uint8_t lower = KEY_TO_SHIFT(key);
>> + ARM64SysReg *reg = &arm64_id_regs[reg_idx];
>> + struct kvm_one_reg kvm_reg;
>> + uint64_t current, old, mask;
>> + uint64_t kvm_idx;
>> + ARM64SysRegField *field = get_field(lower, reg);
>> + int length = field->length;
>> + uint64_t oldfv;
>> + int ret;
>> +
>> + mask = MAKE_64BIT_MASK(lower, length);
>> + value = value << lower;
>> +
>> + old = idregs[reg_idx];
>> +
>> + kvm_idx = idregs_sysreg_to_kvm_reg(id_register_sysreg[reg_idx]);
>> + ret = read_sys_reg64(cs->kvm_fd, ¤t, kvm_idx);
>> + if (ret) {
>> + error_setg(errp, "failed to read the current value of %s",
>> + reg->name);
>> + return ret;
>> + }
>> + oldfv = (current & mask) >> lower;
>> +
>> + idregs[reg_idx] = current & ~mask;
>> + idregs[reg_idx] |= value;
>> + kvm_reg.id = kvm_idx;
>> + kvm_reg.addr = (uintptr_t)&idregs[reg_idx];
>> +
> We can ignore this if value == old value
>
>> + ret = ioctl(cs->kvm_fd, KVM_SET_ONE_REG, &kvm_reg);
>> + if (ret) {
>> + error_setg(errp, "failed to apply new value 0x%"PRIx64" for
>> field %s.%s "
>> + "(previous is 0x%"PRIx64"): %m", idregs[reg_idx],
>> + reg->name, field->name, oldfv);
>> + return ret;
>> + }
>> +
>> + trace_apply_sysreg_prop(reg->name, field->name, old, current,
>> + mask, value, idregs[reg_idx]);
>> + }
>> + return 0;
>> +}
>> +
>> int kvm_arch_init_vcpu(CPUState *cs, Error **errp)
>> {
>> int ret;
>> @@ -2315,6 +2375,11 @@ int kvm_arch_init_vcpu(CPUState *cs, Error **errp)
>> }
>> cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
>>
>> + ret = kvm_arm_apply_sysreg_props(cpu, errp);
>> + if (ret) {
>> + return ret;
>> + }
>> +
> This should also happen on vCPU reset.
>
> Overall, I like this approach as if the user does not provide any property
> overrides, it should behave the same as older builds.
to me the risky part if the initialization of all cpu->isar.idregs[]
from host which can have side effects like the CLDIR_EL1 story.
Eric
>
> Warm Regards,
> Khushit
>> return kvm_arm_init_cpreg_list(cpu);
>> }
>>
>> diff --git a/target/arm/trace-events b/target/arm/trace-events
>> index d72ad6b671..b4d2b45a7d 100644
>> --- a/target/arm/trace-events
>> +++ b/target/arm/trace-events
>> @@ -18,6 +18,7 @@ kvm_arm_writable_idregs_to_cpreg_list(const char *name,
>> uint64_t previous, uint6
>> decode_idreg_writemap(const char* name, int lower, int upper, char
>> *prop_name) "%s [%d:%d] is writable (prop %s)"
>> 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
>> +apply_sysreg_prop(const char *reg_name, const char *field_name, uint64_t
>> old, uint64_t current, uint64_t mask, uint64_t field_value, uint64_t new)
>> "%s.%s old=0x%"PRIx64" current=0x%"PRIx64" mask=0x%"PRIx64" new field
>> value=0x%"PRIx64" new=0x%"PRIx64
>> nb_sysreg_props(const char *name, int count) "%s: %d SYSREG properties"
>>
>> # cpu.c
>> --
>> 2.53.0
>>
