Re: [PATCH v3 21/30] target/ppc: Introduce PowerPCCPUClass::has_work()

[Philippe Mathieu-Daudé <f4bug@xxxxxxxxx>]

On 9/3/21 2:50 AM, David Gibson wrote:
> On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote:
>> Each POWER cpu has its own has_work() implementation. Instead of
>> overloading CPUClass on each PowerPCCPUClass init, register the
>> generic ppc_cpu_has_work() handler, and have it call the POWER
>> specific has_work().
> 
> I don't quite see the rationale for introducing a second layer of
> indirection here.  What's wrong with switching the base has_work for
> each cpu variant?
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@xxxxxxxxx>
>> ---
>>  target/ppc/cpu-qom.h  |  3 +++
>>  target/ppc/cpu_init.c | 26 ++++++++++++++++++--------
>>  2 files changed, 21 insertions(+), 8 deletions(-)

>> @@ -8796,6 +8800,12 @@ static bool ppc_cpu_has_work(CPUState *cs)
>>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>>      CPUPPCState *env = &cpu->env;
>>  
>> +    if (cs->halted) {
>> +        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);

Maybe I should have kept the 'if (cs->halted)' for the next patch,
simply dispatch here, then in the next patch the code simplification
is more apparent. I thought this approach would involve less #ifdef'ry
but haven't checked the other way around. Will do now.

>> +        return pcc->has_work(cs);
>> +    }
>> +
>>      return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
>>  }
>>  #endif /* CONFIG_TCG && !CONFIG_USER_ONLY */
>