Re: [PATCH v3 07/10] arm/kvm: write back modified ID regs to KVM

[Cornelia Huck <cohuck@redhat.com>]

On Wed, Jul 02 2025, Jinqian Yang <yangjinqian1@huawei.com> wrote:

> On 2025/4/15 0:38, Cornelia Huck wrote:
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> We want to give a chance to override the value of host ID regs.
>> In a previous patch we made sure all their values could be fetched
>> through kvm_get_one_reg() calls before their modification. After
>> their potential modification we need to make sure we write back
>> the values through kvm_set_one_reg() calls.
>>
>> Make sure the cpreg_list is modified with updated values and
>> transfer those values back to kvm.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>   target/arm/kvm.c        | 44 ++++++++++++++++++++++++++++++++++++++++-
>>   target/arm/trace-events |  1 +
>>   2 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index b07d5f16db50..9e4cca1705c8 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -1083,6 +1083,39 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu)
>>       }
>>   }
>>   
>> +static void kvm_arm_writable_idregs_to_cpreg_list(ARMCPU *cpu)
>> +{
>> +    if (!cpu->writable_map) {
>> +        return;
>> +    }
>> +    for (int i = 0; i < NR_ID_REGS; i++) {
>> +        uint64_t writable_mask = cpu->writable_map->regs[i];
>> +        uint64_t *cpreg;
>> +
>> +        if (writable_mask) {
>> +            uint64_t previous, new;
>> +            int idx = kvm_idx_to_idregs_idx(i);
>> +            ARM64SysReg *sysregdesc;
>> +            uint32_t sysreg;
>> +
>> +            if (idx == -1) {
>> +                /* sysreg writable, but we don't know it */
>> +                continue;
>> +            }
>> +            sysregdesc = &arm64_id_regs[idx];
>> +            sysreg = sysregdesc->sysreg;
>> +            cpreg = kvm_arm_get_cpreg_ptr(cpu, idregs_sysreg_to_kvm_reg(sysreg));
>> +            previous = *cpreg;
>> +            new = cpu->isar.idregs[idx];
>> +            if (previous != new) {
>> +                *cpreg = new;
>> +                trace_kvm_arm_writable_idregs_to_cpreg_list(sysregdesc->name,
>> +                                                            previous, new);
>> +            }
>> +        }
>> +    }
>> +}
>> +
>>   void kvm_arm_reset_vcpu(ARMCPU *cpu)
>>   {
>>       int ret;
>> @@ -2050,7 +2083,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>       }
>>       cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
>>   
>> -    return kvm_arm_init_cpreg_list(cpu);
>> +    ret = kvm_arm_init_cpreg_list(cpu);
>> +    if (ret) {
>> +        return ret;
>> +    }
>> +    /* overwrite writable ID regs with their updated property values */
>> +    kvm_arm_writable_idregs_to_cpreg_list(cpu);
>> +
>> +    write_list_to_kvmstate(cpu, 3);
>> +
>> +    return 0;
>>   }
>
> Hi,
>
> When I was testing this series, I found a problem. On the host, 
> SYSREG_ID_AA64MMFR3_EL1_TCRX=0x1,
> I configured it to 0x0 in qemu, and qemu cannot start.
> The ID_AA64MMFR3_EL1 TCRX field controls whether the TCR2_EL1 and 
> TCR2_EL2 registers are used.
> In the kernel, when TCRX is 0, TCR2_EL1 is invisible to the guest, and 
> when it is 1, it is visible. When we
> configure this field segment to 0, the configuration is not yet written 
> to KVM, but the cpreg list is initialized,
> adding TCR2_EL1 to the cpreg list. Therefore, after writing the 
> QEMU-configured registers to KVM, the
> cpreg list needs to be updated again.
>
> @@ -2227,7 +2229,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>
>       write_list_to_kvmstate(cpu, 3);
>
> -    return 0;
> +    return kvm_arm_init_cpreg_list(cpu);
>   }
>
> The above modification can solve the problem, but it may not be the best 
> way.

Thanks for testing. Indeed, if we write something that changes the
visibility of registers, we need to do an update. Probably needs some
thinking (e.g. what happens if a register disappears, but the user set
some fields?)