Re: [Qemu-devel] [PATCH] PXA-specific ARM hacks.

["andrzej zaborowski" <balrog@zabor.org>]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On 17/03/07, andrzej zaborowski <balrog@zabor.org> wrote:
> On 17/03/07, Paul Brook <paul@codesourcery.com> wrote:
> > On Friday 16 March 2007 22:06, andrzej zaborowski wrote:
> > > This patch contains some ugly but very useful chnages:
> > >  - in cpu-exec.c, halt the arm CPU on CPU_INTERRUPT_HALT like in other
> > > architectures.
> >
> > This is ok. Though I'd prefer it to actually be common code (like
> > CPU_INTERRUPT_DEBUG).
> >
> > >  - wake the cpu up on CPU_INTERRUPT_EXITTB - this serves waking the
> > > CPU up without asserting IRQ or FIQ, which is possible on PXA. Would
> > > it be better to add a separate interrupt type instead?
> >
> > I think this is probably ok. Could do with a comment somewhere saying so.
> >
> > >  - in target-arm/translate.c, don't terminate the TB after a CP15
> > > write. The purpose of this is to imitate the real processor's
> > > instruction cache (to some degree). OSes should never need rely on the
> > > cache prefetch but unfortunately Linux for pxa does when it's waking
> > > up from sleep or deep idle, when enabling the MMU (simplifies things a
> > > lot for Linux).
> >
> > This is not acceptable in its current form. IIRC on arm cores the effects take
> > effect once all insn have worked through the pipeline (typically a few
> > cycles), and I bet there's code that relies on this. ie:
> > mcr cp15, ...;nop;nop;nop;nop;@expect cp15 write to have taken effect by now
> >
> > A more acceptable solution would be:
> > - Only extend the TB for the specific instruction that linux abuses
> > - Limit the number of instructions that can follow before the end of the TB.
> > - Add a comment saying what this hack is for.
>
> Ok. Attaching a second version of this patch. The number of
> instructions after MMU enable and before TB ends is limited by the
> obligatory CPWAIT rather than in qemu.

Oops, one more try. This time removing the use of cpu_reset() on
entering Standby because cpu_reset() now does more than we want.

Andrzej

[-- Attachment #2: 0015-PXA-specific-ARM-hacks.txt --]
[-- Type: text/plain, Size: 4781 bytes --]

From 8d4d84ee71048638fd101ed27ba37b038ddf4651 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <balrog@zabor.org>
Date: Sat, 17 Mar 2007 22:13:29 +0100
Subject: [PATCH] PXA-specific ARM hacks.

---
 cpu-all.h              |    3 ++-
 cpu-exec.c             |   20 +++++++++++---------
 hw/pxa2xx.c            |    3 ++-
 target-arm/translate.c |   11 ++++++++---
 4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 9b617fc..177bcba 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -763,7 +763,8 @@ void cpu_dump_statistics (CPUState *env, FILE *f,
                           int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
                           int flags);
 
-void cpu_abort(CPUState *env, const char *fmt, ...);
+void cpu_abort(CPUState *env, const char *fmt, ...)
+    __attribute__ ((__format__ (__printf__, 2, 3)));
 extern CPUState *first_cpu;
 extern CPUState *cpu_single_env;
 extern int code_copy_enabled;
diff --git a/cpu-exec.c b/cpu-exec.c
index 48c2a93..10e2f2b 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -275,9 +275,10 @@ int cpu_exec(CPUState *env1)
 #elif defined(TARGET_ARM)
     if (env1->halted) {
         /* An interrupt wakes the CPU even if the I and F CPSR bits are
-           set.  */
-        if (env1->interrupt_request
-            & (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD)) {
+           set.  We use EXITTB to silently wake CPU without causing an
+           actual interrupt.  */
+        if (env1->interrupt_request &
+            (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB)) {
             env1->halted = 0;
         } else {
             return EXCP_HALTED;
@@ -414,6 +415,12 @@ int cpu_exec(CPUState *env1)
                         env->exception_index = EXCP_DEBUG;
                         cpu_loop_exit();
                     }
+                    if (interrupt_request & CPU_INTERRUPT_HALT) {
+                        env->interrupt_request &= ~CPU_INTERRUPT_HALT;
+                        env->halted = 1;
+                        env->exception_index = EXCP_HLT;
+                        cpu_loop_exit();
+                    }
 #if defined(TARGET_I386)
                     if ((interrupt_request & CPU_INTERRUPT_SMI) &&
                         !(env->hflags & HF_SMM_MASK)) {
@@ -511,12 +518,7 @@ int cpu_exec(CPUState *env1)
 		    } else if (interrupt_request & CPU_INTERRUPT_TIMER) {
 			//do_interrupt(0, 0, 0, 0, 0);
 			env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
-		    } else if (interrupt_request & CPU_INTERRUPT_HALT) {
-			env->interrupt_request &= ~CPU_INTERRUPT_HALT;
-			env->halted = 1;
-			env->exception_index = EXCP_HLT;
-			cpu_loop_exit();
-                    }
+		    }
 #elif defined(TARGET_ARM)
                     if (interrupt_request & CPU_INTERRUPT_FIQ
                         && !(env->uncached_cpsr & CPSR_F)) {
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index c8c6eb4..2d2d72a 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -259,7 +259,8 @@ static void pxa2xx_clkpwr_write(void *opaque, int op2, int reg, int crm,
             goto message;
 
         case 3:
-            cpu_reset(s->env);
+            s->env->uncached_cpsr =
+                    ARM_CPU_MODE_SVC | CPSR_A | CPSR_F | CPSR_I;
             s->env->cp15.c1_sys = 0;
             s->env->cp15.c1_coproc = 0;
             s->env->cp15.c2 = 0;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index cd1c04d..3e8367a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -1579,7 +1579,7 @@ static int disas_cp_insn(DisasContext *s, uint32_t insn)
 
 /* Disassemble system coprocessor (cp15) instruction.  Return nonzero if
    instruction is not defined.  */
-static int disas_cp15_insn(DisasContext *s, uint32_t insn)
+static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
 {
     uint32_t rd;
 
@@ -1605,8 +1605,13 @@ static int disas_cp15_insn(DisasContext *s, uint32_t insn)
     } else {
         gen_movl_T0_reg(s, rd);
         gen_op_movl_cp15_T0(insn);
+        /* Normally we would always end the TB here, but Linux
+         * arch/arm/mach-pxa/sleep.S expects two instructions following
+         * an MMU enable to execute from cache.  Imitate this behaviour.  */
+        if (!arm_feature(env, ARM_FEATURE_XSCALE) ||
+                (insn & 0x0fff0fff) != 0x0e010f10)
+            gen_lookup_tb(s);
     }
-    gen_lookup_tb(s);
     return 0;
 }
 
@@ -2907,7 +2912,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                     goto illegal_op;
                 break;
             case 15:
-                if (disas_cp15_insn (s, insn))
+                if (disas_cp15_insn (env, s, insn))
                     goto illegal_op;
                 break;
             default:
-- 
1.4.4.3