Re: [PATCH 28/37] Cavium OCTEON FPU EMU exception as TLB exception

[David Daney <ddaney@caviumnetworks.com>]

Sergei Shtylyov wrote:
> Hello.
> 
> ddaney@caviumnetworks.com wrote:
> 
>> The FPU exceptions come in as TLB exceptions -- see if this is
>> one of them, and act accordingly.
>>
>> Signed-off-by: Tomaso Paoletti <tpaoletti@caviumnetworks.com>
>> Signed-off-by: Paul Gortmaker <Paul.Gortmaker@windriver.com>
>> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
>> ---
>>  arch/mips/mm/fault.c |   15 +++++++++++++++
>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
>> index fa636fc..9ce503a 100644
>> --- a/arch/mips/mm/fault.c
>> +++ b/arch/mips/mm/fault.c
>> @@ -47,6 +47,21 @@ asmlinkage void do_page_fault(struct pt_regs *regs, 
>> unsigned long write,
>>             field, regs->cp0_epc);
>>  #endif
>>  
>> +#ifdef CONFIG_CAVIUM_OCTEON_HW_FIX_UNALIGNED
>>   
> 
>   Why this is not in the same patch that introduces this option?

Because the option was introduced in patch 01/37 along with all the rest 
of the new files.  I decided to add the Kconfig for the entire patch set 
  in a single go.  For an incremental patch, certainly Kconfig changes 
would probably be in the patch that adds their use.  But I think 
rigorous orthodoxy on this point in the initial patch set might make 
things less clear rather than improving the situation.


> 
>> +    /*
>> +     * Normally the FPU emulator uses a load word from address one
>> +     * to retake control of the CPU after executing the
>> +     * instruction in the delay slot of an emulated branch. The
>> +     * Octeon hardware unaligned access fix changes this from an
>> +     * address exception into a TLB exception. This code checks to
>> +     * see if this page fault was caused by an FPU emulation.
>> +     *
>> +     * Terminate if exception was recognized as a delay slot return */
>>   
> 
>   I'm back to nitpicking again, see chapter 8 for the preferred 
> multiuline comment style (you almost got it right :-).
> 
>> +    extern int do_dsemulret(struct pt_regs *);
>>   
> 
>   Won't this cause a warning about the declaration amidst of code? You 
> should be able to put it in this function's declaration block painlessly...
> 

Both of these last point will be addressed.

David Daney