Re: [Qemu-devel] [PATCH v2 01/12] ARM: Export cpu_env

[Andreas Färber]

Am 28.06.2013 16:20, schrieb Peter Maydell:
> On 30 April 2013 13:00, Andreas Färber <address@hidden> wrote:
>> Am 30.04.2013 13:54, schrieb Alexander Graf:
>>> Am 30.04.2013 um 13:42 schrieb Andreas Färber <address@hidden>:
>>>
>>>> Am 30.04.2013 08:36, schrieb John Rigby:
>>>>> From: Alexander Graf <address@hidden>
>>>>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>>>>> index 675773a..36537bd 100644
>>>>> --- a/target-arm/translate.c
>>>>> +++ b/target-arm/translate.c
>>>>> @@ -59,7 +59,7 @@ static uint32_t gen_opc_condexec_bits[OPC_BUF_SIZE];
>>>>> #define DISAS_WFI 4
>>>>> #define DISAS_SWI 5
>>>>>
>>>>> -static TCGv_ptr cpu_env;
>>>>> +TCGv_ptr cpu_env;
>>>>> /* We reuse the same 64-bit temporaries for efficiency.  */
>>>>> static TCGv_i64 cpu_V0, cpu_V1, cpu_M0;
>>>>> static TCGv_i32 cpu_R[16];
>>>>> diff --git a/target-arm/translate.h b/target-arm/translate.h
>>>>> index e727bc6..8ba1433 100644
>>>>> --- a/target-arm/translate.h
>>>>> +++ b/target-arm/translate.h
>>>>> @@ -24,4 +24,6 @@ typedef struct DisasContext {
>>>>>     int vec_stride;
>>>>> } DisasContext;
>>>>>
>>>>> +extern TCGv_ptr cpu_env;
>>>>> +
>>>>> #endif /* TARGET_ARM_TRANSLATE_H */
>>>>
>>>> Alex, have you checked whether the variable can be placed in qom/cpu.c
>>>> instead once for all targets? I'd hope that would be possible with an
>>>> appropriate typedef (since target_long size etc. are unknown there).
>>>
>>> I would prefer to keep the translation context separate from the execution 
>>> context.
>>
>> Not sure if you understood my point? Exposing cpu_env from target-arm
>> would seem to torpedo our efforts to link target-arm and, e.g.,
>> target-microblaze together, since they all have cpu_env. I don't really
>> care if it's qom/cpu.c or tcg/shared.c or renaming to arm_cpu_env. :)
> 
> So we could deal with this by:
>  translate-a64.c has its own 'static TCGv_ptr cpu_env;'
>  translate.c:arm_translate_init() passes cpu_env to
>    a64_translate_init()
>  translate-a64.c:a64_translate_init() stashes the value it
>    gets passed in its own cpu_env static.
> 
> That avoids having it be a non-static global and doesn't require us
> to rename every usage of cpu_env in translate.c either.
> 
> Any objections?

I don't mind that cpu_env change getting committed as interim solution,
so far I did not come up with a better patch - we'd need to split out
host parts from tcg/tcg.h first, for which I did not find time yet.
Just please fix up the subject so that things are consistent.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg