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

[Thomas Huth]

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