[Qemu-devel] [PATCH 0/9] sparc64: tick timers

[Qemu-devel] [PATCH 0/9] sparc64: tick timers

[Igor V. Kovalenko]

The following series deals with tick timers for sparc64.
It is not complete solution yet, comments are welcome.

First two changes are really debug helpers,
so are not strictly required.

Central part is addressing how traps are taken, which
includes taking trap when PSTATE.IE is changed,
PIL mask is changed, and SOFTINT is written to.
I did not used CPU_INTERRUPT_TIMER at all, instead
all interrupts are delivered with CPU_INTERRUPT_HARD.

Last change is reimplementing tick timers without
periodic timer framework. sparc64 timers are essentially
one-shot since they provide trap on tick counter match
only so require reloading for next time period. Still
it may be cleaner to fix sun4u use of periodic timers.

These changes allow recent linux kernel to start
scheduler works, and count about 200 bogomips
before crashing later.

---

Igor V. Kovalenko (9):
      sparc64: trace pstate and global register set changes
      sparc64: add PSR and PIL to cpu state dump
      sparc64: use helper_wrpil to check pending irq on write
      sparc64: check for pending irq when pil, pstate or softint is changed
      sparc64: add macros to deal with softint and timer interrupt
      sparc64: clear exception_index with -1 value
      sparc64: move cpu_interrupts_enabled to cpu.h
      sparc64: interrupt trap handling
      sparc64: reimplement tick timers


 cpu-exec.c               |   40 ++++----
 hw/sun4u.c               |  234 +++++++++++++++++++++++++++++++++++++---------
 target-sparc/cpu.h       |   19 ++++
 target-sparc/exec.h      |   13 ---
 target-sparc/helper.c    |    1 
 target-sparc/helper.h    |    1 
 target-sparc/op_helper.c |   75 ++++++++++++++-
 target-sparc/translate.c |    5 -
 8 files changed, 304 insertions(+), 84 deletions(-)

-- 
Kind regards,
Igor V. Kovalenko

[Qemu-devel] [PATCH 1/9] sparc64: trace pstate and global register set changes

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/op_helper.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index dab2c25..26092e5 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -11,6 +11,7 @@
 //#define DEBUG_UNASSIGNED
 //#define DEBUG_ASI
 //#define DEBUG_PCALL
+//#define DEBUG_PSTATE
 
 #ifdef DEBUG_MMU
 #define DPRINTF_MMU(fmt, ...)                                   \
@@ -31,6 +32,13 @@
     do { printf("ASI: " fmt , ## __VA_ARGS__); } while (0)
 #endif
 
+#ifdef DEBUG_PSTATE
+#define DPRINTF_PSTATE(fmt, ...)                                   \
+    do { printf("PSTATE: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF_PSTATE(fmt, ...) do {} while (0)
+#endif
+
 #ifdef TARGET_SPARC64
 #ifndef TARGET_ABI32
 #define AM_CHECK(env1) ((env1)->pstate & PS_AM)
@@ -3244,6 +3252,12 @@ static inline uint64_t *get_gregset(uint64_t pstate)
 {
     switch (pstate) {
     default:
+        DPRINTF_PSTATE("ERROR in get_gregset: active pstate bits=%lX %s %s %s",
+                pstate,
+                (pstate&PS_IG)?"IG":"",
+                (pstate&PS_MG)?"MG":"",
+                (pstate&PS_AG)?"AG":"" );
+        /* pass through to normal set of global registers */
     case 0:
         return env->bgregs;
     case PS_AG:
@@ -3269,12 +3283,18 @@ static inline void change_pstate(uint64_t new_pstate)
     new_pstate_regs = new_pstate & 0xc01;
 
     if (new_pstate_regs != pstate_regs) {
+        DPRINTF_PSTATE("change_pstate: switching regs old=%lX new=%lX\n",
+                       pstate_regs, new_pstate_regs);
         // Switch global register bank
         src = get_gregset(new_pstate_regs);
         dst = get_gregset(pstate_regs);
         memcpy32(dst, env->gregs);
         memcpy32(env->gregs, src);
     }
+    else {
+        DPRINTF_PSTATE("change_pstate: regs new=%lX (unchanged)\n",
+                       new_pstate_regs);
+    }
     env->pstate = new_pstate;
 }
 

[Qemu-devel] [PATCH 2/9] sparc64: add PSR and PIL to cpu state dump

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/helper.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index a06923a..0f0e583 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -1452,6 +1452,7 @@ void cpu_dump_state(CPUState *env, FILE *f,
 #ifdef TARGET_SPARC64
     cpu_fprintf(f, "pstate: 0x%08x ccr: 0x%02x asi: 0x%02x tl: %d fprs: %d\n",
                 env->pstate, GET_CCR(env), env->asi, env->tl, env->fprs);
+    cpu_fprintf(f, "psr: 0x%08x pil=%x\n", GET_PSR(env), env->psrpil);
     cpu_fprintf(f, "cansave: %d canrestore: %d otherwin: %d wstate %d "
                 "cleanwin %d cwp %d\n",
                 env->cansave, env->canrestore, env->otherwin, env->wstate,

[Qemu-devel] [PATCH 3/9] sparc64: use helper_wrpil to check pending irq on write

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/helper.h    |    1 +
 target-sparc/op_helper.c |   14 ++++++++++++++
 target-sparc/translate.c |    5 +----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 4002b9e..6f103e7 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -5,6 +5,7 @@ DEF_HELPER_0(rett, void)
 DEF_HELPER_1(wrpsr, void, tl)
 DEF_HELPER_0(rdpsr, tl)
 #else
+DEF_HELPER_1(wrpil, void, tl)
 DEF_HELPER_1(wrpstate, void, tl)
 DEF_HELPER_0(done, void)
 DEF_HELPER_0(retry, void)
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 26092e5..a7da0e4 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3303,6 +3303,20 @@ void helper_wrpstate(target_ulong new_state)
     change_pstate(new_state & 0xf3f);
 }
 
+void helper_wrpil(target_ulong new_pil)
+{
+    DPRINTF_PSTATE("helper_wrpil old=%X new=%X\n",
+                   env->psrpil, (uint32_t)new_pil);
+
+    env->psrpil = new_pil;
+
+#if !defined(CONFIG_USER_ONLY)
+    if (env->pstate & PS_IE) {
+        cpu_check_irqs(env);
+    }
+#endif
+}
+
 void helper_done(void)
 {
     trap_state* tsptr = cpu_tsptr(env);
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index bf6df50..7e9f0cf 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -3371,10 +3371,7 @@ static void disas_sparc_insn(DisasContext * dc)
                                                offsetof(CPUSPARCState, tl));
                                 break;
                             case 8: // pil
-                                tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_tmp0);
-                                tcg_gen_st_i32(cpu_tmp32, cpu_env,
-                                               offsetof(CPUSPARCState,
-                                                        psrpil));
+                                gen_helper_wrpil(cpu_tmp0);
                                 break;
                             case 9: // cwp
                                 gen_helper_wrcwp(cpu_tmp0);

[Qemu-devel] [PATCH 4/9] sparc64: check for pending irq when pil, pstate or softint is changed

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/op_helper.c |   39 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index a7da0e4..b1978cb 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3301,6 +3301,12 @@ static inline void change_pstate(uint64_t new_pstate)
 void helper_wrpstate(target_ulong new_state)
 {
     change_pstate(new_state & 0xf3f);
+
+#if !defined(CONFIG_USER_ONLY)
+    if (env->pstate & PS_IE) {
+        cpu_check_irqs(env);
+    }
+#endif
 }
 
 void helper_wrpil(target_ulong new_pil)
@@ -3328,6 +3334,14 @@ void helper_done(void)
     change_pstate((tsptr->tstate >> 8) & 0xf3f);
     PUT_CWP64(env, tsptr->tstate & 0xff);
     env->tl--;
+
+    DPRINTF_PSTATE("... helper_done tl=%d\n", env->tl);
+
+#if !defined(CONFIG_USER_ONLY)
+    if (env->pstate & PS_IE) {
+        cpu_check_irqs(env);
+    }
+#endif
 }
 
 void helper_retry(void)
@@ -3341,21 +3355,40 @@ void helper_retry(void)
     change_pstate((tsptr->tstate >> 8) & 0xf3f);
     PUT_CWP64(env, tsptr->tstate & 0xff);
     env->tl--;
+
+    DPRINTF_PSTATE("... helper_retry tl=%d\n", env->tl);
+
+#if !defined(CONFIG_USER_ONLY)
+    if (env->pstate & PS_IE) {
+        cpu_check_irqs(env);
+    }
+#endif
+}
+
+static void do_modify_softint(const char* operation, uint32_t value)
+{
+    if (env->softint != value) {
+        env->softint = value;
+        DPRINTF_PSTATE(": %s new %08X\n", operation, env->softint);
+#if !defined(CONFIG_USER_ONLY)
+        cpu_check_irqs(env);
+#endif
+    }
 }
 
 void helper_set_softint(uint64_t value)
 {
-    env->softint |= (uint32_t)value;
+    do_modify_softint("helper_set_softint" , env->softint | (uint32_t)value);
 }
 
 void helper_clear_softint(uint64_t value)
 {
-    env->softint &= (uint32_t)~value;
+    do_modify_softint("helper_clear_softint" , env->softint & (uint32_t)~value);
 }
 
 void helper_write_softint(uint64_t value)
 {
-    env->softint = (uint32_t)value;
+    do_modify_softint("helper_write_softint", (uint32_t)value);
 }
 #endif
 

[Qemu-devel] [PATCH 5/9] sparc64: add macros to deal with softint and timer interrupt

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/cpu.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 1fe4d0f..0dba241 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -394,6 +394,8 @@ typedef struct CPUSPARCState {
     uint64_t fprs;
     uint64_t tick_cmpr, stick_cmpr;
     void *tick, *stick;
+#define TICK_NPT_MASK        0x8000000000000000ULL
+#define TICK_SOFTINT_DISABLE 0x8000000000000000ULL
     uint64_t gsr;
     uint32_t gl; // UA2005
     /* UA 2005 hyperprivileged registers */
@@ -402,6 +404,10 @@ typedef struct CPUSPARCState {
     uint32_t softint;
 #define SOFTINT_TIMER   1
 #define SOFTINT_STIMER  (1 << 16)
+#define SOFTINT_INTRMASK (0xFFFE)
+#define SOFTINT_TM       (1 << 0)
+#define SOFTINT_SM       (1 << 16)
+#define SOFTINT_REG_MASK (SOFTINT_SM|SOFTINT_INTRMASK|SOFTINT_TM)
 #endif
     sparc_def_t *def;
 } CPUSPARCState;

[Qemu-devel] [PATCH 6/9] sparc64: clear exception_index with -1 value

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/op_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b1978cb..94f1c7a 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3535,7 +3535,7 @@ void do_interrupt(CPUState *env)
     env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
     env->pc = env->tbr;
     env->npc = env->pc + 4;
-    env->exception_index = 0;
+    env->exception_index = -1;
 }
 #else
 #ifdef DEBUG_PCALL

[Qemu-devel] [PATCH 7/9] sparc64: move cpu_interrupts_enabled to cpu.h

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

- to be used by cpu_check_irqs

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 target-sparc/cpu.h  |   13 +++++++++++++
 target-sparc/exec.h |   13 -------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 0dba241..f1584d4 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -566,6 +566,19 @@ static inline int cpu_mmu_index(CPUState *env1)
 #endif
 }
 
+static inline int cpu_interrupts_enabled(CPUState *env1)
+{
+#if !defined (TARGET_SPARC64)
+    if (env1->psret != 0)
+        return 1;
+#else
+    if (env1->pstate & PS_IE)
+        return 1;
+#endif
+
+    return 0;
+}
+
 static inline int cpu_fpu_enabled(CPUState *env1)
 {
 #if defined(CONFIG_USER_ONLY)
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index e120d6f..3e021e9 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -24,19 +24,6 @@ static inline void regs_to_env(void)
 /* op_helper.c */
 void do_interrupt(CPUState *env);
 
-static inline int cpu_interrupts_enabled(CPUState *env1)
-{
-#if !defined (TARGET_SPARC64)
-    if (env1->psret != 0)
-        return 1;
-#else
-    if (env1->pstate & PS_IE)
-        return 1;
-#endif
-
-    return 0;
-}
-
 static inline int cpu_has_work(CPUState *env1)
 {
     return (env1->interrupt_request & CPU_INTERRUPT_HARD) &&

[Qemu-devel] [PATCH 8/9] sparc64: interrupt trap handling

[Igor V. Kovalenko]

From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

cpu_check_irqs
- handle SOFTINT register TICK and STICK timer bits
- only check interrupt levels greater than PIL value
- handle preemption by higher level traps

cpu_exec
- handle CPU_INTERRUPT_HARD only if interrupts are enabled
- PIL 15 is not special level on sparcv9

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
---
 cpu-exec.c |   40 ++++++++++++++++++++++------------------
 hw/sun4u.c |   52 +++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index af4595b..65192c1 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -449,24 +449,28 @@ int cpu_exec(CPUState *env1)
                         next_tb = 0;
                     }
 #elif defined(TARGET_SPARC)
-                    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
-			cpu_interrupts_enabled(env)) {
-			int pil = env->interrupt_index & 15;
-			int type = env->interrupt_index & 0xf0;
-
-			if (((type == TT_EXTINT) &&
-			     (pil == 15 || pil > env->psrpil)) ||
-			    type != TT_EXTINT) {
-			    env->interrupt_request &= ~CPU_INTERRUPT_HARD;
-                            env->exception_index = env->interrupt_index;
-                            do_interrupt(env);
-			    env->interrupt_index = 0;
-                        next_tb = 0;
-			}
-		    } else if (interrupt_request & CPU_INTERRUPT_TIMER) {
-			//do_interrupt(0, 0, 0, 0, 0);
-			env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
-		    }
+                    if ((interrupt_request & CPU_INTERRUPT_HARD)) {
+                        if (cpu_interrupts_enabled(env)) {
+                            int pil = env->interrupt_index & 0xf;
+                            int type = env->interrupt_index & 0xf0;
+
+                            if (((type == TT_EXTINT) && (pil > env->psrpil)) ||
+                                            type != TT_EXTINT) {
+                                //env->interrupt_request &= ~CPU_INTERRUPT_HARD;
+                                if (env->interrupt_index > 0) {
+                                    env->exception_index = env->interrupt_index;
+                                    do_interrupt(env);
+                                    next_tb = 0;
+                                }
+                            }
+                        }
+                    }
+                    else if (interrupt_request & CPU_INTERRUPT_TIMER) {
+                        env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
+#if !defined(CONFIG_USER_ONLY)
+                        cpu_check_irqs(env);
+#endif
+                    }
 #elif defined(TARGET_ARM)
                     if (interrupt_request & CPU_INTERRUPT_FIQ
                         && !(env->uncached_cpsr & CPSR_F)) {
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 9d46f08..84a8043 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -233,29 +233,51 @@ void irq_info(Monitor *mon)
 
 void cpu_check_irqs(CPUState *env)
 {
-    uint32_t pil = env->pil_in | (env->softint & ~SOFTINT_TIMER) |
-        ((env->softint & SOFTINT_TIMER) << 14);
+    uint32_t pil = env->pil_in | (env->softint & ~(SOFTINT_TM | SOFTINT_SM));
+
+    /* check if TM or SM in SOFTINT are set
+       setting these also causes interrupt 14 */
+    if (env->softint & (SOFTINT_TM | SOFTINT_SM))
+        pil |= 1 << 14;
+
+    if (!pil) {
+        if (env->interrupt_request & CPU_INTERRUPT_HARD) {
+            CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %X)\n",
+                           env->interrupt_index);
+            env->interrupt_index = 0;
+            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
+        }
+        return;
+    }
+
+    if (cpu_interrupts_enabled(env)) {
 
-    if (pil && (env->interrupt_index == 0 ||
-                (env->interrupt_index & ~15) == TT_EXTINT)) {
         unsigned int i;
 
-        for (i = 15; i > 0; i--) {
+        for (i = 15; i > env->psrpil; i--) {
             if (pil & (1 << i)) {
                 int old_interrupt = env->interrupt_index;
-
-                env->interrupt_index = TT_EXTINT | i;
-                if (old_interrupt != env->interrupt_index) {
-                    CPUIRQ_DPRINTF("Set CPU IRQ %d\n", i);
-                    cpu_interrupt(env, CPU_INTERRUPT_HARD);
-                }
+                int new_interrupt = TT_EXTINT | i;
+
+                    if (env->tl > 0 && cpu_tsptr(env)->tt > new_interrupt) {
+                        CPUIRQ_DPRINTF("Not setting CPU IRQ: TL=%d "
+                                "current %X >= pending %X\n",
+                                env->tl, cpu_tsptr(env)->tt, new_interrupt);
+                    }
+                    else if (old_interrupt != new_interrupt) {
+                        env->interrupt_index = new_interrupt;
+                        CPUIRQ_DPRINTF("Set CPU IRQ %d old=%X new=%X\n", i,
+                                old_interrupt, new_interrupt);
+                        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+                    }
                 break;
             }
         }
-    } else if (!pil && (env->interrupt_index & ~15) == TT_EXTINT) {
-        CPUIRQ_DPRINTF("Reset CPU IRQ %d\n", env->interrupt_index & 15);
-        env->interrupt_index = 0;
-        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
+    }
+    else {
+        CPUIRQ_DPRINTF("Interrupts disabled, pil=%08X pil_in=%08X softint=%08X "
+                "current interrupt %X\n",
+                pil, env->pil_in, env->softint, env->interrupt_index);
     }
 }
 

Re: [Qemu-devel] [PATCH 1/9] sparc64: trace pstate and global register set changes

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/op_helper.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index dab2c25..26092e5 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -11,6 +11,7 @@
>  //#define DEBUG_UNASSIGNED
>  //#define DEBUG_ASI
>  //#define DEBUG_PCALL
> +//#define DEBUG_PSTATE
>
>  #ifdef DEBUG_MMU
>  #define DPRINTF_MMU(fmt, ...)                                   \
> @@ -31,6 +32,13 @@
>     do { printf("ASI: " fmt , ## __VA_ARGS__); } while (0)
>  #endif
>
> +#ifdef DEBUG_PSTATE
> +#define DPRINTF_PSTATE(fmt, ...)                                   \
> +    do { printf("PSTATE: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF_PSTATE(fmt, ...) do {} while (0)
> +#endif
> +
>  #ifdef TARGET_SPARC64
>  #ifndef TARGET_ABI32
>  #define AM_CHECK(env1) ((env1)->pstate & PS_AM)
> @@ -3244,6 +3252,12 @@ static inline uint64_t *get_gregset(uint64_t pstate)
>  {
>     switch (pstate) {
>     default:
> +        DPRINTF_PSTATE("ERROR in get_gregset: active pstate bits=%lX %s %s %s",
> +                pstate,
> +                (pstate&PS_IG)?"IG":"",
> +                (pstate&PS_MG)?"MG":"",
> +                (pstate&PS_AG)?"AG":"" );
> +        /* pass through to normal set of global registers */

%lX format is not correct on 64 bit hosts and nobody uses upper case
hex, please use just %x. The spacing is too tight.

On UltraSPARC-IV and T1/2 the bits are interpreted differently (GL) so
the printout would be wrong, but that can be disregarded for now.

>     case 0:
>         return env->bgregs;
>     case PS_AG:
> @@ -3269,12 +3283,18 @@ static inline void change_pstate(uint64_t new_pstate)
>     new_pstate_regs = new_pstate & 0xc01;
>
>     if (new_pstate_regs != pstate_regs) {
> +        DPRINTF_PSTATE("change_pstate: switching regs old=%lX new=%lX\n",
> +                       pstate_regs, new_pstate_regs);

Same format issue here.

>         // Switch global register bank
>         src = get_gregset(new_pstate_regs);
>         dst = get_gregset(pstate_regs);
>         memcpy32(dst, env->gregs);
>         memcpy32(env->gregs, src);
>     }
> +    else {
> +        DPRINTF_PSTATE("change_pstate: regs new=%lX (unchanged)\n",
> +                       new_pstate_regs);
> +    }

And here.

Re: [Qemu-devel] [PATCH 2/9] sparc64: add PSR and PIL to cpu state dump

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/helper.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index a06923a..0f0e583 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -1452,6 +1452,7 @@ void cpu_dump_state(CPUState *env, FILE *f,
>  #ifdef TARGET_SPARC64
>     cpu_fprintf(f, "pstate: 0x%08x ccr: 0x%02x asi: 0x%02x tl: %d fprs: %d\n",
>                 env->pstate, GET_CCR(env), env->asi, env->tl, env->fprs);
> +    cpu_fprintf(f, "psr: 0x%08x pil=%x\n", GET_PSR(env), env->psrpil);

PSR does not exist on Sparc64, instead we have separate registers,
like VER, CWP and CCR. PIL part is OK.

Re: [Qemu-devel] [PATCH 3/9] sparc64: use helper_wrpil to check pending irq on write

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/helper.h    |    1 +
>  target-sparc/op_helper.c |   14 ++++++++++++++
>  target-sparc/translate.c |    5 +----
>  3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/target-sparc/helper.h b/target-sparc/helper.h
> index 4002b9e..6f103e7 100644
> --- a/target-sparc/helper.h
> +++ b/target-sparc/helper.h
> @@ -5,6 +5,7 @@ DEF_HELPER_0(rett, void)
>  DEF_HELPER_1(wrpsr, void, tl)
>  DEF_HELPER_0(rdpsr, tl)
>  #else
> +DEF_HELPER_1(wrpil, void, tl)
>  DEF_HELPER_1(wrpstate, void, tl)
>  DEF_HELPER_0(done, void)
>  DEF_HELPER_0(retry, void)
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index 26092e5..a7da0e4 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -3303,6 +3303,20 @@ void helper_wrpstate(target_ulong new_state)
>     change_pstate(new_state & 0xf3f);
>  }
>
> +void helper_wrpil(target_ulong new_pil)
> +{
> +    DPRINTF_PSTATE("helper_wrpil old=%X new=%X\n",
> +                   env->psrpil, (uint32_t)new_pil);
> +
> +    env->psrpil = new_pil;
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    if (env->pstate & PS_IE) {
> +        cpu_check_irqs(env);
> +    }
> +#endif
> +}

It's not possible to write to PIL in user mode, so the whole code
inside the function should be surrounded by #if
!defined(CONFIG_USER_ONLY)/#endif. Please use lowercase hex.

I'm wondering about using target_ulong instead of uint32_t (type of
env->psrpil). TL makes the generated code shorter, uint32_t would
decrease TCG register pressure on 32 bit hosts. This is not
performance critical, so either way should be OK.

Re: [Qemu-devel] [PATCH 4/9] sparc64: check for pending irq when pil, pstate or softint is changed

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/op_helper.c |   39 ++++++++++++++++++++++++++++++++++++---
>  1 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index a7da0e4..b1978cb 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -3301,6 +3301,12 @@ static inline void change_pstate(uint64_t new_pstate)
>  void helper_wrpstate(target_ulong new_state)
>  {
>     change_pstate(new_state & 0xf3f);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    if (env->pstate & PS_IE) {
> +        cpu_check_irqs(env);
> +    }
> +#endif
>  }
>
>  void helper_wrpil(target_ulong new_pil)
> @@ -3328,6 +3334,14 @@ void helper_done(void)
>     change_pstate((tsptr->tstate >> 8) & 0xf3f);
>     PUT_CWP64(env, tsptr->tstate & 0xff);
>     env->tl--;
> +
> +    DPRINTF_PSTATE("... helper_done tl=%d\n", env->tl);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    if (env->pstate & PS_IE) {
> +        cpu_check_irqs(env);
> +    }
> +#endif
>  }
>
>  void helper_retry(void)
> @@ -3341,21 +3355,40 @@ void helper_retry(void)
>     change_pstate((tsptr->tstate >> 8) & 0xf3f);
>     PUT_CWP64(env, tsptr->tstate & 0xff);
>     env->tl--;
> +
> +    DPRINTF_PSTATE("... helper_retry tl=%d\n", env->tl);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    if (env->pstate & PS_IE) {
> +        cpu_check_irqs(env);
> +    }
> +#endif
> +}
> +
> +static void do_modify_softint(const char* operation, uint32_t value)
> +{
> +    if (env->softint != value) {
> +        env->softint = value;
> +        DPRINTF_PSTATE(": %s new %08X\n", operation, env->softint);

Uppercase hex again. Otherwise the patch looks OK.

Maybe the #if CONFIG_USER_ONLY/#endif could be avoided if
cpu_check_irqs would be #defined as a dummy macro for CONFIG_USER_ONLY
in cpu.h.

Re: [Qemu-devel] [PATCH 5/9] sparc64: add macros to deal with softint and timer interrupt

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/cpu.h |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 1fe4d0f..0dba241 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -394,6 +394,8 @@ typedef struct CPUSPARCState {
>     uint64_t fprs;
>     uint64_t tick_cmpr, stick_cmpr;
>     void *tick, *stick;
> +#define TICK_NPT_MASK        0x8000000000000000ULL
> +#define TICK_SOFTINT_DISABLE 0x8000000000000000ULL

Please move the TICK_NPT and TICK_INT_DIS macros from sun4u.c to here.

>     uint64_t gsr;
>     uint32_t gl; // UA2005
>     /* UA 2005 hyperprivileged registers */
> @@ -402,6 +404,10 @@ typedef struct CPUSPARCState {
>     uint32_t softint;
>  #define SOFTINT_TIMER   1
>  #define SOFTINT_STIMER  (1 << 16)
> +#define SOFTINT_INTRMASK (0xFFFE)
> +#define SOFTINT_TM       (1 << 0)
> +#define SOFTINT_SM       (1 << 16)

Why the duplicate definitions?

> +#define SOFTINT_REG_MASK (SOFTINT_SM|SOFTINT_INTRMASK|SOFTINT_TM)
>  #endif
>     sparc_def_t *def;
>  } CPUSPARCState;
>
>
>
>

Re: [Qemu-devel] [PATCH 8/9] sparc64: interrupt trap handling

[Blue Swirl]

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> cpu_check_irqs
> - handle SOFTINT register TICK and STICK timer bits
> - only check interrupt levels greater than PIL value
> - handle preemption by higher level traps
>
> cpu_exec
> - handle CPU_INTERRUPT_HARD only if interrupts are enabled
> - PIL 15 is not special level on sparcv9
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  cpu-exec.c |   40 ++++++++++++++++++++++------------------
>  hw/sun4u.c |   52 +++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 59 insertions(+), 33 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index af4595b..65192c1 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -449,24 +449,28 @@ int cpu_exec(CPUState *env1)
>                         next_tb = 0;
>                     }
>  #elif defined(TARGET_SPARC)
> -                    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
> -                       cpu_interrupts_enabled(env)) {
> -                       int pil = env->interrupt_index & 15;
> -                       int type = env->interrupt_index & 0xf0;
> -
> -                       if (((type == TT_EXTINT) &&
> -                            (pil == 15 || pil > env->psrpil)) ||
> -                           type != TT_EXTINT) {
> -                           env->interrupt_request &= ~CPU_INTERRUPT_HARD;
> -                            env->exception_index = env->interrupt_index;
> -                            do_interrupt(env);
> -                           env->interrupt_index = 0;
> -                        next_tb = 0;
> -                       }
> -                   } else if (interrupt_request & CPU_INTERRUPT_TIMER) {
> -                       //do_interrupt(0, 0, 0, 0, 0);
> -                       env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
> -                   }
> +                    if ((interrupt_request & CPU_INTERRUPT_HARD)) {
> +                        if (cpu_interrupts_enabled(env)) {
> +                            int pil = env->interrupt_index & 0xf;
> +                            int type = env->interrupt_index & 0xf0;
> +
> +                            if (((type == TT_EXTINT) && (pil > env->psrpil)) ||
> +                                            type != TT_EXTINT) {

This removes the check for level 15, which is non-maskable on V8.

> +                                //env->interrupt_request &= ~CPU_INTERRUPT_HARD;

Remove.

> +                                if (env->interrupt_index > 0) {
> +                                    env->exception_index = env->interrupt_index;
> +                                    do_interrupt(env);
> +                                    next_tb = 0;
> +                                }
> +                            }
> +                        }
> +                    }
> +                    else if (interrupt_request & CPU_INTERRUPT_TIMER) {

'else if' etc. should be on the same line as '}'.

> +                        env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
> +#if !defined(CONFIG_USER_ONLY)
> +                        cpu_check_irqs(env);
> +#endif
> +                    }
>  #elif defined(TARGET_ARM)
>                     if (interrupt_request & CPU_INTERRUPT_FIQ
>                         && !(env->uncached_cpsr & CPSR_F)) {
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index 9d46f08..84a8043 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -233,29 +233,51 @@ void irq_info(Monitor *mon)
>
>  void cpu_check_irqs(CPUState *env)
>  {
> -    uint32_t pil = env->pil_in | (env->softint & ~SOFTINT_TIMER) |
> -        ((env->softint & SOFTINT_TIMER) << 14);
> +    uint32_t pil = env->pil_in | (env->softint & ~(SOFTINT_TM | SOFTINT_SM));
> +
> +    /* check if TM or SM in SOFTINT are set
> +       setting these also causes interrupt 14 */
> +    if (env->softint & (SOFTINT_TM | SOFTINT_SM))
> +        pil |= 1 << 14;
> +
> +    if (!pil) {
> +        if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> +            CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %X)\n",
> +                           env->interrupt_index);
> +            env->interrupt_index = 0;
> +            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);

All other architectures have this code in cpu-exec.c, why move?

Uppercase hex.

> +        }
> +        return;
> +    }
> +
> +    if (cpu_interrupts_enabled(env)) {
>
> -    if (pil && (env->interrupt_index == 0 ||
> -                (env->interrupt_index & ~15) == TT_EXTINT)) {
>         unsigned int i;
>
> -        for (i = 15; i > 0; i--) {
> +        for (i = 15; i > env->psrpil; i--) {
>             if (pil & (1 << i)) {
>                 int old_interrupt = env->interrupt_index;
> -
> -                env->interrupt_index = TT_EXTINT | i;
> -                if (old_interrupt != env->interrupt_index) {
> -                    CPUIRQ_DPRINTF("Set CPU IRQ %d\n", i);
> -                    cpu_interrupt(env, CPU_INTERRUPT_HARD);
> -                }
> +                int new_interrupt = TT_EXTINT | i;
> +
> +                    if (env->tl > 0 && cpu_tsptr(env)->tt > new_interrupt) {
> +                        CPUIRQ_DPRINTF("Not setting CPU IRQ: TL=%d "
> +                                "current %X >= pending %X\n",
> +                                env->tl, cpu_tsptr(env)->tt, new_interrupt);
> +                    }
> +                    else if (old_interrupt != new_interrupt) {

'else if' etc. should be on the same line as '}'.

> +                        env->interrupt_index = new_interrupt;
> +                        CPUIRQ_DPRINTF("Set CPU IRQ %d old=%X new=%X\n", i,
> +                                old_interrupt, new_interrupt);
> +                        cpu_interrupt(env, CPU_INTERRUPT_HARD);
> +                    }
>                 break;
>             }
>         }
> -    } else if (!pil && (env->interrupt_index & ~15) == TT_EXTINT) {
> -        CPUIRQ_DPRINTF("Reset CPU IRQ %d\n", env->interrupt_index & 15);
> -        env->interrupt_index = 0;
> -        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
> +    }
> +    else {
> +        CPUIRQ_DPRINTF("Interrupts disabled, pil=%08X pil_in=%08X softint=%08X "
> +                "current interrupt %X\n",
> +                pil, env->pil_in, env->softint, env->interrupt_index);
>     }
>  }

Overall, it looks like there are some unnecessary changes so that it's
hard to see what is the fix.

Re: [Qemu-devel] [PATCH 6/9] sparc64: clear exception_index with -1 value

[Blue Swirl]

Thanks, applied.

On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> ---
>  target-sparc/op_helper.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index b1978cb..94f1c7a 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -3535,7 +3535,7 @@ void do_interrupt(CPUState *env)
>     env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>     env->pc = env->tbr;
>     env->npc = env->pc + 4;
> -    env->exception_index = 0;
> +    env->exception_index = -1;
>  }
>  #else
>  #ifdef DEBUG_PCALL
>
>
>
>

Re: [Qemu-devel] [PATCH 6/9] sparc64: clear exception_index with -1 value

[Artyom Tarasenko]

What's the effect of the patch? Don't we need it for sparc32 too? The
code looks similar.

2010/1/6 Blue Swirl <blauwirbel@gmail.com>:
> Thanks, applied.
>
> On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
> <igor.v.kovalenko@gmail.com> wrote:
>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>
>> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>> ---
>>  target-sparc/op_helper.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
>> index b1978cb..94f1c7a 100644
>> --- a/target-sparc/op_helper.c
>> +++ b/target-sparc/op_helper.c
>> @@ -3535,7 +3535,7 @@ void do_interrupt(CPUState *env)
>>     env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>>     env->pc = env->tbr;
>>     env->npc = env->pc + 4;
>> -    env->exception_index = 0;
>> +    env->exception_index = -1;
>>  }
>>  #else
>>  #ifdef DEBUG_PCALL
>>
>>
>>
>>
>
>
>



-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

Re: [Qemu-devel] [PATCH 6/9] sparc64: clear exception_index with -1 value

[Igor Kovalenko]

On Thu, Jan 7, 2010 at 2:29 AM, Artyom Tarasenko
<atar4qemu@googlemail.com> wrote:
> What's the effect of the patch? Don't we need it for sparc32 too? The
> code looks similar.
>
> 2010/1/6 Blue Swirl <blauwirbel@gmail.com>:
>> Thanks, applied.
>>
>> On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
>> <igor.v.kovalenko@gmail.com> wrote:
>>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>>
>>> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>> ---
>>>  target-sparc/op_helper.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
>>> index b1978cb..94f1c7a 100644
>>> --- a/target-sparc/op_helper.c
>>> +++ b/target-sparc/op_helper.c
>>> @@ -3535,7 +3535,7 @@ void do_interrupt(CPUState *env)
>>>     env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>>>     env->pc = env->tbr;
>>>     env->npc = env->pc + 4;
>>> -    env->exception_index = 0;
>>> +    env->exception_index = -1;
>>>  }
>>>  #else
>>>  #ifdef DEBUG_PCALL

Right, but that's out of scope for this changeset.

In fact sparc32 should have the same issue, as well as alpha, s390 and
sh4. Check in cpu_exec() for valid exception_index is "greater or
equal to zero" so all those targets should be corrected. On sh4 we
even check for "invalid value of exception_index is -1"

-- 
Kind regards,
Igor V. Kovalenko

Re: [Qemu-devel] [PATCH 8/9] sparc64: interrupt trap handling

[Igor Kovalenko]

On Wed, Jan 6, 2010 at 8:00 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
> <igor.v.kovalenko@gmail.com> wrote:
>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>
>> cpu_check_irqs
>> - handle SOFTINT register TICK and STICK timer bits
>> - only check interrupt levels greater than PIL value
>> - handle preemption by higher level traps
>>
>> cpu_exec
>> - handle CPU_INTERRUPT_HARD only if interrupts are enabled
>> - PIL 15 is not special level on sparcv9
>>
>> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>> ---
>>  cpu-exec.c |   40 ++++++++++++++++++++++------------------
>>  hw/sun4u.c |   52 +++++++++++++++++++++++++++++++++++++---------------
>>  2 files changed, 59 insertions(+), 33 deletions(-)
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index af4595b..65192c1 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -449,24 +449,28 @@ int cpu_exec(CPUState *env1)
>>                         next_tb = 0;
>>                     }
>>  #elif defined(TARGET_SPARC)
>> -                    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
>> -                       cpu_interrupts_enabled(env)) {
>> -                       int pil = env->interrupt_index & 15;
>> -                       int type = env->interrupt_index & 0xf0;
>> -
>> -                       if (((type == TT_EXTINT) &&
>> -                            (pil == 15 || pil > env->psrpil)) ||
>> -                           type != TT_EXTINT) {
>> -                           env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>> -                            env->exception_index = env->interrupt_index;
>> -                            do_interrupt(env);
>> -                           env->interrupt_index = 0;
>> -                        next_tb = 0;
>> -                       }
>> -                   } else if (interrupt_request & CPU_INTERRUPT_TIMER) {
>> -                       //do_interrupt(0, 0, 0, 0, 0);
>> -                       env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
>> -                   }
>> +                    if ((interrupt_request & CPU_INTERRUPT_HARD)) {
>> +                        if (cpu_interrupts_enabled(env)) {
>> +                            int pil = env->interrupt_index & 0xf;
>> +                            int type = env->interrupt_index & 0xf0;
>> +
>> +                            if (((type == TT_EXTINT) && (pil > env->psrpil)) ||
>> +                                            type != TT_EXTINT) {
>
> This removes the check for level 15, which is non-maskable on V8.

I'll implement cpu_pil_allowed() to hide v8 vs v9 difference wrt psrpil.

>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>> index 9d46f08..84a8043 100644
>> --- a/hw/sun4u.c
>> +++ b/hw/sun4u.c
>> @@ -233,29 +233,51 @@ void irq_info(Monitor *mon)
>>
>>  void cpu_check_irqs(CPUState *env)
>>  {
>> -    uint32_t pil = env->pil_in | (env->softint & ~SOFTINT_TIMER) |
>> -        ((env->softint & SOFTINT_TIMER) << 14);
>> +    uint32_t pil = env->pil_in | (env->softint & ~(SOFTINT_TM | SOFTINT_SM));
>> +
>> +    /* check if TM or SM in SOFTINT are set
>> +       setting these also causes interrupt 14 */
>> +    if (env->softint & (SOFTINT_TM | SOFTINT_SM))
>> +        pil |= 1 << 14;
>> +
>> +    if (!pil) {
>> +        if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> +            CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %X)\n",
>> +                           env->interrupt_index);
>> +            env->interrupt_index = 0;
>> +            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>
> All other architectures have this code in cpu-exec.c, why move?

Not really a move (from cpu-exec.c) - see below, it was there already,
and sparc32 code is similar. PPC can do that in cpu-exec.c since it
exposes pending_interrupts flag. Other arches also seem to reset HARD
flag wherever fits.

For sparc64 clearing flag in cpu-exec.c would require pulling some
code to deal with softint bits. That way we could reduce
cpu_check_irqs() to set separate flag "possible interrupt state
change" and return from tb so cpu-exec.c code would pick the flag and
deal with interrupt.

That implementation I'd like less than consolidating trigger logic
into cpu_check_irqs.

> Overall, it looks like there are some unnecessary changes so that it's
> hard to see what is the fix.

Since code used to be tabbed as well, there is a great deal of churn here.
I tried to outline fixes in the changeset header.

-- 
Kind regards,
Igor V. Kovalenko

Re: [Qemu-devel] [PATCH 6/9] sparc64: clear exception_index with -1 value

[Blue Swirl]

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

On Wed, Jan 6, 2010 at 11:57 PM, Igor Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> On Thu, Jan 7, 2010 at 2:29 AM, Artyom Tarasenko
> <atar4qemu@googlemail.com> wrote:
>> What's the effect of the patch? Don't we need it for sparc32 too? The
>> code looks similar.
>>
>> 2010/1/6 Blue Swirl <blauwirbel@gmail.com>:
>>> Thanks, applied.
>>>
>>> On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
>>> <igor.v.kovalenko@gmail.com> wrote:
>>>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>>>
>>>> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>>> ---
>>>>  target-sparc/op_helper.c |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
>>>> index b1978cb..94f1c7a 100644
>>>> --- a/target-sparc/op_helper.c
>>>> +++ b/target-sparc/op_helper.c
>>>> @@ -3535,7 +3535,7 @@ void do_interrupt(CPUState *env)
>>>>     env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>>>>     env->pc = env->tbr;
>>>>     env->npc = env->pc + 4;
>>>> -    env->exception_index = 0;
>>>> +    env->exception_index = -1;
>>>>  }
>>>>  #else
>>>>  #ifdef DEBUG_PCALL
>
> Right, but that's out of scope for this changeset.
>
> In fact sparc32 should have the same issue, as well as alpha, s390 and
> sh4. Check in cpu_exec() for valid exception_index is "greater or
> equal to zero" so all those targets should be corrected. On sh4 we
> even check for "invalid value of exception_index is -1"

Thanks, I applied the fix for Sparc32. Here's also an untested fix for
other architectures.

[-- Attachment #2: 0001-Fix-incorrect-exception_index-use.patch --]
[-- Type: application/x-patch, Size: 2121 bytes --]