Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask

[Fabiano Rosas <farosas@linux.ibm.com>]

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
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);
        }
     ...

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.
   
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.