Re: [PATCH v5 2/9] target/ppc: Add IBM PPE42 family of processors

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

Hi Glenn,

On 9/24/25 20:36, Miles Glenn wrote:
>>>> @@ -6802,53 +6916,63 @@ static void init_ppc_proc(PowerPCCPU *cpu)
>>>>    
>>>>        /* MSR bits & flags consistency checks */
>>>>        if (env->msr_mask & (1 << 25)) {
>>>> -        switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
>>>> +        switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE |
>>>> +                              POWERPC_FLAG_PPE42)) {
>>>>            case POWERPC_FLAG_SPE:
>>>>            case POWERPC_FLAG_VRE:
>>>> +        case POWERPC_FLAG_PPE42:
>>>>                break;
>>>>            default:
>>>>                fprintf(stderr, "PowerPC MSR definition inconsistency\n"
>>>> -                    "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n");
>>>> +                    "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n"
>>>> +                    "or POWERPC_FLAG_PPE42\n");
>>>>                exit(1);
>>>>            }
>>>>        } else if (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
>>> Hey Glenn,
>>>
>>> Did you miss adding the POWERPC_FLAG_PPE42 flag here  ^  ?
>>>
>>> Thanks,
>>> Chinmay
>> No. All PPE42 processors will have bit 1 << 25 set in env->msr_mask, so
>> it will always fall into the previous condition block and never enter
>> the 2nd check.
>>
>> Glenn
>>
> Ah, sorry, I should have looked closer!  This is supposed to be
> checking that if 1 << 25 is not set that we shouldn't be setting the
> PPE42 flag either.  So, yes, I'll add that in v6.

While we are at it, can we also replace all hard-coded bit shifts with
appropriate macros which reflect what these shifts are about. There are
few more such checks in the patch. May be audit other patches as well
for such instances.

regards
Harsh

> 
> Thanks,
> 
> Glenn