Re: [Qemu-ppc] [Qemu-devel] [PATCH] target-ppc: Init dcache and icache size for e500 user mode

[Andreas Färber]

Am 18.04.2012 17:40, schrieb Alexander Graf:
> On 04/15/2012 06:14 PM, Andreas Färber wrote:
>> Am 13.04.2012 14:00, schrieb Meador Inge:
>>> On 04/13/2012 06:40 AM, Andreas Färber wrote:
>>>
>>>> Am 12.04.2012 19:24, schrieb Scott Wood:
>>>>> On 04/12/2012 11:59 AM, Andreas Färber wrote:
>>>>>> Am 10.04.2012 22:04, schrieb Meador Inge:
>>>>>>> commit f7aa558396dd0f6b7a2b22c05cb503c655854102 pulled the dcache
>>>>>>> and icache
>>>>>>> line size initialization inside of a '#if
>>>>>>> !defined(CONFIG_USER_ONLY)' block.
>>>>>>> This is not correct because instructions like 'dcbz' need the
>>>>>>> dcache size
>>>>>>> initialized even for user mode.
>>>>>>>
>>>>>>> Signed-off-by: Meador Inge<address@hidden>
>>>>>> Looks okay and compiles,
>>>>>>
>>>>>> Reviewed-by: Andreas Färber<address@hidden>
>>>>>>
>>>>>> Scott, are you planning to review this e500 patch? Or should I go
>>>>>> ahead
>>>>>> and apply?
>>>>> I'm OK with it, though it may make more sense for USER_ONLY to just
>>>>> pick
>>>>> an arbitrary cache line size (probably 32) than to try to imitate a
>>>>> specific core.
>> [...]
>>>> Scott's suggestion would avoid some #ifdef'ery so I'd prefer that if
>>>> possible. I'm planning for a PULL later today, so let me know.
>>> Sounds good to me.  I think the #ifdef stuff is gross, but I wasn't
>>> sure of way
>>> around it.
>> Not seeing a follow-up yet I've applied the following change to ppc-next:
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 86a915c..ba4b84d 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -4462,36 +4462,32 @@ static void init_proc_e500 (CPUPPCState *env,
>> int version)
>>                    &spr_read_spefscr,&spr_write_spefscr,
>>                    0x00000000);
>>       /* Memory management */
>> -#if !defined(CONFIG_USER_ONLY)
>> +#if defined(CONFIG_USER_ONLY)
>> +    env->dcache_line_size = 32;
>> +    env->icache_line_size = 32;
>> +#else /* !defined(CONFIG_USER_ONLY) */
> 
> I don't think I like this one. If you're running linux-user with a
> specific CPU (not the generic "I am the common ground" CPU) then you're
> asking for behavior of that exact CPU. So the cache line size has to be
> exposed to the program.

Scott, Meador and me disliked the density of #ifdefs though. If you have
a better idea to structure the code feel free to change it.

Andreas

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