[PATCH v2 1/2] powerpc/64s: system call scv tabort fix for corrupt irq soft-mask state

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of September 2, 2021 3:21 am:
> 
> 
> Le 01/09/2021 à 18:54, Nicholas Piggin a écrit :
>> If a system call is made with a transaction active, the kernel
>> immediately aborts it and returns. scv system calls disable irqs even
>> earlier in their interrupt handler, and tabort_syscall does not fix this
>> up.
>> 
>> This can result in irq soft-mask state being messed up on the next
>> kernel entry, and crashing at BUG_ON(arch_irq_disabled_regs(regs)) in
>> the kernel exit handlers, or possibly worse.
>> 
>> Fix this by having tabort_syscall setting irq soft-mask back to enabled
>> (which requires MSR[EE] be disabled first).
>> 
>> Reported-by: Eirik Fuller <efuller at redhat.com>
>> Fixes: 7fa95f9adaee7 ("powerpc/64s: system call support for scv/rfscv instructions")
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>> 
>> Tested the wrong kernel before sending v1 and missed a bug, sorry.
>> 
>>   arch/powerpc/kernel/interrupt_64.S | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
>> index d4212d2ff0b5..9c31d65b4851 100644
>> --- a/arch/powerpc/kernel/interrupt_64.S
>> +++ b/arch/powerpc/kernel/interrupt_64.S
>> @@ -428,16 +428,22 @@ RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
>>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>>   tabort_syscall:
>>   _ASM_NOKPROBE_SYMBOL(tabort_syscall)
>> -	/* Firstly we need to enable TM in the kernel */
>> +	/* We need to enable TM in the kernel, and disable EE (for scv) */
>>   	mfmsr	r10
>>   	li	r9, 1
>>   	rldimi	r10, r9, MSR_TM_LG, 63-MSR_TM_LG
>> +	LOAD_REG_IMMEDIATE(r9, MSR_EE)
>> +	andc	r10, r10, r9
> 
> Why not use 'rlwinm' to mask out MSR_EE ?
> 
> Something like
> 
> 	rlwinm	r10, r10, 0, ~MSR_EE

Mainly because I'm bad at powerpc assembly. Why do you think I'm trying 
to change as much as possible to C?

Actually there should really be no need for mfmsr either, I wanted to
rewrite the thing entirely as

	ld      r10,PACAKMSR(r13)
	LOAD_REG_IMMEDIATE(r9, MSR_TM)
	or	r10,r10,r9
	mtmsrd	r10

But I thought that's not a minimal bug fix.

Thanks,
Nick
> 
>>   	mtmsrd	r10, 0
>>   
>>   	/* tabort, this dooms the transaction, nothing else */
>>   	li	r9, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
>>   	TABORT(R9)
>>   
>> +	/* scv has disabled irqs so must re-enable. sc just remains enabled */
>> +	li	r9,IRQS_ENABLED
>> +	stb	r9,PACAIRQSOFTMASK(r13)
>> +
>>   	/*
>>   	 * Return directly to userspace. We have corrupted user register state,
>>   	 * but userspace will never see that register state. Execution will
>> 
>