Re: [PATCH] powerpc/asm: Mark cr0 as clobbered in mftb()

[Oliver <oohall@gmail.com>]

On Mon, Jul 10, 2017 at 9:50 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Oliver O'Halloran <oohall@gmail.com> writes:
>
>> The workaround for the CELL timebase bug does not correctly mark cr0 as
>> being clobbered. This can result in GCC making some poor^W completely
>> broken optimisations.
>
> Fruit 'n oats, how did we never notice that? Luck I guess. Or subtle
> breakage that no one could pin down :E

Dumb luck probably.  The workaround is inside a feature section which
depends on CPU_FTR_CELL_TB_BUG so you would only ever see a problem
when actually running on Cell (can't be that many people...). CR0
being volatile across function calls probably helped mask it too.

> I'll tag it for stable.
Good idea.

> Your change log is not entirely fair, it's not GCC's fault, we changed
> register state without telling it, so it's on us :)  I'll reword it a
> bit here.

Yeah that was probably a bit harsh.

>
> cheers
>
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 7e50e47375d6..a3b6575c7842 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -1303,7 +1303,7 @@ static inline void msr_check_and_clear(unsigned long bits)
>>                               "       .llong 0\n"                     \
>>                               ".previous"                             \
>>                       : "=r" (rval) \
>> -                     : "i" (CPU_FTR_CELL_TB_BUG), "i" (SPRN_TBRL)); \
>> +                     : "i" (CPU_FTR_CELL_TB_BUG), "i" (SPRN_TBRL) : "cr0"); \
>>                       rval;})
>>  #else
>>  #define mftb()               ({unsigned long rval;   \
>> --
>> 2.9.4