* [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM
@ 2014-07-10 15:49 Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions Alex Bennée
` (9 more replies)
0 siblings, 10 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée
Hi,
This series has already been sent out once before under the guise of
tidying up the pstate access and getting TCG migration working for ARM
v8 hosts. I've since added the final two patches to support KVM based
migration as well.
For KVM migration you will need some patches for the kernel side which
are currently working their way through kvmarm:
https://patches.linaro.org/patchwork/bundle/alex.bennee@linaro.org/armv8-migration/
Changes since v1:
- addressed review comments
- merged xpsr state into changes
- checkpatch fixes
- cleaner handling of integer flag manipulation
- addition of final KVM patches
Alex Bennée (10):
target-arm/cpu.h: document various program state functions
target-arm/cpu.h: common pstate save/restore
target-arm: Support save/load for 64 bit CPUs
target-arm: replace cpsr/xpsr/pstate_read calls
arm/nwfps: replace cpsr_write with set_condition_codes
linux-user/main.c: __kernel_cmpxchg set env->CF directly
target-arm: remove last users of cpsr_write
target-arm: remove final users of pstate_write
target-arm/kvm.c: better error reporting
target-arm/kvm: make reg sync code common between kvm32/64
linux-user/arm/nwfpe/fpa11.h | 2 +-
linux-user/elfload.c | 4 +-
linux-user/main.c | 17 ++--
linux-user/signal.c | 65 ++++++++-------
target-arm/cpu.h | 192 +++++++++++++++++++++++++++++++++++--------
target-arm/gdbstub.c | 10 ++-
target-arm/gdbstub64.c | 6 +-
target-arm/helper-a64.c | 11 +--
target-arm/helper.c | 92 +++------------------
target-arm/kvm.c | 137 ++++++++++++++++++++++++++++++
target-arm/kvm32.c | 96 ++--------------------
target-arm/kvm64.c | 6 +-
target-arm/kvm_arm.h | 12 +++
target-arm/machine.c | 27 +++---
target-arm/op_helper.c | 55 +++++++++++--
target-arm/translate-a64.c | 2 +-
target-arm/translate.c | 2 +-
17 files changed, 448 insertions(+), 288 deletions(-)
--
2.0.1
Cheers,
--
Alex Bennée
QEMU/KVM Hacker for Linaro
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
@ 2014-07-10 15:49 ` Alex Bennée
2014-08-04 12:28 ` Peter Maydell
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Alex Bennée
We have a number of program state saving functions (pstate, cpsr, xpsr)
which are dependant on the mode the CPU is in. This commit adds a little
documentation to each function and asserts to defend against incorrect
use.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2:
- remove xpsr_state asserts
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 369d472..c2312d0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -475,22 +475,34 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
#define PSTATE_MODE_EL1t 4
#define PSTATE_MODE_EL0t 0
-/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
- * interprocessing, so we don't attempt to sync with the cpsr state used by
- * the 32 bit decoder.
+/* ARMv8 ARM D1.7 Process state, PSTATE
+ *
+ * 31 28 27 24 23 22 21 20 22 21 20 19 16 15 8 7 5 4 0
+ * +------+------+-------+-----+--------+---+------+------+-----+------+
+ * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
+ * +------+------+-------+-----+--------+---+------+------+-----+------+
+ *
+ * The PSTATE is an abstraction of a number of Return the current
+ * PSTATE value. This is only valid for A64 hardware although can be
+ * read when in AArch32 mode.
*/
static inline uint32_t pstate_read(CPUARMState *env)
{
int ZF;
+ g_assert(is_a64(env));
+
ZF = (env->ZF == 0);
return (env->NF & 0x80000000) | (ZF << 30)
| (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
| env->pstate | env->daif;
}
+/* Update the current PSTATE value. This doesn't include nRW which is */
static inline void pstate_write(CPUARMState *env, uint32_t val)
{
+ g_assert(is_a64(env));
+
env->ZF = (~val) & PSTATE_Z;
env->NF = val;
env->CF = (val >> 29) & 1;
@@ -499,15 +511,22 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
env->pstate = val & ~CACHED_PSTATE_BITS;
}
-/* Return the current CPSR value. */
+/* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
+ *
+ * Unlike the above PSTATE implementation these functions will attempt
+ * to switch processor mode when the M[4:0] bits are set.
+ */
uint32_t cpsr_read(CPUARMState *env);
/* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
-/* Return the current xPSR value. */
+/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
static inline uint32_t xpsr_read(CPUARMState *env)
{
int ZF;
+
+ g_assert(!is_a64(env));
+
ZF = (env->ZF == 0);
return (env->NF & 0x80000000) | (ZF << 30)
| (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
@@ -519,6 +538,8 @@ static inline uint32_t xpsr_read(CPUARMState *env)
/* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */
static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
{
+ g_assert(!is_a64(env));
+
if (mask & CPSR_NZCV) {
env->ZF = (~val) & CPSR_Z;
env->NF = val;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2b4ce6a..ec1fef5 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -506,8 +506,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->condexec_bits = 0;
}
- pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
env->aarch64 = 1;
+ pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
env->pc = addr;
cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions Alex Bennée
@ 2014-07-10 15:49 ` Alex Bennée
2014-08-04 12:47 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs Alex Bennée
` (7 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Alex Bennée
This adds a universal program state save and restore function. This is
intended to simplify the migration serialisation functionality and avoid
special casing depending on the mode of the CPU at serialisation time.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2:
- reword commentary for restore_state_from_spsr
- rm commented out code
- add set_condition_codes for flags
- add xpsr_read functionality
v3:
- rebase
- checkpatch.pl issues
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c2312d0..fe4d4f3 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -453,19 +453,24 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
#define PSTATE_SP (1U)
#define PSTATE_M (0xFU)
#define PSTATE_nRW (1U << 4)
+#define PSTATE_T (1U << 5)
#define PSTATE_F (1U << 6)
#define PSTATE_I (1U << 7)
#define PSTATE_A (1U << 8)
#define PSTATE_D (1U << 9)
#define PSTATE_IL (1U << 20)
#define PSTATE_SS (1U << 21)
+#define PSTATE_Q (1U << 27)
#define PSTATE_V (1U << 28)
#define PSTATE_C (1U << 29)
#define PSTATE_Z (1U << 30)
#define PSTATE_N (1U << 31)
#define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
-#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
-#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
+#define PSTATE_AIF (PSTATE_A | PSTATE_I | PSTATE_F)
+#define PSTATE_DAIF (PSTATE_D | PSTATE_AIF)
+#define AARCH64_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
+#define AARCH32_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_Q | PSTATE_AIF \
+ | CACHED_CPSR_BITS)
/* Mode values for AArch64 */
#define PSTATE_MODE_EL3h 13
#define PSTATE_MODE_EL3t 12
@@ -508,7 +513,7 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
env->CF = (val >> 29) & 1;
env->VF = (val << 3) & 0x80000000;
env->daif = val & PSTATE_DAIF;
- env->pstate = val & ~CACHED_PSTATE_BITS;
+ env->pstate = val & ~AARCH64_CACHED_PSTATE_BITS;
}
/* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
@@ -698,6 +703,137 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
return arm_feature(env, ARM_FEATURE_AARCH64);
}
+/* ARMv8 ARM D1.6.4, Saved Program Status Registers
+ *
+ * These are formats used to save program status when exceptions are
+ * taken. There are two forms:
+ *
+ * AArch64 -> AArch64 Exception
+ * 31 30 28 29 27 22 21 20 19 10 9 8 7 6 5 4 3 0
+ * +---+---+---+---+------+----+----+------+---+---+---+---+---+------------+
+ * | N | Z | C | V | RES0 | SS | IL | RES0 | D | A | I | F | 0 | 0 | M[3:0] |
+ * +---+---+---+---+------+----+----+------+---+---+---+---+---+------------+
+ *
+ * AArch32 -> AArch64 Exception
+ * (see also ARMv7-AR ARM B1.3.3 CSPR/SPSR formats)
+ * 31 30 29 28 27 26 25 24 23 22 21 20 19 16
+ * +---+---+---+---+---+---------+---+------+----+----+---------+
+ * | N | Z | C | V | Q | IT[1:0] | J | RES0 | SS | IL | GE[3:0] |
+ * +---+---+---+---+---+---------+---+------+----+----+---------+
+ * 15 10 9 8 7 6 5 4 3 0
+ * +---------+---+---+---+---+---+---+--------+
+ * | IT[7:2] | E | A | I | F | T | 1 | M[3:0] |
+ * +---------+---+---+---+---+---+---+--------+
+ *
+ * M[4] = nRW - 0 = 64bit, 1 = 32bit
+ * Bit definitions for ARMv8 SPSR (PSTATE) format.
+ * Only these are valid when in AArch64 mode; in
+ * AArch32 mode SPSRs are basically CPSR-format.
+ *
+ * Also ARMv7-M ARM B1.4.2, special purpose program status register xPSR
+ */
+
+/* Save the program state */
+static inline uint32_t save_state_to_spsr(CPUARMState *env)
+{
+ uint32_t final_spsr = 0;
+
+ /* Calculate integer flags */
+ final_spsr |= (env->NF & 0x80000000) ? PSTATE_N : 0; /* bit 31 */
+ final_spsr |= (env->ZF == 0) ? PSTATE_Z : 0;
+ final_spsr |= env->CF ? PSTATE_C : 0; /* or env->CF << 29 */
+ final_spsr |= (env->VF & 0x80000000) ? PSTATE_V : 0; /* bit 31 */
+
+ if (is_a64(env)) {
+ /* SS - Software Step */
+ /* IL - Illegal execution state */
+ /* DAIF flags - should we mask?*/
+ final_spsr |= (env->daif & PSTATE_DAIF);
+ /* And the final un-cached bits */
+ final_spsr |= (env->pstate & ~AARCH64_CACHED_PSTATE_BITS);
+ } else {
+ /* condexec_bits is split over two parts */
+ final_spsr |= ((env->condexec_bits & 3) << 25);
+ final_spsr |= ((env->condexec_bits & 0xfc) << 8);
+
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ final_spsr |= (env->thumb << 24); /* alt pos */
+ final_spsr |= env->v7m.exception;
+ } else {
+ final_spsr |= env->QF ? PSTATE_Q : 0;
+ /* GE needs shifting into place */
+ final_spsr |= (env->GE << 16);
+ /* AIF is a a subset of DAIF */
+ final_spsr |= (env->daif & PSTATE_AIF);
+ final_spsr |= env->thumb ? PSTATE_T : 0;
+
+ final_spsr |= PSTATE_nRW;
+
+ /* And the final un-cached bits */
+ final_spsr |= (env->uncached_cpsr & ~AARCH32_CACHED_PSTATE_BITS);
+ }
+ }
+ return final_spsr;
+}
+
+/* Restore the program state.
+ *
+ * This restores the program execution state including it's current
+ * execution mode. However validation of mode changes and additional
+ * registers and state that need changing should be done by the
+ * calling function.
+ *
+ * The simple set_condition_codes() function can be used just to
+ * update the cached integer flags which are quite often manipulated
+ * alone.
+ */
+
+/* Only update the cached condition codes. */
+static inline void set_condition_codes(CPUARMState *env, uint32_t new_flags)
+{
+ /* Process the integer flags directly */
+ env->ZF = (~new_flags) & CPSR_Z;
+ env->NF = new_flags;
+ env->CF = (new_flags >> 29) & 1;
+ env->VF = (new_flags << 3) & 0x80000000;
+}
+
+/* Restore the complete program state */
+static inline void restore_state_from_spsr(CPUARMState *env,
+ uint32_t saved_state)
+{
+ set_condition_codes(env, saved_state);
+
+ /* the rest depends on the mode we end up in */
+ env->aarch64 = (saved_state & PSTATE_nRW) ? 0 : 1;
+
+ if (is_a64(env)) {
+ env->daif = saved_state & PSTATE_DAIF;
+ env->pstate = (saved_state & ~AARCH64_CACHED_PSTATE_BITS);
+ } else {
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ fprintf(stderr, "%s: M state not yet implmented\n", __func__);
+ } else {
+ env->QF = saved_state & PSTATE_Q ? 1 : 0;
+ /* condexec_bits is split over two parts */
+ env->condexec_bits = (saved_state & CPSR_IT_0_1) >> 25;
+ env->condexec_bits |= (saved_state & CPSR_IT_2_7) >> 8;
+
+ /* GE needs shifting into place */
+ env->GE = (saved_state >> 16) & 0xf;
+
+ /* AIF is a a subset of DAIF */
+ env->daif = (saved_state & PSTATE_AIF);
+
+ env->thumb = saved_state & PSTATE_T ? 1 : 0;
+
+ /* And the final un-cached bits */
+ env->uncached_cpsr = saved_state & ~AARCH32_CACHED_PSTATE_BITS;
+ }
+ }
+}
+
+
void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
/* Interface between CPU and Interrupt controller. */
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-08-04 12:50 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls Alex Bennée
` (6 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Alex Bennée
This enables the saving and restoring of machine state by including the
current program state (*psr) and xregs. The save_state_to_spsr hides the
details of if the processor is in 32 or 64 bit mode at the time.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2 (ajb)
- use common state save functions
- re-base to latest origin/master
- clean up commented out code
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 3bcc7cc..759610c 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -120,30 +120,27 @@ static const VMStateDescription vmstate_thumb2ee = {
}
};
-static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
+static int get_psr(QEMUFile *f, void *opaque, size_t size)
{
ARMCPU *cpu = opaque;
CPUARMState *env = &cpu->env;
uint32_t val = qemu_get_be32(f);
- /* Avoid mode switch when restoring CPSR */
- env->uncached_cpsr = val & CPSR_M;
- cpsr_write(env, val, 0xffffffff);
+ restore_state_from_spsr(env, val);
return 0;
}
-static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
+static void put_psr(QEMUFile *f, void *opaque, size_t size)
{
ARMCPU *cpu = opaque;
CPUARMState *env = &cpu->env;
-
- qemu_put_be32(f, cpsr_read(env));
+ qemu_put_be32(f, save_state_to_spsr(env));
}
-static const VMStateInfo vmstate_cpsr = {
+static const VMStateInfo vmstate_psr = {
.name = "cpsr",
- .get = get_cpsr,
- .put = put_cpsr,
+ .get = get_psr,
+ .put = put_psr,
};
static void cpu_pre_save(void *opaque)
@@ -218,17 +215,19 @@ static int cpu_post_load(void *opaque, int version_id)
const VMStateDescription vmstate_arm_cpu = {
.name = "cpu",
- .version_id = 20,
- .minimum_version_id = 20,
+ .version_id = 21,
+ .minimum_version_id = 21,
.pre_save = cpu_pre_save,
.post_load = cpu_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
+ VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
+ VMSTATE_UINT64(env.pc, ARMCPU),
{
- .name = "cpsr",
+ .name = "psr",
.version_id = 0,
.size = sizeof(uint32_t),
- .info = &vmstate_cpsr,
+ .info = &vmstate_psr,
.flags = VMS_SINGLE,
.offset = 0,
},
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
` (2 preceding siblings ...)
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-15 13:40 ` Riku Voipio
2014-08-04 12:59 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes Alex Bennée
` (5 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Riku Voipio, Alex Bennée
Use the unified save_state_to_spsr. I've also updated the interrupt
helpers to restore via the restore_state_from_spsr() functions. In the
aarch32 case this also needs to call switch_mode() to do the appropriate
fiddling.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
--
v2
- include xpsr_read conversions
- checkpatch fixes
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 60777fe..577e1d3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -321,7 +321,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
(*regs)[14] = tswapreg(env->regs[14]);
(*regs)[15] = tswapreg(env->regs[15]);
- (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
+ (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));
(*regs)[17] = tswapreg(env->regs[0]); /* XXX */
}
@@ -509,7 +509,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
(*regs)[i] = tswapreg(env->xregs[i]);
}
(*regs)[32] = tswapreg(env->pc);
- (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
+ (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
}
#define USE_ELF_CORE_DUMP
diff --git a/linux-user/main.c b/linux-user/main.c
index b453a39..8848e15 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -546,7 +546,7 @@ do_kernel_trap(CPUARMState *env)
operations. However things like ldrex/strex are much harder so
there's not much point trying. */
start_exclusive();
- cpsr = cpsr_read(env);
+ cpsr = save_state_to_spsr(env);
addr = env->regs[2];
/* FIXME: This should SEGV if the access fails. */
if (get_user_u32(val, addr))
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 86fae3d..9c6727b 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1224,7 +1224,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
}
__put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
__put_user(env->pc, &sf->uc.tuc_mcontext.pc);
- __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
+ __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
__put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
@@ -1572,7 +1572,7 @@ setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
__put_user(env->regs[14], &sc->arm_lr);
__put_user(env->regs[15], &sc->arm_pc);
#ifdef TARGET_CONFIG_CPU_32
- __put_user(cpsr_read(env), &sc->arm_cpsr);
+ __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
#endif
__put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
@@ -1604,7 +1604,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
abi_ulong handler = ka->_sa_handler;
abi_ulong retcode;
int thumb = handler & 1;
- uint32_t cpsr = cpsr_read(env);
+ uint32_t cpsr = save_state_to_spsr(env);
cpsr &= ~CPSR_IT;
if (thumb) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index fe4d4f3..3f23167 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -480,30 +480,8 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
#define PSTATE_MODE_EL1t 4
#define PSTATE_MODE_EL0t 0
-/* ARMv8 ARM D1.7 Process state, PSTATE
- *
- * 31 28 27 24 23 22 21 20 22 21 20 19 16 15 8 7 5 4 0
- * +------+------+-------+-----+--------+---+------+------+-----+------+
- * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
- * +------+------+-------+-----+--------+---+------+------+-----+------+
- *
- * The PSTATE is an abstraction of a number of Return the current
- * PSTATE value. This is only valid for A64 hardware although can be
- * read when in AArch32 mode.
- */
-static inline uint32_t pstate_read(CPUARMState *env)
-{
- int ZF;
-
- g_assert(is_a64(env));
-
- ZF = (env->ZF == 0);
- return (env->NF & 0x80000000) | (ZF << 30)
- | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
- | env->pstate | env->daif;
-}
-
-/* Update the current PSTATE value. This doesn't include nRW which is */
+/* Update the current PSTATE value. This doesn't include nRW which
+ * indicates if we are in 64 or 32 bit mode */
static inline void pstate_write(CPUARMState *env, uint32_t val)
{
g_assert(is_a64(env));
@@ -520,26 +498,10 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
*
* Unlike the above PSTATE implementation these functions will attempt
* to switch processor mode when the M[4:0] bits are set.
- */
-uint32_t cpsr_read(CPUARMState *env);
-/* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
+ *
+ * Note that some bits of mask must be all-set or all-clear. */
void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
-/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
-static inline uint32_t xpsr_read(CPUARMState *env)
-{
- int ZF;
-
- g_assert(!is_a64(env));
-
- ZF = (env->ZF == 0);
- return (env->NF & 0x80000000) | (ZF << 30)
- | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
- | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
- | ((env->condexec_bits & 0xfc) << 8)
- | env->v7m.exception;
-}
-
/* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */
static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
{
diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
index 1c34396..ec25f30 100644
--- a/target-arm/gdbstub.c
+++ b/target-arm/gdbstub.c
@@ -53,7 +53,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
return gdb_get_reg32(mem_buf, 0);
case 25:
/* CPSR */
- return gdb_get_reg32(mem_buf, cpsr_read(env));
+ return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
}
/* Unknown register. */
return 0;
diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
index 8f3b8d1..76d1b91 100644
--- a/target-arm/gdbstub64.c
+++ b/target-arm/gdbstub64.c
@@ -35,7 +35,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
case 32:
return gdb_get_reg64(mem_buf, env->pc);
case 33:
- return gdb_get_reg32(mem_buf, pstate_read(env));
+ return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
}
/* Unknown register. */
return 0;
@@ -62,7 +62,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
env->pc = tmp;
return 8;
case 33:
- /* CPSR */
+ /* SPSR */
pstate_write(env, tmp);
return 4;
}
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index ec1fef5..03cd64f 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -445,6 +445,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
CPUARMState *env = &cpu->env;
target_ulong addr = env->cp15.vbar_el[1];
int i;
+ uint32_t spsr = save_state_to_spsr(env);
if (arm_current_pl(env) == 0) {
if (env->aarch64) {
@@ -452,7 +453,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
} else {
addr += 0x600;
}
- } else if (pstate_read(env) & PSTATE_SP) {
+ } else if (spsr & PSTATE_SP) {
addr += 0x200;
}
@@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
}
if (is_a64(env)) {
- env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
+ env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
env->sp_el[arm_current_pl(env)] = env->xregs[31];
env->xregs[31] = env->sp_el[1];
env->elr_el[1] = env->pc;
} else {
- env->banked_spsr[0] = cpsr_read(env);
+ env->banked_spsr[0] = spsr;
if (!env->thumb) {
env->cp15.esr_el[1] |= 1 << 25;
}
@@ -506,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->condexec_bits = 0;
}
+ // TODO: restore_state_from_spsr()
env->aarch64 = 1;
pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index d343856..030bcdd 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2990,17 +2990,6 @@ static int bad_mode_switch(CPUARMState *env, int mode)
}
}
-uint32_t cpsr_read(CPUARMState *env)
-{
- int ZF;
- ZF = (env->ZF == 0);
- return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
- (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
- | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
- | ((env->condexec_bits & 0xfc) << 8)
- | (env->GE << 16) | (env->daif & CPSR_AIF);
-}
-
void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
{
if (mask & CPSR_NZCV) {
@@ -3278,7 +3267,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
- uint32_t xpsr = xpsr_read(env);
+ uint32_t xpsr = save_state_to_spsr(env);
uint32_t lr;
uint32_t addr;
@@ -3479,7 +3468,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
addr += env->cp15.vbar_el[1];
}
switch_mode (env, new_mode);
- env->spsr = cpsr_read(env);
+ env->spsr = save_state_to_spsr(env);
+ /* TODO: restore_state_from_spsr */
/* Clear IT bits. */
env->condexec_bits = 0;
/* Switch to the new mode, and to the correct instruction set. */
@@ -4212,19 +4202,19 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
switch (reg) {
case 0: /* APSR */
- return xpsr_read(env) & 0xf8000000;
+ return save_state_to_spsr(env) & 0xf8000000;
case 1: /* IAPSR */
- return xpsr_read(env) & 0xf80001ff;
+ return save_state_to_spsr(env) & 0xf80001ff;
case 2: /* EAPSR */
- return xpsr_read(env) & 0xff00fc00;
+ return save_state_to_spsr(env) & 0xff00fc00;
case 3: /* xPSR */
- return xpsr_read(env) & 0xff00fdff;
+ return save_state_to_spsr(env) & 0xff00fdff;
case 5: /* IPSR */
- return xpsr_read(env) & 0x000001ff;
+ return save_state_to_spsr(env) & 0x000001ff;
case 6: /* EPSR */
- return xpsr_read(env) & 0x0700fc00;
+ return save_state_to_spsr(env) & 0x0700fc00;
case 7: /* IEPSR */
- return xpsr_read(env) & 0x0700edff;
+ return save_state_to_spsr(env) & 0x0700edff;
case 8: /* MSP */
return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
case 9: /* PSP */
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 5ec4eb1..789f52f 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -384,7 +384,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
/* Special cases which aren't a single CPUARMState field */
- cpsr = cpsr_read(env);
+ cpsr = save_state_to_spsr(env);
r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
r.addr = (uintptr_t)(&cpsr);
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 5d217ca..eaf6ff8 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -153,7 +153,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
/* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
- val = pstate_read(env);
+ val = save_state_to_spsr(env);
reg.id = AARCH64_CORE_REG(regs.pstate);
reg.addr = (uintptr_t) &val;
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9c1ef52..052a4bd 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -258,7 +258,7 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
uint32_t HELPER(cpsr_read)(CPUARMState *env)
{
- return cpsr_read(env) & ~CPSR_EXEC;
+ return save_state_to_spsr(env) & ~CPSR_EXEC;
}
void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
@@ -386,10 +386,9 @@ void HELPER(exception_return)(CPUARMState *env)
if (spsr & PSTATE_nRW) {
/* TODO: We currently assume EL1/2/3 are running in AArch64. */
- env->aarch64 = 0;
new_el = 0;
- env->uncached_cpsr = 0x10;
- cpsr_write(env, spsr, ~0);
+ switch_mode(env, spsr & CPSR_M);
+
for (i = 0; i < 15; i++) {
env->regs[i] = env->xregs[i];
}
@@ -412,11 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
/* Return to EL0 with M[0] bit set */
goto illegal_return;
}
- env->aarch64 = 1;
- pstate_write(env, spsr);
env->xregs[31] = env->sp_el[new_el];
env->pc = env->elr_el[cur_el];
}
+ /* This will set env->aarch64 as appropriate */
+ restore_state_from_spsr(env, spsr);
return;
@@ -431,8 +430,8 @@ illegal_return:
env->pstate |= PSTATE_IL;
env->pc = env->elr_el[cur_el];
spsr &= PSTATE_NZCV | PSTATE_DAIF;
- spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
- pstate_write(env, spsr);
+ spsr |= save_state_to_spsr(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
+ restore_state_from_spsr(env, spsr);
}
/* ??? Flag setting arithmetic is awkward because we need to do comparisons.
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 33b5025..3b4a676 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -126,7 +126,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
- uint32_t psr = pstate_read(env);
+ uint32_t psr = save_state_to_spsr(env);
int i;
cpu_fprintf(f, "PC=%016"PRIx64" SP=%016"PRIx64"\n",
diff --git a/target-arm/translate.c b/target-arm/translate.c
index cf4e767..8427396 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11192,7 +11192,7 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
else
cpu_fprintf(f, " ");
}
- psr = cpsr_read(env);
+ psr = save_state_to_spsr(env);
cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",
psr,
psr & (1 << 31) ? 'N' : '-',
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
` (3 preceding siblings ...)
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-15 13:41 ` Riku Voipio
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly Alex Bennée
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Alex Bennée
This is a pre-cursor to removing the cpsr_write function completely from
the code base. set_condition_codes() only affects the integer condition
flags.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- fix nwfpe set_condition_codes
diff --git a/linux-user/arm/nwfpe/fpa11.h b/linux-user/arm/nwfpe/fpa11.h
index bb9ac65..c7883d0 100644
--- a/linux-user/arm/nwfpe/fpa11.h
+++ b/linux-user/arm/nwfpe/fpa11.h
@@ -108,7 +108,7 @@ static inline void writeRegister(unsigned int x, unsigned int y)
static inline void writeConditionCodes(unsigned int x)
{
- cpsr_write(user_registers,x,CPSR_NZCV);
+ set_condition_codes(user_registers, x);
}
#define ARM_REG_PC 15
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
` (4 preceding siblings ...)
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-15 13:39 ` Riku Voipio
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write Alex Bennée
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Alex Bennée
As we only need to manipulate the single flag do it directly though env.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2:
- remove unused cpsr
- the direct flag setting seems a little hacky?
diff --git a/linux-user/main.c b/linux-user/main.c
index 8848e15..9101541 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -468,7 +468,7 @@ void cpu_loop(CPUX86State *env)
static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
{
uint64_t oldval, newval, val;
- uint32_t addr, cpsr;
+ uint32_t addr;
target_siginfo_t info;
/* Based on the 32 bit code in do_kernel_trap */
@@ -478,7 +478,6 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
operations. However things like ldrex/strex are much harder so
there's not much point trying. */
start_exclusive();
- cpsr = cpsr_read(env);
addr = env->regs[2];
if (get_user_u64(oldval, env->regs[0])) {
@@ -505,12 +504,11 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
};
env->regs[0] = 0;
- cpsr |= CPSR_C;
+ env->CF = 1;
} else {
env->regs[0] = -1;
- cpsr &= ~CPSR_C;
+ env->CF = 0;
}
- cpsr_write(env, cpsr, CPSR_C);
end_exclusive();
return;
@@ -533,7 +531,6 @@ static int
do_kernel_trap(CPUARMState *env)
{
uint32_t addr;
- uint32_t cpsr;
uint32_t val;
switch (env->regs[15]) {
@@ -546,7 +543,6 @@ do_kernel_trap(CPUARMState *env)
operations. However things like ldrex/strex are much harder so
there's not much point trying. */
start_exclusive();
- cpsr = save_state_to_spsr(env);
addr = env->regs[2];
/* FIXME: This should SEGV if the access fails. */
if (get_user_u32(val, addr))
@@ -556,12 +552,11 @@ do_kernel_trap(CPUARMState *env)
/* FIXME: Check for segfaults. */
put_user_u32(val, addr);
env->regs[0] = 0;
- cpsr |= CPSR_C;
+ env->CF = 1;
} else {
env->regs[0] = -1;
- cpsr &= ~CPSR_C;
+ env->CF = 0;
}
- cpsr_write(env, cpsr, CPSR_C);
end_exclusive();
break;
case 0xffff0fe0: /* __kernel_get_tls */
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
` (5 preceding siblings ...)
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-15 13:43 ` Riku Voipio
2014-08-04 13:01 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 08/10] target-arm: remove final users of pstate_write Alex Bennée
` (2 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Riku Voipio, Alex Bennée
And use the new machinery to to save and restore program state. The old
cpsr_write function did some special handling for mode switches which
has been moved into the helper function.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2:
- rebase
- add mask helper function
- checkpatch fixes
diff --git a/linux-user/main.c b/linux-user/main.c
index 9101541..5f7cc31 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4184,7 +4184,7 @@ int main(int argc, char **argv, char **envp)
#elif defined(TARGET_ARM)
{
int i;
- cpsr_write(env, regs->uregs[16], 0xffffffff);
+ restore_state_from_spsr(env, regs->uregs[16]);
for(i = 0; i < 16; i++) {
env->regs[i] = regs->uregs[i];
}
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9c6727b..b6f9ef4 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1599,38 +1599,39 @@ get_sigframe(struct target_sigaction *ka, CPUARMState *regs, int framesize)
static void
setup_return(CPUARMState *env, struct target_sigaction *ka,
- abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
+ abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
{
- abi_ulong handler = ka->_sa_handler;
- abi_ulong retcode;
- int thumb = handler & 1;
- uint32_t cpsr = save_state_to_spsr(env);
+ abi_ulong handler = ka->_sa_handler;
+ abi_ulong retcode;
+ int thumb = handler & 1;
+ uint32_t cpsr = save_state_to_spsr(env);
- cpsr &= ~CPSR_IT;
- if (thumb) {
- cpsr |= CPSR_T;
- } else {
- cpsr &= ~CPSR_T;
- }
+ cpsr &= ~CPSR_IT;
+ if (thumb) {
+ cpsr |= CPSR_T;
+ } else {
+ cpsr &= ~CPSR_T;
+ }
- if (ka->sa_flags & TARGET_SA_RESTORER) {
- retcode = ka->sa_restorer;
- } else {
- unsigned int idx = thumb;
+ if (ka->sa_flags & TARGET_SA_RESTORER) {
+ retcode = ka->sa_restorer;
+ } else {
+ unsigned int idx = thumb;
- if (ka->sa_flags & TARGET_SA_SIGINFO)
- idx += 2;
+ if (ka->sa_flags & TARGET_SA_SIGINFO) {
+ idx += 2;
+ }
- __put_user(retcodes[idx], rc);
+ __put_user(retcodes[idx], rc);
- retcode = rc_addr + thumb;
- }
+ retcode = rc_addr + thumb;
+ }
- env->regs[0] = usig;
- env->regs[13] = frame_addr;
- env->regs[14] = retcode;
- env->regs[15] = handler & (thumb ? ~1 : ~3);
- cpsr_write(env, cpsr, 0xffffffff);
+ env->regs[0] = usig;
+ env->regs[13] = frame_addr;
+ env->regs[14] = retcode;
+ env->regs[15] = handler & (thumb ? ~1 : ~3);
+ restore_state_from_spsr(env, cpsr);
}
static abi_ulong *setup_sigframe_v2_vfp(abi_ulong *regspace, CPUARMState *env)
@@ -1858,12 +1859,14 @@ restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
__get_user(env->regs[15], &sc->arm_pc);
#ifdef TARGET_CONFIG_CPU_32
__get_user(cpsr, &sc->arm_cpsr);
- cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
+ restore_state_from_masked_spsr(env,
+ (CPSR_USER | CPSR_EXEC),
+ cpsr);
#endif
- err |= !valid_user_regs(env);
+ err |= !valid_user_regs(env);
- return err;
+ return err;
}
static long do_sigreturn_v1(CPUARMState *env)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3f23167..b56f1a8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -795,6 +795,15 @@ static inline void restore_state_from_spsr(CPUARMState *env,
}
}
+/* Restore a few masked bits of the program state */
+static inline void restore_state_from_masked_spsr(CPUARMState *env,
+ uint32_t mask,
+ uint32_t saved_state)
+{
+ uint32_t spsr = (save_state_to_spsr(env) & ~mask);
+ spsr |= (saved_state & mask);
+ return restore_state_from_spsr(env, spsr);
+}
void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
index ec25f30..5e60589 100644
--- a/target-arm/gdbstub.c
+++ b/target-arm/gdbstub.c
@@ -93,8 +93,12 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
}
return 4;
case 25:
- /* CPSR */
- cpsr_write(env, tmp, 0xffffffff);
+ /* CPSR
+ * FIXME?: as restore_state_from_spsr() doesn't do aarch32
+ * special mode fixups this may break. However GDB doesn't
+ * seem to be able to handle tracing over a mode switch anyway
+ */
+ restore_state_from_spsr(env, tmp);
return 4;
}
/* Unknown register. */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 030bcdd..6d755c0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2970,68 +2970,6 @@ void arm_cp_reset_ignore(CPUARMState *env, const ARMCPRegInfo *opaque)
/* Helper coprocessor reset function for do-nothing-on-reset registers */
}
-static int bad_mode_switch(CPUARMState *env, int mode)
-{
- /* Return true if it is not valid for us to switch to
- * this CPU mode (ie all the UNPREDICTABLE cases in
- * the ARM ARM CPSRWriteByInstr pseudocode).
- */
- switch (mode) {
- case ARM_CPU_MODE_USR:
- case ARM_CPU_MODE_SYS:
- case ARM_CPU_MODE_SVC:
- case ARM_CPU_MODE_ABT:
- case ARM_CPU_MODE_UND:
- case ARM_CPU_MODE_IRQ:
- case ARM_CPU_MODE_FIQ:
- return 0;
- default:
- return 1;
- }
-}
-
-void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
-{
- if (mask & CPSR_NZCV) {
- env->ZF = (~val) & CPSR_Z;
- env->NF = val;
- env->CF = (val >> 29) & 1;
- env->VF = (val << 3) & 0x80000000;
- }
- if (mask & CPSR_Q)
- env->QF = ((val & CPSR_Q) != 0);
- if (mask & CPSR_T)
- env->thumb = ((val & CPSR_T) != 0);
- if (mask & CPSR_IT_0_1) {
- env->condexec_bits &= ~3;
- env->condexec_bits |= (val >> 25) & 3;
- }
- if (mask & CPSR_IT_2_7) {
- env->condexec_bits &= 3;
- env->condexec_bits |= (val >> 8) & 0xfc;
- }
- if (mask & CPSR_GE) {
- env->GE = (val >> 16) & 0xf;
- }
-
- env->daif &= ~(CPSR_AIF & mask);
- env->daif |= val & CPSR_AIF & mask;
-
- if ((env->uncached_cpsr ^ val) & mask & CPSR_M) {
- if (bad_mode_switch(env, val & CPSR_M)) {
- /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
- * We choose to ignore the attempt and leave the CPSR M field
- * untouched.
- */
- mask &= ~CPSR_M;
- } else {
- switch_mode(env, val & CPSR_M);
- }
- }
- mask &= ~CACHED_CPSR_BITS;
- env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask);
-}
-
/* Sign/zero extend */
uint32_t HELPER(sxtb16)(uint32_t x)
{
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 789f52f..39117c7 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -464,7 +464,7 @@ int kvm_arch_get_registers(CPUState *cs)
if (ret) {
return ret;
}
- cpsr_write(env, cpsr, 0xffffffff);
+ restore_state_from_spsr(env, cpsr);
/* Make sure the current mode regs are properly set */
mode = env->uncached_cpsr & CPSR_M;
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 052a4bd..c17bdba 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -261,9 +261,47 @@ uint32_t HELPER(cpsr_read)(CPUARMState *env)
return save_state_to_spsr(env) & ~CPSR_EXEC;
}
+static int bad_mode_switch(CPUARMState *env, int mode)
+{
+ /* Return true if it is not valid for us to switch to
+ * this CPU mode (ie all the UNPREDICTABLE cases in
+ * the ARM ARM CPSRWriteByInstr pseudocode).
+ */
+ switch (mode) {
+ case ARM_CPU_MODE_USR:
+ case ARM_CPU_MODE_SYS:
+ case ARM_CPU_MODE_SVC:
+ case ARM_CPU_MODE_ABT:
+ case ARM_CPU_MODE_UND:
+ case ARM_CPU_MODE_IRQ:
+ case ARM_CPU_MODE_FIQ:
+ return 0;
+ default:
+ return 1;
+ }
+}
+
void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
{
- cpsr_write(env, val, mask);
+ uint32_t current_cpsr = save_state_to_spsr(env);
+ uint32_t new_cpsr;
+
+ /* we may be triggering a mode change */
+ if ((current_cpsr ^ val) & mask & CPSR_M) {
+ if (bad_mode_switch(env, val & CPSR_M)) {
+ /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
+ * We choose to ignore the attempt and leave the CPSR M field
+ * untouched.
+ */
+ mask &= ~CPSR_M;
+ } else {
+ switch_mode(env, val & CPSR_M);
+ }
+ }
+
+ new_cpsr = current_cpsr & ~mask;
+ new_cpsr |= (val & mask);
+ restore_state_from_spsr(env, new_cpsr);
}
/* Access to user mode registers from privileged modes. */
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 08/10] target-arm: remove final users of pstate_write
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
` (6 preceding siblings ...)
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-15 13:44 ` Riku Voipio
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
9 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Riku Voipio, Alex Bennée
This converts all users of pstate_write to use the common state
save/restore functionality.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/linux-user/signal.c b/linux-user/signal.c
index b6f9ef4..b1958a5 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1274,7 +1274,7 @@ static int target_restore_sigframe(CPUARMState *env,
__get_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
__get_user(env->pc, &sf->uc.tuc_mcontext.pc);
__get_user(pstate, &sf->uc.tuc_mcontext.pstate);
- pstate_write(env, pstate);
+ restore_state_from_spsr(env, pstate);
__get_user(magic, &aux->fpsimd.head.magic);
__get_user(size, &aux->fpsimd.head.size);
diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
index 76d1b91..366335a 100644
--- a/target-arm/gdbstub64.c
+++ b/target-arm/gdbstub64.c
@@ -63,7 +63,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 8;
case 33:
/* SPSR */
- pstate_write(env, tmp);
+ restore_state_from_spsr(env, tmp);
return 4;
}
/* Unknown register. */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 03cd64f..990b084 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -507,9 +507,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->condexec_bits = 0;
}
- // TODO: restore_state_from_spsr()
- env->aarch64 = 1;
- pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
+ /* start IRQ with a clean program state */
+ restore_state_from_spsr(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
env->pc = addr;
cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index eaf6ff8..7a022a6 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -230,7 +230,7 @@ int kvm_arch_get_registers(CPUState *cs)
if (ret) {
return ret;
}
- pstate_write(env, val);
+ restore_state_from_spsr(env, val);
/* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
* QEMU side we keep the current SP in xregs[31] as well.
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 09/10] target-arm/kvm.c: better error reporting
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
` (8 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Alex Bennée, Peter Maydell, Paolo Bonzini,
open list:Overall
From: Alex Bennée <alex@bennee.com>
When we have a problem syncing CP registers between kvm<->qemu it's a
lot more useful to have the names of the registers in the log than just
a random abort() and core dump.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- less verbose log message
- fix checkpatch warnings
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 319784d..72e242d 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -279,6 +279,16 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
memory_region_ref(kd->mr);
}
+static void failed_cpreg_operation(ARMCPU *cpu, uint64_t regidx, int ret,
+ const char *func)
+{
+ uint32_t cpreg_id = kvm_to_cpreg_id(regidx);
+ ARMCPRegInfo *cpreg = g_hash_table_lookup(cpu->cp_regs, &cpreg_id);
+ qemu_log_mask(LOG_UNIMP,
+ "%s: failed (%d) KVM reg op %"PRIx64" (%s)\n",
+ func, ret, regidx, cpreg ? cpreg->name : "unknown");
+}
+
bool write_kvmstate_to_list(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -309,6 +319,7 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
abort();
}
if (ret) {
+ failed_cpreg_operation(cpu, regidx, ret, __func__);
ok = false;
}
}
@@ -345,6 +356,7 @@ bool write_list_to_kvmstate(ARMCPU *cpu)
* "you tried to set a register which is constant with
* a different value from what it actually contains".
*/
+ failed_cpreg_operation(cpu, regidx, ret, __func__);
ok = false;
}
}
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 09/10] target-arm/kvm.c: better error reporting
@ 2014-07-10 15:50 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Alex Bennée, Alex Bennée,
open list:Overall, Paolo Bonzini
From: Alex Bennée <alex@bennee.com>
When we have a problem syncing CP registers between kvm<->qemu it's a
lot more useful to have the names of the registers in the log than just
a random abort() and core dump.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- less verbose log message
- fix checkpatch warnings
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 319784d..72e242d 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -279,6 +279,16 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
memory_region_ref(kd->mr);
}
+static void failed_cpreg_operation(ARMCPU *cpu, uint64_t regidx, int ret,
+ const char *func)
+{
+ uint32_t cpreg_id = kvm_to_cpreg_id(regidx);
+ ARMCPRegInfo *cpreg = g_hash_table_lookup(cpu->cp_regs, &cpreg_id);
+ qemu_log_mask(LOG_UNIMP,
+ "%s: failed (%d) KVM reg op %"PRIx64" (%s)\n",
+ func, ret, regidx, cpreg ? cpreg->name : "unknown");
+}
+
bool write_kvmstate_to_list(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -309,6 +319,7 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
abort();
}
if (ret) {
+ failed_cpreg_operation(cpu, regidx, ret, __func__);
ok = false;
}
}
@@ -345,6 +356,7 @@ bool write_list_to_kvmstate(ARMCPU *cpu)
* "you tried to set a register which is constant with
* a different value from what it actually contains".
*/
+ failed_cpreg_operation(cpu, regidx, ret, __func__);
ok = false;
}
}
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 10/10] target-arm/kvm: make reg sync code common between kvm32/64
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
@ 2014-07-10 15:50 ` Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
` (8 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Peter Maydell, Paolo Bonzini, open list:Overall
Before we launch a guest we query KVM for the list of "co-processor"
registers it knows about which is used later for save/restore of machine
state. The logic is identical for both 32-bit and 64-bit so I've moved
it all into the common code and simplified the exit paths (as failure =>
exit).
This list may well have more registers than are known by the TCG
emulation which is not necessarily a problem but it does stop us from
migrating between KVM and TCG hosted guests. I've added some additional
checking to report those registers under -d unimp.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 72e242d..a2895dc 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -21,6 +21,7 @@
#include "sysemu/kvm.h"
#include "kvm_arm.h"
#include "cpu.h"
+#include "internals.h"
#include "hw/arm/arm.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
@@ -289,6 +290,130 @@ static void failed_cpreg_operation(ARMCPU *cpu, uint64_t regidx, int ret,
func, ret, regidx, cpreg ? cpreg->name : "unknown");
}
+static int compare_u64(const void *a, const void *b)
+{
+ if (*(uint64_t *)a > *(uint64_t *)b) {
+ return 1;
+ }
+ if (*(uint64_t *)a < *(uint64_t *)b) {
+ return -1;
+ }
+ return 0;
+}
+
+static bool reg_syncs_via_tuple_list(uint64_t regidx)
+{
+ /* Return true if the regidx is a register we should synchronize
+ * via the cpreg_tuples array (ie is not a core reg we sync by
+ * hand in kvm_arch_get/put_registers())
+ */
+ switch (regidx & KVM_REG_ARM_COPROC_MASK) {
+ case KVM_REG_ARM_CORE:
+#ifdef KVM_REG_ARM_VFP
+ case KVM_REG_ARM_VFP:
+#endif
+ return false;
+ default:
+ return true;
+ }
+}
+
+/*
+ * Fetch a list of registers from KVM that we will need to be able to
+ * migrate the state. These registers may or may not map onto real
+ * hardware registers but either way QEMU uses the KVM_GET/SET_ONE_REG
+ * api to copy their state back and forth when required.
+ *
+ * For migration between KVM and TCG both models need to understand
+ * the same set of registers.
+ *
+ * If we exit due to failure we would leak memory but we'll be exiting
+ * anyway so the return path is kept simple.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs)
+{
+ struct kvm_reg_list rl;
+ struct kvm_reg_list *rlp;
+ int i, j, ret, arraylen;
+ ARMCPU *cpu = ARM_CPU(cs);
+
+ /* Populate the cpreg list based on the kernel's idea
+ * of what registers exist (and throw away the TCG-created list).
+ */
+ rl.n = 0;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
+ if (ret != -E2BIG) {
+ return FALSE;
+ }
+
+ rlp = g_malloc(sizeof(struct kvm_reg_list) + (rl.n * sizeof(uint64_t)));
+ rlp->n = rl.n;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
+ if (ret) {
+ fprintf(stderr, "%s: failed to get register list\n", __func__);
+ return FALSE;
+ }
+ /* Sort the list we get back from the kernel, since cpreg_tuples
+ * must be in strictly ascending order.
+ */
+ qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
+
+ /* Count how many of these registers we'll actually sync through
+ * the cpreg_indexes mechanism and overwrite the existing TCG
+ * built array of registers.
+ */
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ uint64_t regidx = rlp->reg[i];
+ if (reg_syncs_via_tuple_list(regidx)) {
+ gboolean found = FALSE;
+ arraylen++;
+ for (j = 0; j < cpu->cpreg_array_len; j++) {
+ if (regidx == cpu->cpreg_indexes[j]) {
+ found = TRUE;
+ break;
+ }
+ }
+ if (!found) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: TCG missing definition of %"PRIx64"\n",
+ __func__, regidx);
+ }
+ }
+ }
+
+ cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
+ cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
+ cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
+ arraylen);
+ cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
+ arraylen);
+ cpu->cpreg_array_len = arraylen;
+ cpu->cpreg_vmstate_array_len = arraylen;
+
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ uint64_t regidx = rlp->reg[i];
+ if (!reg_syncs_via_tuple_list(regidx)) {
+ continue;
+ }
+ switch (regidx & KVM_REG_SIZE_MASK) {
+ case KVM_REG_SIZE_U32:
+ case KVM_REG_SIZE_U64:
+ break;
+ default:
+ fprintf(stderr,
+ "%s: un-handled register size (%"PRIx64") in kernel list\n",
+ __func__, regidx);
+ return FALSE;
+ }
+ cpu->cpreg_indexes[arraylen] = regidx;
+ arraylen++;
+ }
+
+ g_assert(cpu->cpreg_array_len == arraylen);
+
+ return TRUE;
+}
+
bool write_kvmstate_to_list(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 39117c7..adfc902 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -138,39 +138,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
return true;
}
-static bool reg_syncs_via_tuple_list(uint64_t regidx)
-{
- /* Return true if the regidx is a register we should synchronize
- * via the cpreg_tuples array (ie is not a core reg we sync by
- * hand in kvm_arch_get/put_registers())
- */
- switch (regidx & KVM_REG_ARM_COPROC_MASK) {
- case KVM_REG_ARM_CORE:
- case KVM_REG_ARM_VFP:
- return false;
- default:
- return true;
- }
-}
-
-static int compare_u64(const void *a, const void *b)
-{
- if (*(uint64_t *)a > *(uint64_t *)b) {
- return 1;
- }
- if (*(uint64_t *)a < *(uint64_t *)b) {
- return -1;
- }
- return 0;
-}
-
int kvm_arch_init_vcpu(CPUState *cs)
{
- int i, ret, arraylen;
+ int i, ret;
uint64_t v;
struct kvm_one_reg r;
- struct kvm_reg_list rl;
- struct kvm_reg_list *rlp;
ARMCPU *cpu = ARM_CPU(cs);
if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
@@ -206,73 +178,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
return -EINVAL;
}
- /* Populate the cpreg list based on the kernel's idea
- * of what registers exist (and throw away the TCG-created list).
- */
- rl.n = 0;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
- if (ret != -E2BIG) {
- return ret;
- }
- rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
- rlp->n = rl.n;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
- if (ret) {
- goto out;
- }
- /* Sort the list we get back from the kernel, since cpreg_tuples
- * must be in strictly ascending order.
- */
- qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
- continue;
- }
- switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
- case KVM_REG_SIZE_U32:
- case KVM_REG_SIZE_U64:
- break;
- default:
- fprintf(stderr, "Can't handle size of register in kernel list\n");
- ret = -EINVAL;
- goto out;
- }
-
- arraylen++;
- }
-
- cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
- cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
- cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
- arraylen);
- cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
- arraylen);
- cpu->cpreg_array_len = arraylen;
- cpu->cpreg_vmstate_array_len = arraylen;
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- uint64_t regidx = rlp->reg[i];
- if (!reg_syncs_via_tuple_list(regidx)) {
- continue;
- }
- cpu->cpreg_indexes[arraylen] = regidx;
- arraylen++;
+ if (!kvm_arm_sync_register_list(cpu)) {
+ return -EINVAL;
}
- assert(cpu->cpreg_array_len == arraylen);
if (!write_kvmstate_to_list(cpu)) {
/* Shouldn't happen unless kernel is inconsistent about
* what registers exist.
*/
fprintf(stderr, "Initial read of kernel register state failed\n");
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
-
-out:
- g_free(rlp);
- return ret;
}
typedef struct Reg {
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 7a022a6..0e28901 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -102,7 +102,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
return ret;
}
- /* TODO : support for save/restore/reset of system regs via tuple list */
+ kvm_arm_sync_register_list(cs);
return 0;
}
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index af93105..2efd0b7 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -47,6 +47,18 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
uint64_t attr, int dev_fd);
/**
+ * kvm_arm_sync_register_list:
+ * @cs: CPUState
+ *
+ * Before migration can occur we need to sync the list of additional
+ * registers that KVM knows about which we can then use when we start
+ * doing migration. It's OK for the TCG side not to know about
+ * registers exposed by the KVM side although this will break
+ * migration between the two VM types.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs);
+
+/**
* write_list_to_kvmstate:
* @cpu: ARMCPU
*
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 10/10] target-arm/kvm: make reg sync code common between kvm32/64
@ 2014-07-10 15:50 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2014-07-10 15:50 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Alex Bennée, open list:Overall, Paolo Bonzini
Before we launch a guest we query KVM for the list of "co-processor"
registers it knows about which is used later for save/restore of machine
state. The logic is identical for both 32-bit and 64-bit so I've moved
it all into the common code and simplified the exit paths (as failure =>
exit).
This list may well have more registers than are known by the TCG
emulation which is not necessarily a problem but it does stop us from
migrating between KVM and TCG hosted guests. I've added some additional
checking to report those registers under -d unimp.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 72e242d..a2895dc 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -21,6 +21,7 @@
#include "sysemu/kvm.h"
#include "kvm_arm.h"
#include "cpu.h"
+#include "internals.h"
#include "hw/arm/arm.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
@@ -289,6 +290,130 @@ static void failed_cpreg_operation(ARMCPU *cpu, uint64_t regidx, int ret,
func, ret, regidx, cpreg ? cpreg->name : "unknown");
}
+static int compare_u64(const void *a, const void *b)
+{
+ if (*(uint64_t *)a > *(uint64_t *)b) {
+ return 1;
+ }
+ if (*(uint64_t *)a < *(uint64_t *)b) {
+ return -1;
+ }
+ return 0;
+}
+
+static bool reg_syncs_via_tuple_list(uint64_t regidx)
+{
+ /* Return true if the regidx is a register we should synchronize
+ * via the cpreg_tuples array (ie is not a core reg we sync by
+ * hand in kvm_arch_get/put_registers())
+ */
+ switch (regidx & KVM_REG_ARM_COPROC_MASK) {
+ case KVM_REG_ARM_CORE:
+#ifdef KVM_REG_ARM_VFP
+ case KVM_REG_ARM_VFP:
+#endif
+ return false;
+ default:
+ return true;
+ }
+}
+
+/*
+ * Fetch a list of registers from KVM that we will need to be able to
+ * migrate the state. These registers may or may not map onto real
+ * hardware registers but either way QEMU uses the KVM_GET/SET_ONE_REG
+ * api to copy their state back and forth when required.
+ *
+ * For migration between KVM and TCG both models need to understand
+ * the same set of registers.
+ *
+ * If we exit due to failure we would leak memory but we'll be exiting
+ * anyway so the return path is kept simple.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs)
+{
+ struct kvm_reg_list rl;
+ struct kvm_reg_list *rlp;
+ int i, j, ret, arraylen;
+ ARMCPU *cpu = ARM_CPU(cs);
+
+ /* Populate the cpreg list based on the kernel's idea
+ * of what registers exist (and throw away the TCG-created list).
+ */
+ rl.n = 0;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
+ if (ret != -E2BIG) {
+ return FALSE;
+ }
+
+ rlp = g_malloc(sizeof(struct kvm_reg_list) + (rl.n * sizeof(uint64_t)));
+ rlp->n = rl.n;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
+ if (ret) {
+ fprintf(stderr, "%s: failed to get register list\n", __func__);
+ return FALSE;
+ }
+ /* Sort the list we get back from the kernel, since cpreg_tuples
+ * must be in strictly ascending order.
+ */
+ qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
+
+ /* Count how many of these registers we'll actually sync through
+ * the cpreg_indexes mechanism and overwrite the existing TCG
+ * built array of registers.
+ */
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ uint64_t regidx = rlp->reg[i];
+ if (reg_syncs_via_tuple_list(regidx)) {
+ gboolean found = FALSE;
+ arraylen++;
+ for (j = 0; j < cpu->cpreg_array_len; j++) {
+ if (regidx == cpu->cpreg_indexes[j]) {
+ found = TRUE;
+ break;
+ }
+ }
+ if (!found) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: TCG missing definition of %"PRIx64"\n",
+ __func__, regidx);
+ }
+ }
+ }
+
+ cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
+ cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
+ cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
+ arraylen);
+ cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
+ arraylen);
+ cpu->cpreg_array_len = arraylen;
+ cpu->cpreg_vmstate_array_len = arraylen;
+
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ uint64_t regidx = rlp->reg[i];
+ if (!reg_syncs_via_tuple_list(regidx)) {
+ continue;
+ }
+ switch (regidx & KVM_REG_SIZE_MASK) {
+ case KVM_REG_SIZE_U32:
+ case KVM_REG_SIZE_U64:
+ break;
+ default:
+ fprintf(stderr,
+ "%s: un-handled register size (%"PRIx64") in kernel list\n",
+ __func__, regidx);
+ return FALSE;
+ }
+ cpu->cpreg_indexes[arraylen] = regidx;
+ arraylen++;
+ }
+
+ g_assert(cpu->cpreg_array_len == arraylen);
+
+ return TRUE;
+}
+
bool write_kvmstate_to_list(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 39117c7..adfc902 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -138,39 +138,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
return true;
}
-static bool reg_syncs_via_tuple_list(uint64_t regidx)
-{
- /* Return true if the regidx is a register we should synchronize
- * via the cpreg_tuples array (ie is not a core reg we sync by
- * hand in kvm_arch_get/put_registers())
- */
- switch (regidx & KVM_REG_ARM_COPROC_MASK) {
- case KVM_REG_ARM_CORE:
- case KVM_REG_ARM_VFP:
- return false;
- default:
- return true;
- }
-}
-
-static int compare_u64(const void *a, const void *b)
-{
- if (*(uint64_t *)a > *(uint64_t *)b) {
- return 1;
- }
- if (*(uint64_t *)a < *(uint64_t *)b) {
- return -1;
- }
- return 0;
-}
-
int kvm_arch_init_vcpu(CPUState *cs)
{
- int i, ret, arraylen;
+ int i, ret;
uint64_t v;
struct kvm_one_reg r;
- struct kvm_reg_list rl;
- struct kvm_reg_list *rlp;
ARMCPU *cpu = ARM_CPU(cs);
if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
@@ -206,73 +178,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
return -EINVAL;
}
- /* Populate the cpreg list based on the kernel's idea
- * of what registers exist (and throw away the TCG-created list).
- */
- rl.n = 0;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
- if (ret != -E2BIG) {
- return ret;
- }
- rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
- rlp->n = rl.n;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
- if (ret) {
- goto out;
- }
- /* Sort the list we get back from the kernel, since cpreg_tuples
- * must be in strictly ascending order.
- */
- qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
- continue;
- }
- switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
- case KVM_REG_SIZE_U32:
- case KVM_REG_SIZE_U64:
- break;
- default:
- fprintf(stderr, "Can't handle size of register in kernel list\n");
- ret = -EINVAL;
- goto out;
- }
-
- arraylen++;
- }
-
- cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
- cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
- cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
- arraylen);
- cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
- arraylen);
- cpu->cpreg_array_len = arraylen;
- cpu->cpreg_vmstate_array_len = arraylen;
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- uint64_t regidx = rlp->reg[i];
- if (!reg_syncs_via_tuple_list(regidx)) {
- continue;
- }
- cpu->cpreg_indexes[arraylen] = regidx;
- arraylen++;
+ if (!kvm_arm_sync_register_list(cpu)) {
+ return -EINVAL;
}
- assert(cpu->cpreg_array_len == arraylen);
if (!write_kvmstate_to_list(cpu)) {
/* Shouldn't happen unless kernel is inconsistent about
* what registers exist.
*/
fprintf(stderr, "Initial read of kernel register state failed\n");
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
-
-out:
- g_free(rlp);
- return ret;
}
typedef struct Reg {
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 7a022a6..0e28901 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -102,7 +102,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
return ret;
}
- /* TODO : support for save/restore/reset of system regs via tuple list */
+ kvm_arm_sync_register_list(cs);
return 0;
}
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index af93105..2efd0b7 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -47,6 +47,18 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
uint64_t attr, int dev_fd);
/**
+ * kvm_arm_sync_register_list:
+ * @cs: CPUState
+ *
+ * Before migration can occur we need to sync the list of additional
+ * registers that KVM knows about which we can then use when we start
+ * doing migration. It's OK for the TCG side not to know about
+ * registers exposed by the KVM side although this will break
+ * migration between the two VM types.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs);
+
+/**
* write_list_to_kvmstate:
* @cpu: ARMCPU
*
--
2.0.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly Alex Bennée
@ 2014-07-15 13:39 ` Riku Voipio
0 siblings, 0 replies; 27+ messages in thread
From: Riku Voipio @ 2014-07-15 13:39 UTC (permalink / raw)
To: Alex Bennée; +Cc: Riku Voipio, qemu-devel
On Thu, Jul 10, 2014 at 04:50:03PM +0100, Alex Bennée wrote:
> As we only need to manipulate the single flag do it directly though env.
Acked-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
>
> v2:
> - remove unused cpsr
> - the direct flag setting seems a little hacky?
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8848e15..9101541 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -468,7 +468,7 @@ void cpu_loop(CPUX86State *env)
> static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
> {
> uint64_t oldval, newval, val;
> - uint32_t addr, cpsr;
> + uint32_t addr;
> target_siginfo_t info;
>
> /* Based on the 32 bit code in do_kernel_trap */
> @@ -478,7 +478,6 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
> operations. However things like ldrex/strex are much harder so
> there's not much point trying. */
> start_exclusive();
> - cpsr = cpsr_read(env);
> addr = env->regs[2];
>
> if (get_user_u64(oldval, env->regs[0])) {
> @@ -505,12 +504,11 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
> };
>
> env->regs[0] = 0;
> - cpsr |= CPSR_C;
> + env->CF = 1;
> } else {
> env->regs[0] = -1;
> - cpsr &= ~CPSR_C;
> + env->CF = 0;
> }
> - cpsr_write(env, cpsr, CPSR_C);
> end_exclusive();
> return;
>
> @@ -533,7 +531,6 @@ static int
> do_kernel_trap(CPUARMState *env)
> {
> uint32_t addr;
> - uint32_t cpsr;
> uint32_t val;
>
> switch (env->regs[15]) {
> @@ -546,7 +543,6 @@ do_kernel_trap(CPUARMState *env)
> operations. However things like ldrex/strex are much harder so
> there's not much point trying. */
> start_exclusive();
> - cpsr = save_state_to_spsr(env);
> addr = env->regs[2];
> /* FIXME: This should SEGV if the access fails. */
> if (get_user_u32(val, addr))
> @@ -556,12 +552,11 @@ do_kernel_trap(CPUARMState *env)
> /* FIXME: Check for segfaults. */
> put_user_u32(val, addr);
> env->regs[0] = 0;
> - cpsr |= CPSR_C;
> + env->CF = 1;
> } else {
> env->regs[0] = -1;
> - cpsr &= ~CPSR_C;
> + env->CF = 0;
> }
> - cpsr_write(env, cpsr, CPSR_C);
> end_exclusive();
> break;
> case 0xffff0fe0: /* __kernel_get_tls */
> --
> 2.0.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls Alex Bennée
@ 2014-07-15 13:40 ` Riku Voipio
2014-08-04 12:59 ` Peter Maydell
1 sibling, 0 replies; 27+ messages in thread
From: Riku Voipio @ 2014-07-15 13:40 UTC (permalink / raw)
To: Alex Bennée; +Cc: Peter Maydell, qemu-devel
On Thu, Jul 10, 2014 at 04:50:01PM +0100, Alex Bennée wrote:
> Use the unified save_state_to_spsr. I've also updated the interrupt
> helpers to restore via the restore_state_from_spsr() functions. In the
> aarch32 case this also needs to call switch_mode() to do the appropriate
> fiddling.
For the linux-user part,
Acked-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
>
> v2
> - include xpsr_read conversions
> - checkpatch fixes
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60777fe..577e1d3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -321,7 +321,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
> (*regs)[14] = tswapreg(env->regs[14]);
> (*regs)[15] = tswapreg(env->regs[15]);
>
> - (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
> + (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));
> (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
> }
>
> @@ -509,7 +509,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
> (*regs)[i] = tswapreg(env->xregs[i]);
> }
> (*regs)[32] = tswapreg(env->pc);
> - (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
> + (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
> }
>
> #define USE_ELF_CORE_DUMP
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b453a39..8848e15 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -546,7 +546,7 @@ do_kernel_trap(CPUARMState *env)
> operations. However things like ldrex/strex are much harder so
> there's not much point trying. */
> start_exclusive();
> - cpsr = cpsr_read(env);
> + cpsr = save_state_to_spsr(env);
> addr = env->regs[2];
> /* FIXME: This should SEGV if the access fails. */
> if (get_user_u32(val, addr))
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 86fae3d..9c6727b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1224,7 +1224,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
> }
> __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
> __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
> - __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
> + __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
>
> __put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
>
> @@ -1572,7 +1572,7 @@ setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
> __put_user(env->regs[14], &sc->arm_lr);
> __put_user(env->regs[15], &sc->arm_pc);
> #ifdef TARGET_CONFIG_CPU_32
> - __put_user(cpsr_read(env), &sc->arm_cpsr);
> + __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
> #endif
>
> __put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
> @@ -1604,7 +1604,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
> abi_ulong handler = ka->_sa_handler;
> abi_ulong retcode;
> int thumb = handler & 1;
> - uint32_t cpsr = cpsr_read(env);
> + uint32_t cpsr = save_state_to_spsr(env);
>
> cpsr &= ~CPSR_IT;
> if (thumb) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fe4d4f3..3f23167 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -480,30 +480,8 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> #define PSTATE_MODE_EL1t 4
> #define PSTATE_MODE_EL0t 0
>
> -/* ARMv8 ARM D1.7 Process state, PSTATE
> - *
> - * 31 28 27 24 23 22 21 20 22 21 20 19 16 15 8 7 5 4 0
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - *
> - * The PSTATE is an abstraction of a number of Return the current
> - * PSTATE value. This is only valid for A64 hardware although can be
> - * read when in AArch32 mode.
> - */
> -static inline uint32_t pstate_read(CPUARMState *env)
> -{
> - int ZF;
> -
> - g_assert(is_a64(env));
> -
> - ZF = (env->ZF == 0);
> - return (env->NF & 0x80000000) | (ZF << 30)
> - | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> - | env->pstate | env->daif;
> -}
> -
> -/* Update the current PSTATE value. This doesn't include nRW which is */
> +/* Update the current PSTATE value. This doesn't include nRW which
> + * indicates if we are in 64 or 32 bit mode */
> static inline void pstate_write(CPUARMState *env, uint32_t val)
> {
> g_assert(is_a64(env));
> @@ -520,26 +498,10 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
> *
> * Unlike the above PSTATE implementation these functions will attempt
> * to switch processor mode when the M[4:0] bits are set.
> - */
> -uint32_t cpsr_read(CPUARMState *env);
> -/* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
> + *
> + * Note that some bits of mask must be all-set or all-clear. */
> void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
>
> -/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
> -static inline uint32_t xpsr_read(CPUARMState *env)
> -{
> - int ZF;
> -
> - g_assert(!is_a64(env));
> -
> - ZF = (env->ZF == 0);
> - return (env->NF & 0x80000000) | (ZF << 30)
> - | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> - | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
> - | ((env->condexec_bits & 0xfc) << 8)
> - | env->v7m.exception;
> -}
> -
> /* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */
> static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
> index 1c34396..ec25f30 100644
> --- a/target-arm/gdbstub.c
> +++ b/target-arm/gdbstub.c
> @@ -53,7 +53,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> return gdb_get_reg32(mem_buf, 0);
> case 25:
> /* CPSR */
> - return gdb_get_reg32(mem_buf, cpsr_read(env));
> + return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
> }
> /* Unknown register. */
> return 0;
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 8f3b8d1..76d1b91 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -35,7 +35,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> case 32:
> return gdb_get_reg64(mem_buf, env->pc);
> case 33:
> - return gdb_get_reg32(mem_buf, pstate_read(env));
> + return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
> }
> /* Unknown register. */
> return 0;
> @@ -62,7 +62,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> env->pc = tmp;
> return 8;
> case 33:
> - /* CPSR */
> + /* SPSR */
> pstate_write(env, tmp);
> return 4;
> }
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index ec1fef5..03cd64f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -445,6 +445,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> CPUARMState *env = &cpu->env;
> target_ulong addr = env->cp15.vbar_el[1];
> int i;
> + uint32_t spsr = save_state_to_spsr(env);
>
> if (arm_current_pl(env) == 0) {
> if (env->aarch64) {
> @@ -452,7 +453,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> } else {
> addr += 0x600;
> }
> - } else if (pstate_read(env) & PSTATE_SP) {
> + } else if (spsr & PSTATE_SP) {
> addr += 0x200;
> }
>
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> }
>
> if (is_a64(env)) {
> - env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
> + env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
> env->sp_el[arm_current_pl(env)] = env->xregs[31];
> env->xregs[31] = env->sp_el[1];
> env->elr_el[1] = env->pc;
> } else {
> - env->banked_spsr[0] = cpsr_read(env);
> + env->banked_spsr[0] = spsr;
> if (!env->thumb) {
> env->cp15.esr_el[1] |= 1 << 25;
> }
> @@ -506,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> env->condexec_bits = 0;
> }
>
> + // TODO: restore_state_from_spsr()
> env->aarch64 = 1;
> pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d343856..030bcdd 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2990,17 +2990,6 @@ static int bad_mode_switch(CPUARMState *env, int mode)
> }
> }
>
> -uint32_t cpsr_read(CPUARMState *env)
> -{
> - int ZF;
> - ZF = (env->ZF == 0);
> - return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
> - (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> - | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
> - | ((env->condexec_bits & 0xfc) << 8)
> - | (env->GE << 16) | (env->daif & CPSR_AIF);
> -}
> -
> void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> if (mask & CPSR_NZCV) {
> @@ -3278,7 +3267,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> - uint32_t xpsr = xpsr_read(env);
> + uint32_t xpsr = save_state_to_spsr(env);
> uint32_t lr;
> uint32_t addr;
>
> @@ -3479,7 +3468,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
> addr += env->cp15.vbar_el[1];
> }
> switch_mode (env, new_mode);
> - env->spsr = cpsr_read(env);
> + env->spsr = save_state_to_spsr(env);
> + /* TODO: restore_state_from_spsr */
> /* Clear IT bits. */
> env->condexec_bits = 0;
> /* Switch to the new mode, and to the correct instruction set. */
> @@ -4212,19 +4202,19 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>
> switch (reg) {
> case 0: /* APSR */
> - return xpsr_read(env) & 0xf8000000;
> + return save_state_to_spsr(env) & 0xf8000000;
> case 1: /* IAPSR */
> - return xpsr_read(env) & 0xf80001ff;
> + return save_state_to_spsr(env) & 0xf80001ff;
> case 2: /* EAPSR */
> - return xpsr_read(env) & 0xff00fc00;
> + return save_state_to_spsr(env) & 0xff00fc00;
> case 3: /* xPSR */
> - return xpsr_read(env) & 0xff00fdff;
> + return save_state_to_spsr(env) & 0xff00fdff;
> case 5: /* IPSR */
> - return xpsr_read(env) & 0x000001ff;
> + return save_state_to_spsr(env) & 0x000001ff;
> case 6: /* EPSR */
> - return xpsr_read(env) & 0x0700fc00;
> + return save_state_to_spsr(env) & 0x0700fc00;
> case 7: /* IEPSR */
> - return xpsr_read(env) & 0x0700edff;
> + return save_state_to_spsr(env) & 0x0700edff;
> case 8: /* MSP */
> return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
> case 9: /* PSP */
> diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
> index 5ec4eb1..789f52f 100644
> --- a/target-arm/kvm32.c
> +++ b/target-arm/kvm32.c
> @@ -384,7 +384,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> }
>
> /* Special cases which aren't a single CPUARMState field */
> - cpsr = cpsr_read(env);
> + cpsr = save_state_to_spsr(env);
> r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
> r.addr = (uintptr_t)(&cpsr);
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 5d217ca..eaf6ff8 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -153,7 +153,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> }
>
> /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
> - val = pstate_read(env);
> + val = save_state_to_spsr(env);
> reg.id = AARCH64_CORE_REG(regs.pstate);
> reg.addr = (uintptr_t) &val;
> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 9c1ef52..052a4bd 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -258,7 +258,7 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
>
> uint32_t HELPER(cpsr_read)(CPUARMState *env)
> {
> - return cpsr_read(env) & ~CPSR_EXEC;
> + return save_state_to_spsr(env) & ~CPSR_EXEC;
> }
>
> void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
> @@ -386,10 +386,9 @@ void HELPER(exception_return)(CPUARMState *env)
>
> if (spsr & PSTATE_nRW) {
> /* TODO: We currently assume EL1/2/3 are running in AArch64. */
> - env->aarch64 = 0;
> new_el = 0;
> - env->uncached_cpsr = 0x10;
> - cpsr_write(env, spsr, ~0);
> + switch_mode(env, spsr & CPSR_M);
> +
> for (i = 0; i < 15; i++) {
> env->regs[i] = env->xregs[i];
> }
> @@ -412,11 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
> /* Return to EL0 with M[0] bit set */
> goto illegal_return;
> }
> - env->aarch64 = 1;
> - pstate_write(env, spsr);
> env->xregs[31] = env->sp_el[new_el];
> env->pc = env->elr_el[cur_el];
> }
> + /* This will set env->aarch64 as appropriate */
> + restore_state_from_spsr(env, spsr);
>
> return;
>
> @@ -431,8 +430,8 @@ illegal_return:
> env->pstate |= PSTATE_IL;
> env->pc = env->elr_el[cur_el];
> spsr &= PSTATE_NZCV | PSTATE_DAIF;
> - spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> - pstate_write(env, spsr);
> + spsr |= save_state_to_spsr(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> + restore_state_from_spsr(env, spsr);
> }
>
> /* ??? Flag setting arithmetic is awkward because we need to do comparisons.
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 33b5025..3b4a676 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -126,7 +126,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> - uint32_t psr = pstate_read(env);
> + uint32_t psr = save_state_to_spsr(env);
> int i;
>
> cpu_fprintf(f, "PC=%016"PRIx64" SP=%016"PRIx64"\n",
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index cf4e767..8427396 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -11192,7 +11192,7 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
> else
> cpu_fprintf(f, " ");
> }
> - psr = cpsr_read(env);
> + psr = save_state_to_spsr(env);
> cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",
> psr,
> psr & (1 << 31) ? 'N' : '-',
> --
> 2.0.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes Alex Bennée
@ 2014-07-15 13:41 ` Riku Voipio
0 siblings, 0 replies; 27+ messages in thread
From: Riku Voipio @ 2014-07-15 13:41 UTC (permalink / raw)
To: Alex Bennée; +Cc: Riku Voipio, qemu-devel
On Thu, Jul 10, 2014 at 04:50:02PM +0100, Alex Bennée wrote:
> This is a pre-cursor to removing the cpsr_write function completely from
> the code base. set_condition_codes() only affects the integer condition
> flags.
Acked-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
>
> v2
> - fix nwfpe set_condition_codes
>
> diff --git a/linux-user/arm/nwfpe/fpa11.h b/linux-user/arm/nwfpe/fpa11.h
> index bb9ac65..c7883d0 100644
> --- a/linux-user/arm/nwfpe/fpa11.h
> +++ b/linux-user/arm/nwfpe/fpa11.h
> @@ -108,7 +108,7 @@ static inline void writeRegister(unsigned int x, unsigned int y)
>
> static inline void writeConditionCodes(unsigned int x)
> {
> - cpsr_write(user_registers,x,CPSR_NZCV);
> + set_condition_codes(user_registers, x);
> }
>
> #define ARM_REG_PC 15
> --
> 2.0.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write Alex Bennée
@ 2014-07-15 13:43 ` Riku Voipio
2014-08-04 13:01 ` Peter Maydell
1 sibling, 0 replies; 27+ messages in thread
From: Riku Voipio @ 2014-07-15 13:43 UTC (permalink / raw)
To: Alex Bennée; +Cc: Peter Maydell, Riku Voipio, qemu-devel
On Thu, Jul 10, 2014 at 04:50:04PM +0100, Alex Bennée wrote:
> And use the new machinery to to save and restore program state. The old
> cpsr_write function did some special handling for mode switches which
> has been moved into the helper function.
Again for the linux-user part,
Acked-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2:
> - rebase
> - add mask helper function
> - checkpatch fixes
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 9101541..5f7cc31 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4184,7 +4184,7 @@ int main(int argc, char **argv, char **envp)
> #elif defined(TARGET_ARM)
> {
> int i;
> - cpsr_write(env, regs->uregs[16], 0xffffffff);
> + restore_state_from_spsr(env, regs->uregs[16]);
> for(i = 0; i < 16; i++) {
> env->regs[i] = regs->uregs[i];
> }
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 9c6727b..b6f9ef4 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1599,38 +1599,39 @@ get_sigframe(struct target_sigaction *ka, CPUARMState *regs, int framesize)
>
> static void
> setup_return(CPUARMState *env, struct target_sigaction *ka,
> - abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
> + abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
> {
> - abi_ulong handler = ka->_sa_handler;
> - abi_ulong retcode;
> - int thumb = handler & 1;
> - uint32_t cpsr = save_state_to_spsr(env);
> + abi_ulong handler = ka->_sa_handler;
> + abi_ulong retcode;
> + int thumb = handler & 1;
> + uint32_t cpsr = save_state_to_spsr(env);
>
> - cpsr &= ~CPSR_IT;
> - if (thumb) {
> - cpsr |= CPSR_T;
> - } else {
> - cpsr &= ~CPSR_T;
> - }
> + cpsr &= ~CPSR_IT;
> + if (thumb) {
> + cpsr |= CPSR_T;
> + } else {
> + cpsr &= ~CPSR_T;
> + }
>
> - if (ka->sa_flags & TARGET_SA_RESTORER) {
> - retcode = ka->sa_restorer;
> - } else {
> - unsigned int idx = thumb;
> + if (ka->sa_flags & TARGET_SA_RESTORER) {
> + retcode = ka->sa_restorer;
> + } else {
> + unsigned int idx = thumb;
>
> - if (ka->sa_flags & TARGET_SA_SIGINFO)
> - idx += 2;
> + if (ka->sa_flags & TARGET_SA_SIGINFO) {
> + idx += 2;
> + }
>
> - __put_user(retcodes[idx], rc);
> + __put_user(retcodes[idx], rc);
>
> - retcode = rc_addr + thumb;
> - }
> + retcode = rc_addr + thumb;
> + }
>
> - env->regs[0] = usig;
> - env->regs[13] = frame_addr;
> - env->regs[14] = retcode;
> - env->regs[15] = handler & (thumb ? ~1 : ~3);
> - cpsr_write(env, cpsr, 0xffffffff);
> + env->regs[0] = usig;
> + env->regs[13] = frame_addr;
> + env->regs[14] = retcode;
> + env->regs[15] = handler & (thumb ? ~1 : ~3);
> + restore_state_from_spsr(env, cpsr);
> }
>
> static abi_ulong *setup_sigframe_v2_vfp(abi_ulong *regspace, CPUARMState *env)
> @@ -1858,12 +1859,14 @@ restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
> __get_user(env->regs[15], &sc->arm_pc);
> #ifdef TARGET_CONFIG_CPU_32
> __get_user(cpsr, &sc->arm_cpsr);
> - cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
> + restore_state_from_masked_spsr(env,
> + (CPSR_USER | CPSR_EXEC),
> + cpsr);
> #endif
>
> - err |= !valid_user_regs(env);
> + err |= !valid_user_regs(env);
>
> - return err;
> + return err;
> }
>
> static long do_sigreturn_v1(CPUARMState *env)
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 3f23167..b56f1a8 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -795,6 +795,15 @@ static inline void restore_state_from_spsr(CPUARMState *env,
> }
> }
>
> +/* Restore a few masked bits of the program state */
> +static inline void restore_state_from_masked_spsr(CPUARMState *env,
> + uint32_t mask,
> + uint32_t saved_state)
> +{
> + uint32_t spsr = (save_state_to_spsr(env) & ~mask);
> + spsr |= (saved_state & mask);
> + return restore_state_from_spsr(env, spsr);
> +}
>
> void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>
> diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
> index ec25f30..5e60589 100644
> --- a/target-arm/gdbstub.c
> +++ b/target-arm/gdbstub.c
> @@ -93,8 +93,12 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> }
> return 4;
> case 25:
> - /* CPSR */
> - cpsr_write(env, tmp, 0xffffffff);
> + /* CPSR
> + * FIXME?: as restore_state_from_spsr() doesn't do aarch32
> + * special mode fixups this may break. However GDB doesn't
> + * seem to be able to handle tracing over a mode switch anyway
> + */
> + restore_state_from_spsr(env, tmp);
> return 4;
> }
> /* Unknown register. */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 030bcdd..6d755c0 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2970,68 +2970,6 @@ void arm_cp_reset_ignore(CPUARMState *env, const ARMCPRegInfo *opaque)
> /* Helper coprocessor reset function for do-nothing-on-reset registers */
> }
>
> -static int bad_mode_switch(CPUARMState *env, int mode)
> -{
> - /* Return true if it is not valid for us to switch to
> - * this CPU mode (ie all the UNPREDICTABLE cases in
> - * the ARM ARM CPSRWriteByInstr pseudocode).
> - */
> - switch (mode) {
> - case ARM_CPU_MODE_USR:
> - case ARM_CPU_MODE_SYS:
> - case ARM_CPU_MODE_SVC:
> - case ARM_CPU_MODE_ABT:
> - case ARM_CPU_MODE_UND:
> - case ARM_CPU_MODE_IRQ:
> - case ARM_CPU_MODE_FIQ:
> - return 0;
> - default:
> - return 1;
> - }
> -}
> -
> -void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> -{
> - if (mask & CPSR_NZCV) {
> - env->ZF = (~val) & CPSR_Z;
> - env->NF = val;
> - env->CF = (val >> 29) & 1;
> - env->VF = (val << 3) & 0x80000000;
> - }
> - if (mask & CPSR_Q)
> - env->QF = ((val & CPSR_Q) != 0);
> - if (mask & CPSR_T)
> - env->thumb = ((val & CPSR_T) != 0);
> - if (mask & CPSR_IT_0_1) {
> - env->condexec_bits &= ~3;
> - env->condexec_bits |= (val >> 25) & 3;
> - }
> - if (mask & CPSR_IT_2_7) {
> - env->condexec_bits &= 3;
> - env->condexec_bits |= (val >> 8) & 0xfc;
> - }
> - if (mask & CPSR_GE) {
> - env->GE = (val >> 16) & 0xf;
> - }
> -
> - env->daif &= ~(CPSR_AIF & mask);
> - env->daif |= val & CPSR_AIF & mask;
> -
> - if ((env->uncached_cpsr ^ val) & mask & CPSR_M) {
> - if (bad_mode_switch(env, val & CPSR_M)) {
> - /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
> - * We choose to ignore the attempt and leave the CPSR M field
> - * untouched.
> - */
> - mask &= ~CPSR_M;
> - } else {
> - switch_mode(env, val & CPSR_M);
> - }
> - }
> - mask &= ~CACHED_CPSR_BITS;
> - env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask);
> -}
> -
> /* Sign/zero extend */
> uint32_t HELPER(sxtb16)(uint32_t x)
> {
> diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
> index 789f52f..39117c7 100644
> --- a/target-arm/kvm32.c
> +++ b/target-arm/kvm32.c
> @@ -464,7 +464,7 @@ int kvm_arch_get_registers(CPUState *cs)
> if (ret) {
> return ret;
> }
> - cpsr_write(env, cpsr, 0xffffffff);
> + restore_state_from_spsr(env, cpsr);
>
> /* Make sure the current mode regs are properly set */
> mode = env->uncached_cpsr & CPSR_M;
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 052a4bd..c17bdba 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -261,9 +261,47 @@ uint32_t HELPER(cpsr_read)(CPUARMState *env)
> return save_state_to_spsr(env) & ~CPSR_EXEC;
> }
>
> +static int bad_mode_switch(CPUARMState *env, int mode)
> +{
> + /* Return true if it is not valid for us to switch to
> + * this CPU mode (ie all the UNPREDICTABLE cases in
> + * the ARM ARM CPSRWriteByInstr pseudocode).
> + */
> + switch (mode) {
> + case ARM_CPU_MODE_USR:
> + case ARM_CPU_MODE_SYS:
> + case ARM_CPU_MODE_SVC:
> + case ARM_CPU_MODE_ABT:
> + case ARM_CPU_MODE_UND:
> + case ARM_CPU_MODE_IRQ:
> + case ARM_CPU_MODE_FIQ:
> + return 0;
> + default:
> + return 1;
> + }
> +}
> +
> void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> - cpsr_write(env, val, mask);
> + uint32_t current_cpsr = save_state_to_spsr(env);
> + uint32_t new_cpsr;
> +
> + /* we may be triggering a mode change */
> + if ((current_cpsr ^ val) & mask & CPSR_M) {
> + if (bad_mode_switch(env, val & CPSR_M)) {
> + /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
> + * We choose to ignore the attempt and leave the CPSR M field
> + * untouched.
> + */
> + mask &= ~CPSR_M;
> + } else {
> + switch_mode(env, val & CPSR_M);
> + }
> + }
> +
> + new_cpsr = current_cpsr & ~mask;
> + new_cpsr |= (val & mask);
> + restore_state_from_spsr(env, new_cpsr);
> }
>
> /* Access to user mode registers from privileged modes. */
> --
> 2.0.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/10] target-arm: remove final users of pstate_write
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 08/10] target-arm: remove final users of pstate_write Alex Bennée
@ 2014-07-15 13:44 ` Riku Voipio
0 siblings, 0 replies; 27+ messages in thread
From: Riku Voipio @ 2014-07-15 13:44 UTC (permalink / raw)
To: Alex Bennée; +Cc: Peter Maydell, Riku Voipio, qemu-devel
On Thu, Jul 10, 2014 at 04:50:05PM +0100, Alex Bennée wrote:
> This converts all users of pstate_write to use the common state
> save/restore functionality.
Acked-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index b6f9ef4..b1958a5 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1274,7 +1274,7 @@ static int target_restore_sigframe(CPUARMState *env,
> __get_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
> __get_user(env->pc, &sf->uc.tuc_mcontext.pc);
> __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
> - pstate_write(env, pstate);
> + restore_state_from_spsr(env, pstate);
>
> __get_user(magic, &aux->fpsimd.head.magic);
> __get_user(size, &aux->fpsimd.head.size);
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 76d1b91..366335a 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -63,7 +63,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> return 8;
> case 33:
> /* SPSR */
> - pstate_write(env, tmp);
> + restore_state_from_spsr(env, tmp);
> return 4;
> }
> /* Unknown register. */
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 03cd64f..990b084 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -507,9 +507,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> env->condexec_bits = 0;
> }
>
> - // TODO: restore_state_from_spsr()
> - env->aarch64 = 1;
> - pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
> + /* start IRQ with a clean program state */
> + restore_state_from_spsr(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>
> env->pc = addr;
> cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index eaf6ff8..7a022a6 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -230,7 +230,7 @@ int kvm_arch_get_registers(CPUState *cs)
> if (ret) {
> return ret;
> }
> - pstate_write(env, val);
> + restore_state_from_spsr(env, val);
>
> /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
> * QEMU side we keep the current SP in xregs[31] as well.
> --
> 2.0.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions Alex Bennée
@ 2014-08-04 12:28 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 12:28 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers
On 10 July 2014 16:49, Alex Bennée <alex.bennee@linaro.org> wrote:
> We have a number of program state saving functions (pstate, cpsr, xpsr)
> which are dependant on the mode the CPU is in. This commit adds a little
> documentation to each function and asserts to defend against incorrect
> use.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2:
> - remove xpsr_state asserts
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 369d472..c2312d0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -475,22 +475,34 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> #define PSTATE_MODE_EL1t 4
> #define PSTATE_MODE_EL0t 0
>
> -/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
> - * interprocessing, so we don't attempt to sync with the cpsr state used by
> - * the 32 bit decoder.
> +/* ARMv8 ARM D1.7 Process state, PSTATE
> + *
> + * 31 28 27 24 23 22 21 20 22 21 20 19 16 15 8 7 5 4 0
> + * +------+------+-------+-----+--------+---+------+------+-----+------+
> + * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
> + * +------+------+-------+-----+--------+---+------+------+-----+------+
This is a bit misleading because the ARM ARM very deliberately
doesn't define a bit format for PSTATE. I suspect what we're actually
dealing with here is the PSTATE in SPSR format.
> + *
> + * The PSTATE is an abstraction of a number of Return the current
Mangled comment text?
> + * PSTATE value. This is only valid for A64 hardware although can be
> + * read when in AArch32 mode.
> */
> static inline uint32_t pstate_read(CPUARMState *env)
> {
> int ZF;
>
> + g_assert(is_a64(env));
> +
> ZF = (env->ZF == 0);
> return (env->NF & 0x80000000) | (ZF << 30)
> | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> | env->pstate | env->daif;
> }
>
> +/* Update the current PSTATE value. This doesn't include nRW which is */
Another truncated comment.
> static inline void pstate_write(CPUARMState *env, uint32_t val)
> {
> + g_assert(is_a64(env));
> +
> env->ZF = (~val) & PSTATE_Z;
> env->NF = val;
> env->CF = (val >> 29) & 1;
> @@ -499,15 +511,22 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
> env->pstate = val & ~CACHED_PSTATE_BITS;
> }
>
> -/* Return the current CPSR value. */
> +/* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
> + *
> + * Unlike the above PSTATE implementation these functions will attempt
> + * to switch processor mode when the M[4:0] bits are set.
> + */
> uint32_t cpsr_read(CPUARMState *env);
> /* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
> void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
>
> -/* Return the current xPSR value. */
> +/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
> static inline uint32_t xpsr_read(CPUARMState *env)
> {
> int ZF;
> +
> + g_assert(!is_a64(env));
> +
> ZF = (env->ZF == 0);
> return (env->NF & 0x80000000) | (ZF << 30)
> | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> @@ -519,6 +538,8 @@ static inline uint32_t xpsr_read(CPUARMState *env)
> /* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */
> static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> + g_assert(!is_a64(env));
> +
> if (mask & CPSR_NZCV) {
> env->ZF = (~val) & CPSR_Z;
> env->NF = val;
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 2b4ce6a..ec1fef5 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -506,8 +506,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> env->condexec_bits = 0;
> }
>
> - pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
> env->aarch64 = 1;
> + pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>
> env->pc = addr;
> cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> --
> 2.0.1
>
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
@ 2014-08-04 12:47 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 12:47 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers
On 10 July 2014 16:49, Alex Bennée <alex.bennee@linaro.org> wrote:
> This adds a universal program state save and restore function. This is
> intended to simplify the migration serialisation functionality and avoid
> special casing depending on the mode of the CPU at serialisation time.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2:
> - reword commentary for restore_state_from_spsr
> - rm commented out code
> - add set_condition_codes for flags
> - add xpsr_read functionality
>
> v3:
> - rebase
> - checkpatch.pl issues
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c2312d0..fe4d4f3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -453,19 +453,24 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> #define PSTATE_SP (1U)
> #define PSTATE_M (0xFU)
> #define PSTATE_nRW (1U << 4)
> +#define PSTATE_T (1U << 5)
> #define PSTATE_F (1U << 6)
> #define PSTATE_I (1U << 7)
> #define PSTATE_A (1U << 8)
> #define PSTATE_D (1U << 9)
> #define PSTATE_IL (1U << 20)
> #define PSTATE_SS (1U << 21)
> +#define PSTATE_Q (1U << 27)
> #define PSTATE_V (1U << 28)
> #define PSTATE_C (1U << 29)
> #define PSTATE_Z (1U << 30)
> #define PSTATE_N (1U << 31)
> #define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
> -#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
> -#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
> +#define PSTATE_AIF (PSTATE_A | PSTATE_I | PSTATE_F)
> +#define PSTATE_DAIF (PSTATE_D | PSTATE_AIF)
> +#define AARCH64_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
> +#define AARCH32_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_Q | PSTATE_AIF \
> + | CACHED_CPSR_BITS)
> /* Mode values for AArch64 */
> #define PSTATE_MODE_EL3h 13
> #define PSTATE_MODE_EL3t 12
> @@ -508,7 +513,7 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
> env->CF = (val >> 29) & 1;
> env->VF = (val << 3) & 0x80000000;
> env->daif = val & PSTATE_DAIF;
> - env->pstate = val & ~CACHED_PSTATE_BITS;
> + env->pstate = val & ~AARCH64_CACHED_PSTATE_BITS;
> }
>
> /* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
> @@ -698,6 +703,137 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
> return arm_feature(env, ARM_FEATURE_AARCH64);
> }
>
> +/* ARMv8 ARM D1.6.4, Saved Program Status Registers
> + *
> + * These are formats used to save program status when exceptions are
> + * taken. There are two forms:
> + *
> + * AArch64 -> AArch64 Exception
> + * 31 30 28 29 27 22 21 20 19 10 9 8 7 6 5 4 3 0
> + * +---+---+---+---+------+----+----+------+---+---+---+---+---+------------+
> + * | N | Z | C | V | RES0 | SS | IL | RES0 | D | A | I | F | 0 | 0 | M[3:0] |
> + * +---+---+---+---+------+----+----+------+---+---+---+---+---+------------+
> + *
> + * AArch32 -> AArch64 Exception
> + * (see also ARMv7-AR ARM B1.3.3 CSPR/SPSR formats)
> + * 31 30 29 28 27 26 25 24 23 22 21 20 19 16
> + * +---+---+---+---+---+---------+---+------+----+----+---------+
> + * | N | Z | C | V | Q | IT[1:0] | J | RES0 | SS | IL | GE[3:0] |
> + * +---+---+---+---+---+---------+---+------+----+----+---------+
> + * 15 10 9 8 7 6 5 4 3 0
> + * +---------+---+---+---+---+---+---+--------+
> + * | IT[7:2] | E | A | I | F | T | 1 | M[3:0] |
> + * +---------+---+---+---+---+---+---+--------+
> + *
> + * M[4] = nRW - 0 = 64bit, 1 = 32bit
> + * Bit definitions for ARMv8 SPSR (PSTATE) format.
> + * Only these are valid when in AArch64 mode; in
> + * AArch32 mode SPSRs are basically CPSR-format.
> + *
> + * Also ARMv7-M ARM B1.4.2, special purpose program status register xPSR
> + */
> +
> +/* Save the program state */
> +static inline uint32_t save_state_to_spsr(CPUARMState *env)
> +{
> + uint32_t final_spsr = 0;
> +
> + /* Calculate integer flags */
> + final_spsr |= (env->NF & 0x80000000) ? PSTATE_N : 0; /* bit 31 */
> + final_spsr |= (env->ZF == 0) ? PSTATE_Z : 0;
> + final_spsr |= env->CF ? PSTATE_C : 0; /* or env->CF << 29 */
> + final_spsr |= (env->VF & 0x80000000) ? PSTATE_V : 0; /* bit 31 */
> +
> + if (is_a64(env)) {
> + /* SS - Software Step */
> + /* IL - Illegal execution state */
Not sure you need specific comments here for SS and IL -- I
would expect those to live in env->pstate with the other
uncached bits.
> + /* DAIF flags - should we mask?*/
> + final_spsr |= (env->daif & PSTATE_DAIF);
We don't mask env->daif in pstate_read() so we don't need to
here either.
> + /* And the final un-cached bits */
> + final_spsr |= (env->pstate & ~AARCH64_CACHED_PSTATE_BITS);
> + } else {
> + /* condexec_bits is split over two parts */
> + final_spsr |= ((env->condexec_bits & 3) << 25);
> + final_spsr |= ((env->condexec_bits & 0xfc) << 8);
> +
> + if (arm_feature(env, ARM_FEATURE_M)) {
> + final_spsr |= (env->thumb << 24); /* alt pos */
This comment is too cryptic.
> + final_spsr |= env->v7m.exception;
> + } else {
> + final_spsr |= env->QF ? PSTATE_Q : 0;
> + /* GE needs shifting into place */
> + final_spsr |= (env->GE << 16);
> + /* AIF is a a subset of DAIF */
More interesting would be to point out that the D bit doesn't
exist in an AArch32 SPSR and it's used for the E bit instead.
> + final_spsr |= (env->daif & PSTATE_AIF);
> + final_spsr |= env->thumb ? PSTATE_T : 0;
> +
> + final_spsr |= PSTATE_nRW;
> +
> + /* And the final un-cached bits */
> + final_spsr |= (env->uncached_cpsr & ~AARCH32_CACHED_PSTATE_BITS);
> + }
> + }
> + return final_spsr;
> +}
> +
> +/* Restore the program state.
> + *
> + * This restores the program execution state including it's current
"its"
> + * execution mode. However validation of mode changes and additional
> + * registers and state that need changing should be done by the
> + * calling function.
> + *
> + * The simple set_condition_codes() function can be used just to
> + * update the cached integer flags which are quite often manipulated
> + * alone.
> + */
> +
> +/* Only update the cached condition codes. */
> +static inline void set_condition_codes(CPUARMState *env, uint32_t new_flags)
> +{
> + /* Process the integer flags directly */
> + env->ZF = (~new_flags) & CPSR_Z;
> + env->NF = new_flags;
> + env->CF = (new_flags >> 29) & 1;
> + env->VF = (new_flags << 3) & 0x80000000;
> +}
> +
> +/* Restore the complete program state */
> +static inline void restore_state_from_spsr(CPUARMState *env,
> + uint32_t saved_state)
> +{
> + set_condition_codes(env, saved_state);
> +
> + /* the rest depends on the mode we end up in */
> + env->aarch64 = (saved_state & PSTATE_nRW) ? 0 : 1;
> +
> + if (is_a64(env)) {
> + env->daif = saved_state & PSTATE_DAIF;
> + env->pstate = (saved_state & ~AARCH64_CACHED_PSTATE_BITS);
> + } else {
> + if (arm_feature(env, ARM_FEATURE_M)) {
> + fprintf(stderr, "%s: M state not yet implmented\n", __func__);
> + } else {
> + env->QF = saved_state & PSTATE_Q ? 1 : 0;
> + /* condexec_bits is split over two parts */
> + env->condexec_bits = (saved_state & CPSR_IT_0_1) >> 25;
> + env->condexec_bits |= (saved_state & CPSR_IT_2_7) >> 8;
> +
> + /* GE needs shifting into place */
> + env->GE = (saved_state >> 16) & 0xf;
> +
> + /* AIF is a a subset of DAIF */
> + env->daif = (saved_state & PSTATE_AIF);
The interesting bit here is that PSTATE.D is not accessible
(directly or indirectly by having it affect behaviour) in AArch32,
so it has an undefined value. We're choosing to set it to zero
here for consistency/to avoid being confusing.
> +
> + env->thumb = saved_state & PSTATE_T ? 1 : 0;
> +
> + /* And the final un-cached bits */
> + env->uncached_cpsr = saved_state & ~AARCH32_CACHED_PSTATE_BITS;
> + }
> + }
> +}
> +
> +
Stray blank line.
> void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>
> /* Interface between CPU and Interrupt controller. */
> --
> 2.0.1
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs Alex Bennée
@ 2014-08-04 12:50 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 12:50 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> This enables the saving and restoring of machine state by including the
> current program state (*psr) and xregs. The save_state_to_spsr hides the
> details of if the processor is in 32 or 64 bit mode at the time.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
>
> v2 (ajb)
> - use common state save functions
> - re-base to latest origin/master
> - clean up commented out code
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 3bcc7cc..759610c 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -120,30 +120,27 @@ static const VMStateDescription vmstate_thumb2ee = {
> }
> };
>
> -static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
> +static int get_psr(QEMUFile *f, void *opaque, size_t size)
> {
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
> uint32_t val = qemu_get_be32(f);
>
> - /* Avoid mode switch when restoring CPSR */
> - env->uncached_cpsr = val & CPSR_M;
> - cpsr_write(env, val, 0xffffffff);
> + restore_state_from_spsr(env, val);
> return 0;
> }
>
> -static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
> +static void put_psr(QEMUFile *f, void *opaque, size_t size)
> {
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
> -
> - qemu_put_be32(f, cpsr_read(env));
> + qemu_put_be32(f, save_state_to_spsr(env));
> }
>
> -static const VMStateInfo vmstate_cpsr = {
> +static const VMStateInfo vmstate_psr = {
> .name = "cpsr",
> - .get = get_cpsr,
> - .put = put_cpsr,
> + .get = get_psr,
> + .put = put_psr,
> };
>
> static void cpu_pre_save(void *opaque)
> @@ -218,17 +215,19 @@ static int cpu_post_load(void *opaque, int version_id)
>
> const VMStateDescription vmstate_arm_cpu = {
> .name = "cpu",
> - .version_id = 20,
> - .minimum_version_id = 20,
> + .version_id = 21,
> + .minimum_version_id = 21,
> .pre_save = cpu_pre_save,
> .post_load = cpu_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
> + VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
> + VMSTATE_UINT64(env.pc, ARMCPU),
> {
> - .name = "cpsr",
> + .name = "psr",
Why do we rename "cpsr" to "psr" here but not in the
vmstate_psr itself above? (Personally I would call this
"pstate" or just leave it as "cpsr", but naming here isn't
a big deal I think.)
> .version_id = 0,
> .size = sizeof(uint32_t),
> - .info = &vmstate_cpsr,
> + .info = &vmstate_psr,
> .flags = VMS_SINGLE,
> .offset = 0,
> },
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls Alex Bennée
2014-07-15 13:40 ` Riku Voipio
@ 2014-08-04 12:59 ` Peter Maydell
1 sibling, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 12:59 UTC (permalink / raw)
To: Alex Bennée; +Cc: Riku Voipio, QEMU Developers
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> Use the unified save_state_to_spsr. I've also updated the interrupt
> helpers to restore via the restore_state_from_spsr() functions. In the
> aarch32 case this also needs to call switch_mode() to do the appropriate
> fiddling.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
>
> v2
> - include xpsr_read conversions
> - checkpatch fixes
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60777fe..577e1d3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -321,7 +321,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
> (*regs)[14] = tswapreg(env->regs[14]);
> (*regs)[15] = tswapreg(env->regs[15]);
>
> - (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
> + (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));
Using a function named "save_something" to *read* CPU
state is really confusing.
Also, the CPSR format is not the same as the AArch32 SPSR
format -- for instance bit 20 is IL in the SPSR, but we must not
allow a guest in AArch32 to read or write PSTATE.IL via a
CPSR read or write insn. We should check whether the value
the kernel exposes in places like the signal handler context
structs is the SPSR format or not.
> (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
> }
>
> @@ -509,7 +509,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
> (*regs)[i] = tswapreg(env->xregs[i]);
> }
> (*regs)[32] = tswapreg(env->pc);
> - (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
> + (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
> }
>
> #define USE_ELF_CORE_DUMP
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b453a39..8848e15 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -546,7 +546,7 @@ do_kernel_trap(CPUARMState *env)
> operations. However things like ldrex/strex are much harder so
> there's not much point trying. */
> start_exclusive();
> - cpsr = cpsr_read(env);
> + cpsr = save_state_to_spsr(env);
> addr = env->regs[2];
> /* FIXME: This should SEGV if the access fails. */
> if (get_user_u32(val, addr))
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 86fae3d..9c6727b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1224,7 +1224,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
> }
> __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
> __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
> - __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
> + __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
>
> __put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
>
> @@ -1572,7 +1572,7 @@ setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
> __put_user(env->regs[14], &sc->arm_lr);
> __put_user(env->regs[15], &sc->arm_pc);
> #ifdef TARGET_CONFIG_CPU_32
> - __put_user(cpsr_read(env), &sc->arm_cpsr);
> + __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
> #endif
>
> __put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
> @@ -1604,7 +1604,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
> abi_ulong handler = ka->_sa_handler;
> abi_ulong retcode;
> int thumb = handler & 1;
> - uint32_t cpsr = cpsr_read(env);
> + uint32_t cpsr = save_state_to_spsr(env);
>
> cpsr &= ~CPSR_IT;
> if (thumb) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fe4d4f3..3f23167 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -480,30 +480,8 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> #define PSTATE_MODE_EL1t 4
> #define PSTATE_MODE_EL0t 0
>
> -/* ARMv8 ARM D1.7 Process state, PSTATE
> - *
> - * 31 28 27 24 23 22 21 20 22 21 20 19 16 15 8 7 5 4 0
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - *
> - * The PSTATE is an abstraction of a number of Return the current
> - * PSTATE value. This is only valid for A64 hardware although can be
> - * read when in AArch32 mode.
> - */
> -static inline uint32_t pstate_read(CPUARMState *env)
> -{
> - int ZF;
> -
> - g_assert(is_a64(env));
> -
> - ZF = (env->ZF == 0);
> - return (env->NF & 0x80000000) | (ZF << 30)
> - | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> - | env->pstate | env->daif;
> -}
??? You just added this function and comment in an earlier
patch. Rebasing damage?
> -
> -/* Update the current PSTATE value. This doesn't include nRW which is */
> +/* Update the current PSTATE value. This doesn't include nRW which
> + * indicates if we are in 64 or 32 bit mode */
> static inline void pstate_write(CPUARMState *env, uint32_t val)
> {
> g_assert(is_a64(env));
> @@ -520,26 +498,10 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
> *
> * Unlike the above PSTATE implementation these functions will attempt
> * to switch processor mode when the M[4:0] bits are set.
> - */
> -uint32_t cpsr_read(CPUARMState *env);
> -/* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
> + *
> + * Note that some bits of mask must be all-set or all-clear. */
> void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
>
> -/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
> -static inline uint32_t xpsr_read(CPUARMState *env)
> -{
> - int ZF;
> -
> - g_assert(!is_a64(env));
> -
> - ZF = (env->ZF == 0);
> - return (env->NF & 0x80000000) | (ZF << 30)
> - | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> - | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
> - | ((env->condexec_bits & 0xfc) << 8)
> - | env->v7m.exception;
> -}
> -
> /* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */
> static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
> index 1c34396..ec25f30 100644
> --- a/target-arm/gdbstub.c
> +++ b/target-arm/gdbstub.c
> @@ -53,7 +53,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> return gdb_get_reg32(mem_buf, 0);
> case 25:
> /* CPSR */
> - return gdb_get_reg32(mem_buf, cpsr_read(env));
> + return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
> }
> /* Unknown register. */
> return 0;
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 8f3b8d1..76d1b91 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -35,7 +35,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> case 32:
> return gdb_get_reg64(mem_buf, env->pc);
> case 33:
> - return gdb_get_reg32(mem_buf, pstate_read(env));
> + return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
> }
> /* Unknown register. */
> return 0;
> @@ -62,7 +62,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> env->pc = tmp;
> return 8;
> case 33:
> - /* CPSR */
> + /* SPSR */
> pstate_write(env, tmp);
> return 4;
> }
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index ec1fef5..03cd64f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -445,6 +445,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> CPUARMState *env = &cpu->env;
> target_ulong addr = env->cp15.vbar_el[1];
> int i;
> + uint32_t spsr = save_state_to_spsr(env);
>
> if (arm_current_pl(env) == 0) {
> if (env->aarch64) {
> @@ -452,7 +453,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> } else {
> addr += 0x600;
> }
> - } else if (pstate_read(env) & PSTATE_SP) {
> + } else if (spsr & PSTATE_SP) {
> addr += 0x200;
> }
>
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> }
>
> if (is_a64(env)) {
> - env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
> + env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
> env->sp_el[arm_current_pl(env)] = env->xregs[31];
> env->xregs[31] = env->sp_el[1];
> env->elr_el[1] = env->pc;
> } else {
> - env->banked_spsr[0] = cpsr_read(env);
> + env->banked_spsr[0] = spsr;
> if (!env->thumb) {
> env->cp15.esr_el[1] |= 1 << 25;
> }
> @@ -506,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> env->condexec_bits = 0;
> }
>
> + // TODO: restore_state_from_spsr()
Don't leave TODO comments in patches...
> env->aarch64 = 1;
> pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d343856..030bcdd 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2990,17 +2990,6 @@ static int bad_mode_switch(CPUARMState *env, int mode)
> }
> }
>
> -uint32_t cpsr_read(CPUARMState *env)
> -{
> - int ZF;
> - ZF = (env->ZF == 0);
> - return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
> - (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> - | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
> - | ((env->condexec_bits & 0xfc) << 8)
> - | (env->GE << 16) | (env->daif & CPSR_AIF);
> -}
> -
> void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> {
> if (mask & CPSR_NZCV) {
> @@ -3278,7 +3267,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> - uint32_t xpsr = xpsr_read(env);
> + uint32_t xpsr = save_state_to_spsr(env);
> uint32_t lr;
> uint32_t addr;
>
> @@ -3479,7 +3468,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
> addr += env->cp15.vbar_el[1];
> }
> switch_mode (env, new_mode);
> - env->spsr = cpsr_read(env);
> + env->spsr = save_state_to_spsr(env);
> + /* TODO: restore_state_from_spsr */
Another TODO comment here...
> /* Clear IT bits. */
> env->condexec_bits = 0;
> /* Switch to the new mode, and to the correct instruction set. */
> @@ -4212,19 +4202,19 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>
> switch (reg) {
> case 0: /* APSR */
> - return xpsr_read(env) & 0xf8000000;
> + return save_state_to_spsr(env) & 0xf8000000;
> case 1: /* IAPSR */
> - return xpsr_read(env) & 0xf80001ff;
> + return save_state_to_spsr(env) & 0xf80001ff;
> case 2: /* EAPSR */
> - return xpsr_read(env) & 0xff00fc00;
> + return save_state_to_spsr(env) & 0xff00fc00;
> case 3: /* xPSR */
> - return xpsr_read(env) & 0xff00fdff;
> + return save_state_to_spsr(env) & 0xff00fdff;
> case 5: /* IPSR */
> - return xpsr_read(env) & 0x000001ff;
> + return save_state_to_spsr(env) & 0x000001ff;
> case 6: /* EPSR */
> - return xpsr_read(env) & 0x0700fc00;
> + return save_state_to_spsr(env) & 0x0700fc00;
> case 7: /* IEPSR */
> - return xpsr_read(env) & 0x0700edff;
> + return save_state_to_spsr(env) & 0x0700edff;
> case 8: /* MSP */
> return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
> case 9: /* PSP */
This all seems more confusing rather than less. What's wrong
with still having functions like xpsr_read() ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write Alex Bennée
2014-07-15 13:43 ` Riku Voipio
@ 2014-08-04 13:01 ` Peter Maydell
1 sibling, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 13:01 UTC (permalink / raw)
To: Alex Bennée; +Cc: Riku Voipio, QEMU Developers
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> And use the new machinery to to save and restore program state. The old
> cpsr_write function did some special handling for mode switches which
> has been moved into the helper function.
Are you really sure all the old places we did cpsr_write() don't
want the mode switch handling? That was kind of the whole
point of that function...
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 09/10] target-arm/kvm.c: better error reporting
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
@ 2014-08-04 13:03 ` Peter Maydell
-1 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 13:03 UTC (permalink / raw)
To: Alex Bennée
Cc: QEMU Developers, Alex Bennée, Paolo Bonzini, open list:Overall
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> From: Alex Bennée <alex@bennee.com>
>
> When we have a problem syncing CP registers between kvm<->qemu it's a
> lot more useful to have the names of the registers in the log than just
> a random abort() and core dump.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
No particular objection but it seems out of place in this patchset.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 09/10] target-arm/kvm.c: better error reporting
@ 2014-08-04 13:03 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 13:03 UTC (permalink / raw)
To: Alex Bennée
Cc: Paolo Bonzini, Alex Bennée, QEMU Developers, open list:Overall
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> From: Alex Bennée <alex@bennee.com>
>
> When we have a problem syncing CP registers between kvm<->qemu it's a
> lot more useful to have the names of the registers in the log than just
> a random abort() and core dump.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
No particular objection but it seems out of place in this patchset.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 10/10] target-arm/kvm: make reg sync code common between kvm32/64
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
@ 2014-08-04 13:04 ` Peter Maydell
-1 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 13:04 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Paolo Bonzini, open list:Overall
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> Before we launch a guest we query KVM for the list of "co-processor"
> registers it knows about which is used later for save/restore of machine
> state. The logic is identical for both 32-bit and 64-bit so I've moved
> it all into the common code and simplified the exit paths (as failure =>
> exit).
>
> This list may well have more registers than are known by the TCG
> emulation which is not necessarily a problem but it does stop us from
> migrating between KVM and TCG hosted guests. I've added some additional
> checking to report those registers under -d unimp.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
This definitely shouldn't be in this patchset...
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/10] target-arm/kvm: make reg sync code common between kvm32/64
@ 2014-08-04 13:04 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2014-08-04 13:04 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, QEMU Developers, open list:Overall
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> Before we launch a guest we query KVM for the list of "co-processor"
> registers it knows about which is used later for save/restore of machine
> state. The logic is identical for both 32-bit and 64-bit so I've moved
> it all into the common code and simplified the exit paths (as failure =>
> exit).
>
> This list may well have more registers than are known by the TCG
> emulation which is not necessarily a problem but it does stop us from
> migrating between KVM and TCG hosted guests. I've added some additional
> checking to report those registers under -d unimp.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
This definitely shouldn't be in this patchset...
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2014-08-04 13:04 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-10 15:49 [Qemu-devel] [PATCH v2 00/10] aarch64 migration for TCG and KVM Alex Bennée
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions Alex Bennée
2014-08-04 12:28 ` Peter Maydell
2014-07-10 15:49 ` [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore Alex Bennée
2014-08-04 12:47 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs Alex Bennée
2014-08-04 12:50 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 04/10] target-arm: replace cpsr/xpsr/pstate_read calls Alex Bennée
2014-07-15 13:40 ` Riku Voipio
2014-08-04 12:59 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 05/10] arm/nwfps: replace cpsr_write with set_condition_codes Alex Bennée
2014-07-15 13:41 ` Riku Voipio
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 06/10] linux-user/main.c: __kernel_cmpxchg set env->CF directly Alex Bennée
2014-07-15 13:39 ` Riku Voipio
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write Alex Bennée
2014-07-15 13:43 ` Riku Voipio
2014-08-04 13:01 ` Peter Maydell
2014-07-10 15:50 ` [Qemu-devel] [PATCH v2 08/10] target-arm: remove final users of pstate_write Alex Bennée
2014-07-15 13:44 ` Riku Voipio
2014-07-10 15:50 ` [PATCH v2 09/10] target-arm/kvm.c: better error reporting Alex Bennée
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
2014-08-04 13:03 ` Peter Maydell
2014-08-04 13:03 ` [Qemu-devel] " Peter Maydell
2014-07-10 15:50 ` [PATCH v2 10/10] target-arm/kvm: make reg sync code common between kvm32/64 Alex Bennée
2014-07-10 15:50 ` [Qemu-devel] " Alex Bennée
2014-08-04 13:04 ` Peter Maydell
2014-08-04 13:04 ` [Qemu-devel] " Peter Maydell
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.