target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code

[PATCH for-8.2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code

Posted by Thomas Huth 9 months, 1 week ago

KVM_SYNC_GPRS, KVM_SYNC_ACRS, KVM_SYNC_CRS and KVM_SYNC_PREFIX are
available since kernel 3.10. Since we already require at least kernel
3.15 in the s390x KVM code, we can assume that the KVM_CAP_SYNC_REGS
sync code is always possible for these registers, and remove the
related checks and fallbacks via KVM_SET_REGS and KVM_GET_REGS.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/kvm/kvm.c | 119 ++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 85 deletions(-)

diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index a9e5880349..ff415f7b30 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -148,7 +148,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
 
-static int cap_sync_regs;
 static int cap_async_pf;
 static int cap_mem_op;
 static int cap_mem_op_extension;
@@ -342,21 +341,28 @@ static void ccw_machine_class_foreach(ObjectClass *oc, void *opaque)
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
+    int required_caps[] = {
+        KVM_CAP_DEVICE_CTRL,
+        KVM_CAP_SYNC_REGS,
+    };
+
+    for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
+        if (!kvm_check_extension(s, required_caps[i])) {
+            error_report("KVM is missing capability #%d - "
+                         "please use kernel 3.15 or newer", required_caps[i]);
+            return -1;
+        }
+    }
+
     object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
                          false, NULL);
 
-    if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
-        error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
-                     "please use kernel 3.15 or newer");
-        return -1;
-    }
     if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
         error_report("KVM is missing capability KVM_CAP_S390_COW - "
                      "unsupported environment");
         return -1;
     }
 
-    cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
     cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
     cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
     cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);
@@ -463,15 +469,15 @@ void kvm_s390_reset_vcpu_normal(S390CPU *cpu)
 
 static int can_sync_regs(CPUState *cs, int regs)
 {
-    return cap_sync_regs && (cs->kvm_run->kvm_valid_regs & regs) == regs;
+    return (cs->kvm_run->kvm_valid_regs & regs) == regs;
 }
 
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
+    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
+                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
-    struct kvm_sregs sregs;
-    struct kvm_regs regs;
     struct kvm_fpu fpu = {};
     int r;
     int i;
@@ -480,21 +486,16 @@ 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 & required_syncs) == required_syncs);
+    cs->kvm_run->kvm_dirty_regs |= required_syncs;
+    for (i = 0; i < 16; i++) {
+        cs->kvm_run->s.regs.gprs[i] = env->regs[i];
+        cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
+        cs->kvm_run->s.regs.crs[i] = env->cregs[i];
     }
 
+    cs->kvm_run->s.regs.prefix = env->psa;
+
     if (can_sync_regs(cs, KVM_SYNC_VRS)) {
         for (i = 0; i < 32; i++) {
             cs->kvm_run->s.regs.vrs[i][0] = env->vregs[i][0];
@@ -572,25 +573,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         }
     }
 
-    /* access registers and control registers*/
-    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
-        for (i = 0; i < 16; i++) {
-            cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
-            cs->kvm_run->s.regs.crs[i] = env->cregs[i];
-        }
-        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
-        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_CRS;
-    } else {
-        for (i = 0; i < 16; i++) {
-            sregs.acrs[i] = env->aregs[i];
-            sregs.crs[i] = env->cregs[i];
-        }
-        r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
-        if (r < 0) {
-            return r;
-        }
-    }
-
     if (can_sync_regs(cs, KVM_SYNC_GSCB)) {
         memcpy(cs->kvm_run->s.regs.gscb, env->gscb, 32);
         cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
@@ -612,22 +594,15 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
     }
 
-    /* Finally the prefix */
-    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
-        cs->kvm_run->s.regs.prefix = env->psa;
-        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
-    } else {
-        /* prefix is only supported via sync regs */
-    }
     return 0;
 }
 
 int kvm_arch_get_registers(CPUState *cs)
 {
+    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
+                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
-    struct kvm_sregs sregs;
-    struct kvm_regs regs;
     struct kvm_fpu fpu;
     int i, r;
 
@@ -635,37 +610,16 @@ int kvm_arch_get_registers(CPUState *cs)
     env->psw.addr = cs->kvm_run->psw_addr;
     env->psw.mask = cs->kvm_run->psw_mask;
 
-    /* the GPRS */
-    if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
-        for (i = 0; i < 16; i++) {
-            env->regs[i] = cs->kvm_run->s.regs.gprs[i];
-        }
-    } else {
-        r = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
-        if (r < 0) {
-            return r;
-        }
-         for (i = 0; i < 16; i++) {
-            env->regs[i] = regs.gprs[i];
-        }
+    /* the GPRS, ACRS and CRS */
+    g_assert((cs->kvm_run->kvm_valid_regs & required_syncs) == required_syncs);
+    for (i = 0; i < 16; i++) {
+        env->regs[i] = cs->kvm_run->s.regs.gprs[i];
+        env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
+        env->cregs[i] = cs->kvm_run->s.regs.crs[i];
     }
 
-    /* The ACRS and CRS */
-    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
-        for (i = 0; i < 16; i++) {
-            env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
-            env->cregs[i] = cs->kvm_run->s.regs.crs[i];
-        }
-    } else {
-        r = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
-        if (r < 0) {
-            return r;
-        }
-         for (i = 0; i < 16; i++) {
-            env->aregs[i] = sregs.acrs[i];
-            env->cregs[i] = sregs.crs[i];
-        }
-    }
+    /* The prefix */
+    env->psa = cs->kvm_run->s.regs.prefix;
 
     /* Floating point and vector registers */
     if (can_sync_regs(cs, KVM_SYNC_VRS)) {
@@ -690,11 +644,6 @@ int kvm_arch_get_registers(CPUState *cs)
         env->fpc = fpu.fpc;
     }
 
-    /* The prefix */
-    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
-        env->psa = cs->kvm_run->s.regs.prefix;
-    }
-
     if (can_sync_regs(cs, KVM_SYNC_ARCH0)) {
         env->cputm = cs->kvm_run->s.regs.cputm;
         env->ckc = cs->kvm_run->s.regs.ckc;
-- 
2.39.3

Re: [PATCH for-8.2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code

Posted by Thomas Huth 8 months, 3 weeks ago

On 14/08/2023 21.06, Thomas Huth wrote:
> KVM_SYNC_GPRS, KVM_SYNC_ACRS, KVM_SYNC_CRS and KVM_SYNC_PREFIX are
> available since kernel 3.10. Since we already require at least kernel
> 3.15 in the s390x KVM code, we can assume that the KVM_CAP_SYNC_REGS
> sync code is always possible for these registers, and remove the
> related checks and fallbacks via KVM_SET_REGS and KVM_GET_REGS.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   target/s390x/kvm/kvm.c | 119 ++++++++++++-----------------------------
>   1 file changed, 34 insertions(+), 85 deletions(-)

Ping! Anybody any comments on this one?

  Thomas

> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index a9e5880349..ff415f7b30 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -148,7 +148,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>       KVM_CAP_LAST_INFO
>   };
>   
> -static int cap_sync_regs;
>   static int cap_async_pf;
>   static int cap_mem_op;
>   static int cap_mem_op_extension;
> @@ -342,21 +341,28 @@ static void ccw_machine_class_foreach(ObjectClass *oc, void *opaque)
>   
>   int kvm_arch_init(MachineState *ms, KVMState *s)
>   {
> +    int required_caps[] = {
> +        KVM_CAP_DEVICE_CTRL,
> +        KVM_CAP_SYNC_REGS,
> +    };
> +
> +    for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
> +        if (!kvm_check_extension(s, required_caps[i])) {
> +            error_report("KVM is missing capability #%d - "
> +                         "please use kernel 3.15 or newer", required_caps[i]);
> +            return -1;
> +        }
> +    }
> +
>       object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
>                            false, NULL);
>   
> -    if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
> -        error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
> -                     "please use kernel 3.15 or newer");
> -        return -1;
> -    }
>       if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
>           error_report("KVM is missing capability KVM_CAP_S390_COW - "
>                        "unsupported environment");
>           return -1;
>       }
>   
> -    cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>       cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>       cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
>       cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);
> @@ -463,15 +469,15 @@ void kvm_s390_reset_vcpu_normal(S390CPU *cpu)
>   
>   static int can_sync_regs(CPUState *cs, int regs)
>   {
> -    return cap_sync_regs && (cs->kvm_run->kvm_valid_regs & regs) == regs;
> +    return (cs->kvm_run->kvm_valid_regs & regs) == regs;
>   }
>   
>   int kvm_arch_put_registers(CPUState *cs, int level)
>   {
> +    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
> +                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;
>       S390CPU *cpu = S390_CPU(cs);
>       CPUS390XState *env = &cpu->env;
> -    struct kvm_sregs sregs;
> -    struct kvm_regs regs;
>       struct kvm_fpu fpu = {};
>       int r;
>       int i;
> @@ -480,21 +486,16 @@ 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 & required_syncs) == required_syncs);
> +    cs->kvm_run->kvm_dirty_regs |= required_syncs;
> +    for (i = 0; i < 16; i++) {
> +        cs->kvm_run->s.regs.gprs[i] = env->regs[i];
> +        cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
> +        cs->kvm_run->s.regs.crs[i] = env->cregs[i];
>       }
>   
> +    cs->kvm_run->s.regs.prefix = env->psa;
> +
>       if (can_sync_regs(cs, KVM_SYNC_VRS)) {
>           for (i = 0; i < 32; i++) {
>               cs->kvm_run->s.regs.vrs[i][0] = env->vregs[i][0];
> @@ -572,25 +573,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>           }
>       }
>   
> -    /* access registers and control registers*/
> -    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
> -        for (i = 0; i < 16; i++) {
> -            cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
> -            cs->kvm_run->s.regs.crs[i] = env->cregs[i];
> -        }
> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_CRS;
> -    } else {
> -        for (i = 0; i < 16; i++) {
> -            sregs.acrs[i] = env->aregs[i];
> -            sregs.crs[i] = env->cregs[i];
> -        }
> -        r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
> -        if (r < 0) {
> -            return r;
> -        }
> -    }
> -
>       if (can_sync_regs(cs, KVM_SYNC_GSCB)) {
>           memcpy(cs->kvm_run->s.regs.gscb, env->gscb, 32);
>           cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
> @@ -612,22 +594,15 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>           cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
>       }
>   
> -    /* Finally the prefix */
> -    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
> -        cs->kvm_run->s.regs.prefix = env->psa;
> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
> -    } else {
> -        /* prefix is only supported via sync regs */
> -    }
>       return 0;
>   }
>   
>   int kvm_arch_get_registers(CPUState *cs)
>   {
> +    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
> +                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;
>       S390CPU *cpu = S390_CPU(cs);
>       CPUS390XState *env = &cpu->env;
> -    struct kvm_sregs sregs;
> -    struct kvm_regs regs;
>       struct kvm_fpu fpu;
>       int i, r;
>   
> @@ -635,37 +610,16 @@ int kvm_arch_get_registers(CPUState *cs)
>       env->psw.addr = cs->kvm_run->psw_addr;
>       env->psw.mask = cs->kvm_run->psw_mask;
>   
> -    /* the GPRS */
> -    if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
> -        for (i = 0; i < 16; i++) {
> -            env->regs[i] = cs->kvm_run->s.regs.gprs[i];
> -        }
> -    } else {
> -        r = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
> -        if (r < 0) {
> -            return r;
> -        }
> -         for (i = 0; i < 16; i++) {
> -            env->regs[i] = regs.gprs[i];
> -        }
> +    /* the GPRS, ACRS and CRS */
> +    g_assert((cs->kvm_run->kvm_valid_regs & required_syncs) == required_syncs);
> +    for (i = 0; i < 16; i++) {
> +        env->regs[i] = cs->kvm_run->s.regs.gprs[i];
> +        env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
> +        env->cregs[i] = cs->kvm_run->s.regs.crs[i];
>       }
>   
> -    /* The ACRS and CRS */
> -    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
> -        for (i = 0; i < 16; i++) {
> -            env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
> -            env->cregs[i] = cs->kvm_run->s.regs.crs[i];
> -        }
> -    } else {
> -        r = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
> -        if (r < 0) {
> -            return r;
> -        }
> -         for (i = 0; i < 16; i++) {
> -            env->aregs[i] = sregs.acrs[i];
> -            env->cregs[i] = sregs.crs[i];
> -        }
> -    }
> +    /* The prefix */
> +    env->psa = cs->kvm_run->s.regs.prefix;
>   
>       /* Floating point and vector registers */
>       if (can_sync_regs(cs, KVM_SYNC_VRS)) {
> @@ -690,11 +644,6 @@ int kvm_arch_get_registers(CPUState *cs)
>           env->fpc = fpu.fpc;
>       }
>   
> -    /* The prefix */
> -    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
> -        env->psa = cs->kvm_run->s.regs.prefix;
> -    }
> -
>       if (can_sync_regs(cs, KVM_SYNC_ARCH0)) {
>           env->cputm = cs->kvm_run->s.regs.cputm;
>           env->ckc = cs->kvm_run->s.regs.ckc;

Re: [PATCH for-8.2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code

Posted by Cédric Le Goater 8 months, 3 weeks ago

Hello Thomas,

On 8/28/23 20:20, Thomas Huth wrote:
> On 14/08/2023 21.06, Thomas Huth wrote:
>> KVM_SYNC_GPRS, KVM_SYNC_ACRS, KVM_SYNC_CRS and KVM_SYNC_PREFIX are
>> available since kernel 3.10. Since we already require at least kernel
>> 3.15 in the s390x KVM code, we can assume that the KVM_CAP_SYNC_REGS
>> sync code is always possible for these registers, and remove the
>> related checks and fallbacks via KVM_SET_REGS and KVM_GET_REGS.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/s390x/kvm/kvm.c | 119 ++++++++++++-----------------------------
>>   1 file changed, 34 insertions(+), 85 deletions(-)
> 
> Ping! Anybody any comments on this one?


Only minor comments, see below.

> 
>   Thomas
> 
>> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
>> index a9e5880349..ff415f7b30 100644
>> --- a/target/s390x/kvm/kvm.c
>> +++ b/target/s390x/kvm/kvm.c
>> @@ -148,7 +148,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>>       KVM_CAP_LAST_INFO
>>   };
>> -static int cap_sync_regs;
>>   static int cap_async_pf;
>>   static int cap_mem_op;
>>   static int cap_mem_op_extension;
>> @@ -342,21 +341,28 @@ static void ccw_machine_class_foreach(ObjectClass *oc, void *opaque)
>>   int kvm_arch_init(MachineState *ms, KVMState *s)
>>   {
>> +    int required_caps[] = {
>> +        KVM_CAP_DEVICE_CTRL,
>> +        KVM_CAP_SYNC_REGS,
>> +    };
>> +
>> +    for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
>> +        if (!kvm_check_extension(s, required_caps[i])) {
>> +            error_report("KVM is missing capability #%d - "
>> +                         "please use kernel 3.15 or newer", required_caps[i]);
>> +            return -1;
>> +        }
>> +    }
>> +
>>       object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
>>                            false, NULL);
>> -    if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>> -        error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
>> -                     "please use kernel 3.15 or newer");
>> -        return -1;
>> -    }
>>       if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
>>           error_report("KVM is missing capability KVM_CAP_S390_COW - "
>>                        "unsupported environment");
>>           return -1;
>>       }
>> -    cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>>       cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>>       cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
>>       cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);

I would introduce a first patch for the changes above, checking the
KVM capabilities, and a second for get/put register changes.


>> @@ -463,15 +469,15 @@ void kvm_s390_reset_vcpu_normal(S390CPU *cpu)
>>   static int can_sync_regs(CPUState *cs, int regs)
>>   {
>> -    return cap_sync_regs && (cs->kvm_run->kvm_valid_regs & regs) == regs;
>> +    return (cs->kvm_run->kvm_valid_regs & regs) == regs;
>>   }
>>   int kvm_arch_put_registers(CPUState *cs, int level)
>>   {
>> +    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
>> +                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;

May be introduce a KVM_SYNC_s390_REQUIRED_FIELDS define for these fields,
as we already have KVM_SYNC_S390_VALID_FIELDS.

>>       S390CPU *cpu = S390_CPU(cs);
>>       CPUS390XState *env = &cpu->env;
>> -    struct kvm_sregs sregs;
>> -    struct kvm_regs regs;
>>       struct kvm_fpu fpu = {};
>>       int r;
>>       int i;
>> @@ -480,21 +486,16 @@ 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 & required_syncs) == required_syncs);

this also is :

           g_assert(can_sync_regs(cs, KVM_SYNC_s390_REQUIRED_FIELDS));

the rest looks good.

Thanks,

C.

>> +    cs->kvm_run->kvm_dirty_regs |= required_syncs;
>> +    for (i = 0; i < 16; i++) {
>> +        cs->kvm_run->s.regs.gprs[i] = env->regs[i];
>> +        cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
>> +        cs->kvm_run->s.regs.crs[i] = env->cregs[i];
>>       }
>> +    cs->kvm_run->s.regs.prefix = env->psa;
>> +
>>       if (can_sync_regs(cs, KVM_SYNC_VRS)) {
>>           for (i = 0; i < 32; i++) {
>>               cs->kvm_run->s.regs.vrs[i][0] = env->vregs[i][0];
>> @@ -572,25 +573,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>           }
>>       }
>> -    /* access registers and control registers*/
>> -    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
>> -        for (i = 0; i < 16; i++) {
>> -            cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
>> -            cs->kvm_run->s.regs.crs[i] = env->cregs[i];
>> -        }
>> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
>> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_CRS;
>> -    } else {
>> -        for (i = 0; i < 16; i++) {
>> -            sregs.acrs[i] = env->aregs[i];
>> -            sregs.crs[i] = env->cregs[i];
>> -        }
>> -        r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
>> -        if (r < 0) {
>> -            return r;
>> -        }
>> -    }
>> -
>>       if (can_sync_regs(cs, KVM_SYNC_GSCB)) {
>>           memcpy(cs->kvm_run->s.regs.gscb, env->gscb, 32);
>>           cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
>> @@ -612,22 +594,15 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>           cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
>>       }
>> -    /* Finally the prefix */
>> -    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>> -        cs->kvm_run->s.regs.prefix = env->psa;
>> -        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
>> -    } else {
>> -        /* prefix is only supported via sync regs */
>> -    }
>>       return 0;
>>   }
>>   int kvm_arch_get_registers(CPUState *cs)
>>   {
>> +    const int required_syncs = KVM_SYNC_GPRS | KVM_SYNC_ACRS |
>> +                               KVM_SYNC_CRS | KVM_SYNC_PREFIX;
>>       S390CPU *cpu = S390_CPU(cs);
>>       CPUS390XState *env = &cpu->env;
>> -    struct kvm_sregs sregs;
>> -    struct kvm_regs regs;
>>       struct kvm_fpu fpu;
>>       int i, r;
>> @@ -635,37 +610,16 @@ int kvm_arch_get_registers(CPUState *cs)
>>       env->psw.addr = cs->kvm_run->psw_addr;
>>       env->psw.mask = cs->kvm_run->psw_mask;
>> -    /* the GPRS */
>> -    if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
>> -        for (i = 0; i < 16; i++) {
>> -            env->regs[i] = cs->kvm_run->s.regs.gprs[i];
>> -        }
>> -    } else {
>> -        r = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
>> -        if (r < 0) {
>> -            return r;
>> -        }
>> -         for (i = 0; i < 16; i++) {
>> -            env->regs[i] = regs.gprs[i];
>> -        }
>> +    /* the GPRS, ACRS and CRS */
>> +    g_assert((cs->kvm_run->kvm_valid_regs & required_syncs) == required_syncs);
>> +    for (i = 0; i < 16; i++) {
>> +        env->regs[i] = cs->kvm_run->s.regs.gprs[i];
>> +        env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
>> +        env->cregs[i] = cs->kvm_run->s.regs.crs[i];
>>       }
>> -    /* The ACRS and CRS */
>> -    if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
>> -        for (i = 0; i < 16; i++) {
>> -            env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
>> -            env->cregs[i] = cs->kvm_run->s.regs.crs[i];
>> -        }
>> -    } else {
>> -        r = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
>> -        if (r < 0) {
>> -            return r;
>> -        }
>> -         for (i = 0; i < 16; i++) {
>> -            env->aregs[i] = sregs.acrs[i];
>> -            env->cregs[i] = sregs.crs[i];
>> -        }
>> -    }
>> +    /* The prefix */
>> +    env->psa = cs->kvm_run->s.regs.prefix;
>>       /* Floating point and vector registers */
>>       if (can_sync_regs(cs, KVM_SYNC_VRS)) {
>> @@ -690,11 +644,6 @@ int kvm_arch_get_registers(CPUState *cs)
>>           env->fpc = fpu.fpc;
>>       }
>> -    /* The prefix */
>> -    if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>> -        env->psa = cs->kvm_run->s.regs.prefix;
>> -    }
>> -
>>       if (can_sync_regs(cs, KVM_SYNC_ARCH0)) {
>>           env->cputm = cs->kvm_run->s.regs.cputm;
>>           env->ckc = cs->kvm_run->s.regs.ckc;
>