Re: [RFC PATCH 2/4] target/ppc: divided mmu_helper.c in 2 files

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4235 bytes --]

On Mon, Jun 07, 2021 at 03:35:06PM -0300, Lucas Mateus Martins Araujo e Castro wrote:
> 
> On 06/06/2021 23:31, David Gibson wrote:
> > On Wed, Jun 02, 2021 at 04:26:02PM -0300, Lucas Mateus Castro (alqotel) wrote:
> > > Moved functions in mmu_helper.c that should be compiled in build to
> > > mmu_common.c, moved declaration of functions that both files use to
> > > cpu.h and moved struct declarations and inline functions needed by
> > > both to target/ppc/internal.h. Updated meson.build to compile the
> > > new file.
> > > 
> > > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> > > ---
> > > Had to turn a few functions non static as it was used by both
> > > mmu_common.c and mmu_helper.c. Added the declaration of mmu_ctx_t to
> > > cpu.h so functions there can reference it and added the definition to
> > > internal.h so functions in both mmu_* files can access it.
> > > And maybe the tlb functions should be declared in internal.h instead of
> > > cpu.h.
> > > ---
> > >   target/ppc/cpu.h        |   35 +
> > >   target/ppc/internal.h   |   26 +
> > >   target/ppc/meson.build  |    6 +-
> > >   target/ppc/mmu_common.c | 1606 ++++++++++++++++++++++++++++++++++
> > >   target/ppc/mmu_helper.c | 1814 +++------------------------------------
> > >   5 files changed, 1774 insertions(+), 1713 deletions(-)
> > >   create mode 100644 target/ppc/mmu_common.c
> > > 
> > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > > index b0934d9be4..cfc35ef83e 100644
> > > --- a/target/ppc/cpu.h
> > > +++ b/target/ppc/cpu.h
> > > @@ -1329,6 +1329,41 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
> > >   void ppc_tlb_invalidate_all(CPUPPCState *env);
> > >   void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
> > >   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
> > > +
> > > +typedef struct mmu_ctx_t mmu_ctx_t;
> > > +int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> > > +                            hwaddr *raddrp,
> > > +                            target_ulong address, uint32_t pid, int ext,
> > > +                            int i);
> > > +int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t *ctx,
> > > +                                     target_ulong eaddr,
> > > +                                     MMUAccessType access_type, int type,
> > > +                                     int mmu_idx);
> > > +hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> > > +                                        ppcmas_tlb_t *tlb);
> > > +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
> > > +                            hwaddr *raddrp, target_ulong address,
> > > +                            uint32_t pid);
> > > +int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> > > +                                target_ulong eaddr, MMUAccessType access_type,
> > > +                                int type);
> > > +static inline int ppc6xx_tlb_getnum(CPUPPCState *env, target_ulong eaddr,
> > > +                                    int way, int is_code)
> > > +{
> > > +    int nr;
> > > +
> > > +    /* Select TLB num in a way from address */
> > > +    nr = (eaddr >> TARGET_PAGE_BITS) & (env->tlb_per_way - 1);
> > > +    /* Select TLB way */
> > > +    nr += env->tlb_per_way * way;
> > > +    /* 6xx have separate TLBs for instructions and data */
> > > +    if (is_code && env->id_tlbs == 1) {
> > > +        nr += env->nb_tlb;
> > > +    }
> > > +
> > > +    return nr;
> > > +}
> > This is a rather complex and model specific function to have inline in
> > a header.
> What is the best way to deal with this function? It's used by both
> mmu_helper.c and mmu_common.c so I put it there as a way to keep it being an
> inline function, so it would be best to put it in target/ppc/internal.h or
> maybe just turn it into a not inline function?

There's no good reason for this to be inline.  Just put it in
mmu_common.c and export.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]