Re: [Qemu-devel] [PATCH] target-arm: Don't decode old cp15 WFI instructions on v7 cores

2011-03-06 Thread Aurelien Jarno 
On Fri, Feb 25, 2011 at 03:04:12PM +, Peter Maydell wrote:
 In v7 of the ARM architecture, WFI (wait for interrupt) is a first-class
 instruction, but in previous versions this functionality was provided
 via a cp15 coprocessor register. Add correct feature checks to the
 decoding of the cp15 WFI instructions so that they behave correctly
 for newer cores. In particular, the old 0,c7,c8,2 encoding used on
 ARM940 has been reused for VA-to-PA translation in v6 and v7.
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 This patch stands alone as a fix to target-arm; it's a prerequisite
 for Adam's VA-PA translation patch, because otherwise attempting a
 user-read translation will get you a WFI instead...
 
  target-arm/translate.c |   35 ++-
  1 files changed, 30 insertions(+), 5 deletions(-)

Thanks, applied.

 diff --git a/target-arm/translate.c b/target-arm/translate.c
 index dbd958b..baa1256 100644
 --- a/target-arm/translate.c
 +++ b/target-arm/translate.c
 @@ -2538,13 +2538,38 @@ static int disas_cp15_insn(CPUState *env, 
 DisasContext *s, uint32_t insn)
  if (IS_USER(s)  !cp15_user_ok(insn)) {
  return 1;
  }
 -if ((insn  0x0fff0fff) == 0x0e070f90
 -|| (insn  0x0fff0fff) == 0x0e070f58) {
 -/* Wait for interrupt.  */
 -gen_set_pc_im(s-pc);
 -s-is_jmp = DISAS_WFI;
 +
 +/* Pre-v7 versions of the architecture implemented WFI via coprocessor
 + * instructions rather than a separate instruction.
 + */
 +if ((insn  0x0fff0fff) == 0x0e070f90) {
 +/* 0,c7,c0,4: Standard v6 WFI (also used in some pre-v6 cores).
 + * In v7, this must NOP.
 + */
 +if (!arm_feature(env, ARM_FEATURE_V7)) {
 +/* Wait for interrupt.  */
 +gen_set_pc_im(s-pc);
 +s-is_jmp = DISAS_WFI;
 +}
  return 0;
  }
 +
 +if ((insn  0x0fff0fff) == 0x0e070f58) {
 +/* 0,c7,c8,2: Not all pre-v6 cores implemented this WFI,
 + * so this is slightly over-broad.
 + */
 +if (!arm_feature(env, ARM_FEATURE_V6)) {
 +/* Wait for interrupt.  */
 +gen_set_pc_im(s-pc);
 +s-is_jmp = DISAS_WFI;
 +return 0;
 +}
 +/* Otherwise fall through to handle via helper function.
 + * In particular, on v7 and some v6 cores this is one of
 + * the VA-PA registers.
 + */
 +}
 +
  rd = (insn  12)  0xf;
  
  if (cp15_tls_load_store(env, s, insn, rd))
 -- 
 1.7.1
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [PATCH] target-arm: Don't decode old cp15 WFI instructions on v7 cores

2011-02-25 Thread Peter Maydell 
In v7 of the ARM architecture, WFI (wait for interrupt) is a first-class
instruction, but in previous versions this functionality was provided
via a cp15 coprocessor register. Add correct feature checks to the
decoding of the cp15 WFI instructions so that they behave correctly
for newer cores. In particular, the old 0,c7,c8,2 encoding used on
ARM940 has been reused for VA-to-PA translation in v6 and v7.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This patch stands alone as a fix to target-arm; it's a prerequisite
for Adam's VA-PA translation patch, because otherwise attempting a
user-read translation will get you a WFI instead...

 target-arm/translate.c |   35 ++-
 1 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index dbd958b..baa1256 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2538,13 +2538,38 @@ static int disas_cp15_insn(CPUState *env, DisasContext 
*s, uint32_t insn)
 if (IS_USER(s)  !cp15_user_ok(insn)) {
 return 1;
 }
-if ((insn  0x0fff0fff) == 0x0e070f90
-|| (insn  0x0fff0fff) == 0x0e070f58) {
-/* Wait for interrupt.  */
-gen_set_pc_im(s-pc);
-s-is_jmp = DISAS_WFI;
+
+/* Pre-v7 versions of the architecture implemented WFI via coprocessor
+ * instructions rather than a separate instruction.
+ */
+if ((insn  0x0fff0fff) == 0x0e070f90) {
+/* 0,c7,c0,4: Standard v6 WFI (also used in some pre-v6 cores).
+ * In v7, this must NOP.
+ */
+if (!arm_feature(env, ARM_FEATURE_V7)) {
+/* Wait for interrupt.  */
+gen_set_pc_im(s-pc);
+s-is_jmp = DISAS_WFI;
+}
 return 0;
 }
+
+if ((insn  0x0fff0fff) == 0x0e070f58) {
+/* 0,c7,c8,2: Not all pre-v6 cores implemented this WFI,
+ * so this is slightly over-broad.
+ */
+if (!arm_feature(env, ARM_FEATURE_V6)) {
+/* Wait for interrupt.  */
+gen_set_pc_im(s-pc);
+s-is_jmp = DISAS_WFI;
+return 0;
+}
+/* Otherwise fall through to handle via helper function.
+ * In particular, on v7 and some v6 cores this is one of
+ * the VA-PA registers.
+ */
+}
+
 rd = (insn  12)  0xf;
 
 if (cp15_tls_load_store(env, s, insn, rd))
-- 
1.7.1