[PATCH V2] ppc64/lv1call: Fix PS3 LV1 clobbering SRR

[René Rebe]

Hi Christophe,

> On 19. Nov 2025, at 09:08, Christophe Leroy <christophe.leroy at csgroup.eu> wrote:
> 
> Le 18/11/2025 à 18:49, René Rebe a écrit :
>> For a while, likely since 6eaaf9de3599 ("powerpc/64s/interrupt: Check
>> and fix srr_valid without crashing") booting a PS3 instantly yields:
> 
> Can you confirm the 'likely' ?

I could, it “only” takes some hours ;-)

> checkout 6eaaf9de3599 and confirm the bug is there.
> checkout 6eaaf9de3599~ and confirm the bug is not there.

Given 6eaaf9de3599 introduced the check without debug, this
will “likely” confirm it. However, I would find it more interesting
to test if enabling the previous DEBUG option for that already
uncovered this, or if applying this check to such a slightly or
much older kernel would already have notified about this long
before.

Actually looking more into git log of powerpc, the original sin
appears to be likely 59dc5bfca0cb ("powerpc/64s: avoid reloading
(H)SRR registers if they are still valid”)

Either way testing this 100% will be a bit of work. Do we think this
is really necessary for (PS3) or just stick a Fixes: label onto it?

Thanks,
	René

>> <4>SRR0 was: c0000000000971b4 should be: c0000000002d55c4
>> <4>SRR1 was: 8000000000008032 should be: 8000000000008032
>> Fix LV1 HV clobbering SRR by srr_regs_clobbered() each lv1 call.
>> Attribute srr_regs_clobbered __always_inline as used by modules.
>> Fixes: 6eaaf9de3599 ("powerpc/64s/interrupt: Check and fix srr_valid without crashing")
>> Signed-off-by: René Rebe <rene at exactco.de>
> 
> Reviewed-by: Christophe Leroy <christophe.leroy at csgroup.eu>
> 
>> ---
>> v2: use __always_inline, ppc64, Fixes:
>> ---
>>  arch/powerpc/include/asm/interrupt.h | 1 +
>>  arch/powerpc/include/asm/lv1call.h   | 3 ++-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index eb0e4a20b818..783c6f32a395 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -106,6 +106,6 @@ static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>>   return search_kernel_soft_mask_table(regs->nip);
>>  }
>>  -static inline void srr_regs_clobbered(void)
>> +static __always_inline void srr_regs_clobbered(void)
>>  {
>>   local_paca->srr_valid = 0;
>>   local_paca->hsrr_valid = 0;
>> diff --git a/arch/powerpc/include/asm/lv1call.h b/arch/powerpc/include/asm/lv1call.h
>> index ae70120953a8..48dd55a2b72e 100644
>> --- a/arch/powerpc/include/asm/lv1call.h
>> +++ b/arch/powerpc/include/asm/lv1call.h
>> @@ -14,6 +14,7 @@
>>    #include <linux/types.h>
>>  #include <linux/export.h>
>> +#include <asm/interrupt.h>
>>    /* lv1 call declaration macros */
>>  @@ -208,7 +209,7 @@
>>  #define LV1_CALL(name, in, out, num)                               \
>>    extern s64 _lv1_##name(LV1_##in##_IN_##out##_OUT_ARG_DECL);      \
>>    static inline int lv1_##name(LV1_##in##_IN_##out##_OUT_ARG_DECL) \
>> -    {return _lv1_##name(LV1_##in##_IN_##out##_OUT_ARGS);}
>> +    {srr_regs_clobbered(); return _lv1_##name(LV1_##in##_IN_##out##_OUT_ARGS);}
>>  #endif
>>    #endif /* !defined(__ASSEMBLER__) */
> 

-- 
https://exactco.de - https://t2linux.com - https://rene.rebe.de