All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
	marc.zyngier@arm.com, "Alex Bennée" <alex.bennee@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>
Subject: [PATCH v2 5/6] target-arm: kvm64 fix save/restore of SPSR regs
Date: Wed,  4 Mar 2015 14:35:52 +0000	[thread overview]
Message-ID: <1425479753-18349-6-git-send-email-alex.bennee@linaro.org> (raw)
In-Reply-To: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org>

From: Christoffer Dall <christoffer.dall@linaro.org>

The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.

Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2 (ajb)
  - minor tweaks and clarifications
v3
  - Use the correct bank index function for setting/getting env->spsr
  - only deal with spsrs in elevated exception levels

diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..45e5c3f 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     uint64_t val;
     int i;
     int ret;
+    unsigned int el;
 
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -206,9 +207,27 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    /* Saved Program State Registers
+     *
+     * Before we restore from the banked_spsr[] array we need to
+     * ensure that any modifications to env->spsr are correctly
+     * reflected and map aarch64 exception levels if required.
+     */
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
+            env->banked_spsr[1] = env->banked_spsr[0];
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+            env->banked_spsr[i] = env->spsr;
+        }
+    }
+
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
         if (ret) {
             return ret;
@@ -253,6 +272,7 @@ int kvm_arch_get_registers(CPUState *cs)
     struct kvm_one_reg reg;
     uint64_t val;
     uint32_t fpr;
+    unsigned int el;
     int i;
     int ret;
 
@@ -325,15 +345,35 @@ int kvm_arch_get_registers(CPUState *cs)
         return ret;
     }
 
+    /* Fetch the SPSR registers
+     *
+     * KVM has an array of state indexed for all the possible aarch32
+     * privilage levels. Although not all are valid at all points
+     * there are some transitions possible which can access old state
+     * so it is worth keeping them all.
+     */
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
         if (ret) {
             return ret;
         }
     }
 
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+            env->banked_spsr[0] = env->banked_spsr[1];
+            i = aarch64_banked_spsr_index(el);
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+        }
+        env->spsr = env->banked_spsr[i];
+    }
+
     /* Advanced SIMD and FP registers */
     for (i = 0; i < 32; i++) {
         reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-- 
2.3.1


WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	kvm@vger.kernel.org, marc.zyngier@arm.com,
	linux-arm-kernel@lists.infradead.org,
	"Alex Bennée" <alex.bennee@linaro.org>,
	kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org
Subject: [Qemu-devel] [PATCH v2 5/6] target-arm: kvm64 fix save/restore of SPSR regs
Date: Wed,  4 Mar 2015 14:35:52 +0000	[thread overview]
Message-ID: <1425479753-18349-6-git-send-email-alex.bennee@linaro.org> (raw)
In-Reply-To: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org>

From: Christoffer Dall <christoffer.dall@linaro.org>

The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.

Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2 (ajb)
  - minor tweaks and clarifications
v3
  - Use the correct bank index function for setting/getting env->spsr
  - only deal with spsrs in elevated exception levels

diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..45e5c3f 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     uint64_t val;
     int i;
     int ret;
+    unsigned int el;
 
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -206,9 +207,27 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    /* Saved Program State Registers
+     *
+     * Before we restore from the banked_spsr[] array we need to
+     * ensure that any modifications to env->spsr are correctly
+     * reflected and map aarch64 exception levels if required.
+     */
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
+            env->banked_spsr[1] = env->banked_spsr[0];
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+            env->banked_spsr[i] = env->spsr;
+        }
+    }
+
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
         if (ret) {
             return ret;
@@ -253,6 +272,7 @@ int kvm_arch_get_registers(CPUState *cs)
     struct kvm_one_reg reg;
     uint64_t val;
     uint32_t fpr;
+    unsigned int el;
     int i;
     int ret;
 
@@ -325,15 +345,35 @@ int kvm_arch_get_registers(CPUState *cs)
         return ret;
     }
 
+    /* Fetch the SPSR registers
+     *
+     * KVM has an array of state indexed for all the possible aarch32
+     * privilage levels. Although not all are valid at all points
+     * there are some transitions possible which can access old state
+     * so it is worth keeping them all.
+     */
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
         if (ret) {
             return ret;
         }
     }
 
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+            env->banked_spsr[0] = env->banked_spsr[1];
+            i = aarch64_banked_spsr_index(el);
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+        }
+        env->spsr = env->banked_spsr[i];
+    }
+
     /* Advanced SIMD and FP registers */
     for (i = 0; i < 32; i++) {
         reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-- 
2.3.1

WARNING: multiple messages have this Message-ID (diff)
From: alex.bennee@linaro.org (Alex Bennée)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 5/6] target-arm: kvm64 fix save/restore of SPSR regs
Date: Wed,  4 Mar 2015 14:35:52 +0000	[thread overview]
Message-ID: <1425479753-18349-6-git-send-email-alex.bennee@linaro.org> (raw)
In-Reply-To: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org>

From: Christoffer Dall <christoffer.dall@linaro.org>

The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.

Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>

---
v2 (ajb)
  - minor tweaks and clarifications
v3
  - Use the correct bank index function for setting/getting env->spsr
  - only deal with spsrs in elevated exception levels

diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..45e5c3f 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     uint64_t val;
     int i;
     int ret;
+    unsigned int el;
 
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -206,9 +207,27 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    /* Saved Program State Registers
+     *
+     * Before we restore from the banked_spsr[] array we need to
+     * ensure that any modifications to env->spsr are correctly
+     * reflected and map aarch64 exception levels if required.
+     */
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
+            env->banked_spsr[1] = env->banked_spsr[0];
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+            env->banked_spsr[i] = env->spsr;
+        }
+    }
+
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
         if (ret) {
             return ret;
@@ -253,6 +272,7 @@ int kvm_arch_get_registers(CPUState *cs)
     struct kvm_one_reg reg;
     uint64_t val;
     uint32_t fpr;
+    unsigned int el;
     int i;
     int ret;
 
@@ -325,15 +345,35 @@ int kvm_arch_get_registers(CPUState *cs)
         return ret;
     }
 
+    /* Fetch the SPSR registers
+     *
+     * KVM has an array of state indexed for all the possible aarch32
+     * privilage levels. Although not all are valid at all points
+     * there are some transitions possible which can access old state
+     * so it is worth keeping them all.
+     */
     for (i = 0; i < KVM_NR_SPSR; i++) {
         reg.id = AARCH64_CORE_REG(spsr[i]);
-        reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+        reg.addr = (uintptr_t) &env->banked_spsr[i+1];
         ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
         if (ret) {
             return ret;
         }
     }
 
+    el = arm_current_el(env);
+    if (el > 0) {
+        if (is_a64(env)) {
+            g_assert(el == 1);
+            /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+            env->banked_spsr[0] = env->banked_spsr[1];
+            i = aarch64_banked_spsr_index(el);
+        } else {
+            i = bank_number(env->uncached_cpsr & CPSR_M);
+        }
+        env->spsr = env->banked_spsr[i];
+    }
+
     /* Advanced SIMD and FP registers */
     for (i = 0; i < 32; i++) {
         reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-- 
2.3.1

  parent reply	other threads:[~2015-03-04 14:36 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-04 14:35 [PATCH v2 0/6] QEMU ARM64 Migration Fixes Alex Bennée
2015-03-04 14:35 ` Alex Bennée
2015-03-04 14:35 ` [Qemu-devel] " Alex Bennée
2015-03-04 14:35 ` [PATCH v2 1/6] target-arm: kvm: save/restore mp state Alex Bennée
2015-03-04 14:35   ` Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-11 13:42   ` Greg Bellows
2015-03-11 13:42     ` [Qemu-devel] " Greg Bellows
2015-03-12 15:43   ` Peter Maydell
2015-03-12 15:43     ` Peter Maydell
2015-03-12 15:43     ` [Qemu-devel] " Peter Maydell
2015-03-13 10:40     ` Alex Bennée
2015-03-13 10:40       ` Alex Bennée
2015-03-13 10:40       ` [Qemu-devel] " Alex Bennée
2015-03-04 14:35 ` [PATCH v2 2/6] hw/intc: arm_gic_kvm.c restore config first Alex Bennée
2015-03-04 14:35   ` Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-11 13:59   ` Greg Bellows
2015-03-11 13:59     ` Greg Bellows
2015-03-11 13:59     ` Greg Bellows
2015-03-04 14:35 ` [PATCH v2 3/6] hw/char: pl011 don't keep setting the IRQ if nothing changed Alex Bennée
2015-03-04 14:35   ` Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-11 14:44   ` Greg Bellows
2015-03-11 14:44     ` Greg Bellows
2015-03-11 14:44     ` Greg Bellows
2015-03-12 15:51   ` Peter Maydell
2015-03-12 15:51     ` Peter Maydell
2015-03-12 15:51     ` [Qemu-devel] " Peter Maydell
2015-03-12 20:27     ` Peter Maydell
2015-03-12 20:27       ` Peter Maydell
2015-03-12 20:27       ` [Qemu-devel] " Peter Maydell
2015-03-13 10:38       ` Alex Bennée
2015-03-13 10:38         ` Alex Bennée
2015-03-13 10:38         ` Alex Bennée
2015-03-13 10:38         ` [Qemu-devel] " Alex Bennée
2015-03-04 14:35 ` [PATCH v2 4/6] target-arm: kvm64 sync FP register state Alex Bennée
2015-03-04 14:35   ` Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-11 15:17   ` Greg Bellows
2015-03-11 15:17     ` Greg Bellows
2015-03-11 15:17     ` Greg Bellows
2015-03-04 14:35 ` Alex Bennée [this message]
2015-03-04 14:35   ` [PATCH v2 5/6] target-arm: kvm64 fix save/restore of SPSR regs Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-09 13:26   ` Christoffer Dall
2015-03-09 13:26     ` Christoffer Dall
2015-03-09 13:26     ` [Qemu-devel] " Christoffer Dall
2015-03-11 19:41     ` Greg Bellows
2015-03-11 19:41       ` Greg Bellows
2015-03-11 19:41       ` Greg Bellows
2015-03-04 14:35 ` [PATCH v2 6/6] target-arm: cpu.h document why env->spsr exists Alex Bennée
2015-03-04 14:35   ` Alex Bennée
2015-03-04 14:35   ` [Qemu-devel] " Alex Bennée
2015-03-04 14:46   ` Peter Maydell
2015-03-04 14:46     ` Peter Maydell
2015-03-04 14:46     ` [Qemu-devel] " Peter Maydell
2015-03-04 16:27     ` Alex Bennée
2015-03-04 16:27       ` Alex Bennée
2015-03-04 16:27       ` Alex Bennée
2015-03-04 16:27       ` [Qemu-devel] " Alex Bennée

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=1425479753-18349-6-git-send-email-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=christoffer.dall@linaro.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=peter.maydell@linaro.org \
    --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.