Patchwork [3/7,v2] KVM regsync: S390: Implement kvm_arch_[get/put]_registers regmap paramter

login
register
mail settings
Submitter Jason J. Herne
Date Jan. 10, 2013, 3:28 p.m.
Message ID <1357831732-3917-1-git-send-email-jjherne@us.ibm.com>
Download mbox | patch
Permalink /patch/211076/
State New
Headers show

Comments

Jason J. Herne - Jan. 10, 2013, 3:28 p.m.
From: "Jason J. Herne" <jjherne@us.ibm.com>

S390 re-implementation of kvm_arch_get_registers and kvm_arch_put_registers
functions to take advantage of the register map parameter.

Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c |  165 ++++++++++++++++++++++++++--------------------------
 1 file changed, 84 insertions(+), 81 deletions(-)
Alexander Graf - Jan. 24, 2013, 12:11 p.m.
On 10.01.2013, at 16:28, Jason J. Herne wrote:

> From: "Jason J. Herne" <jjherne@us.ibm.com>
> 
> S390 re-implementation of kvm_arch_get_registers and kvm_arch_put_registers
> functions to take advantage of the register map parameter.
> 
> Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> target-s390x/kvm.c |  165 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 84 insertions(+), 81 deletions(-)
> 
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 4b87f1c..ab6b27b 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -88,7 +88,7 @@ void kvm_arch_reset_vcpu(CPUState *cpu)
>     /* FIXME: add code to reset vcpu. */
> }
> 
> -int kvm_arch_put_registers(CPUState *cs, int level)
> +int kvm_arch_put_registers(CPUState *cs, int regmap)
> {
>     S390CPU *cpu = S390_CPU(cs);
>     CPUS390XState *env = &cpu->env;
> @@ -97,57 +97,56 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>     int ret;
>     int i;
> 
> -    /* always save the PSW  and the GPRS*/
> -    cs->kvm_run->psw_addr = env->psw.addr;
> -    cs->kvm_run->psw_mask = env->psw.mask;
> +    if (regmap & KVM_REGSYNC_S390_RUNTIME_REGS) {
> +        cs->kvm_run->psw_addr = env->psw.addr;
> +        cs->kvm_run->psw_mask = env->psw.mask;
> 
> -    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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];
> -        }
> -        ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
> -        if (ret < 0) {
> -            return ret;
> +        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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];
> +            }
> +            ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
> +            if (ret < 0) {
> +                return ret;
> +            }
>         }
>     }
> 
> -    /* Do we need to save more than that? */
> -    if (level == KVM_REGSYNC_RUNTIME_STATE) {
> -        return 0;
> -    }
> -
> -    if (cap_sync_regs &&
> -        cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> -        cs->kvm_run->kvm_valid_regs & 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];
> +    if (regmap & KVM_REGSYNC_S390_RESET_REGS) {
> +        if (cap_sync_regs &&
> +            cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> +            cs->kvm_run->kvm_valid_regs & 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];
> +            }
> +            ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
> +            if (ret < 0) {
> +                return ret;
> +            }
>         }
> -        ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
> -        if (ret < 0) {
> -            return ret;
> +
> +        /* Finally the prefix */
> +        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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 */
>         }
>     }
> 
> -    /* Finally the prefix */
> -    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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;
> }
> 
> @@ -160,49 +159,53 @@ int kvm_arch_get_registers(CPUState *cs, int regmap)
>     int ret;
>     int i;
> 
> -    /* get the PSW */
> -    env->psw.addr = cs->kvm_run->psw_addr;
> -    env->psw.mask = cs->kvm_run->psw_mask;
> +    if (regmap & KVM_REGSYNC_S390_RUNTIME_REGS) {
> +        /* get the PSW */
> +        env->psw.addr = cs->kvm_run->psw_addr;
> +        env->psw.mask = cs->kvm_run->psw_mask;
> 
> -    /* the GPRS */
> -    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
> -        for (i = 0; i < 16; i++) {
> -            env->regs[i] = cs->kvm_run->s.regs.gprs[i];
> -        }
> -    } else {
> -        ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
> -        if (ret < 0) {
> -            return ret;
> -        }
> -         for (i = 0; i < 16; i++) {
> -            env->regs[i] = regs.gprs[i];
> +        /* the GPRS */
> +        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
> +            for (i = 0; i < 16; i++) {
> +                env->regs[i] = cs->kvm_run->s.regs.gprs[i];
> +            }
> +        } else {
> +            ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +             for (i = 0; i < 16; i++) {
> +                env->regs[i] = regs.gprs[i];
> +            }
>         }
>     }
> 
> -    /* The ACRS and CRS */
> -    if (cap_sync_regs &&
> -        cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> -        cs->kvm_run->kvm_valid_regs & 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 {
> -        ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
> -        if (ret < 0) {
> -            return ret;
> -        }
> -         for (i = 0; i < 16; i++) {
> -            env->aregs[i] = sregs.acrs[i];
> -            env->cregs[i] = sregs.crs[i];
> +    if (regmap & KVM_REGSYNC_S390_RESET_REGS) {

Here you're changing the way arch_get_registers() behaves in a patch that should be neutral for external callers. Please extract the bits where you individually get registers into a separate patch with separate explanation why you're restricting it.

The basic idea behind RESET_REGS is that you don't have to sync them. They are usually write-only. So get(RESET) should really never happen.


Alex

> +        /* The ACRS and CRS */
> +        if (cap_sync_regs &&
> +            cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> +            cs->kvm_run->kvm_valid_regs & 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 {
> +            ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +             for (i = 0; i < 16; i++) {
> +                env->aregs[i] = sregs.acrs[i];
> +                env->cregs[i] = sregs.crs[i];
> +            }
>         }
> -    }
> 
> -    /* Finally the prefix */
> -    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
> -        env->psa = cs->kvm_run->s.regs.prefix;
> -    } else {
> -        /* no prefix without sync regs */
> +        /* Finally the prefix */
> +        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
> +            env->psa = cs->kvm_run->s.regs.prefix;
> +        } else {
> +            /* no prefix without sync regs */
> +        }
>     }
> 
>     return 0;
> -- 
> 1.7.9.5
>

Patch

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 4b87f1c..ab6b27b 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -88,7 +88,7 @@  void kvm_arch_reset_vcpu(CPUState *cpu)
     /* FIXME: add code to reset vcpu. */
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level)
+int kvm_arch_put_registers(CPUState *cs, int regmap)
 {
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
@@ -97,57 +97,56 @@  int kvm_arch_put_registers(CPUState *cs, int level)
     int ret;
     int i;
 
-    /* always save the PSW  and the GPRS*/
-    cs->kvm_run->psw_addr = env->psw.addr;
-    cs->kvm_run->psw_mask = env->psw.mask;
+    if (regmap & KVM_REGSYNC_S390_RUNTIME_REGS) {
+        cs->kvm_run->psw_addr = env->psw.addr;
+        cs->kvm_run->psw_mask = env->psw.mask;
 
-    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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];
-        }
-        ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
-        if (ret < 0) {
-            return ret;
+        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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];
+            }
+            ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
+            if (ret < 0) {
+                return ret;
+            }
         }
     }
 
-    /* Do we need to save more than that? */
-    if (level == KVM_REGSYNC_RUNTIME_STATE) {
-        return 0;
-    }
-
-    if (cap_sync_regs &&
-        cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
-        cs->kvm_run->kvm_valid_regs & 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];
+    if (regmap & KVM_REGSYNC_S390_RESET_REGS) {
+        if (cap_sync_regs &&
+            cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
+            cs->kvm_run->kvm_valid_regs & 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];
+            }
+            ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
+            if (ret < 0) {
+                return ret;
+            }
         }
-        ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
-        if (ret < 0) {
-            return ret;
+
+        /* Finally the prefix */
+        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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 */
         }
     }
 
-    /* Finally the prefix */
-    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & 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;
 }
 
@@ -160,49 +159,53 @@  int kvm_arch_get_registers(CPUState *cs, int regmap)
     int ret;
     int i;
 
-    /* get the PSW */
-    env->psw.addr = cs->kvm_run->psw_addr;
-    env->psw.mask = cs->kvm_run->psw_mask;
+    if (regmap & KVM_REGSYNC_S390_RUNTIME_REGS) {
+        /* get the PSW */
+        env->psw.addr = cs->kvm_run->psw_addr;
+        env->psw.mask = cs->kvm_run->psw_mask;
 
-    /* the GPRS */
-    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
-        for (i = 0; i < 16; i++) {
-            env->regs[i] = cs->kvm_run->s.regs.gprs[i];
-        }
-    } else {
-        ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
-        if (ret < 0) {
-            return ret;
-        }
-         for (i = 0; i < 16; i++) {
-            env->regs[i] = regs.gprs[i];
+        /* the GPRS */
+        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
+            for (i = 0; i < 16; i++) {
+                env->regs[i] = cs->kvm_run->s.regs.gprs[i];
+            }
+        } else {
+            ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, &regs);
+            if (ret < 0) {
+                return ret;
+            }
+             for (i = 0; i < 16; i++) {
+                env->regs[i] = regs.gprs[i];
+            }
         }
     }
 
-    /* The ACRS and CRS */
-    if (cap_sync_regs &&
-        cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
-        cs->kvm_run->kvm_valid_regs & 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 {
-        ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
-        if (ret < 0) {
-            return ret;
-        }
-         for (i = 0; i < 16; i++) {
-            env->aregs[i] = sregs.acrs[i];
-            env->cregs[i] = sregs.crs[i];
+    if (regmap & KVM_REGSYNC_S390_RESET_REGS) {
+        /* The ACRS and CRS */
+        if (cap_sync_regs &&
+            cs->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
+            cs->kvm_run->kvm_valid_regs & 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 {
+            ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
+            if (ret < 0) {
+                return ret;
+            }
+             for (i = 0; i < 16; i++) {
+                env->aregs[i] = sregs.acrs[i];
+                env->cregs[i] = sregs.crs[i];
+            }
         }
-    }
 
-    /* Finally the prefix */
-    if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
-        env->psa = cs->kvm_run->s.regs.prefix;
-    } else {
-        /* no prefix without sync regs */
+        /* Finally the prefix */
+        if (cap_sync_regs && cs->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
+            env->psa = cs->kvm_run->s.regs.prefix;
+        } else {
+            /* no prefix without sync regs */
+        }
     }
 
     return 0;