From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UlGQy-0002RY-RP for qemu-devel@nongnu.org; Sat, 08 Jun 2013 06:31:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UlGQw-0005AG-TH for qemu-devel@nongnu.org; Sat, 08 Jun 2013 06:31:44 -0400 Message-ID: <51B30809.80207@suse.de> Date: Sat, 08 Jun 2013 12:31:37 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1370348465-31652-1-git-send-email-aik@ozlabs.ru> <1370348465-31652-4-git-send-email-aik@ozlabs.ru> In-Reply-To: <1370348465-31652-4-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 03/13] target-ppc: Convert ppc cpu savevm to VMStateDescription List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Paul Mackerras , David Gibson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf Am 04.06.2013 14:20, schrieb Alexey Kardashevskiy: > Author: David Gibson From: >=20 > The savevm code for the powerpc cpu emulation is currently based around > the old register_savevm() rather than register_vmstate() method. It's = also > rather broken, missing some important state on some CPU models. >=20 > This patch completely rewrites the savevm for target-ppc, using the new > VMStateDescription approach. Exactly what needs to be saved in what > configurations has been more carefully examined, too. This introduces = a > new version (5) of the cpu save format. The old load function is retai= ned > to support version 4 images. >=20 > Signed-off-by: David Gibson > Signed-off-by: Alexey Kardashevskiy > --- > target-ppc/cpu-qom.h | 4 + > target-ppc/cpu.h | 8 +- > target-ppc/machine.c | 539 +++++++++++++++++++++++++++++++++++= +------- > target-ppc/translate_init.c | 2 + > 4 files changed, 460 insertions(+), 93 deletions(-) >=20 > diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h > index eb03a00..2b96b04 100644 > --- a/target-ppc/cpu-qom.h > +++ b/target-ppc/cpu-qom.h > @@ -102,4 +102,8 @@ PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)= ; > =20 > void ppc_cpu_do_interrupt(CPUState *cpu); > =20 > +#ifndef CONFIG_USER_ONLY > +extern const struct VMStateDescription vmstate_ppc_cpu; > +#endif > + > #endif > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index aa1d013..e47853a 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -946,7 +946,7 @@ struct CPUPPCState { > #if defined(TARGET_PPC64) > /* PowerPC 64 SLB area */ > ppc_slb_t slb[64]; > - int slb_nr; > + int32_t slb_nr; > #endif > /* segment registers */ > hwaddr htab_base; > @@ -955,11 +955,11 @@ struct CPUPPCState { > /* externally stored hash table */ > uint8_t *external_htab; > /* BATs */ > - int nb_BATs; > + uint32_t nb_BATs; Here you changed to uint32_t but above and below to int32_t? > target_ulong DBAT[2][8]; > target_ulong IBAT[2][8]; > /* PowerPC TLB registers (for 4xx, e500 and 60x software driven TL= Bs) */ > - int nb_tlb; /* Total number of TLB = */ > + int32_t nb_tlb; /* Total number of TLB = */ > int tlb_per_way; /* Speed-up helper: used to avoid divisions at ru= n time */ > int nb_ways; /* Number of ways in the TLB set = */ > int last_way; /* Last used way used to allocate TLB in a LRU wa= y */ > @@ -1174,8 +1174,6 @@ static inline CPUPPCState *cpu_init(const char *c= pu_model) > #define cpu_signal_handler cpu_ppc_signal_handler > #define cpu_list ppc_cpu_list > =20 > -#define CPU_SAVE_VERSION 4 > - > /* MMU modes definitions */ > #define MMU_MODE0_SUFFIX _user > #define MMU_MODE1_SUFFIX _kernel > diff --git a/target-ppc/machine.c b/target-ppc/machine.c > index 2d10adb..d58e652 100644 > --- a/target-ppc/machine.c > +++ b/target-ppc/machine.c [...] > +static bool fpu_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return (env->insns_flags & PPC_FLOAT); If you only need env once, you could just access cpu->env.foo directly, saving a line. Applies to most such helpers below except for those that access env two times. > +} [...] > +static bool altivec_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return (env->insns_flags & PPC_ALTIVEC); > +} [...] > +static bool vsx_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return (env->insns_flags2 & PPC2_VSX); > +} [...] > +static bool sr_needed(void *opaque) > +{ > +#ifdef TARGET_PPC64 > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return !(env->mmu_model & POWERPC_MMU_64); > +#else > + return true; > +#endif > +} [...] > +static bool slb_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + /* We don't support any of the old segment table based 64-bit CPUs= */ > + return (env->mmu_model & POWERPC_MMU_64); > +} [...] > +static bool tlb6xx_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return env->nb_tlb && (env->tlb_type =3D=3D TLB_6XX); > +} [...] > +static bool tlbemb_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return env->nb_tlb && (env->tlb_type =3D=3D TLB_EMB); > +} > + > +static bool pbr403_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + uint32_t pvr =3D env->spr[SPR_PVR]; > + > + return (pvr & 0xffff0000) =3D=3D 0x00200000; > +} [...] > +static bool tlbmas_needed(void *opaque) > +{ > + PowerPCCPU *cpu =3D opaque; > + CPUPPCState *env =3D &cpu->env; > + > + return env->nb_tlb && (env->tlb_type =3D=3D TLB_MAS); > +} [...] > +const VMStateDescription vmstate_ppc_cpu =3D { > + .name =3D "cpu", > + .version_id =3D 5, > + .minimum_version_id =3D 5, > + .minimum_version_id_old =3D 4, > + .load_state_old =3D cpu_load_old, > + .pre_save =3D cpu_pre_save, > + .post_load =3D cpu_post_load, > + .fields =3D (VMStateField []) { [...] > +}; > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index 021a31e..4f89b14 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -8309,6 +8309,8 @@ static void ppc_cpu_class_init(ObjectClass *oc, v= oid *data) > =20 > cc->class_by_name =3D ppc_cpu_class_by_name; > cc->do_interrupt =3D ppc_cpu_do_interrupt; > + > + cpu_class_set_vmsd(cc, &vmstate_ppc_cpu); > } > =20 > static const TypeInfo ppc_cpu_type_info =3D { The way to hook it up looks fine now. Device-based CPU VMState is still not ready and would be incompatible from what I understand, so with or without my minor comments addressed this looks good to go to me now! Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg