Re: [RFC PATCH 08/11] target/ppc: wrapped some TCG only logic with ifdefs

[Bruno Piazera Larsen]

On 14/05/2021 11:44, Richard Henderson wrote:

> On
>       5/14/21 8:29 AM, Bruno Piazera Larsen wrote:
>       
> >         
> >         On 12/05/2021 15:57, Bruno Piazera Larsen wrote:
> >         
> > > On 12/05/2021 15:33, Richard Henderson
> > >           wrote:
> > >           
> > > > On 5/12/21 9:08 AM, Bruno Larsen
> > > >             (billionai) wrote:
> > > >             
> > > > > diff --git
> > > > >               a/include/exec/helper-proto.h
> > > > >               b/include/exec/helper-proto.h
> > > > >               
> > > > >               index ba100793a7..ce287222ee 100644
> > > > >               
> > > > >               --- a/include/exec/helper-proto.h
> > > > >               
> > > > >               +++ b/include/exec/helper-proto.h
> > > > >               
> > > > >               @@ -38,7 +38,9 @@ dh_ctype(ret) HELPER(name)
> > > > >               (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
> > > > >               
> > > > >                 #define IN_HELPER_PROTO
> > > > >               
> > > > >                   #include "helper.h"
> > > > >               
> > > > >               +#ifdef CONFIG_TCG
> > > > >               
> > > > >                 #include "trace/generated-helpers.h"
> > > > >               
> > > > >               +#endif
> > > > >               
> > > > >                 #include "accel/tcg/tcg-runtime.h"
> > > > >               
> > > > >                 #include "accel/tcg/plugin-helpers.h"
> > > > >               
> > > > >             
> > > >             
> > > >             Um.. this file is exclusively TCG already.
> > > >             
> > > >             Are you missing some use of helper_foo()?
> > > >             
> > > >           
> > >           A lot of files that we are compiling (mainly mmu-*,
> > >           excp_helper and gdbstub IIRC). We could comb through all of
> > >           them and remove all declarations of helpers and wrap the
> > >           inclusion of helper-proto itself in ifdefs, but it felt
> > >           unnecessarily long. If it is preferable, we can do it.
> > >           
> > >           
> > >         
> >         So, I just looked and we'd need to change excp_helper.c and
> >         mmu-hash64.c, encasing 14 and 8 helper_foo() declarations. Is it
> >         better to work on those 2 files, or to change helper-proto?
> >         
> >       
>       
>       Let's work on excp_helper.c and mmu-hash64.c.
>       
>       
>       For excp_helper.c, ideally everything in there would be tcg
>       related.  Either explicitly as helper_foo() or by being one of the
>       TCGCPUOps functions like ppc_cpu_exec_interrupt.
>       
>

Removing excp_helper.c gives linker errors for the functions:

* ppc_cpu_do_system_reset, on hw/ppc/pnv.c and hw/ppc/spapr.c

* ppc_do_cpu_interrupt, on hw/ppc/spapr_events.c and target/ppc/kvm.c, and from what was discussed on the IRC this should still be compiled

* ppc_cpu_do_fwnmi_machine_check, on hw/ppc/spapr_events.c

I think ppc_do_cpu_interrupt could go to cpu.c, but no clue where the others should go.

>       
>       For mmu-hash64.c... I guess the easiest thing in the short term is
>       to put big ifdefs around helper_slbi{a,e,eg}.  Or they could be
>       moved to mmu_helper.c, with slb_lookup declared in mmu-hash64.h.
>       
>       
>     
    The ifdefs are easy enough, and not compiling it gives way too many
    linker errors. Moving the helpers and static functions sounds the
    cleanest though, so I'll look into it while I wait for the
    excp_helper help
>       
>       r~
>       
>

--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer