On 21.06.2012, at 00:26, Scott Wood wrote: > On 06/20/2012 03:11 PM, Alexander Graf wrote: >> + /* XXX better abstract into Emb.xxx features */ >> + if (version == fsl_e5500) { >> + spr_register(env, SPR_BOOKE_EPCR, "EPCR", >> + SPR_NOACCESS, SPR_NOACCESS, >> + &spr_read_generic, &spr_write_generic, >> + 0x00000000); >> + spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3", >> + SPR_NOACCESS, SPR_NOACCESS, >> + &spr_read_mas73, &spr_write_mas73, >> + 0x00000000); >> + env->reset_msr = (1ULL < MSR_CM); > > That's a funny way of writing "env->reset_msr = 0". :-) > > Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why? We > enter the kernel in 32-bit mode. It resets in 32-bit mode as well, if > we ever implement that for e5500 QEMU.
Hrm. At least my self-compiled kernel did issue an "ld" instruction before going into MSR_CM mode, hence I figured we need it. > You have the same issue in the previous patch with "1ULL < MSR_SF", > though I don't know if those chips actually do reset into 64-bit mode. They do :). > >> + } >> >> #if !defined(CONFIG_USER_ONLY) >> env->nb_tlb = 0; >> @@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int >> version) >> #endif >> >> init_excp_e200(env); >> + >> +#if !defined(CONFIG_USER_ONLY) >> + /* We support 64bit wide IVPR on 64bit platforms */ >> + if (version == fsl_e5500) { >> + env->ivpr_mask = (target_ulong)~0xFFFFULL; >> + } >> +#endif > > So, I'm guessing you don't do this unconditionally because QEMU will > generate 64-bit code if compiled that way, regardless of the actual > target -- and you don't want stray garbage in the upper 32 bits being > written into IVPR. But why isn't this an issue with all the other SPRs? > Why don't we have a problem with junk being written into the upper half > of MAS3, for example (there's MAS3_RPN_MASK, but it's not used)? I was thinking of making it unconditional, but this way seemed cleaner to me, as it actually follows exactly what the spec says. Not sure what would happen if you have -1 in your 32-bit register value and you try to write that to IVPR otherwise. It'd probably break :). > Speaking of which, I don't see where you change MAS2_EPN_MASK to take > MSR_CM into account (or to accept 64-bit MAS2 at all). Ugh. Good question why this works at all. Do we by accident truncate everything to 32bit? Alex