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

[Philippe Mathieu-Daudé]

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,