diff mbox

[PULL,3/4] s390x/kvm: rework KVM synchronize to tracing for some ONEREGS

Message ID 1398427363-11178-4-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck April 25, 2014, 12:02 p.m. UTC 
From: Christian Borntraeger <borntraeger@de.ibm.com>

Some ONE_REGS on s390 are not protected by a capability. Older kernels
might not provide those and return an error. Fortunately these registers
are only critical for the migration path. There is no need to error out
on reset and normal runtime. Furthermore, these kernels don't provide
a proper dirty bitmap anyway, so let's use tracing for those errors.

Also provide generic one reg helper to simplify the code.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 target-s390x/kvm.c |  143 ++++++++++++++++++++++++----------------------------
 trace-events       |    4 ++
 2 files changed, 69 insertions(+), 78 deletions(-)

Comments

Alexander Graf April 25, 2014, 12:04 p.m. UTC | #1 
On 25.04.14 14:02, Cornelia Huck wrote:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Some ONE_REGS on s390 are not protected by a capability. Older kernels
> might not provide those and return an error. Fortunately these registers
> are only critical for the migration path. There is no need to error out
> on reset and normal runtime. Furthermore, these kernels don't provide
> a proper dirty bitmap anyway, so let's use tracing for those errors.
>
> Also provide generic one reg helper to simplify the code.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

CC stable?


Alex
Christian Borntraeger April 25, 2014, 12:24 p.m. UTC | #2 
On 25/04/14 14:04, Alexander Graf wrote:
> 
> On 25.04.14 14:02, Cornelia Huck wrote:
>> From: Christian Borntraeger <borntraeger@de.ibm.com>
>>
>> Some ONE_REGS on s390 are not protected by a capability. Older kernels
>> might not provide those and return an error. Fortunately these registers
>> are only critical for the migration path. There is no need to error out
>> on reset and normal runtime. Furthermore, these kernels don't provide
>> a proper dirty bitmap anyway, so let's use tracing for those errors.
>>
>> Also provide generic one reg helper to simplify the code.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> CC stable?

Makes sense. get_registers is fine as the ONE_REGS are at the end of the function,
but put_registers will not store the SREGS and the prefix in that case.

I guess its ok to send the patch and the commit id to qemu stable after
this hits master?

Christian
Alexander Graf April 25, 2014, 12:26 p.m. UTC | #3 
On 25.04.14 14:24, Christian Borntraeger wrote:
> On 25/04/14 14:04, Alexander Graf wrote:
>> On 25.04.14 14:02, Cornelia Huck wrote:
>>> From: Christian Borntraeger <borntraeger@de.ibm.com>
>>>
>>> Some ONE_REGS on s390 are not protected by a capability. Older kernels
>>> might not provide those and return an error. Fortunately these registers
>>> are only critical for the migration path. There is no need to error out
>>> on reset and normal runtime. Furthermore, these kernels don't provide
>>> a proper dirty bitmap anyway, so let's use tracing for those errors.
>>>
>>> Also provide generic one reg helper to simplify the code.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> CC stable?
> Makes sense. get_registers is fine as the ONE_REGS are at the end of the function,
> but put_registers will not store the SREGS and the prefix in that case.
>
> I guess its ok to send the patch and the commit id to qemu stable after
> this hits master?

The "normal process" is that the maintainer adds a CC: line to the 
commit when he puts it into his queue. That way git-send-email 
automatically CCs stable and there's proper documentation of this in the 
master tree.

But of course doing it manually also works.


Alex
diff mbox

Patch

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 2b2dcdc..daaabbd 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -36,6 +36,7 @@ 
 #include "sysemu/device_tree.h"
 #include "qapi/qmp/qjson.h"
 #include "monitor/monitor.h"
+#include "trace.h"
 
 /* #define DEBUG_KVM */
 
@@ -128,14 +129,42 @@  void kvm_arch_reset_vcpu(CPUState *cpu)
     }
 }
 
+static int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
+{
+    struct kvm_one_reg reg;
+    int r;
+
+    reg.id = id;
+    reg.addr = (uint64_t) source;
+    r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
+    if (r) {
+        trace_kvm_failed_reg_set(id, strerror(errno));
+    }
+    return r;
+}
+
+static int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
+{
+    struct kvm_one_reg reg;
+    int r;
+
+    reg.id = id;
+    reg.addr = (uint64_t) target;
+    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+    if (r) {
+        trace_kvm_failed_reg_get(id, strerror(errno));
+    }
+    return r;
+}
+
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
-    struct kvm_one_reg reg;
     struct kvm_sregs sregs;
     struct kvm_regs regs;
-    int ret;
+    int r;
     int i;
 
     /* always save the PSW  and the GPRS*/
@@ -151,9 +180,9 @@  int kvm_arch_put_registers(CPUState *cs, int level)
         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;
+        r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
+        if (r < 0) {
+            return r;
         }
     }
 
@@ -162,47 +191,27 @@  int kvm_arch_put_registers(CPUState *cs, int level)
         return 0;
     }
 
-    reg.id = KVM_REG_S390_CPU_TIMER;
-    reg.addr = (__u64)&(env->cputm);
-    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-    if (ret < 0) {
-        return ret;
-    }
-
-    reg.id = KVM_REG_S390_CLOCK_COMP;
-    reg.addr = (__u64)&(env->ckc);
-    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-    if (ret < 0) {
-        return ret;
-    }
-
-    reg.id = KVM_REG_S390_TODPR;
-    reg.addr = (__u64)&(env->todpr);
-    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-    if (ret < 0) {
-        return ret;
-    }
+    /*
+     * These ONE_REGS are not protected by a capability. As they are only
+     * necessary for migration we just trace a possible error, but don't
+     * return with an error return code.
+     */
+    kvm_set_one_reg(cs, KVM_REG_S390_CPU_TIMER, &env->cputm);
+    kvm_set_one_reg(cs, KVM_REG_S390_CLOCK_COMP, &env->ckc);
+    kvm_set_one_reg(cs, KVM_REG_S390_TODPR, &env->todpr);
 
     if (cap_async_pf) {
-        reg.id = KVM_REG_S390_PFTOKEN;
-        reg.addr = (__u64)&(env->pfault_token);
-        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-        if (ret < 0) {
-            return ret;
+        r = kvm_set_one_reg(cs, KVM_REG_S390_PFTOKEN, &env->pfault_token);
+        if (r < 0) {
+            return r;
         }
-
-        reg.id = KVM_REG_S390_PFCOMPARE;
-        reg.addr = (__u64)&(env->pfault_compare);
-        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-        if (ret < 0) {
-            return ret;
+        r = kvm_set_one_reg(cs, KVM_REG_S390_PFCOMPARE, &env->pfault_compare);
+        if (r < 0) {
+            return r;
         }
-
-        reg.id = KVM_REG_S390_PFSELECT;
-        reg.addr = (__u64)&(env->pfault_select);
-        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
-        if (ret < 0) {
-            return ret;
+        r = kvm_set_one_reg(cs, KVM_REG_S390_PFSELECT, &env->pfault_select);
+        if (r < 0) {
+            return r;
         }
     }
 
@@ -220,9 +229,9 @@  int kvm_arch_put_registers(CPUState *cs, int level)
             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;
+        r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
+        if (r < 0) {
+            return r;
         }
     }
 
@@ -240,7 +249,6 @@  int kvm_arch_get_registers(CPUState *cs)
 {
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
-    struct kvm_one_reg reg;
     struct kvm_sregs sregs;
     struct kvm_regs regs;
     int i, r;
@@ -288,46 +296,25 @@  int kvm_arch_get_registers(CPUState *cs)
         env->psa = cs->kvm_run->s.regs.prefix;
     }
 
-    /* One Regs */
-    reg.id = KVM_REG_S390_CPU_TIMER;
-    reg.addr = (__u64)&(env->cputm);
-    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
-    if (r < 0) {
-        return r;
-    }
-
-    reg.id = KVM_REG_S390_CLOCK_COMP;
-    reg.addr = (__u64)&(env->ckc);
-    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
-    if (r < 0) {
-        return r;
-    }
-
-    reg.id = KVM_REG_S390_TODPR;
-    reg.addr = (__u64)&(env->todpr);
-    r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
-    if (r < 0) {
-        return r;
-    }
+    /*
+     * These ONE_REGS are not protected by a capability. As they are only
+     * necessary for migration we just trace a possible error, but don't
+     * return with an error return code.
+     */
+    kvm_get_one_reg(cs, KVM_REG_S390_CPU_TIMER, &env->cputm);
+    kvm_get_one_reg(cs, KVM_REG_S390_CLOCK_COMP, &env->ckc);
+    kvm_get_one_reg(cs, KVM_REG_S390_TODPR, &env->todpr);
 
     if (cap_async_pf) {
-        reg.id = KVM_REG_S390_PFTOKEN;
-        reg.addr = (__u64)&(env->pfault_token);
-        r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+        r = kvm_get_one_reg(cs, KVM_REG_S390_PFTOKEN, &env->pfault_token);
         if (r < 0) {
             return r;
         }
-
-        reg.id = KVM_REG_S390_PFCOMPARE;
-        reg.addr = (__u64)&(env->pfault_compare);
-        r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+        r = kvm_get_one_reg(cs, KVM_REG_S390_PFCOMPARE, &env->pfault_compare);
         if (r < 0) {
             return r;
         }
-
-        reg.id = KVM_REG_S390_PFSELECT;
-        reg.addr = (__u64)&(env->pfault_select);
-        r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+        r = kvm_get_one_reg(cs, KVM_REG_S390_PFSELECT, &env->pfault_select);
         if (r < 0) {
             return r;
         }
diff --git a/trace-events b/trace-events
index 6ecaab2..a5218ba 100644
--- a/trace-events
+++ b/trace-events
@@ -1243,3 +1243,7 @@  xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (ad
 # hw/pci/pci_host.c
 pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x -> 0x%x"
 pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x <- 0x%x"
+
+# target-s390/kvm.c
+kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve ONEREG %" PRIu64 " from KVM: %s"
+kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set ONEREG %" PRIu64 " to KVM: %s"