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

[Alexander Graf]

On 18.04.2012, at 22:16, Andreas Färber wrote:

> 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.

Sure - just do 2 switch statements :).


Alex