On Wed, Jul 02 2025, Jinqian Yang <yangjinqi...@huawei.com> wrote:
> On 2025/4/15 0:38, Cornelia Huck wrote:
>> From: Eric Auger <eric.au...@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.au...@redhat.com>
>> Signed-off-by: Cornelia Huck <coh...@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?)
