[Qemu-devel] [PATCH v2 0/5] target-mips: Preparations for CPUState part 4b series

[Qemu-devel] [PATCH v2 0/5] target-mips: Preparations for CPUState part 4b series

[Andreas Färber]

Hello Aurélien,

This series picks up some preparatory patches for QOM CPUState refactoring,
originally posted in May. They still applied cleanly, but I optimized them
a bit and expanded the explanations.

In short it is about MIPSCPU vs. CPUMIPSState; more fields will be moved
from CPU_COMMON macro to CPUState struct, and to access CPUState we need
a MIPSCPU. Thus MIPSCPU is preferable for arguments of static helpers
(not TCG helpers) because we save some redundant accessor/cast macros.

Can you please ack/apply and keep in mind for the current patch review?

Available for testing from:
git://github.com/afaerber/qemu-cpu.git qom-cpu-mips
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-mips

Regards,
Andreas

Cc: Aurélien Jarno <aurelien@aurel32.net>

Cc: Jia Liu <proljc@gmail.com>
Cc: Eric Johnson <ericj@mips.com>
Cc: Richard Henderson <rth@twiddle.net>

v1 -> v2:
* Cherry-picked from my CPUState part 4 series
* Avoided calling mips_env_get_cpu() in both if branches of helper_m[t]tc0_tchalt()
* Prepended patch to clean up resulting variable naming mess
* Placed variable declarations in the closest block (requested by Alex elsewhere)

Andreas Färber (5):
  target-mips: Clean up other_cpu in helper_{d,e}vpe()
  target-mips: Pass MIPSCPU to mips_tc_wake()
  target-mips: Pass MIPSCPU to mips_vpe_is_wfi()
  target-mips: Pass MIPSCPU to mips_tc_sleep()
  target-mips: Pass MIPSCPU to mips_vpe_sleep()

 target-mips/op_helper.c |   63 +++++++++++++++++++++++++++++------------------
 1 Datei geändert, 39 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 1/5] target-mips: Clean up other_cpu in helper_{d, e}vpe()

[Andreas Färber]

Free the variable name "other_cpu" for later use for MIPSCPU.

Fix off-by-one indentation while at it.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-mips/op_helper.c |   28 ++++++++++++++--------------
 1 Datei geändert, 14 Zeilen hinzugefügt(+), 14 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index ce5ddaf..1051c44 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1874,35 +1874,35 @@ target_ulong helper_emt(void)
 
 target_ulong helper_dvpe(CPUMIPSState *env)
 {
-    CPUMIPSState *other_cpu = first_cpu;
+    CPUMIPSState *other_cpu_env = first_cpu;
     target_ulong prev = env->mvp->CP0_MVPControl;
 
     do {
         /* Turn off all VPEs except the one executing the dvpe.  */
-        if (other_cpu != env) {
-            other_cpu->mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
-            mips_vpe_sleep(other_cpu);
+        if (other_cpu_env != env) {
+            other_cpu_env->mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
+            mips_vpe_sleep(other_cpu_env);
         }
-        other_cpu = other_cpu->next_cpu;
-    } while (other_cpu);
+        other_cpu_env = other_cpu_env->next_cpu;
+    } while (other_cpu_env);
     return prev;
 }
 
 target_ulong helper_evpe(CPUMIPSState *env)
 {
-    CPUMIPSState *other_cpu = first_cpu;
+    CPUMIPSState *other_cpu_env = first_cpu;
     target_ulong prev = env->mvp->CP0_MVPControl;
 
     do {
-        if (other_cpu != env
-           /* If the VPE is WFI, don't disturb its sleep.  */
-           && !mips_vpe_is_wfi(other_cpu)) {
+        if (other_cpu_env != env
+            /* If the VPE is WFI, don't disturb its sleep.  */
+            && !mips_vpe_is_wfi(other_cpu_env)) {
             /* Enable the VPE.  */
-            other_cpu->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
-            mips_vpe_wake(other_cpu); /* And wake it up.  */
+            other_cpu_env->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
+            mips_vpe_wake(other_cpu_env); /* And wake it up.  */
         }
-        other_cpu = other_cpu->next_cpu;
-    } while (other_cpu);
+        other_cpu_env = other_cpu_env->next_cpu;
+    } while (other_cpu_env);
     return prev;
 }
 #endif /* !CONFIG_USER_ONLY */
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 2/5] target-mips: Pass MIPSCPU to mips_tc_wake()

[Andreas Färber]

Needed for changing mips_vpe_is_wfi() argument type to MIPSCPU.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-mips/op_helper.c |   11 ++++++++---
 1 Datei geändert, 8 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 1051c44..e721a4d 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -737,8 +737,10 @@ static inline void mips_vpe_sleep(CPUMIPSState *c)
     cpu_reset_interrupt(c, CPU_INTERRUPT_WAKE);
 }
 
-static inline void mips_tc_wake(CPUMIPSState *c, int tc)
+static inline void mips_tc_wake(MIPSCPU *cpu, int tc)
 {
+    CPUMIPSState *c = &cpu->env;
+
     /* FIXME: TC reschedule.  */
     if (mips_vpe_active(c) && !mips_vpe_is_wfi(c)) {
         mips_vpe_wake(c);
@@ -1342,13 +1344,15 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1)
 {
+    MIPSCPU *cpu = mips_env_get_cpu(env);
+
     env->active_tc.CP0_TCHalt = arg1 & 0x1;
 
     // TODO: Halt TC / Restart (if allocated+active) TC.
     if (env->active_tc.CP0_TCHalt & 1) {
         mips_tc_sleep(env, env->current_tc);
     } else {
-        mips_tc_wake(env, env->current_tc);
+        mips_tc_wake(cpu, env->current_tc);
     }
 }
 
@@ -1356,6 +1360,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1)
 {
     int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
     CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
+    MIPSCPU *other_cpu = mips_env_get_cpu(other);
 
     // TODO: Halt TC / Restart (if allocated+active) TC.
 
@@ -1367,7 +1372,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1)
     if (arg1 & 1) {
         mips_tc_sleep(other, other_tc);
     } else {
-        mips_tc_wake(other, other_tc);
+        mips_tc_wake(other_cpu, other_tc);
     }
 }
 
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/5] target-mips: Pass MIPSCPU to mips_vpe_is_wfi()

[Andreas Färber]

Needed for moving halted field to CPUState.
The variable name "c" is retained for MIPSCPU to leave "cpu" for CPUState.

Also change return type to bool while at it.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-mips/op_helper.c |   12 ++++++++----
 1 Datei geändert, 8 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index e721a4d..9770741 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -714,11 +714,13 @@ void helper_sdm(CPUMIPSState *env, target_ulong addr, target_ulong reglist,
 
 #ifndef CONFIG_USER_ONLY
 /* SMP helpers.  */
-static int mips_vpe_is_wfi(CPUMIPSState *c)
+static bool mips_vpe_is_wfi(MIPSCPU *c)
 {
+    CPUMIPSState *env = &c->env;
+
     /* If the VPE is halted but otherwise active, it means it's waiting for
        an interrupt.  */
-    return c->halted && mips_vpe_active(c);
+    return env->halted && mips_vpe_active(env);
 }
 
 static inline void mips_vpe_wake(CPUMIPSState *c)
@@ -742,7 +744,7 @@ static inline void mips_tc_wake(MIPSCPU *cpu, int tc)
     CPUMIPSState *c = &cpu->env;
 
     /* FIXME: TC reschedule.  */
-    if (mips_vpe_active(c) && !mips_vpe_is_wfi(c)) {
+    if (mips_vpe_active(c) && !mips_vpe_is_wfi(cpu)) {
         mips_vpe_wake(c);
     }
 }
@@ -1899,9 +1901,11 @@ target_ulong helper_evpe(CPUMIPSState *env)
     target_ulong prev = env->mvp->CP0_MVPControl;
 
     do {
+        MIPSCPU *other_cpu = mips_env_get_cpu(other_cpu_env);
+
         if (other_cpu_env != env
             /* If the VPE is WFI, don't disturb its sleep.  */
-            && !mips_vpe_is_wfi(other_cpu_env)) {
+            && !mips_vpe_is_wfi(other_cpu)) {
             /* Enable the VPE.  */
             other_cpu_env->mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
             mips_vpe_wake(other_cpu_env); /* And wake it up.  */
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 4/5] target-mips: Pass MIPSCPU to mips_tc_sleep()

[Andreas Färber]

Needed for changing mips_vpe_sleep() argument type to MIPSCPU.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-mips/op_helper.c |    8 +++++---
 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 9770741..5710dd0 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -749,8 +749,10 @@ static inline void mips_tc_wake(MIPSCPU *cpu, int tc)
     }
 }
 
-static inline void mips_tc_sleep(CPUMIPSState *c, int tc)
+static inline void mips_tc_sleep(MIPSCPU *cpu, int tc)
 {
+    CPUMIPSState *c = &cpu->env;
+
     /* FIXME: TC reschedule.  */
     if (!mips_vpe_active(c)) {
         mips_vpe_sleep(c);
@@ -1352,7 +1354,7 @@ void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1)
 
     // TODO: Halt TC / Restart (if allocated+active) TC.
     if (env->active_tc.CP0_TCHalt & 1) {
-        mips_tc_sleep(env, env->current_tc);
+        mips_tc_sleep(cpu, env->current_tc);
     } else {
         mips_tc_wake(cpu, env->current_tc);
     }
@@ -1372,7 +1374,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1)
         other->tcs[other_tc].CP0_TCHalt = arg1;
 
     if (arg1 & 1) {
-        mips_tc_sleep(other, other_tc);
+        mips_tc_sleep(other_cpu, other_tc);
     } else {
         mips_tc_wake(other_cpu, other_tc);
     }
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 5/5] target-mips: Pass MIPSCPU to mips_vpe_sleep()

[Andreas Färber]

Needed for moving halted field to CPUState.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-mips/op_helper.c |   10 +++++++---
 1 Datei geändert, 7 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5710dd0..05b7730 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -731,8 +731,10 @@ static inline void mips_vpe_wake(CPUMIPSState *c)
     cpu_interrupt(c, CPU_INTERRUPT_WAKE);
 }
 
-static inline void mips_vpe_sleep(CPUMIPSState *c)
+static inline void mips_vpe_sleep(MIPSCPU *cpu)
 {
+    CPUMIPSState *c = &cpu->env;
+
     /* The VPE was shut off, really go to bed.
        Reset any old _WAKE requests.  */
     c->halted = 1;
@@ -755,7 +757,7 @@ static inline void mips_tc_sleep(MIPSCPU *cpu, int tc)
 
     /* FIXME: TC reschedule.  */
     if (!mips_vpe_active(c)) {
-        mips_vpe_sleep(c);
+        mips_vpe_sleep(cpu);
     }
 }
 
@@ -1889,8 +1891,10 @@ target_ulong helper_dvpe(CPUMIPSState *env)
     do {
         /* Turn off all VPEs except the one executing the dvpe.  */
         if (other_cpu_env != env) {
+            MIPSCPU *other_cpu = mips_env_get_cpu(other_cpu_env);
+
             other_cpu_env->mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
-            mips_vpe_sleep(other_cpu_env);
+            mips_vpe_sleep(other_cpu);
         }
         other_cpu_env = other_cpu_env->next_cpu;
     } while (other_cpu_env);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 0/5] target-mips: Preparations for CPUState part 4b series

[Aurelien Jarno]

On Fri, Oct 12, 2012 at 12:56:32AM +0200, Andreas Färber wrote:
> Hello Aurélien,
> 
> This series picks up some preparatory patches for QOM CPUState refactoring,
> originally posted in May. They still applied cleanly, but I optimized them
> a bit and expanded the explanations.
> 
> In short it is about MIPSCPU vs. CPUMIPSState; more fields will be moved
> from CPU_COMMON macro to CPUState struct, and to access CPUState we need
> a MIPSCPU. Thus MIPSCPU is preferable for arguments of static helpers
> (not TCG helpers) because we save some redundant accessor/cast macros.
> 
> Can you please ack/apply and keep in mind for the current patch review?
> 
> Available for testing from:
> git://github.com/afaerber/qemu-cpu.git qom-cpu-mips
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-mips
> 
> Regards,
> Andreas
> 
> Cc: Aurélien Jarno <aurelien@aurel32.net>
> 
> Cc: Jia Liu <proljc@gmail.com>
> Cc: Eric Johnson <ericj@mips.com>
> Cc: Richard Henderson <rth@twiddle.net>
> 
> v1 -> v2:
> * Cherry-picked from my CPUState part 4 series
> * Avoided calling mips_env_get_cpu() in both if branches of helper_m[t]tc0_tchalt()
> * Prepended patch to clean up resulting variable naming mess
> * Placed variable declarations in the closest block (requested by Alex elsewhere)
> 
> Andreas Färber (5):
>   target-mips: Clean up other_cpu in helper_{d,e}vpe()
>   target-mips: Pass MIPSCPU to mips_tc_wake()
>   target-mips: Pass MIPSCPU to mips_vpe_is_wfi()
>   target-mips: Pass MIPSCPU to mips_tc_sleep()
>   target-mips: Pass MIPSCPU to mips_vpe_sleep()
> 
>  target-mips/op_helper.c |   63 +++++++++++++++++++++++++++++------------------
>  1 Datei geändert, 39 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
> 
> -- 
> 1.7.10.4

Thanks, all applied.

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