From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nxihl-0000X6-Qg for qemu-devel@nongnu.org; Fri, 02 Apr 2010 11:22:41 -0400 Received: from [140.186.70.92] (port=54369 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nxihj-0000Wy-Qt for qemu-devel@nongnu.org; Fri, 02 Apr 2010 11:22:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Nxihg-0008Kj-VC for qemu-devel@nongnu.org; Fri, 02 Apr 2010 11:22:39 -0400 Received: from mail-pw0-f45.google.com ([209.85.160.45]:45351) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Nxihg-0008Kc-HP for qemu-devel@nongnu.org; Fri, 02 Apr 2010 11:22:36 -0400 Received: by pwi6 with SMTP id 6so1713173pwi.4 for ; Fri, 02 Apr 2010 08:22:35 -0700 (PDT) Message-ID: <4BB60BB9.2050301@codemonkey.ws> Date: Fri, 02 Apr 2010 10:22:33 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] provide opaque CPUState to files that are compiled once References: <1270219540-30027-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1270219540-30027-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 04/02/2010 09:45 AM, Paolo Bonzini wrote: > This patch allows to unpoison CPUState and env in once-compiled files. > To achieve this, it defines an opaque struct CPUState in cpu-common.h. > This also requires tweaking the relationship between CPUState and > CPUXYZState in target files. > > Unpoisoning env is needed because it is widely used as the name for > CPUState arguments. This is anyway safe, because references to the > global register variable will not creep into target-independent files. > To this end, the patch rationalizes inclusions at the head of > target-*/exec.h. All exec.h files now include cpu.h before defining the > global register variable env, so that inclusions from machine-independent > context will error out in cpu.h even before env is defined. > hw/* should never access CPUState. Can you give examples of when qemu-kvm needs this? Regards, Anthony Liguori > Signed-off-by: Paolo Bonzini > --- > This patch is helpful for qemu-kvm, because some of the changes > there use CPUState in hw/* files that are now compiled only > once. > > It can also be used to compile more files once-only, but I'm > not doing that here. > > cpu-common.h | 3 +++ > cpu-defs.h | 1 + > hw/poison.h | 3 --- > target-alpha/cpu.h | 4 +--- > target-alpha/exec.h | 6 ++---- > target-arm/cpu.h | 6 +++--- > target-arm/exec.h | 5 ++--- > target-cris/cpu.h | 6 +++--- > target-cris/exec.h | 6 +++--- > target-i386/cpu.h | 6 +++--- > target-i386/exec.h | 7 ++----- > target-m68k/cpu.h | 6 +++--- > target-m68k/exec.h | 6 +++--- > target-microblaze/cpu.h | 7 +++---- > target-microblaze/exec.h | 6 +++--- > target-mips/cpu.h | 5 +---- > target-mips/exec.h | 6 ++---- > target-ppc/cpu.h | 3 +-- > target-ppc/exec.h | 2 -- > target-s390x/cpu.h | 6 +++--- > target-s390x/exec.h | 7 +++---- > target-sh4/cpu.h | 6 +++--- > target-sh4/exec.h | 5 ++--- > target-sparc/cpu.h | 6 +++--- > target-sparc/exec.h | 6 +++--- > 25 files changed, 56 insertions(+), 74 deletions(-) > > diff --git a/cpu-common.h b/cpu-common.h > index b730ca0..415feb5 100644 > --- a/cpu-common.h > +++ b/cpu-common.h > @@ -18,6 +18,9 @@ > #include "bswap.h" > #include "qemu-queue.h" > > +struct CPUState; > +typedef struct CPUState CPUState; > + > #if !defined(CONFIG_USER_ONLY) > > /* address in the RAM (different from a physical address) */ > diff --git a/cpu-defs.h b/cpu-defs.h > index 2e94585..e8da6af 100644 > --- a/cpu-defs.h > +++ b/cpu-defs.h > @@ -30,6 +30,7 @@ > #include "osdep.h" > #include "qemu-queue.h" > #include "targphys.h" > +#include "cpu-common.h" > > #ifndef TARGET_LONG_BITS > #error TARGET_LONG_BITS must be defined before including this header > diff --git a/hw/poison.h b/hw/poison.h > index d7db7f4..e7814cb 100644 > --- a/hw/poison.h > +++ b/hw/poison.h > @@ -33,9 +33,6 @@ > #pragma GCC poison TARGET_PAGE_BITS > #pragma GCC poison TARGET_PAGE_ALIGN > > -#pragma GCC poison CPUState > -#pragma GCC poison env > - > #pragma GCC poison CPU_INTERRUPT_HARD > #pragma GCC poison CPU_INTERRUPT_EXITTB > #pragma GCC poison CPU_INTERRUPT_TIMER > diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h > index 8afe16d..92283e2 100644 > --- a/target-alpha/cpu.h > +++ b/target-alpha/cpu.h > @@ -24,7 +24,7 @@ > > #define TARGET_LONG_BITS 64 > > -#define CPUState struct CPUAlphaState > +#define CPUAlphaState CPUState > > #include "cpu-defs.h" > > @@ -317,8 +317,6 @@ enum { > IPR_LAST, > }; > > -typedef struct CPUAlphaState CPUAlphaState; > - > typedef struct pal_handler_t pal_handler_t; > struct pal_handler_t { > /* Reset */ > diff --git a/target-alpha/exec.h b/target-alpha/exec.h > index 66526e2..789305f 100644 > --- a/target-alpha/exec.h > +++ b/target-alpha/exec.h > @@ -21,8 +21,9 @@ > #define __ALPHA_EXEC_H__ > > #include "config.h" > - > #include "dyngen-exec.h" > +#include "cpu.h" > +#include "exec-all.h" > > #define TARGET_LONG_BITS 64 > > @@ -32,9 +33,6 @@ register struct CPUAlphaState *env asm(AREG0); > #define SPARAM(n) ((int32_t)PARAM##n) > #define FP_STATUS (env->fp_status) > > -#include "cpu.h" > -#include "exec-all.h" > - > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif /* !defined(CONFIG_USER_ONLY) */ > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 3892db4..d068b6e 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -23,7 +23,7 @@ > > #define ELF_MACHINE EM_ARM > > -#define CPUState struct CPUARMState > +#define CPUARMState CPUState > > #include "cpu-defs.h" > > @@ -70,7 +70,7 @@ struct arm_boot_info; > s<2n+1> maps to the most significant half of d > */ > > -typedef struct CPUARMState { > +struct CPUARMState { > /* Regs for current mode. */ > uint32_t regs[16]; > /* Frequently accessed CPSR bits are stored separately for efficiently. > @@ -206,7 +206,7 @@ typedef struct CPUARMState { > > /* These fields after the common ones so they are preserved on reset. */ > struct arm_boot_info *boot_info; > -} CPUARMState; > +}; > > CPUARMState *cpu_arm_init(const char *cpu_model); > void arm_translate_init(void); > diff --git a/target-arm/exec.h b/target-arm/exec.h > index 0225c3f..4042eca 100644 > --- a/target-arm/exec.h > +++ b/target-arm/exec.h > @@ -18,14 +18,13 @@ > */ > #include "config.h" > #include "dyngen-exec.h" > +#include "cpu.h" > +#include "exec-all.h" > > register struct CPUARMState *env asm(AREG0); > > #define M0 env->iwmmxt.val > > -#include "cpu.h" > -#include "exec-all.h" > - > static inline int cpu_has_work(CPUState *env) > { > return (env->interrupt_request& > diff --git a/target-cris/cpu.h b/target-cris/cpu.h > index 063a240..fa9b5d6 100644 > --- a/target-cris/cpu.h > +++ b/target-cris/cpu.h > @@ -22,7 +22,7 @@ > > #define TARGET_LONG_BITS 32 > > -#define CPUState struct CPUCRISState > +#define CPUCRISState CPUState > > #include "cpu-defs.h" > > @@ -96,7 +96,7 @@ > > #define NB_MMU_MODES 2 > > -typedef struct CPUCRISState { > +struct CPUCRISState { > uint32_t regs[16]; > /* P0 - P15 are referred to as special registers in the docs. */ > uint32_t pregs[16]; > @@ -156,7 +156,7 @@ typedef struct CPUCRISState { > } tlbsets[2][4][16]; > > CPU_COMMON > -} CPUCRISState; > +}; > > CPUCRISState *cpu_cris_init(const char *cpu_model); > int cpu_cris_exec(CPUCRISState *s); > diff --git a/target-cris/exec.h b/target-cris/exec.h > index 728aa80..af0103d 100644 > --- a/target-cris/exec.h > +++ b/target-cris/exec.h > @@ -17,13 +17,13 @@ > * You should have received a copy of the GNU Lesser General Public > * License along with this library; if not, see. > */ > +#include "config.h" > #include "dyngen-exec.h" > - > -register struct CPUCRISState *env asm(AREG0); > - > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUCRISState *env asm(AREG0); > + > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 548ab80..6bfb970 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -41,7 +41,7 @@ > #define ELF_MACHINE EM_386 > #endif > > -#define CPUState struct CPUX86State > +#define CPUX86State CPUState > > #include "cpu-defs.h" > > @@ -581,7 +581,7 @@ typedef struct { > > #define NB_MMU_MODES 2 > > -typedef struct CPUX86State { > +struct CPUX86State { > /* standard registers */ > target_ulong regs[CPU_NB_REGS]; > target_ulong eip; > @@ -718,7 +718,7 @@ typedef struct CPUX86State { > uint16_t fpus_vmstate; > uint16_t fptag_vmstate; > uint16_t fpregs_format_vmstate; > -} CPUX86State; > +}; > > CPUX86State *cpu_x86_init(const char *cpu_model); > int cpu_x86_exec(CPUX86State *s); > diff --git a/target-i386/exec.h b/target-i386/exec.h > index 4ff3c57..29b741a 100644 > --- a/target-i386/exec.h > +++ b/target-i386/exec.h > @@ -18,6 +18,8 @@ > */ > #include "config.h" > #include "dyngen-exec.h" > +#include "cpu.h" > +#include "exec-all.h" > > /* XXX: factorize this mess */ > #ifdef TARGET_X86_64 > @@ -26,8 +28,6 @@ > #define TARGET_LONG_BITS 32 > #endif > > -#include "cpu-defs.h" > - > register struct CPUX86State *env asm(AREG0); > > #include "qemu-common.h" > @@ -63,9 +63,6 @@ register struct CPUX86State *env asm(AREG0); > #define ST(n) (env->fpregs[(env->fpstt + (n))& 7].d) > #define ST1 ST(1) > > -#include "cpu.h" > -#include "exec-all.h" > - > /* op_helper.c */ > void do_interrupt(int intno, int is_int, int error_code, > target_ulong next_eip, int is_hw); > diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h > index b2f37ec..ac2fa84 100644 > --- a/target-m68k/cpu.h > +++ b/target-m68k/cpu.h > @@ -22,7 +22,7 @@ > > #define TARGET_LONG_BITS 32 > > -#define CPUState struct CPUM68KState > +#define CPUM68KState CPUState > > #include "cpu-defs.h" > > @@ -56,7 +56,7 @@ > > #define NB_MMU_MODES 2 > > -typedef struct CPUM68KState { > +struct CPUM68KState { > uint32_t dregs[8]; > uint32_t aregs[8]; > uint32_t pc; > @@ -112,7 +112,7 @@ typedef struct CPUM68KState { > CPU_COMMON > > uint32_t features; > -} CPUM68KState; > +}; > > void m68k_tcg_init(void); > CPUM68KState *cpu_m68k_init(const char *cpu_model); > diff --git a/target-m68k/exec.h b/target-m68k/exec.h > index ece9aa0..b611282 100644 > --- a/target-m68k/exec.h > +++ b/target-m68k/exec.h > @@ -17,13 +17,13 @@ > * You should have received a copy of the GNU Lesser General Public > * License along with this library; if not, see. > */ > +#include "config.h" > #include "dyngen-exec.h" > - > -register struct CPUM68KState *env asm(AREG0); > - > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUM68KState *env asm(AREG0); > + > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif > diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h > index ec2ca18..732bbca 100644 > --- a/target-microblaze/cpu.h > +++ b/target-microblaze/cpu.h > @@ -21,10 +21,9 @@ > > #define TARGET_LONG_BITS 32 > > -#define CPUState struct CPUMBState > +#define CPUMBState CPUState > > #include "cpu-defs.h" > -struct CPUMBState; > #if !defined(CONFIG_USER_ONLY) > #include "mmu.h" > #endif > @@ -199,7 +198,7 @@ struct CPUMBState; > #define CC_EQ 0 > > #define NB_MMU_MODES 3 > -typedef struct CPUMBState { > +struct CPUMBState { > uint32_t debug; > uint32_t btaken; > uint32_t btarget; > @@ -231,7 +230,7 @@ typedef struct CPUMBState { > #endif > > CPU_COMMON > -} CPUMBState; > +}; > > CPUState *cpu_mb_init(const char *cpu_model); > int cpu_mb_exec(CPUState *s); > diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h > index 646701c..813d3d6 100644 > --- a/target-microblaze/exec.h > +++ b/target-microblaze/exec.h > @@ -16,13 +16,13 @@ > * You should have received a copy of the GNU Lesser General Public > * License along with this library; if not, see. > */ > +#include "config.h" > #include "dyngen-exec.h" > - > -register struct CPUMBState *env asm(AREG0); > - > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUMBState *env asm(AREG0); > + > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif > diff --git a/target-mips/cpu.h b/target-mips/cpu.h > index 7285636..3b2b331 100644 > --- a/target-mips/cpu.h > +++ b/target-mips/cpu.h > @@ -5,7 +5,7 @@ > > #define ELF_MACHINE EM_MIPS > > -#define CPUState struct CPUMIPSState > +#define CPUMIPSState CPUState > > #include "config.h" > #include "mips-defs.h" > @@ -19,8 +19,6 @@ typedef unsigned char uint_fast8_t; > typedef unsigned int uint_fast16_t; > #endif > > -struct CPUMIPSState; > - > typedef struct r4k_tlb_t r4k_tlb_t; > struct r4k_tlb_t { > target_ulong VPN; > @@ -172,7 +170,6 @@ struct TCState { > int32_t CP0_Debug_tcstatus; > }; > > -typedef struct CPUMIPSState CPUMIPSState; > struct CPUMIPSState { > TCState active_tc; > CPUMIPSFPUContext active_fpu; > diff --git a/target-mips/exec.h b/target-mips/exec.h > index 01e9c4d..070e425 100644 > --- a/target-mips/exec.h > +++ b/target-mips/exec.h > @@ -6,13 +6,11 @@ > #include "config.h" > #include "mips-defs.h" > #include "dyngen-exec.h" > -#include "cpu-defs.h" > - > -register struct CPUMIPSState *env asm(AREG0); > - > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUMIPSState *env asm(AREG0); > + > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif /* !defined(CONFIG_USER_ONLY) */ > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index 2ad4486..a47f12b 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -69,7 +69,7 @@ > > #endif /* defined (TARGET_PPC64) */ > > -#define CPUState struct CPUPPCState > +#define CPUPPCState CPUState > > #include "cpu-defs.h" > > @@ -300,7 +300,6 @@ typedef struct opc_handler_t opc_handler_t; > > /*****************************************************************************/ > /* Types used to describe some PowerPC registers */ > -typedef struct CPUPPCState CPUPPCState; > typedef struct ppc_tb_t ppc_tb_t; > typedef struct ppc_spr_t ppc_spr_t; > typedef struct ppc_dcr_t ppc_dcr_t; > diff --git a/target-ppc/exec.h b/target-ppc/exec.h > index 09f592c..651f91a 100644 > --- a/target-ppc/exec.h > +++ b/target-ppc/exec.h > @@ -20,9 +20,7 @@ > #define __PPC_H__ > > #include "config.h" > - > #include "dyngen-exec.h" > - > #include "cpu.h" > #include "exec-all.h" > > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index dd407b2..d30e9da 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -23,7 +23,7 @@ > > #define ELF_MACHINE EM_S390 > > -#define CPUState struct CPUS390XState > +#define CPUS390XState CPUState > > #include "cpu-defs.h" > > @@ -45,7 +45,7 @@ typedef union FPReg { > uint64_t i; > } FPReg; > > -typedef struct CPUS390XState { > +struct CPUS390XState { > uint64_t regs[16]; /* GP registers */ > > uint32_t aregs[16]; /* access registers */ > @@ -64,7 +64,7 @@ typedef struct CPUS390XState { > uint64_t __excp_addr; > > CPU_COMMON > -} CPUS390XState; > +}; > > #if defined(CONFIG_USER_ONLY) > static inline void cpu_clone_regs(CPUState *env, target_ulong newsp) > diff --git a/target-s390x/exec.h b/target-s390x/exec.h > index 837f853..a848f73 100644 > --- a/target-s390x/exec.h > +++ b/target-s390x/exec.h > @@ -17,14 +17,13 @@ > * License along with this library; if not, see. > */ > > -#include "dyngen-exec.h" > - > -register struct CPUS390XState *env asm(AREG0); > - > #include "config.h" > +#include "dyngen-exec.h" > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUS390XState *env asm(AREG0); > + > #if !defined(CONFIG_USER_ONLY) > #include "softmmu_exec.h" > #endif /* !defined(CONFIG_USER_ONLY) */ > diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h > index f8b1680..5535ed1 100644 > --- a/target-sh4/cpu.h > +++ b/target-sh4/cpu.h > @@ -36,7 +36,7 @@ > #define SH_CPU_SH7750_ALL (SH_CPU_SH7750 | SH_CPU_SH7750S | SH_CPU_SH7750R) > #define SH_CPU_SH7751_ALL (SH_CPU_SH7751 | SH_CPU_SH7751R) > > -#define CPUState struct CPUSH4State > +#define CPUSH4State CPUState > > #include "cpu-defs.h" > > @@ -107,7 +107,7 @@ typedef struct memory_content { > struct memory_content *next; > } memory_content; > > -typedef struct CPUSH4State { > +struct CPUSH4State { > int id; /* CPU model */ > > uint32_t flags; /* general execution flags */ > @@ -157,7 +157,7 @@ typedef struct CPUSH4State { > int intr_at_halt; /* SR_BL ignored during sleep */ > memory_content *movcal_backup; > memory_content **movcal_backup_tail; > -} CPUSH4State; > +}; > > CPUSH4State *cpu_sh4_init(const char *cpu_model); > int cpu_sh4_exec(CPUSH4State * s); > diff --git a/target-sh4/exec.h b/target-sh4/exec.h > index edd667d..b2eb306 100644 > --- a/target-sh4/exec.h > +++ b/target-sh4/exec.h > @@ -21,12 +21,11 @@ > > #include "config.h" > #include "dyngen-exec.h" > - > -register struct CPUSH4State *env asm(AREG0); > - > #include "cpu.h" > #include "exec-all.h" > > +register struct CPUSH4State *env asm(AREG0); > + > static inline int cpu_has_work(CPUState *env) > { > return (env->interrupt_request& CPU_INTERRUPT_HARD); > diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h > index 580f4d4..de329a2 100644 > --- a/target-sparc/cpu.h > +++ b/target-sparc/cpu.h > @@ -21,7 +21,7 @@ > #define TARGET_VIRT_ADDR_SPACE_BITS 32 > #endif > > -#define CPUState struct CPUSPARCState > +#define CPUSPARCState CPUState > > #include "cpu-defs.h" > > @@ -316,7 +316,7 @@ struct QEMUFile; > void cpu_put_timer(struct QEMUFile *f, CPUTimer *s); > void cpu_get_timer(struct QEMUFile *f, CPUTimer *s); > > -typedef struct CPUSPARCState { > +struct CPUSPARCState { > target_ulong gregs[8]; /* general registers */ > target_ulong *regwptr; /* pointer to current register window */ > target_ulong pc; /* program counter */ > @@ -432,7 +432,7 @@ typedef struct CPUSPARCState { > #define SOFTINT_REG_MASK (SOFTINT_STIMER|SOFTINT_INTRMASK|SOFTINT_TIMER) > #endif > sparc_def_t *def; > -} CPUSPARCState; > +}; > > /* helper.c */ > CPUSPARCState *cpu_sparc_init(const char *cpu_model); > diff --git a/target-sparc/exec.h b/target-sparc/exec.h > index 70df828..850f7f6 100644 > --- a/target-sparc/exec.h > +++ b/target-sparc/exec.h > @@ -1,7 +1,10 @@ > #ifndef EXEC_SPARC_H > #define EXEC_SPARC_H 1 > + > #include "config.h" > #include "dyngen-exec.h" > +#include "cpu.h" > +#include "exec-all.h" > > register struct CPUSPARCState *env asm(AREG0); > > @@ -10,9 +13,6 @@ register struct CPUSPARCState *env asm(AREG0); > #define QT0 (env->qt0) > #define QT1 (env->qt1) > > -#include "cpu.h" > -#include "exec-all.h" > - > /* op_helper.c */ > void do_interrupt(CPUState *env); > >