On Mon, Jan 17, 2022 at 06:12:20PM -0300, Fabiano Rosas wrote: > Fabiano Rosas <faro...@linux.ibm.com> writes: > > > Some bits described in the user manual are missing from msr_mask. Add > > them. > > > > Signed-off-by: Fabiano Rosas <faro...@linux.ibm.com> > > --- > > target/ppc/cpu_init.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > > index e30e86fe9d..a50ddaeaae 100644 > > --- a/target/ppc/cpu_init.c > > +++ b/target/ppc/cpu_init.c > > @@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data) > > PPC_MEM_SYNC | PPC_MEM_EIEIO | > > PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC | > > PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP; > > - pcc->msr_mask = (1ull << MSR_POW) | > > + pcc->msr_mask = (1ull << MSR_AP) | > > + (1ull << MSR_POW) | > > (1ull << MSR_CE) | > > (1ull << MSR_EE) | > > (1ull << MSR_PR) | > > (1ull << MSR_FP) | > > + (1ull << MSR_ME) | > > (1ull << MSR_DWE) | > > (1ull << MSR_DE) | > > + (1ull << MSR_FE1) | > > (1ull << MSR_IR) | > > (1ull << MSR_DR); > > This patch brings an unexpected complication: > > MSR_AP here is not correct, it is defined as: > > #define MSR_AP 23 /* Access privilege state on 602 */ > > That is bit 8. While MSR_AP in the 405 is bit 6. So I would need to
Uh oh...
> introduce a new MSR_AP_405 defined as:
>
> #define MSR_AP_405 25 /* Auxiliar processor available on 405 */
>
> But 25 is the same as MSR_SPE, so it triggers this code in
> init_ppc_proc:
>
> /* MSR bits & flags consistency checks */
> if (env->msr_mask & (1 << 25)) {
> switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
> case POWERPC_FLAG_SPE:
> case POWERPC_FLAG_VRE:
> break;
> default:
> fprintf(stderr, "PowerPC MSR definition inconsistency\n"
> "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n");
> exit(1);
> }
> ...
Ah. Which suggests this section itself should probably be taken out
of the common path and moved to code specific to the cpu families with
SPE.
> The commit that introduced that sanity check is 25ba3a6812 ("Remove
> synonymous in PowerPC MSR bits definitions..."), which sort of assumes
> that MSR bits will not have different purposes between any of the (now
> 47) CPUs, while itself leaving other duplicated bits around.
Ah, oops. Even in 2007 I could have told you that wasn't a safe
assumption. Oh well.
> So my idea is to drop this patch and only include the MSR_ME that is of
> practical effect at patch 6. I think going into the rabbit hole of
> disambiguating MSR bits falls out of the scope of the exception series.
That seems fair for the time being. I suspect splitting the exception
paths will make cleaning up MSR collisions like this easier.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
