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

-- 
2.17.1

"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,
---

On 04/05/2021 17:38, Fabiano Rosas wrote:
> "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:

These patches are all in preparation for disabling TCG. I wouldn't 
expect the project to support that flag yet, so the errors make sense 
and are being worked on.

I guess I did things in a weird order, so let me explain my thought 
process for the work so far: In my first RFC (when I still thought it 
was an easy-ish fix) I tried to define what needed to be done so the 
project would build. Then I went back to the drawing board, and decided 
to implement good solutions, and only expect the --disable-tcg to work 
at the very end of these, otherwise the bodged solutions could be 
forgotten and committed into the final tree by accident. Now that I know 
the code motion was done in a satisfactory manner, I'll move on to these 
issues and what else shows up.

>
> $ ../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,
> ---

Also ran into this problem, I intend to fix it as well. If you find 
anything else, let me know :)

-- 

Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO 
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>