Subject: Re: [PATCH 8/9] ps3: cleanup ps3fb before clearing HPTE

From: Geoff Levand <[email-masked]>
Date: Mon, 26 Mar 2007 18:06:33 -0700

Milton Miller wrote:
> Paul, after reading this note I send a big fat
> 
> NACK
> 
> to this patch.

Sorry for not being more clear.  That was what I was kind of
saying, that there is something strange here, and I am looking
into it.

> On Mar 24, 2007, at 6:46 PM, Geoff Levand wrote:
> 
>> Christoph Hellwig wrote:
>>>>>  static void ps3_hpte_clear(void)
>>>>  {
>>>> +
>>>> +#ifdef CONFIG_FB_PS3
>>>> +    ps3fb_cleanup();
>>>> +#endif
>>>
>>> And well, adding framebuffer calls into the mmu code is rather fucked.
>>> This at least needs a big comment explaning the hypervisor braindamage
>>> that is responsible for this in colourfull language.
>>
>> Sorry for such a late reply, but I'm now doing the kexec code and see
>> why it was put here (by the original author).  It is so you will have
>> debugging output until the very last possible moment during a kexec
>> shutdown.
> 
> But that is REALLY to late for that.
> 
>> The real problem is really that this routine (and the corresponding 
>> ppc_md
>> variable) are inappropriately named.
> 
> Ok, this should be
> ppc_md.kexec_cleanup_nothing_but_static_data_bss_real_mode_cleanup()
> 
> 
>> In the general case all that is left
>> to do is to call ppc_md.hpte_clear_all, but for ps3 we also want to do 
>> the
>> framebuffer shutdown here, so something like this should be better:
>>
>> -static void ps3_hpte_clear(void)
>> +static void ps3_kexec_final(void)
>>
>> -       ppc_md.hpte_clear_all = ps3_hpte_clear;
>> +       ppc_md.hpte_clear_all = ps3_kexec_final;
>>
> 
> NACK.

Yes, I just wanted to report why this strange code was in ps3_hpte_clear,
since Christoph never did get an answer, and give my idea on what might
work.  This is not a solution, just a thought.  I haven't really got
into it enough to say what the best way would be.  Thanks for the
comments though.
 
> Oh, I just looked at your ps3 kexec hooks.  Your ps3_kexec_cpu_down 
> looks
> totally broken.  

Yes, disregard anything you find regarding kexec on ps3.  I'll post
some new stuff for review, hopefully sometime soon.

-Geoff