Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] target-ppc: Synchronize FPU state with KVM

[David Gibson]

On Wed, Jan 23, 2013 at 11:56:04AM -0600, Scott Wood wrote:
> On 01/22/2013 11:05:00 PM, David Gibson wrote:
> >Currently qemu does not get and put the state of the floating
> >point and
> >vector registers to KVM.  This is obviously a problem for savevm,
> >as well
> >as possibly being problematic for debugging of FP-using guests.
> >
> >This patch fixes this by using new extensions to the ONE_REG
> >interface to
> >synchronize the qemu floating point state with KVM.
> >
> >Signed-off-by: David Gibson <address@hidden>
> >---
> > target-ppc/kvm.c |  123
> >++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 123 insertions(+)
> >
> >diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> >index e84b993..7ed76be 100644
> >--- a/target-ppc/kvm.c
> >+++ b/target-ppc/kvm.c
> >@@ -518,6 +518,125 @@ static void kvm_put_one_spr(CPUState *cs,
> >uint64_t id, int spr)
> >     }
> > }
> >
> >+static void kvm_put_fp(CPUState *cs)
> >+{
> >+    PowerPCCPU *cpu = POWERPC_CPU(cs);
> >+    CPUPPCState *env = &cpu->env;
> >+    struct kvm_one_reg reg;
> >+    int i;
> >+    int ret;
> >+
> >+    if (env->insns_flags & PPC_FLOAT) {
> >+        uint64_t fpscr = env->fpscr;
> >+        bool vsx = !!(env->insns_flags2 & PPC2_VSX);
> >+
> >+        reg.id = KVM_REG_PPC_FPSCR;
> >+        reg.addr = (uintptr_t)&fpscr;
> >+        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> >+        if (ret < 0) {
> >+            fprintf(stderr, "Warning: Unable to set FPSCR to KVM:
> >%s\n",
> >+                    strerror(errno));
> >+        }
> >+        for (i = 0; i < 32; i++) {
> >+            uint64_t vsr[2];
> >+
> >+            vsr[0] = float64_val(env->fpr[i]);
> >+            vsr[1] = env->vsr[i];
> >+            reg.addr = (uintptr_t) &vsr;
> >+            reg.id = vsx ? KVM_REG_PPC_VSR(i) : KVM_REG_PPC_FPR(i);
> >+
> >+            ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> >+            if (ret < 0) {
> >+                fprintf(stderr, "Warning Unable to set %s%d to
> >KVM: %s\n",
> >+                        vsx ? "VSR" : "FPR", i, strerror(errno));
> >+            }
> >+        }
> >+    }
> 
> I don't see where you check a capability, so users of pre-one-reg
> KVM now get (at least) 33 warning messages every time context gets
> synchronized?

Given that a bunch of important state is missing from that
synchronization, I think a slew of warnings is entirely appropriate.
There are no per-register capabilities so there's nothing we can check
here.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: Digital signature