[PATCH v3 3/9] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of June 30, 2021 5:56 pm:
> 
> 
> Le 30/06/2021 à 09:46, Nicholas Piggin a écrit :
>> The implicit soft-masking to speed up interrupt return was going to be
>> used by 64e as well, but it has not been extensively tested on that
>> platform and is not considered ready. It was intended to be disabled
>> before merge. Disable it for now.
>> 
>> Most of the restart code is common with 64s, so with more correctness
>> and performance testing this could be re-enabled again by adding the
>> extra soft-mask checks to interrupt handlers and flipping
>> exit_must_hard_disable().
>> 
>> Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>   arch/powerpc/include/asm/interrupt.h | 33 ++++++++++++++++++++--------
>>   arch/powerpc/kernel/exceptions-64e.S | 12 +---------
>>   arch/powerpc/kernel/interrupt.c      |  2 +-
>>   arch/powerpc/kernel/interrupt_64.S   | 16 ++++++++++++--
>>   4 files changed, 40 insertions(+), 23 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 8b4b1e84e110..f13c93b033c7 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -73,20 +73,34 @@
>>   #include <asm/kprobes.h>
>>   #include <asm/runlatch.h>
>>   
>> -#ifdef CONFIG_PPC64
>> +#ifdef CONFIG_PPC_BOOK3S_64
> 
> Can we avoid that ifdef and use IS_ENABLED(CONFIG_PPC_BOOK3S_64) below ?

Hey Christophe,

Thanks for the review, sorry it was a bit rushed to get these fixes in
before the pull. I agree with this and there's a few other cleanups we
might do as well. Something to look at next.

> 
>>   extern char __end_soft_masked[];
>>   unsigned long search_kernel_restart_table(unsigned long addr);
>> -#endif
>>   
>> -#ifdef CONFIG_PPC_BOOK3S_64
>>   DECLARE_STATIC_KEY_FALSE(interrupt_exit_not_reentrant);
>>   
>> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>> +{
>> +	if (regs->msr & MSR_PR)
>> +		return false;
>> +
>> +	if (regs->nip >= (unsigned long)__end_soft_masked)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>>   static inline void srr_regs_clobbered(void)
>>   {
>>   	local_paca->srr_valid = 0;
>>   	local_paca->hsrr_valid = 0;
>>   }
>>   #else
>> +static inline bool is_implicit_soft_masked(struct pt_regs *regs)
>> +{
>> +	return false;
>> +}
>> +
>>   static inline void srr_regs_clobbered(void)
>>   {
>>   }
>> @@ -150,11 +164,13 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>>   		 */
>>   		if (TRAP(regs) != INTERRUPT_PROGRAM) {
>>   			CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>> -			BUG_ON(regs->nip < (unsigned long)__end_soft_masked);
>> +			BUG_ON(is_implicit_soft_masked(regs));
>>   		}
>> +#ifdef CONFIG_PPC_BOOK3S
> 
> Allthough we are already in a PPC64 section, wouldn't it be better to use CONFIG_PPC_BOOK3S_64 ?
> 
> Can we use IS_ENABLED(CONFIG_PPC_BOOK3S_64) instead ?

Good question, it's a matter of preference. I have used PPC_BOOK3S in 
other places, but maybe in files shared by 32-bit it would be better to 
have the _64?

On the other hand, in cases where you have an else or #else, then you
still need the PPC64 context to understand that.

I don't really have a preference, I would go with either. Making some
convention and using it everywhere is probably a good idea though.

Thanks,
Nick