Re: [PATCH v2 7/7] target/ppc: isolated cpu init from translation logic

From: Bruno Piazera Larsen <bruno.larsen@eldorado.org.br>
To: Fabiano Rosas <farosas@linux.ibm.com>, qemu-devel@nongnu.org
Cc: lucas.araujo@eldorado.org.br, luis.pires@eldorado.org.br,
	fernando.valle@eldorado.org.br, qemu-ppc@nongnu.org,
	matheus.ferst@eldorado.org.br, david@gibson.dropbear.id.au
Subject: Re: [PATCH v2 7/7] target/ppc: isolated cpu init from translation logic
Date: Fri, 30 Apr 2021 14:12:54 -0300	[thread overview]
Message-ID: <dd389368-8778-57e3-5aed-bf56cde1df94@eldorado.org.br> (raw)
In-Reply-To: <874kfo3hvh.fsf@linux.ibm.com>

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

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 */
-- 
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>

[-- Attachment #2: Type: text/html, Size: 5298 bytes --]