| Message ID | 20210429162130.2412-8-bruno.larsen@eldorado.org.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: untangle cpu init from translation | expand |
"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> writes: > finished isolation of CPU initialization logic from > translation logic. CPU initialization now only has common code > which may or may not call accelerator-specific code, as the > build options require, and is compiled separately. > > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> > --- > target/ppc/{translate_init.c.inc => cpu_init.c} | 12 +++++++++++- > target/ppc/meson.build | 1 + > target/ppc/translate.c | 4 +++- > 3 files changed, 15 insertions(+), 2 deletions(-) > rename target/ppc/{translate_init.c.inc => cpu_init.c} (99%) > > diff --git a/target/ppc/translate_init.c.inc b/target/ppc/cpu_init.c > similarity index 99% > rename from target/ppc/translate_init.c.inc > rename to target/ppc/cpu_init.c > index b329e953cb..f0f8fc481e 100644 > --- a/target/ppc/translate_init.c.inc > +++ b/target/ppc/cpu_init.c > @@ -18,6 +18,7 @@ > * License along with this library; if not, see <http://www.gnu.org/licenses/>. > */ > > +#include "qemu/osdep.h" > #include "disas/dis-asm.h" > #include "exec/gdbstub.h" > #include "kvm_ppc.h" > @@ -42,6 +43,10 @@ > #include "fpu/softfloat.h" > #include "qapi/qapi-commands-machine-target.h" > > +#include "helper_regs.h" > +#include "internal.h" > +#include "spr_tcg.h" These two includes look like they belong in patch 3 and 4 respectively. And we probably want an #ifdef CONFIG_TCG around them. > + > /* #define PPC_DEBUG_SPR */ > /* #define USE_APPLE_GDB */ > > @@ -49,7 +54,12 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > { > /* Altivec always uses round-to-nearest */ > set_float_rounding_mode(float_round_nearest_even, &env->vec_status); > - helper_mtvscr(env, val); > + /* > + * This comment is here just so the project will build. > + * The current solution is in another patch and will be > + * added when we figure out an internal fork of qemu > + */ > + /* helper_mtvscr(env, val); */ > } > > #ifdef CONFIG_TCG > diff --git a/target/ppc/meson.build b/target/ppc/meson.build > index bbfef90e08..ad53629298 100644 > --- a/target/ppc/meson.build > +++ b/target/ppc/meson.build > @@ -2,6 +2,7 @@ ppc_ss = ss.source_set() > ppc_ss.add(files( > 'cpu-models.c', > 'cpu.c', > + 'cpu_init.c', > 'dfp_helper.c', > 'excp_helper.c', > 'fpu_helper.c', > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index a6e677fa6d..afb8a2aa27 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -38,6 +38,9 @@ > #include "qemu/atomic128.h" > #include "internal.h" > > +#include "qemu/qemu-print.h" > +#include "qapi/error.h" > +#include "internal.h" This one is already included. > > #define CPU_SINGLE_STEP 0x1 > #define CPU_BRANCH_STEP 0x2 > @@ -7595,7 +7598,6 @@ GEN_HANDLER2_E(trechkpt, "trechkpt", 0x1F, 0x0E, 0x1F, 0x03FFF800, \ > > #include "helper_regs.h" > #include "spr_tcg.c.inc" > -#include "translate_init.c.inc" > > /*****************************************************************************/ > /* Misc PowerPC helpers */
On 4/29/21 9:21 AM, Bruno Larsen (billionai) wrote: > @@ -49,7 +54,12 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > { > /* Altivec always uses round-to-nearest */ > set_float_rounding_mode(float_round_nearest_even, &env->vec_status); > - helper_mtvscr(env, val); > + /* > + * This comment is here just so the project will build. > + * The current solution is in another patch and will be > + * added when we figure out an internal fork of qemu > + */ > + /* helper_mtvscr(env, val); */ > } (1) this is a separate change to splitting out cpu_init.c. (2) you can't even do this without introducing a regression. r~
On 30/04/2021 01:25, Richard Henderson wrote: > On 4/29/21 9:21 AM, Bruno Larsen (billionai) wrote: >> @@ -49,7 +54,12 @@ static inline void vscr_init(CPUPPCState *env, >> uint32_t val) >> { >> /* Altivec always uses round-to-nearest */ >> set_float_rounding_mode(float_round_nearest_even, >> &env->vec_status); >> - helper_mtvscr(env, val); >> + /* >> + * This comment is here just so the project will build. >> + * The current solution is in another patch and will be >> + * added when we figure out an internal fork of qemu >> + */ >> + /* helper_mtvscr(env, val); */ >> } > > (1) this is a separate change to splitting out cpu_init.c. Oh, yeah. I was going to remove this change for now, it was for building with disable-tcg, which is still not working > (2) you can't even do this without introducing a regression. The plan is to not just remove, but change with a common function. on a future series, though. I'm just slightly concerned now that make check has not seen anything... > > > r~
On 29/04/2021 18:23, Fabiano Rosas wrote: > "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> writes: > >> finished isolation of CPU initialization logic from >> translation logic. CPU initialization now only has common code >> which may or may not call accelerator-specific code, as the >> build options require, and is compiled separately. >> >> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> >> --- >> target/ppc/{translate_init.c.inc => cpu_init.c} | 12 +++++++++++- >> target/ppc/meson.build | 1 + >> target/ppc/translate.c | 4 +++- >> 3 files changed, 15 insertions(+), 2 deletions(-) >> rename target/ppc/{translate_init.c.inc => cpu_init.c} (99%) >> >> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/cpu_init.c >> similarity index 99% >> rename from target/ppc/translate_init.c.inc >> rename to target/ppc/cpu_init.c >> index b329e953cb..f0f8fc481e 100644 >> --- a/target/ppc/translate_init.c.inc >> +++ b/target/ppc/cpu_init.c >> @@ -18,6 +18,7 @@ >> * License along with this library; if not, see <http://www.gnu.org/licenses/>. >> */ >> >> +#include "qemu/osdep.h" >> #include "disas/dis-asm.h" >> #include "exec/gdbstub.h" >> #include "kvm_ppc.h" >> @@ -42,6 +43,10 @@ >> #include "fpu/softfloat.h" >> #include "qapi/qapi-commands-machine-target.h" >> >> +#include "helper_regs.h" >> +#include "internal.h" >> +#include "spr_tcg.h" > These two includes look like they belong in patch 3 and 4 respectively. > > And we probably want an #ifdef CONFIG_TCG around them. Just to make sure, you mean spr_tcg.h and internal.h, right? Internal.h needs to be included regardless, since it holds some functions always required for init_proc, like ppc_gdb_init. These bits will be removed on the patch series that specifically disable them if we can. spr_tcg.h only has function prototypes, so I don't think it's a problem to include it in case of !TCG. Some .h were removed in the other RFC because they needed files that weren't in the include path. If we should remove it anyway, I can add that :) >> + >> /* #define PPC_DEBUG_SPR */ >> /* #define USE_APPLE_GDB */ >> >> @@ -49,7 +54,12 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) >> { >> /* Altivec always uses round-to-nearest */ >> set_float_rounding_mode(float_round_nearest_even, &env->vec_status); >> - helper_mtvscr(env, val); >> + /* >> + * This comment is here just so the project will build. >> + * The current solution is in another patch and will be >> + * added when we figure out an internal fork of qemu >> + */ >> + /* helper_mtvscr(env, val); */ >> } >> >> #ifdef CONFIG_TCG >> diff --git a/target/ppc/meson.build b/target/ppc/meson.build >> index bbfef90e08..ad53629298 100644 >> --- a/target/ppc/meson.build >> +++ b/target/ppc/meson.build >> @@ -2,6 +2,7 @@ ppc_ss = ss.source_set() >> ppc_ss.add(files( >> 'cpu-models.c', >> 'cpu.c', >> + 'cpu_init.c', >> 'dfp_helper.c', >> 'excp_helper.c', >> 'fpu_helper.c', >> diff --git a/target/ppc/translate.c b/target/ppc/translate.c >> index a6e677fa6d..afb8a2aa27 100644 >> --- a/target/ppc/translate.c >> +++ b/target/ppc/translate.c >> @@ -38,6 +38,9 @@ >> #include "qemu/atomic128.h" >> #include "internal.h" >> >> +#include "qemu/qemu-print.h" >> +#include "qapi/error.h" >> +#include "internal.h" > This one is already included. oops, removed. > >> >> #define CPU_SINGLE_STEP 0x1 >> #define CPU_BRANCH_STEP 0x2 >> @@ -7595,7 +7598,6 @@ GEN_HANDLER2_E(trechkpt, "trechkpt", 0x1F, 0x0E, 0x1F, 0x03FFF800, \ >> >> #include "helper_regs.h" >> #include "spr_tcg.c.inc" >> -#include "translate_init.c.inc" >> >> /*****************************************************************************/ >> /* Misc PowerPC helpers */
On 4/30/21 10:12 AM, Bruno Piazera Larsen wrote: >>> +#include "helper_regs.h" >>> +#include "internal.h" >>> +#include "spr_tcg.h" >> These two includes look like they belong in patch 3 and 4 respectively. >> >> And we probably want an #ifdef CONFIG_TCG around them. > > Just to make sure, you mean spr_tcg.h and internal.h, right? > > Internal.h needs to be included regardless, since it holds some functions > always required for init_proc, like ppc_gdb_init. These bits will be removed on > the patch series that specifically disable them if we can. > > spr_tcg.h only has function prototypes, so I don't think it's a problem to > include it in case of !TCG. Some .h were removed in the other RFC because they > needed files that weren't in the include path. If we should remove it anyway, I > can add that :) I wouldn't add an ifdef for CONFIG_TCG. That would just force you to add more conditional compilation elsewhere. So long as the symbols are either unused or optimized away, we'll be fine. r~
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/cpu_init.c similarity index 99% rename from target/ppc/translate_init.c.inc rename to target/ppc/cpu_init.c index b329e953cb..f0f8fc481e 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/cpu_init.c @@ -18,6 +18,7 @@ * License along with this library; if not, see <http://www.gnu.org/licenses/>. */ +#include "qemu/osdep.h" #include "disas/dis-asm.h" #include "exec/gdbstub.h" #include "kvm_ppc.h" @@ -42,6 +43,10 @@ #include "fpu/softfloat.h" #include "qapi/qapi-commands-machine-target.h" +#include "helper_regs.h" +#include "internal.h" +#include "spr_tcg.h" + /* #define PPC_DEBUG_SPR */ /* #define USE_APPLE_GDB */ @@ -49,7 +54,12 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) { /* Altivec always uses round-to-nearest */ set_float_rounding_mode(float_round_nearest_even, &env->vec_status); - helper_mtvscr(env, val); + /* + * This comment is here just so the project will build. + * The current solution is in another patch and will be + * added when we figure out an internal fork of qemu + */ + /* helper_mtvscr(env, val); */ } #ifdef CONFIG_TCG diff --git a/target/ppc/meson.build b/target/ppc/meson.build index bbfef90e08..ad53629298 100644 --- a/target/ppc/meson.build +++ b/target/ppc/meson.build @@ -2,6 +2,7 @@ ppc_ss = ss.source_set() ppc_ss.add(files( 'cpu-models.c', 'cpu.c', + 'cpu_init.c', 'dfp_helper.c', 'excp_helper.c', 'fpu_helper.c', diff --git a/target/ppc/translate.c b/target/ppc/translate.c index a6e677fa6d..afb8a2aa27 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -38,6 +38,9 @@ #include "qemu/atomic128.h" #include "internal.h" +#include "qemu/qemu-print.h" +#include "qapi/error.h" +#include "internal.h" #define CPU_SINGLE_STEP 0x1 #define CPU_BRANCH_STEP 0x2 @@ -7595,7 +7598,6 @@ GEN_HANDLER2_E(trechkpt, "trechkpt", 0x1F, 0x0E, 0x1F, 0x03FFF800, \ #include "helper_regs.h" #include "spr_tcg.c.inc" -#include "translate_init.c.inc" /*****************************************************************************/ /* Misc PowerPC helpers */
finished isolation of CPU initialization logic from translation logic. CPU initialization now only has common code which may or may not call accelerator-specific code, as the build options require, and is compiled separately. Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> --- target/ppc/{translate_init.c.inc => cpu_init.c} | 12 +++++++++++- target/ppc/meson.build | 1 + target/ppc/translate.c | 4 +++- 3 files changed, 15 insertions(+), 2 deletions(-) rename target/ppc/{translate_init.c.inc => cpu_init.c} (99%)