[PATCH] powerpc/64: allow rtas to be called in real-mode, use this in machine check

[Nicholas Piggin]

Ganesh's on March 23, 2020 8:19 pm:
> 
> 
> On 3/20/20 8:58 PM, Nicholas Piggin wrote:
>> rtas_call allocates and uses memory in failure paths, which is
>> not safe for RMA. It also calls local_irq_save() which may not be safe
>> in all real mode contexts.
>>
>> Particularly machine check may run with interrupts not "reconciled",
>> and it may have hit while it was in tracing code that should not be
>> rentered.
>>
>> Create minimal rtas call that should be usable by guest machine check
>> code, use it there to call "ibm,nmi-interlock".
>>
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>   arch/powerpc/include/asm/rtas.h      |  1 +
>>   arch/powerpc/kernel/entry_64.S       | 12 ++++++--
>>   arch/powerpc/kernel/rtas.c           | 43 ++++++++++++++++++++++++++++
>>   arch/powerpc/platforms/pseries/ras.c |  2 +-
>>   4 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>> index 3c1887351c71..4ffc499ce1ac 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -352,6 +352,7 @@ extern struct rtas_t rtas;
>>   extern int rtas_token(const char *service);
>>   extern int rtas_service_present(const char *service);
>>   extern int rtas_call(int token, int, int, int *, ...);
>> +extern int raw_rtas_call(int token, int, int, int *, ...);
>>   void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
>>   			int nret, ...);
>>   extern void __noreturn rtas_restart(char *cmd);
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index 51c5b681f70c..309abb677788 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -759,6 +759,13 @@ _GLOBAL(enter_rtas)
>>   	li	r0,0
>>   	mtcr	r0
>>   
>> +	/* enter_rtas called from real-mode may not have irqs reconciled
>> +	 * but will always have interrupts disabled.
>> +	 */
>> +	mfmsr	r6
>> +	andi.	r7,r6,(MSR_IR|MSR_DR)
>> +	beq	2f
>> +
>>   #ifdef CONFIG_BUG
>>   	/* There is no way it is acceptable to get here with interrupts enabled,
>>   	 * check it with the asm equivalent of WARN_ON
>> @@ -769,10 +776,10 @@ _GLOBAL(enter_rtas)
>>   #endif
>>   
>>   	/* Hard-disable interrupts */
>> -	mfmsr	r6
>>   	rldicl	r7,r6,48,1
>>   	rotldi	r7,r7,16
>>   	mtmsrd	r7,1
>> +2:
>>   
>>   	/* Unfortunately, the stack pointer and the MSR are also clobbered,
>>   	 * so they are saved in the PACA which allows us to restore
>> @@ -795,7 +802,6 @@ _GLOBAL(enter_rtas)
>>   	ori	r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE
>>   	andc	r6,r0,r9
>>   
>> -__enter_rtas:
>>   	sync				/* disable interrupts so SRR0/1 */
>>   	mtmsrd	r0			/* don't get trashed */
>>   
>> @@ -837,7 +843,7 @@ rtas_return_loc:
>>   	mtspr	SPRN_SRR1,r4
>>   	RFI_TO_KERNEL
>>   	b	.	/* prevent speculative execution */
>> -_ASM_NOKPROBE_SYMBOL(__enter_rtas)
>> +_ASM_NOKPROBE_SYMBOL(enter_rtas)
>>   _ASM_NOKPROBE_SYMBOL(rtas_return_loc)
>>   
>>   	.align	3
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index c5fa251b8950..a058dcfb6726 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -450,6 +450,8 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
>>   	char *buff_copy = NULL;
>>   	int ret;
>>   
>> +	WARN_ON_ONCE((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR));
>> +
>>   	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
>>   		return -1;
>>   
>> @@ -483,6 +485,47 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
>>   }
>>   EXPORT_SYMBOL(rtas_call);
>>   
>> +/*
>> + * Like rtas_call but no kmalloc or printk etc in error handling, so
>> + * error won't go through log_error. No tracing, may be called in real mode.
>> + */
>> +int notrace raw_rtas_call(int token, int nargs, int nret, int *outputs, ...)
>> +{
>> +	va_list list;
>> +	int i;
>> +	struct rtas_args *rtas_args;
>> +	int ret;
>> +
>> +	WARN_ON_ONCE((mfmsr() & MSR_EE));
>> +
>> +	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
>> +		return -1;
>> +
>> +	/*
>> +	 * Real mode must have MSR[EE]=0 and we prefer not to touch any
>> +	 * irq or preempt state (this may be called in machine check).
>> +	 */
>> +	preempt_disable_notrace();
>> +	arch_spin_lock(&rtas.lock);
> 
> I wonder, if its ok to attempt for this lock in nested MCE.

It's not. It will deadlock if an MCE hits after "ibm,nmi-interlock" is
called and before the lock is released on the same CPU.

The struct actually isn't even that big, 84 bytes or so, and we're in
the dedicated machine check stack here so we could just put it on
stack AFAIKS?

Thanks,
Nick