[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
From: |
David Gibson |
Subject: |
Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask |
Date: |
Tue, 18 Jan 2022 19:40:16 +1100 |
On Mon, Jan 17, 2022 at 06:12:20PM -0300, Fabiano Rosas wrote:
> Fabiano Rosas <farosas@linux.ibm.com> writes:
>
> > Some bits described in the user manual are missing from msr_mask. Add
> > them.
> >
> > Signed-off-by: Fabiano Rosas <farosas@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