Re: [PATCH 1/4] target/ppc: Include missing headers in mmu-hash[32,64].h

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 18/12/24 17:41, Richard Henderson wrote:
> On 12/18/24 09:51, Philippe Mathieu-Daudé wrote:
>> CPUState* is dereferenced, so we need the structure definition
>> from "cpu.h". PowerPCCPU is declared in "cpu-qom.h". Include
>> them in order to avoid when refactoring:
>>
>>    In file included from ../../target/ppc/cpu_init.c:27:
>>    target/ppc/mmu-hash32.h:6:23: error: unknown type name 'PowerPCCPU'
>>        6 | bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, 
>> MMUAccessType access_type,
>>          |                       ^
>>    target/ppc/mmu-hash32.h:66:15: error: incomplete definition of type 
>> 'struct ArchCPU'
>>       66 |     return cpu->env.spr[SPR_SDR1] & SDR_32_HTABORG;
>>          |            ~~~^
>>    target/ppc/mmu-hash64.h:173:36: error: unknown type name 
>> ‘PowerPCCPU’; did you mean ‘PowerPCCPUAlias’?
>>      173 | static inline void ppc_hash64_init(PowerPCCPU *cpu)
>>          |                                    ^~~~~~~~~~
>>          |                                    PowerPCCPUAlias
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/ppc/mmu-hash32.h | 3 +++
>>   target/ppc/mmu-hash64.h | 2 ++
>>   2 files changed, 5 insertions(+)
>>
>> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
>> index 2838de031c7..abbff206d4e 100644
>> --- a/target/ppc/mmu-hash32.h
>> +++ b/target/ppc/mmu-hash32.h
>> @@ -3,6 +3,9 @@
>>   #ifndef CONFIG_USER_ONLY
>> +#include "target/ppc/cpu-qom.h"
>> +#include "cpu.h"
> 
> cpu.h handles cpu-qom.h.
> Do we really need both?

"cpu-qom.h" is the one I want. Currently we need "cpu.h" for all the
inlined functions dereferencing ArchCPU, like ppc_hash32_hpt_base(),
but long term it should be avoided in .h because sources become too
specific. Here we want to link the different MMU 32/64 in the same
PPC binary, having 2 different ArchCPU implementations. This patch
is quite old now so I don't remember well, I'll revisit and see if
it is still necessary.

>> +
>>   bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType 
>> access_type,
>>                         hwaddr *raddrp, int *psizep, int *protp, int 
>> mmu_idx,
>>                         bool guest_visible);
>> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
>> index ae8d4b37aed..b32e17c2c58 100644
>> --- a/target/ppc/mmu-hash64.h
>> +++ b/target/ppc/mmu-hash64.h
>> @@ -3,6 +3,8 @@
>>   #ifndef CONFIG_USER_ONLY
>> +#include "target/ppc/cpu-qom.h"
>> +
>>   #ifdef TARGET_PPC64
>>   void dump_slb(PowerPCCPU *cpu);
>>   int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>