Re: [PATCH v4 0/5] target/ppc: Untangle CPU init from translation

[Fabiano Rosas <farosas@linux.ibm.com>]

"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> writes:

> Based-on: ppc-for-6.1 tree
>
> This patch series aims to remove the logic of initializing CPU from
> the file related to TCG translation. To achieve this, we have to make
> it so registering SPRs isn't directly tied to TCG, and move code only
> related to translation out of translate_init.c.inc and into translate.c.
> This is in preparation to compile this target without TCG.
>
> Changes for v4:
>  * reordered patches, to make partially applying simpler
>  * removed patches that were already applied
>  * undone creation of spt_tcg.c.inc, now waiting for further cleanup
>  * moved SPR_NOACCESS motion to last patch, and to spr_tcg.h
>
> Changes for v3:
>  * fixed the parameters of _spr_register
>  * remove some redundant #include statements
>  * removed some functions that were mentioned in v2 as unnecessary
>  * added copyright header to relevant files
>  * removed first patch, that was already applied
>  * removed a changed that would add a regression
>
>  Changes for v2:
>  * split and reordered patches, to make it easier to review
>  * improved commit messages 
>  * Undid creation of spr_common, as it was unnecessary
>  * kept more functions as static
>  * ensured that the project builds after every commit
>
> Bruno Larsen (billionai) (5):
>   target/ppc: Fold gen_*_xer into their callers
>   target/ppc: renamed SPR registration functions
>   target/ppc: move SPR R/W callbacks to translate.c
>   target/ppc: turned SPR R/W callbacks not static
>   target/ppc: isolated cpu init from translation logic
>
>  .../ppc/{translate_init.c.inc => cpu_init.c}  | 1848 ++++-------------
>  target/ppc/meson.build                        |    1 +
>  target/ppc/spr_tcg.h                          |  136 ++
>  target/ppc/translate.c                        | 1072 +++++++++-
>  4 files changed, 1598 insertions(+), 1459 deletions(-)
>  rename target/ppc/{translate_init.c.inc => cpu_init.c} (89%)
>  create mode 100644 target/ppc/spr_tcg.h

We're still missing some changes:

- some files (hw/ppc/pnv.c, hw/ppc/spapr.c) use oea_read to check if an
SPR exists. This needs to be changed to something that is present in
both configs (I believe Bruno is working on this).

- The commit 6113563982 ("target/ppc: Clean up _spr_register et al")
from the ppc-for-6.1 branch missed some TCG-specific code in
gen_spr_BookE206:

$ ../configure --target-list=ppc64-softmmu --disable-tcg
$ make
(...)
[193/264] Compiling C object libqemu-ppc64-softmmu.fa.p/target_ppc_cpu_init.c.o
FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_cpu_init.c.o 
(...)
../target/ppc/cpu_init.c: In function ‘register_BookE206_sprs’:
../target/ppc/cpu_init.c:1207:16: error: variable ‘uea_write’ set but not used [-Werror=unused-but-set-variable]
         void (*uea_write)(DisasContext *ctx, int sprn, int gprn) =
                ^~~~~~~~~
cc1: all warnings being treated as errors

We need something like:

--- a/target/ppc/translate_init.c.inc   2021-05-04 16:24:53.549556292 -0400
+++ b/target/ppc/translate_init.c.inc   2021-05-04 16:26:41.005280971 -0400
@@ -2025,11 +2025,13 @@
     /* TLB assist registers */
     /* XXX : not implemented */
     for (i = 0; i < 8; i++) {
+#ifdef CONFIG_TCG
         void (*uea_write)(DisasContext *ctx, int sprn, int gprn) =
             &spr_write_generic32;
         if (i == 2 && (mas_mask & (1 << i)) && (env->insns_flags & PPC_64B)) {
             uea_write = &spr_write_generic;
         }
+#endif
         if (mas_mask & (1 << i)) {
             spr_register(env, mas_sprn[i], mas_names[i],
                          SPR_NOACCESS, SPR_NOACCESS,
---