[Qemu-devel] [PATCH 0/4] introduce NEED_GLOBAL_ENV

[Qemu-devel] [PATCH 0/4] introduce NEED_GLOBAL_ENV

[Paolo Bonzini]

Let's start the cleanups from the feature required by Blue Swirl.
I also include here a baby step towards removing eminently TCG-related
stuff from cpu.h.

After this series, only a bunch of files will include exec-all.h,
instead of getting it indirectly from cpu.h.

Note that (as sworn in the previous submission) exec.h is only included
by files that need the global register variable (i.e. cpu-exec.c and
target-*/op_helper.c), and this is the same subset that gets
NEED_GLOBAL_ENV in this patchset.

i386 and sparc have functions declared in cpu.h that are in op_helper.c.
I checked that these do not need the global variable, but it would be
nice to cleanup those too.

Paolo Bonzini (4):
  remove unused stuff from */exec.h
  move cpu_pc_from_tb to target-*/exec.h
  remove exec-all.h inclusion from cpu.h
  require #define NEED_GLOBAL_ENV for files that need the global
    register variable

 cpu-exec.c                    |    2 ++
 exec-all.h                    |    4 ++++
 gdbstub.c                     |    1 +
 hw/xen_domainbuild.c          |    1 +
 kvm-stub.c                    |    1 +
 monitor.c                     |    1 +
 target-alpha/cpu.h            |    6 ------
 target-alpha/exec.h           |    9 +++++----
 target-alpha/op_helper.c      |    1 +
 target-arm/cpu.h              |    6 ------
 target-arm/exec.h             |    8 ++++++--
 target-arm/op_helper.c        |    1 +
 target-cris/cpu.h             |    6 ------
 target-cris/exec.h            |   11 ++++++-----
 target-cris/op_helper.c       |    1 +
 target-i386/cpu.h             |    7 -------
 target-i386/exec.h            |   17 ++++++-----------
 target-i386/op_helper.c       |    3 ++-
 target-m68k/cpu.h             |    6 ------
 target-m68k/exec.h            |    8 ++++++--
 target-m68k/op_helper.c       |    2 ++
 target-microblaze/cpu.h       |    6 ------
 target-microblaze/exec.h      |   10 ++++++----
 target-microblaze/op_helper.c |    1 +
 target-mips/cpu.h             |    8 --------
 target-mips/exec.h            |   17 +++++++----------
 target-mips/op_helper.c       |    7 +++++++
 target-ppc/cpu.h              |    6 ------
 target-ppc/exec.h             |    7 +++++--
 target-ppc/op_helper.c        |    2 ++
 target-s390x/cpu.h            |    6 ------
 target-s390x/exec.h           |    8 ++++++--
 target-s390x/op_helper.c      |    1 +
 target-sh4/cpu.h              |    7 -------
 target-sh4/exec.h             |    8 ++++++--
 target-sh4/op_helper.c        |    2 ++
 target-sparc/cpu.h            |    7 -------
 target-sparc/exec.h           |    8 ++++++--
 target-sparc/op_helper.c      |    1 +
 39 files changed, 96 insertions(+), 118 deletions(-)

[Qemu-devel] [PATCH 1/4] remove unused stuff from */exec.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-alpha/exec.h      |    2 --
 target-cris/exec.h       |    3 ---
 target-i386/exec.h       |    9 ---------
 target-i386/op_helper.c  |    2 +-
 target-microblaze/exec.h |    2 --
 target-mips/exec.h       |    8 --------
 target-mips/op_helper.c  |    5 +++++
 7 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 66526e2..0fb459d 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -28,8 +28,6 @@
 
 register struct CPUAlphaState *env asm(AREG0);
 
-#define PARAM(n) ((uint64_t)PARAM##n)
-#define SPARAM(n) ((int32_t)PARAM##n)
 #define FP_STATUS (env->fp_status)
 
 #include "cpu.h"
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 728aa80..55776ba 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -28,9 +28,6 @@ register struct CPUCRISState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif
 
-void cpu_cris_flush_flags(CPUCRISState *env, int cc_op);
-void helper_movec(CPUCRISState *env, int reg, uint32_t val);
-
 static inline int cpu_has_work(CPUState *env)
 {
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
diff --git a/target-i386/exec.h b/target-i386/exec.h
index 4ff3c57..cb34605 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -33,23 +33,14 @@ register struct CPUX86State *env asm(AREG0);
 #include "qemu-common.h"
 #include "qemu-log.h"
 
-#undef EAX
 #define EAX (env->regs[R_EAX])
-#undef ECX
 #define ECX (env->regs[R_ECX])
-#undef EDX
 #define EDX (env->regs[R_EDX])
-#undef EBX
 #define EBX (env->regs[R_EBX])
-#undef ESP
 #define ESP (env->regs[R_ESP])
-#undef EBP
 #define EBP (env->regs[R_EBP])
-#undef ESI
 #define ESI (env->regs[R_ESI])
-#undef EDI
 #define EDI (env->regs[R_EDI])
-#undef EIP
 #define EIP (env->eip)
 #define DF  (env->df)
 
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index c1256f4..00fc671 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -16,7 +16,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
-#define CPU_NO_GLOBAL_REGS
+
 #include "exec.h"
 #include "exec-all.h"
 #include "host-utils.h"
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index 646701c..db1c99e 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -27,8 +27,6 @@ register struct CPUMBState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif
 
-void cpu_mb_flush_flags(CPUMBState *env, int cc_op);
-
 static inline int cpu_has_work(CPUState *env)
 {
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
diff --git a/target-mips/exec.h b/target-mips/exec.h
index 01e9c4d..a07761d 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -17,14 +17,6 @@ register struct CPUMIPSState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */
 
-void dump_fpu(CPUState *env);
-void fpu_dump_state(CPUState *env, FILE *f,
-                    int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
-                    int flags);
-
-void cpu_mips_clock_init (CPUState *env);
-void cpu_mips_tlb_flush (CPUState *env, int flush_global);
-
 static inline int cpu_has_work(CPUState *env)
 {
     return (env->interrupt_request &
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index d09d6ed..344e0bd 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -22,6 +22,11 @@
 #include "host-utils.h"
 
 #include "helper.h"
+
+#ifndef CONFIG_USER_ONLY
+static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
+#endif
+
 /*****************************************************************************/
 /* Exceptions processing helpers */
 
-- 
1.7.0.1

[Qemu-devel] Re: [PATCH 1/4] remove unused stuff from */exec.h

[Blue Swirl]

On Mon, Jun 28, 2010 at 5:17 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-alpha/exec.h      |    2 --
>  target-cris/exec.h       |    3 ---
>  target-i386/exec.h       |    9 ---------
>  target-i386/op_helper.c  |    2 +-
>  target-microblaze/exec.h |    2 --
>  target-mips/exec.h       |    8 --------
>  target-mips/op_helper.c  |    5 +++++
>  7 files changed, 6 insertions(+), 25 deletions(-)
>
> diff --git a/target-alpha/exec.h b/target-alpha/exec.h
> index 66526e2..0fb459d 100644
> --- a/target-alpha/exec.h
> +++ b/target-alpha/exec.h
> @@ -28,8 +28,6 @@
>
>  register struct CPUAlphaState *env asm(AREG0);
>
> -#define PARAM(n) ((uint64_t)PARAM##n)
> -#define SPARAM(n) ((int32_t)PARAM##n)
>  #define FP_STATUS (env->fp_status)
>
>  #include "cpu.h"
> diff --git a/target-cris/exec.h b/target-cris/exec.h
> index 728aa80..55776ba 100644
> --- a/target-cris/exec.h
> +++ b/target-cris/exec.h
> @@ -28,9 +28,6 @@ register struct CPUCRISState *env asm(AREG0);
>  #include "softmmu_exec.h"
>  #endif
>
> -void cpu_cris_flush_flags(CPUCRISState *env, int cc_op);
> -void helper_movec(CPUCRISState *env, int reg, uint32_t val);
> -
>  static inline int cpu_has_work(CPUState *env)
>  {
>     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
> diff --git a/target-i386/exec.h b/target-i386/exec.h
> index 4ff3c57..cb34605 100644
> --- a/target-i386/exec.h
> +++ b/target-i386/exec.h
> @@ -33,23 +33,14 @@ register struct CPUX86State *env asm(AREG0);
>  #include "qemu-common.h"
>  #include "qemu-log.h"
>
> -#undef EAX
>  #define EAX (env->regs[R_EAX])
> -#undef ECX
>  #define ECX (env->regs[R_ECX])
> -#undef EDX
>  #define EDX (env->regs[R_EDX])
> -#undef EBX
>  #define EBX (env->regs[R_EBX])
> -#undef ESP
>  #define ESP (env->regs[R_ESP])
> -#undef EBP
>  #define EBP (env->regs[R_EBP])
> -#undef ESI
>  #define ESI (env->regs[R_ESI])
> -#undef EDI
>  #define EDI (env->regs[R_EDI])
> -#undef EIP
>  #define EIP (env->eip)
>  #define DF  (env->df)

IIRC these #undefs were added because some system includes on some
hosts also defined them. I think they should be replaced by open coded
versions (rather than using the QEMU_ prefix). Or just leave the
#undefs in place.

>
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index c1256f4..00fc671 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -16,7 +16,7 @@
>  * You should have received a copy of the GNU Lesser General Public
>  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>  */
> -#define CPU_NO_GLOBAL_REGS
> +
>  #include "exec.h"
>  #include "exec-all.h"
>  #include "host-utils.h"
> diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
> index 646701c..db1c99e 100644
> --- a/target-microblaze/exec.h
> +++ b/target-microblaze/exec.h
> @@ -27,8 +27,6 @@ register struct CPUMBState *env asm(AREG0);
>  #include "softmmu_exec.h"
>  #endif
>
> -void cpu_mb_flush_flags(CPUMBState *env, int cc_op);
> -
>  static inline int cpu_has_work(CPUState *env)
>  {
>     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
> diff --git a/target-mips/exec.h b/target-mips/exec.h
> index 01e9c4d..a07761d 100644
> --- a/target-mips/exec.h
> +++ b/target-mips/exec.h
> @@ -17,14 +17,6 @@ register struct CPUMIPSState *env asm(AREG0);
>  #include "softmmu_exec.h"
>  #endif /* !defined(CONFIG_USER_ONLY) */
>
> -void dump_fpu(CPUState *env);
> -void fpu_dump_state(CPUState *env, FILE *f,
> -                    int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
> -                    int flags);
> -
> -void cpu_mips_clock_init (CPUState *env);
> -void cpu_mips_tlb_flush (CPUState *env, int flush_global);
> -
>  static inline int cpu_has_work(CPUState *env)
>  {
>     return (env->interrupt_request &
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index d09d6ed..344e0bd 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -22,6 +22,11 @@
>  #include "host-utils.h"
>
>  #include "helper.h"
> +
> +#ifndef CONFIG_USER_ONLY
> +static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
> +#endif
> +

Does 'inline' make sense with a function prototype?

Please also make the implementation of cpu_mips_tlb_flush() 'static'.

Otherwise, very nice cleanup. PARAM stuff dates from pre-TCG times.

[Qemu-devel] Re: [PATCH 1/4] remove unused stuff from */exec.h

[Paolo Bonzini]

On 06/28/2010 07:43 PM, Blue Swirl wrote:
> IIRC these #undefs were added because some system includes on some
> hosts also defined them. I think they should be replaced by open coded
> versions (rather than using the QEMU_ prefix). Or just leave the
> #undefs in place.

Right, I confused them with reg_EAX/reg_EBX/etc.  They were introduced in:

commit aba1d00a41d5aff5fc78bf924627b89a298a5a5a
Author: Blue Swirl <blauwirbel@gmail.com>
Date:   Sat Sep 12 12:36:11 2009 +0000

     Work around OpenSolaris sys/regset.h namespace pollution

     Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

Paolo

[Qemu-devel] [PATCH 2/4] move cpu_pc_from_tb to target-*/exec.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-alpha/cpu.h       |    5 -----
 target-alpha/exec.h      |    5 +++++
 target-arm/cpu.h         |    5 -----
 target-arm/exec.h        |    6 ++++++
 target-cris/cpu.h        |    5 -----
 target-cris/exec.h       |    6 ++++++
 target-i386/cpu.h        |    5 -----
 target-i386/exec.h       |    6 ++++++
 target-m68k/cpu.h        |    5 -----
 target-m68k/exec.h       |    6 ++++++
 target-microblaze/cpu.h  |    5 -----
 target-microblaze/exec.h |    6 ++++++
 target-mips/cpu.h        |    7 -------
 target-mips/exec.h       |    7 +++++++
 target-ppc/cpu.h         |    5 -----
 target-ppc/exec.h        |    5 +++++
 target-s390x/cpu.h       |    5 -----
 target-s390x/exec.h      |    6 ++++++
 target-sh4/cpu.h         |    6 ------
 target-sh4/exec.h        |    6 ++++++
 target-sparc/cpu.h       |    6 ------
 target-sparc/exec.h      |    6 ++++++
 22 files changed, 65 insertions(+), 59 deletions(-)

diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 314d6ac..c333396 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -512,11 +512,6 @@ void pal_init (CPUState *env);
 void call_pal (CPUState *env);
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 0fb459d..a8a38d2 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -53,4 +53,9 @@ static inline int cpu_halted(CPUState *env)
     return EXCP_HALTED;
 }
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->pc = tb->pc;
+}
+
 #endif /* !defined (__ALPHA_EXEC_H__) */
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index f3d138d..ddf764e 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -437,11 +437,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->regs[15] = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-arm/exec.h b/target-arm/exec.h
index 0225c3f..e4c35a3 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -50,3 +50,9 @@ static inline int cpu_halted(CPUState *env) {
 #endif
 
 void raise_exception(int);
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->regs[15] = tb->pc;
+}
+
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index a62d57c..f86c52a 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -252,11 +252,6 @@ static inline void cpu_set_tls(CPUCRISState *env, target_ulong newtls)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 55776ba..93ce768 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -45,3 +45,9 @@ static inline int cpu_halted(CPUState *env) {
 	}
 	return EXCP_HALTED;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->pc = tb->pc;
+}
+
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8dafa0d..e320c80 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -936,11 +936,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #include "hw/apic.h"
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->eip = tb->pc - tb->cs_base;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-i386/exec.h b/target-i386/exec.h
index cb34605..04a566f 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -318,3 +318,9 @@ static inline void cpu_load_efer(CPUState *env, uint64_t val)
     if (env->efer & MSR_EFER_SVME)
         env->hflags |= HF_SVME_MASK;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->eip = tb->pc - tb->cs_base;
+}
+
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index b2f37ec..0f84514 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -244,11 +244,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index ece9aa0..f31e06e 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -42,3 +42,9 @@ static inline int cpu_halted(CPUState *env) {
     }
     return EXCP_HALTED;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->pc = tb->pc;
+}
+
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index ff8c8c8..a2677cf 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -307,11 +307,6 @@ static inline int cpu_interrupts_enabled(CPUState *env)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->sregs[SR_PC] = tb->pc;
-}
-
 static inline target_ulong cpu_get_pc(CPUState *env)
 {
     return env->sregs[SR_PC];
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index db1c99e..87b2494 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -44,3 +44,9 @@ static inline int cpu_halted(CPUState *env) {
 	}
 	return EXCP_HALTED;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->sregs[SR_PC] = tb->pc;
+}
+
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index c21b8e4..1aaca77 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -612,13 +612,6 @@ target_phys_addr_t cpu_mips_translate_address (CPUState *env, target_ulong addre
 		                               int rw);
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->active_tc.PC = tb->pc;
-    env->hflags &= ~MIPS_HFLAG_BMASK;
-    env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-mips/exec.h b/target-mips/exec.h
index a07761d..af61b54 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -76,4 +76,11 @@ static inline void compute_hflags(CPUState *env)
     }
 }
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->active_tc.PC = tb->pc;
+    env->hflags &= ~MIPS_HFLAG_BMASK;
+    env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
+}
+
 #endif /* !defined(__QEMU_MIPS_EXEC_H__) */
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2ad4486..ca0eb1e 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1601,11 +1601,6 @@ enum {
 
 /*****************************************************************************/
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->nip = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 09f592c..44cc5e9 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -52,4 +52,9 @@ static inline int cpu_halted(CPUState *env)
     return EXCP_HALTED;
 }
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->nip = tb->pc;
+}
+
 #endif /* !defined (__PPC_H__) */
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index dd407b2..49d3128 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -123,11 +123,6 @@ extern CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
 #define EXCP_ADDR 5 /* addressing exception */
 #define EXCP_EXECUTE_SVC 0xff00000 /* supervisor call via execute insn */
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock* tb)
-{
-    env->psw.addr = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState* env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-s390x/exec.h b/target-s390x/exec.h
index 837f853..bf3f264 100644
--- a/target-s390x/exec.h
+++ b/target-s390x/exec.h
@@ -45,3 +45,9 @@ static inline int cpu_halted(CPUState *env)
     }
     return EXCP_HALTED;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock* tb)
+{
+    env->psw.addr = tb->pc;
+}
+
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index f8b1680..f2dee37 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -303,12 +303,6 @@ static inline int cpu_ptel_pr (uint32_t ptel)
 #define PTEA_TC        (1 << 3)
 #define cpu_ptea_tc(ptea) (((ptea) & PTEA_TC) >> 3)
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->pc = tb->pc;
-    env->flags = tb->flags;
-}
-
 #define TB_FLAG_PENDING_MOVCA  (1 << 4)
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
diff --git a/target-sh4/exec.h b/target-sh4/exec.h
index edd667d..2999c02 100644
--- a/target-sh4/exec.h
+++ b/target-sh4/exec.h
@@ -47,4 +47,10 @@ static inline int cpu_halted(CPUState *env) {
 #include "softmmu_exec.h"
 #endif
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->pc = tb->pc;
+    env->flags = tb->flags;
+}
+
 #endif				/* _EXEC_SH4_H */
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 8f0484b..95fe3dc 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -625,12 +625,6 @@ void cpu_tick_set_limit(CPUTimer *timer, uint64_t limit);
 trap_state* cpu_tsptr(CPUState* env);
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-    env->pc = tb->pc;
-    env->npc = tb->cs_base;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index c84e055..f811571 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -32,4 +32,10 @@ static inline int cpu_halted(CPUState *env1) {
     return EXCP_HALTED;
 }
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+    env->pc = tb->pc;
+    env->npc = tb->cs_base;
+}
+
 #endif
-- 
1.7.0.1

[Qemu-devel] [PATCH 3/4] remove exec-all.h inclusion from cpu.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 gdbstub.c               |    1 +
 hw/xen_domainbuild.c    |    1 +
 kvm-stub.c              |    1 +
 monitor.c               |    1 +
 target-alpha/cpu.h      |    1 -
 target-arm/cpu.h        |    1 -
 target-cris/cpu.h       |    1 -
 target-i386/cpu.h       |    2 --
 target-m68k/cpu.h       |    1 -
 target-microblaze/cpu.h |    1 -
 target-mips/cpu.h       |    1 -
 target-ppc/cpu.h        |    1 -
 target-s390x/cpu.h      |    1 -
 target-sh4/cpu.h        |    1 -
 target-sparc/cpu.h      |    1 -
 15 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c1852c2..2b03ef2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -37,6 +37,7 @@
 
 #define MAX_PACKET_LENGTH 4096
 
+#include "exec-all.h"
 #include "qemu_socket.h"
 #include "kvm.h"
 
diff --git a/hw/xen_domainbuild.c b/hw/xen_domainbuild.c
index 2f59856..7f1fd66 100644
--- a/hw/xen_domainbuild.c
+++ b/hw/xen_domainbuild.c
@@ -3,6 +3,7 @@
 #include "xen_domainbuild.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
+#include "qemu-log.h"
 
 #include <xenguest.h>
 
diff --git a/kvm-stub.c b/kvm-stub.c
index 7be5f5d..3378bd3 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -13,6 +13,7 @@
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "hw/hw.h"
+#include "exec-all.h"
 #include "gdbstub.h"
 #include "kvm.h"
 
diff --git a/monitor.c b/monitor.c
index 170b269..8657c86 100644
--- a/monitor.c
+++ b/monitor.c
@@ -55,6 +55,7 @@
 #include "json-streamer.h"
 #include "json-parser.h"
 #include "osdep.h"
+#include "exec-all.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index c333396..686fb4a 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -413,7 +413,6 @@ static inline int cpu_mmu_index (CPUState *env)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 enum {
     FEATURE_ASN    = 0x00000001,
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ddf764e..39c4a0e 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -435,7 +435,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index f86c52a..fce0804 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -250,7 +250,6 @@ static inline void cpu_set_tls(CPUCRISState *env, target_ulong newtls)
 #define SFR_RW_MM_TLB_HI   env->pregs[PR_SRS]][6
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e320c80..e0a039e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -928,8 +928,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
-
 #include "svm.h"
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 0f84514..33c41b2 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -242,7 +242,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index a2677cf..360ac0a 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -305,7 +305,6 @@ static inline int cpu_interrupts_enabled(CPUState *env)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline target_ulong cpu_get_pc(CPUState *env)
 {
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 1aaca77..81051aa 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -526,7 +526,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 /* Memory access type :
  * may be needed for precise access rights control and precise exceptions.
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index ca0eb1e..9c8d774 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -849,7 +849,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 /*****************************************************************************/
 /* CRF definitions */
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 49d3128..8d73fad 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -116,7 +116,6 @@ extern CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
 #define cpu_gen_code cpu_s390x_gen_code
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 #define EXCP_OPEX 1 /* operation exception (sigill) */
 #define EXCP_SVC 2 /* supervisor call (syscall) */
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index f2dee37..64a609b 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -211,7 +211,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 /* Memory access type */
 enum {
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 95fe3dc..7e0d17c 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -615,7 +615,6 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 #ifdef TARGET_SPARC64
 /* sun4u.c */
-- 
1.7.0.1

[Qemu-devel] [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c                    |    2 ++
 exec-all.h                    |    4 ++++
 target-alpha/exec.h           |    2 --
 target-alpha/op_helper.c      |    1 +
 target-arm/exec.h             |    2 --
 target-arm/op_helper.c        |    1 +
 target-cris/exec.h            |    2 --
 target-cris/op_helper.c       |    1 +
 target-i386/exec.h            |    2 --
 target-i386/op_helper.c       |    1 +
 target-m68k/exec.h            |    2 --
 target-m68k/op_helper.c       |    2 ++
 target-microblaze/exec.h      |    2 --
 target-microblaze/op_helper.c |    1 +
 target-mips/exec.h            |    2 --
 target-mips/op_helper.c       |    2 ++
 target-ppc/exec.h             |    2 --
 target-ppc/op_helper.c        |    2 ++
 target-s390x/exec.h           |    2 --
 target-s390x/op_helper.c      |    1 +
 target-sh4/exec.h             |    2 --
 target-sh4/op_helper.c        |    2 ++
 target-sparc/exec.h           |    2 --
 target-sparc/op_helper.c      |    1 +
 24 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 026980a..5d5170f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -16,7 +16,9 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
 #include "config.h"
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "disas.h"
 #include "tcg.h"
diff --git a/exec-all.h b/exec-all.h
index a775582..ebe88ad 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -353,4 +353,8 @@ extern int singlestep;
 /* cpu-exec.c */
 extern volatile sig_atomic_t exit_request;
 
+#ifdef NEED_GLOBAL_ENV
+register CPUState *env asm(AREG0);
+#endif
+
 #endif
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index a8a38d2..1950f83 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -26,8 +26,6 @@
 
 #define TARGET_LONG_BITS 64
 
-register struct CPUAlphaState *env asm(AREG0);
-
 #define FP_STATUS (env->fp_status)
 
 #include "cpu.h"
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index ff5ae26..9870d97 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "host-utils.h"
 #include "softfloat.h"
diff --git a/target-arm/exec.h b/target-arm/exec.h
index e4c35a3..eda5632 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -19,8 +19,6 @@
 #include "config.h"
 #include "dyngen-exec.h"
 
-register struct CPUARMState *env asm(AREG0);
-
 #define M0   env->iwmmxt.val
 
 #include "cpu.h"
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9b1a014..235cffc 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -16,6 +16,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 93ce768..360f45a 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -19,8 +19,6 @@
  */
 #include "dyngen-exec.h"
 
-register struct CPUCRISState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..8bb3876 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -18,6 +18,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "mmu.h"
 #include "helper.h"
diff --git a/target-i386/exec.h b/target-i386/exec.h
index 04a566f..63f2363 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -28,8 +28,6 @@
 
 #include "cpu-defs.h"
 
-register struct CPUX86State *env asm(AREG0);
-
 #include "qemu-common.h"
 #include "qemu-log.h"
 
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 00fc671..05b6340 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "exec-all.h"
 #include "host-utils.h"
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index f31e06e..ac1cf12 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -19,8 +19,6 @@
  */
 #include "dyngen-exec.h"
 
-register struct CPUM68KState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 0711107..df4489f 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index 87b2494..31a14fe 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -18,8 +18,6 @@
  */
 #include "dyngen-exec.h"
 
-register struct CPUMBState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index be0c829..07992b7 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define NEED_GLOBAL_ENV
 #include <assert.h>
 #include "exec.h"
 #include "helper.h"
diff --git a/target-mips/exec.h b/target-mips/exec.h
index af61b54..3cb3201 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -8,8 +8,6 @@
 #include "dyngen-exec.h"
 #include "cpu-defs.h"
 
-register struct CPUMIPSState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 344e0bd..b06bb5d 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#define NEED_GLOBAL_ENV
 #include <stdlib.h>
 #include "exec.h"
 
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 44cc5e9..b835273 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -29,8 +29,6 @@
 /* Precise emulation is needed to correctly emulate exception flags */
 #define USE_PRECISE_EMULATION 1
 
-register struct CPUPPCState *env asm(AREG0);
-
 #if !defined(CONFIG_USER_ONLY)
 #include "softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 3c3aa60..a10c233 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#define NEED_GLOBAL_ENV
 #include <string.h>
 #include "exec.h"
 #include "host-utils.h"
diff --git a/target-s390x/exec.h b/target-s390x/exec.h
index bf3f264..e87622a 100644
--- a/target-s390x/exec.h
+++ b/target-s390x/exec.h
@@ -19,8 +19,6 @@
 
 #include "dyngen-exec.h"
 
-register struct CPUS390XState *env asm(AREG0);
-
 #include "config.h"
 #include "cpu.h"
 #include "exec-all.h"
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 402df2d..a3b66b8 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 
 /*****************************************************************************/
diff --git a/target-sh4/exec.h b/target-sh4/exec.h
index 2999c02..73c06ad 100644
--- a/target-sh4/exec.h
+++ b/target-sh4/exec.h
@@ -22,8 +22,6 @@
 #include "config.h"
 #include "dyngen-exec.h"
 
-register struct CPUSH4State *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index 2e5f555..d29d75e 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -18,6 +18,8 @@
  */
 #include <assert.h>
 #include <stdlib.h>
+
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "helper.h"
 
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index f811571..db6ed49 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -3,8 +3,6 @@
 #include "config.h"
 #include "dyngen-exec.h"
 
-register struct CPUSPARCState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index be3c1e0..5ad1fac 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1,3 +1,4 @@
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "host-utils.h"
 #include "helper.h"
-- 
1.7.0.1

Re: [Qemu-devel] [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paul Brook]

> diff --git a/exec-all.h b/exec-all.h
> index a775582..ebe88ad 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -353,4 +353,8 @@ extern int singlestep;
>  /* cpu-exec.c */
>  extern volatile sig_atomic_t exit_request;
> 
> +#ifdef NEED_GLOBAL_ENV
> +register CPUState *env asm(AREG0);
> +#endif

Wouldn't it be better to just put this in dyngen-exec.h ?
AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include 
"exec.h".

Paul

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

On 06/28/2010 10:29 PM, Paul Brook wrote:
>> diff --git a/exec-all.h b/exec-all.h
>> index a775582..ebe88ad 100644
>> --- a/exec-all.h
>> +++ b/exec-all.h
>> @@ -353,4 +353,8 @@ extern int singlestep;
>>   /* cpu-exec.c */
>>   extern volatile sig_atomic_t exit_request;
>>
>> +#ifdef NEED_GLOBAL_ENV
>> +register CPUState *env asm(AREG0);
>> +#endif
>
> Wouldn't it be better to just put this in dyngen-exec.h ?
> AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
> "exec.h".

True, see cover letter in 0/4.  I was told to make each file request 
explicitly the global variable though.  So I'd have to leave the #ifdef 
even if I moved it into dyngen-exec.h.

Paolo

Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paul Brook]

> On 06/28/2010 10:29 PM, Paul Brook wrote:
> >> diff --git a/exec-all.h b/exec-all.h
> >> index a775582..ebe88ad 100644
> >> --- a/exec-all.h
> >> +++ b/exec-all.h
> >> @@ -353,4 +353,8 @@ extern int singlestep;
> >> 
> >>   /* cpu-exec.c */
> >>   extern volatile sig_atomic_t exit_request;
> >> 
> >> +#ifdef NEED_GLOBAL_ENV
> >> +register CPUState *env asm(AREG0);
> >> +#endif
> > 
> > Wouldn't it be better to just put this in dyngen-exec.h ?
> > AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
> > "exec.h".
> 
> True, see cover letter in 0/4.  I was told to make each file request
> explicitly the global variable though.  So I'd have to leave the #ifdef
> even if I moved it into dyngen-exec.h.

I don't understand what this is supposed to achieve.
The inclusion of exec.h is what defines whether this global variable is 
available.  Just as importantly, it also prevents code clobbering this value.  
Having one without the other makes no sense.

Making "env" available without including exec.h would be a different problem, 
but we never do that and would probably indicate we're doing something else 
wrong.

Paul

Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

On 06/29/2010 01:30 PM, Paul Brook wrote:
> I don't understand what this is supposed to achieve.
> The inclusion of exec.h is what defines whether this global variable is
> available.  Just as importantly, it also prevents code clobbering this value.
> Having one without the other makes no sense.
>
> Making "env" available without including exec.h would be a different problem,
> but we never do that and would probably indicate we're doing something else
> wrong.

Paul, I agree but I was told to do something different.

Paolo

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

On 06/29/2010 03:51 PM, Paolo Bonzini wrote:
> On 06/29/2010 01:30 PM, Paul Brook wrote:
>> I don't understand what this is supposed to achieve. The inclusion
>> of exec.h is what defines whether this global variable is
>> available. Just as importantly, it also prevents code clobbering
>> this value. Having one without the other makes no sense.
>>
>> Making "env" available without including exec.h would be a
>> different problem, but we never do that and would probably indicate
>> we're doing something else wrong.
>
> Paul, I agree but I was told to do something different.

BTW, this may be useful for one thing: being able to include exec.h 
without bringing in the global variable.

I didn't really see a use right now for that, but cpu_get_tb_cpu_state 
could be something that may belong in target-*/exec.h more than in 
target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved 
right now because it is called in exec.c.

That said, I think everybody at least agrees on the first 3 patches, is 
it possible to get at least (v2 of) thos applied?

Paolo

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paul Brook]

> On 06/29/2010 03:51 PM, Paolo Bonzini wrote:
> > On 06/29/2010 01:30 PM, Paul Brook wrote:
> >> I don't understand what this is supposed to achieve. The inclusion
> >> of exec.h is what defines whether this global variable is
> >> available. Just as importantly, it also prevents code clobbering
> >> this value. Having one without the other makes no sense.
> >> 
> >> Making "env" available without including exec.h would be a
> >> different problem, but we never do that and would probably indicate
> >> we're doing something else wrong.
> > 
> > Paul, I agree but I was told to do something different.
> 
> BTW, this may be useful for one thing: being able to include exec.h
> without bringing in the global variable.

I'd argue that's not a desirable feature. I'd much rather have exec.h be the 
controlling factor than have all files include the same set of headers and 
have semantics determined by some combination of preprocessor macros.

I realise we already have this with NEED_CPU_H, however I don't think that's a 
precedent we want to encourage.

> I didn't really see a use right now for that, but cpu_get_tb_cpu_state
> could be something that may belong in target-*/exec.h more than in
> target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
> right now because it is called in exec.c.

How is putting it in exec.h better?

Paul

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

On 06/29/2010 05:28 PM, Paul Brook wrote:
>> On 06/29/2010 03:51 PM, Paolo Bonzini wrote:
>>> On 06/29/2010 01:30 PM, Paul Brook wrote:
>>>> I don't understand what this is supposed to achieve. The inclusion
>>>> of exec.h is what defines whether this global variable is
>>>> available. Just as importantly, it also prevents code clobbering
>>>> this value. Having one without the other makes no sense.
>>>>
>>>> Making "env" available without including exec.h would be a
>>>> different problem, but we never do that and would probably indicate
>>>> we're doing something else wrong.
>>>
>>> Paul, I agree but I was told to do something different.
>>
>> BTW, this may be useful for one thing: being able to include exec.h
>> without bringing in the global variable.
>
> I'd argue that's not a desirable feature. I'd much rather have exec.h be the
> controlling factor than have all files include the same set of headers and
> have semantics determined by some combination of preprocessor macros.
>
> I realise we already have this with NEED_CPU_H, however I don't think that's a
> precedent we want to encourage.
>
>> I didn't really see a use right now for that, but cpu_get_tb_cpu_state
>> could be something that may belong in target-*/exec.h more than in
>> target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
>> right now because it is called in exec.c.
>
> How is putting it in exec.h better?

I see cpu.h as holding things related to the CPU as a hardware device, 
and exec.h as holding things related to emulation of the CPU.

In theory, a hypothetical KVM-only/no-TCG version of QEMU would not need 
to include neither exec-all.h nor target-*/exec.h.

Paolo

Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paul Brook]

> >> BTW, this may be useful for one thing: being able to include exec.h
> >> without bringing in the global variable.
> > 
> > I'd argue that's not a desirable feature. I'd much rather have exec.h be
> > the controlling factor than have all files include the same set of
> > headers and have semantics determined by some combination of
> > preprocessor macros.
> > 
> > I realise we already have this with NEED_CPU_H, however I don't think
> > that's a precedent we want to encourage.
> > 
> >> I didn't really see a use right now for that, but cpu_get_tb_cpu_state
> >> could be something that may belong in target-*/exec.h more than in
> >> target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
> >> right now because it is called in exec.c.
> > 
> > How is putting it in exec.h better?
> 
> I see cpu.h as holding things related to the CPU as a hardware device,
> and exec.h as holding things related to emulation of the CPU.

If you really want NEED_GLOBAL_ENV then your implementation is nowhere near 
sufficient.

One of the main reasons we have exec.h is that you may not call GLOBAL_ENV 
code from regular code[1]. Doing so will result in qemu crashing.  The current 
system is simple to understand and audit: All prototypes for code that 
includes exec.h must go in exec.h.

Once you introduce NEED_GLOBAL_ENV you have both safe and unsafe code in 
exec.h.  All unsafe code should be protected by proper #idefs. I have low 
confidence in being able to get this right.

If you want to split out the TCG specific bits of cpu.h (including things like 
cpu_halted which are currently in exec.h but don't use global env) then I 
suggest creating a new header file for this.

Paul

[1] cpu_exec() is a special case, and jumps though hoops to make this work.

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Blue Swirl]

On Tue, Jun 29, 2010 at 7:44 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 06/28/2010 10:29 PM, Paul Brook wrote:
>>>
>>> diff --git a/exec-all.h b/exec-all.h
>>> index a775582..ebe88ad 100644
>>> --- a/exec-all.h
>>> +++ b/exec-all.h
>>> @@ -353,4 +353,8 @@ extern int singlestep;
>>>  /* cpu-exec.c */
>>>  extern volatile sig_atomic_t exit_request;
>>>
>>> +#ifdef NEED_GLOBAL_ENV
>>> +register CPUState *env asm(AREG0);
>>> +#endif
>>
>> Wouldn't it be better to just put this in dyngen-exec.h ?
>> AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
>> "exec.h".
>
> True, see cover letter in 0/4.  I was told to make each file request
> explicitly the global variable though.  So I'd have to leave the #ifdef even
> if I moved it into dyngen-exec.h.

Well, I only said I'd rename global 'env' to 'global_reg_env', not
something about each file requesting it. But NEED_GLOBAL_ENV wasn't so
bad idea in my opinion.

[Qemu-devel] Re: [PATCH 0/4] introduce NEED_GLOBAL_ENV

[Blue Swirl]

On Mon, Jun 28, 2010 at 5:17 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Let's start the cleanups from the feature required by Blue Swirl.
> I also include here a baby step towards removing eminently TCG-related
> stuff from cpu.h.
>
> After this series, only a bunch of files will include exec-all.h,
> instead of getting it indirectly from cpu.h.
>
> Note that (as sworn in the previous submission) exec.h is only included
> by files that need the global register variable (i.e. cpu-exec.c and
> target-*/op_helper.c), and this is the same subset that gets
> NEED_GLOBAL_ENV in this patchset.
>
> i386 and sparc have functions declared in cpu.h that are in op_helper.c.
> I checked that these do not need the global variable, but it would be
> nice to cleanup those too.

Maybe some of those could be moved to helper.c?

> Paolo Bonzini (4):
>  remove unused stuff from */exec.h
>  move cpu_pc_from_tb to target-*/exec.h
>  remove exec-all.h inclusion from cpu.h
>  require #define NEED_GLOBAL_ENV for files that need the global
>    register variable

Good stuff. I had some comments to 1/4.