Re: [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code

[Thomas Huth <thuth@redhat.com>]

On 10/10/2023 13.41, Christian Borntraeger wrote:
> 
> 
> Am 09.10.23 um 19:07 schrieb Thomas Huth:
> [...]
> 
>> @@ -483,20 +482,14 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>       cs->kvm_run->psw_addr = env->psw.addr;
>>       cs->kvm_run->psw_mask = env->psw.mask;
>> -    if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
>> -        for (i = 0; i < 16; i++) {
>> -            cs->kvm_run->s.regs.gprs[i] = env->regs[i];
>> -            cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
>> -        }
>> -    } else {
>> -        for (i = 0; i < 16; i++) {
>> -            regs.gprs[i] = env->regs[i];
>> -        }
>> -        r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
>> -        if (r < 0) {
>> -            return r;
>> -        }
>> -    }
>> +    g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
>> +             KVM_SYNC_REQUIRED_BITS);
>> +    cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_REQUIRED_BITS;
>> +    memcpy(cs->kvm_run->s.regs.gprs, env->regs, 
>> sizeof(cs->kvm_run->s.regs.gprs));
> 
> 
>> +    memcpy(cs->kvm_run->s.regs.acrs, env->aregs, 
>> sizeof(cs->kvm_run->s.regs.acrs));
>> +    memcpy(cs->kvm_run->s.regs.crs, env->cregs, 
>> sizeof(cs->kvm_run->s.regs.crs));
>> +
>> +    cs->kvm_run->s.regs.prefix = env->psa;
> 
> These 3 have only been saved for level> KVM_PUT_RUNTIME_STATE. By moving the 
> memcpy
> into this position you would always write them. Probably ok but a change in
> behaviour. Was this intentional?

Oops, no, I missed that early return in the function. I'll fix it and send a v3.

  Thanks!
   Thomas