Re: [Qemu-devel] [PATCH v5 1/6] target-arm: Store SPSR_EL1 state in banked_spsr[1] (SPSR_svc)

2015-03-24 Thread Greg Bellows 
On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée alex.ben...@linaro.org wrote:
 From: Peter Maydell peter.mayd...@linaro.org

 The AArch64 SPSR_EL1 register is architecturally mandated to
 be mapped to the AArch32 SPSR_svc register. This means its
 state should live in QEMU's env-banked_spsr[1] field.
 Correct the various places in the code that incorrectly
 put it in banked_spsr[0].

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org

 diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
 index 7e0d038..861f6fa 100644
 --- a/target-arm/helper-a64.c
 +++ b/target-arm/helper-a64.c
 @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
  aarch64_save_sp(env, arm_current_el(env));
  env-elr_el[new_el] = env-pc;
  } else {
 -env-banked_spsr[0] = cpsr_read(env);
 +env-banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);

Are the other banks (2-5) only used for KVM?  It seems we go out of
our way to manage this larger SPSR array then not use all of the slots
in QEMU itself.

  if (!env-thumb) {
  env-cp15.esr_el[new_el] |= 1  25;
  }
 diff --git a/target-arm/helper.c b/target-arm/helper.c
 index 10886c5..d77c6de 100644
 --- a/target-arm/helper.c
 +++ b/target-arm/helper.c
 @@ -2438,7 +2438,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
  { .name = SPSR_EL1, .state = ARM_CP_STATE_AA64,
.type = ARM_CP_ALIAS,
.opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
 -  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[0]) 
 },
 +  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[1]) 
 },
  /* We rely on the access checks not allowing the guest to write to the
   * state field when SPSel indicates that it's being used as the stack
   * pointer.
 diff --git a/target-arm/internals.h b/target-arm/internals.h
 index bb171a7..2cc3017 100644
 --- a/target-arm/internals.h
 +++ b/target-arm/internals.h
 @@ -82,11 +82,14 @@ static inline void arm_log_exception(int idx)

  /*
   * For AArch64, map a given EL to an index in the banked_spsr array.
 + * Note that this mapping and the AArch32 mapping defined in bank_number()
 + * must agree such that the AArch64-AArch32 SPSRs have the architecturally
 + * mandated mapping between each other.
   */
  static inline unsigned int aarch64_banked_spsr_index(unsigned int el)
  {
  static const unsigned int map[4] = {
 -[1] = 0, /* EL1.  */
 +[1] = 1, /* EL1.  */
  [2] = 6, /* EL2.  */
  [3] = 7, /* EL3.  */
  };
 --
 2.3.2


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH v5 1/6] target-arm: Store SPSR_EL1 state in banked_spsr[1] (SPSR_svc)

2015-03-24 Thread Peter Maydell 
On 24 March 2015 at 14:32, Greg Bellows greg.bell...@linaro.org wrote:
 On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée alex.ben...@linaro.org wrote:
 From: Peter Maydell peter.mayd...@linaro.org

 @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
  aarch64_save_sp(env, arm_current_el(env));
  env-elr_el[new_el] = env-pc;
  } else {
 -env-banked_spsr[0] = cpsr_read(env);
 +env-banked_spsr[aarch64_banked_spsr_index(new_el)] = 
 cpsr_read(env);

 Are the other banks (2-5) only used for KVM?  It seems we go out of
 our way to manage this larger SPSR array then not use all of the slots
 in QEMU itself.

They're used in AArch32 (where they are the SPSR for various
32 bit modes). In AArch64 you can access those registers via
MSR/MRS (we probably haven't implemented those yet because they
are only accessible at EL2 and above) so hypervisors can do
worldswitches. But for exception entry and return (which is
what this code is) we only use SPSR_EL0/SPSR_EL1/SPSR_EL2/SPSR_EL3
which is a subset of the AArch32 SPSRs.

-- PMM
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/6] target-arm: Store SPSR_EL1 state in banked_spsr[1] (SPSR_svc)

2015-03-23 Thread Alex Bennée 
From: Peter Maydell peter.mayd...@linaro.org

The AArch64 SPSR_EL1 register is architecturally mandated to
be mapped to the AArch32 SPSR_svc register. This means its
state should live in QEMU's env-banked_spsr[1] field.
Correct the various places in the code that incorrectly
put it in banked_spsr[0].

Signed-off-by: Peter Maydell peter.mayd...@linaro.org

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 7e0d038..861f6fa 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 aarch64_save_sp(env, arm_current_el(env));
 env-elr_el[new_el] = env-pc;
 } else {
-env-banked_spsr[0] = cpsr_read(env);
+env-banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
 if (!env-thumb) {
 env-cp15.esr_el[new_el] |= 1  25;
 }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 10886c5..d77c6de 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2438,7 +2438,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = SPSR_EL1, .state = ARM_CP_STATE_AA64,
   .type = ARM_CP_ALIAS,
   .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
-  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[0]) },
+  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[1]) },
 /* We rely on the access checks not allowing the guest to write to the
  * state field when SPSel indicates that it's being used as the stack
  * pointer.
diff --git a/target-arm/internals.h b/target-arm/internals.h
index bb171a7..2cc3017 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -82,11 +82,14 @@ static inline void arm_log_exception(int idx)
 
 /*
  * For AArch64, map a given EL to an index in the banked_spsr array.
+ * Note that this mapping and the AArch32 mapping defined in bank_number()
+ * must agree such that the AArch64-AArch32 SPSRs have the architecturally
+ * mandated mapping between each other.
  */
 static inline unsigned int aarch64_banked_spsr_index(unsigned int el)
 {
 static const unsigned int map[4] = {
-[1] = 0, /* EL1.  */
+[1] = 1, /* EL1.  */
 [2] = 6, /* EL2.  */
 [3] = 7, /* EL3.  */
 };
-- 
2.3.2

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html