All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Avi Kivity <avi@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>
Cc: kvm <kvm@vger.kernel.org>, qemu-devel <qemu-devel@nongnu.org>
Subject: [PATCH v2][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
Date: Wed, 15 Jun 2011 15:17:26 +0200	[thread overview]
Message-ID: <4DF8B0E6.1080806@siemens.com> (raw)
In-Reply-To: <4DF33413.9070605@web.de>

These FPU states are properly maintained by KVM but not yet by TCG. So
far we unconditionally set them to 0 in the guest which may cause
state corruptions, though not with modern guests.

To avoid breaking backward migration, use a conditional subsection that
is only written if any of the three fields is non-zero. The guest's
FNINIT clears them frequently, and cleared IA32_MISC_ENABLE MSR[2]
reduces the probability of non-zero values further so that this
subsection is not expected to restrict migration in any common scenario.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - use conditional subsection

 target-i386/cpu.h     |    4 ++++
 target-i386/kvm.c     |   20 +++++++++++++++-----
 target-i386/machine.c |   23 +++++++++++++++++++++++
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9c3340d..cdf68ff 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -641,6 +641,10 @@ typedef struct CPUX86State {
     uint16_t fpuc;
     uint8_t fptags[8];   /* 0 = valid, 1 = empty */
     FPReg fpregs[8];
+    /* KVM-only so far */
+    uint16_t fpop;
+    uint64_t fpip;
+    uint64_t fpdp;
 
     /* emulator internal variables */
     float_status fp_status;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5ebb054..938e0a3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -718,6 +718,9 @@ static int kvm_put_fpu(CPUState *env)
     fpu.fsw = env->fpus & ~(7 << 11);
     fpu.fsw |= (env->fpstt & 7) << 11;
     fpu.fcw = env->fpuc;
+    fpu.last_opcode = env->fpop;
+    fpu.last_ip = env->fpip;
+    fpu.last_dp = env->fpdp;
     for (i = 0; i < 8; ++i) {
         fpu.ftwx |= (!env->fptags[i]) << i;
     }
@@ -740,7 +743,7 @@ static int kvm_put_xsave(CPUState *env)
 {
     int i, r;
     struct kvm_xsave* xsave;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_put_fpu(env);
@@ -748,7 +751,7 @@ static int kvm_put_xsave(CPUState *env)
 
     xsave = qemu_memalign(4096, sizeof(struct kvm_xsave));
     memset(xsave, 0, sizeof(struct kvm_xsave));
-    cwd = swd = twd = fop = 0;
+    cwd = swd = twd = 0;
     swd = env->fpus & ~(7 << 11);
     swd |= (env->fpstt & 7) << 11;
     cwd = env->fpuc;
@@ -756,7 +759,9 @@ static int kvm_put_xsave(CPUState *env)
         twd |= (!env->fptags[i]) << i;
     }
     xsave->region[0] = (uint32_t)(swd << 16) + cwd;
-    xsave->region[1] = (uint32_t)(fop << 16) + twd;
+    xsave->region[1] = (uint32_t)(env->fpop << 16) + twd;
+    memcpy(&xsave->region[XSAVE_CWD_RIP], &env->fpip, sizeof(env->fpip));
+    memcpy(&xsave->region[XSAVE_CWD_RDP], &env->fpdp, sizeof(env->fpdp));
     memcpy(&xsave->region[XSAVE_ST_SPACE], env->fpregs,
             sizeof env->fpregs);
     memcpy(&xsave->region[XSAVE_XMM_SPACE], env->xmm_regs,
@@ -921,6 +926,9 @@ static int kvm_get_fpu(CPUState *env)
     env->fpstt = (fpu.fsw >> 11) & 7;
     env->fpus = fpu.fsw;
     env->fpuc = fpu.fcw;
+    env->fpop = fpu.last_opcode;
+    env->fpip = fpu.last_ip;
+    env->fpdp = fpu.last_dp;
     for (i = 0; i < 8; ++i) {
         env->fptags[i] = !((fpu.ftwx >> i) & 1);
     }
@@ -935,7 +943,7 @@ static int kvm_get_xsave(CPUState *env)
 {
     struct kvm_xsave* xsave;
     int ret, i;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_get_fpu(env);
@@ -951,13 +959,15 @@ static int kvm_get_xsave(CPUState *env)
     cwd = (uint16_t)xsave->region[0];
     swd = (uint16_t)(xsave->region[0] >> 16);
     twd = (uint16_t)xsave->region[1];
-    fop = (uint16_t)(xsave->region[1] >> 16);
+    env->fpop = (uint16_t)(xsave->region[1] >> 16);
     env->fpstt = (swd >> 11) & 7;
     env->fpus = swd;
     env->fpuc = cwd;
     for (i = 0; i < 8; ++i) {
         env->fptags[i] = !((twd >> i) & 1);
     }
+    memcpy(&env->fpip, &xsave->region[XSAVE_CWD_RIP], sizeof(env->fpip));
+    memcpy(&env->fpdp, &xsave->region[XSAVE_CWD_RDP], sizeof(env->fpdp));
     env->mxcsr = xsave->region[XSAVE_MXCSR];
     memcpy(env->fpregs, &xsave->region[XSAVE_ST_SPACE],
             sizeof env->fpregs);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index bbeae88..d22a731 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -290,6 +290,26 @@ static const VMStateDescription vmstate_async_pf_msr = {
     }
 };
 
+static bool fpop_ip_dp_needed(void *opaque)
+{
+    CPUState *env = opaque;
+
+    return env->fpop != 0 || env->fpip != 0 || env->fpdp != 0;
+}
+
+static const VMStateDescription vmstate_fpop_ip_dp = {
+    .name = "cpu/fpop_ip_dp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT16_V(fpop, CPUState, 13),
+        VMSTATE_UINT64_V(fpip, CPUState, 13),
+        VMSTATE_UINT64_V(fpdp, CPUState, 13),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_cpu = {
     .name = "cpu",
     .version_id = CPU_SAVE_VERSION,
@@ -398,6 +418,9 @@ static const VMStateDescription vmstate_cpu = {
             .vmsd = &vmstate_async_pf_msr,
             .needed = async_pf_msr_needed,
         } , {
+            .vmsd = &vmstate_fpop_ip_dp,
+            .needed = fpop_ip_dp_needed,
+        } , {
             /* empty */
         }
     }


WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Avi Kivity <avi@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>, kvm <kvm@vger.kernel.org>
Subject: [Qemu-devel] [PATCH v2][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
Date: Wed, 15 Jun 2011 15:17:26 +0200	[thread overview]
Message-ID: <4DF8B0E6.1080806@siemens.com> (raw)
In-Reply-To: <4DF33413.9070605@web.de>

These FPU states are properly maintained by KVM but not yet by TCG. So
far we unconditionally set them to 0 in the guest which may cause
state corruptions, though not with modern guests.

To avoid breaking backward migration, use a conditional subsection that
is only written if any of the three fields is non-zero. The guest's
FNINIT clears them frequently, and cleared IA32_MISC_ENABLE MSR[2]
reduces the probability of non-zero values further so that this
subsection is not expected to restrict migration in any common scenario.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - use conditional subsection

 target-i386/cpu.h     |    4 ++++
 target-i386/kvm.c     |   20 +++++++++++++++-----
 target-i386/machine.c |   23 +++++++++++++++++++++++
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9c3340d..cdf68ff 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -641,6 +641,10 @@ typedef struct CPUX86State {
     uint16_t fpuc;
     uint8_t fptags[8];   /* 0 = valid, 1 = empty */
     FPReg fpregs[8];
+    /* KVM-only so far */
+    uint16_t fpop;
+    uint64_t fpip;
+    uint64_t fpdp;
 
     /* emulator internal variables */
     float_status fp_status;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5ebb054..938e0a3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -718,6 +718,9 @@ static int kvm_put_fpu(CPUState *env)
     fpu.fsw = env->fpus & ~(7 << 11);
     fpu.fsw |= (env->fpstt & 7) << 11;
     fpu.fcw = env->fpuc;
+    fpu.last_opcode = env->fpop;
+    fpu.last_ip = env->fpip;
+    fpu.last_dp = env->fpdp;
     for (i = 0; i < 8; ++i) {
         fpu.ftwx |= (!env->fptags[i]) << i;
     }
@@ -740,7 +743,7 @@ static int kvm_put_xsave(CPUState *env)
 {
     int i, r;
     struct kvm_xsave* xsave;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_put_fpu(env);
@@ -748,7 +751,7 @@ static int kvm_put_xsave(CPUState *env)
 
     xsave = qemu_memalign(4096, sizeof(struct kvm_xsave));
     memset(xsave, 0, sizeof(struct kvm_xsave));
-    cwd = swd = twd = fop = 0;
+    cwd = swd = twd = 0;
     swd = env->fpus & ~(7 << 11);
     swd |= (env->fpstt & 7) << 11;
     cwd = env->fpuc;
@@ -756,7 +759,9 @@ static int kvm_put_xsave(CPUState *env)
         twd |= (!env->fptags[i]) << i;
     }
     xsave->region[0] = (uint32_t)(swd << 16) + cwd;
-    xsave->region[1] = (uint32_t)(fop << 16) + twd;
+    xsave->region[1] = (uint32_t)(env->fpop << 16) + twd;
+    memcpy(&xsave->region[XSAVE_CWD_RIP], &env->fpip, sizeof(env->fpip));
+    memcpy(&xsave->region[XSAVE_CWD_RDP], &env->fpdp, sizeof(env->fpdp));
     memcpy(&xsave->region[XSAVE_ST_SPACE], env->fpregs,
             sizeof env->fpregs);
     memcpy(&xsave->region[XSAVE_XMM_SPACE], env->xmm_regs,
@@ -921,6 +926,9 @@ static int kvm_get_fpu(CPUState *env)
     env->fpstt = (fpu.fsw >> 11) & 7;
     env->fpus = fpu.fsw;
     env->fpuc = fpu.fcw;
+    env->fpop = fpu.last_opcode;
+    env->fpip = fpu.last_ip;
+    env->fpdp = fpu.last_dp;
     for (i = 0; i < 8; ++i) {
         env->fptags[i] = !((fpu.ftwx >> i) & 1);
     }
@@ -935,7 +943,7 @@ static int kvm_get_xsave(CPUState *env)
 {
     struct kvm_xsave* xsave;
     int ret, i;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_get_fpu(env);
@@ -951,13 +959,15 @@ static int kvm_get_xsave(CPUState *env)
     cwd = (uint16_t)xsave->region[0];
     swd = (uint16_t)(xsave->region[0] >> 16);
     twd = (uint16_t)xsave->region[1];
-    fop = (uint16_t)(xsave->region[1] >> 16);
+    env->fpop = (uint16_t)(xsave->region[1] >> 16);
     env->fpstt = (swd >> 11) & 7;
     env->fpus = swd;
     env->fpuc = cwd;
     for (i = 0; i < 8; ++i) {
         env->fptags[i] = !((twd >> i) & 1);
     }
+    memcpy(&env->fpip, &xsave->region[XSAVE_CWD_RIP], sizeof(env->fpip));
+    memcpy(&env->fpdp, &xsave->region[XSAVE_CWD_RDP], sizeof(env->fpdp));
     env->mxcsr = xsave->region[XSAVE_MXCSR];
     memcpy(env->fpregs, &xsave->region[XSAVE_ST_SPACE],
             sizeof env->fpregs);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index bbeae88..d22a731 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -290,6 +290,26 @@ static const VMStateDescription vmstate_async_pf_msr = {
     }
 };
 
+static bool fpop_ip_dp_needed(void *opaque)
+{
+    CPUState *env = opaque;
+
+    return env->fpop != 0 || env->fpip != 0 || env->fpdp != 0;
+}
+
+static const VMStateDescription vmstate_fpop_ip_dp = {
+    .name = "cpu/fpop_ip_dp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT16_V(fpop, CPUState, 13),
+        VMSTATE_UINT64_V(fpip, CPUState, 13),
+        VMSTATE_UINT64_V(fpdp, CPUState, 13),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_cpu = {
     .name = "cpu",
     .version_id = CPU_SAVE_VERSION,
@@ -398,6 +418,9 @@ static const VMStateDescription vmstate_cpu = {
             .vmsd = &vmstate_async_pf_msr,
             .needed = async_pf_msr_needed,
         } , {
+            .vmsd = &vmstate_fpop_ip_dp,
+            .needed = fpop_ip_dp_needed,
+        } , {
             /* empty */
         }
     }

  parent reply	other threads:[~2011-06-15 13:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-11  9:23 [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP Jan Kiszka
2011-06-11  9:23 ` [Qemu-devel] " Jan Kiszka
2011-06-13  8:45 ` Avi Kivity
2011-06-13  8:45   ` [Qemu-devel] " Avi Kivity
2011-06-14  6:10   ` Jan Kiszka
2011-06-14  6:10     ` [Qemu-devel] " Jan Kiszka
2011-06-14  8:23     ` Avi Kivity
2011-06-14  8:23       ` [Qemu-devel] " Avi Kivity
2011-06-14  8:28       ` Jan Kiszka
2011-06-14  8:28         ` [Qemu-devel] " Jan Kiszka
2011-06-15  9:10     ` Avi Kivity
2011-06-15  9:10       ` [Qemu-devel] " Avi Kivity
2011-06-15 10:20       ` Jan Kiszka
2011-06-15 10:20         ` [Qemu-devel] " Jan Kiszka
2011-06-15 10:39         ` Jan Kiszka
2011-06-15 10:39           ` [Qemu-devel] " Jan Kiszka
2011-06-15 11:26         ` Avi Kivity
2011-06-15 11:26           ` [Qemu-devel] " Avi Kivity
2011-06-15 11:32           ` Jan Kiszka
2011-06-15 11:32             ` [Qemu-devel] " Jan Kiszka
2011-06-15 11:33             ` Avi Kivity
2011-06-15 11:33               ` [Qemu-devel] " Avi Kivity
2011-06-15 11:45               ` Jan Kiszka
2011-06-15 11:45                 ` [Qemu-devel] " Jan Kiszka
2011-06-15 13:17 ` Jan Kiszka [this message]
2011-06-15 13:17   ` [Qemu-devel] [PATCH v2][uq/master] " Jan Kiszka
2011-06-16  9:42   ` Christophe Fergeau
2011-06-16  9:42     ` Christophe Fergeau
2011-06-19 12:58   ` Avi Kivity
2011-06-19 12:58     ` [Qemu-devel] " Avi Kivity
2011-06-16  9:35 ` [PATCH][uq/master] " Christophe Fergeau
2011-06-16  9:35   ` [Qemu-devel] " Christophe Fergeau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DF8B0E6.1080806@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.