On Mon Jun 5, 2023 at 6:55 PM AEST, Christophe Leroy wrote:
> A disassembly of interrupt_exit_kernel_prepare() shows a useless read
> of MSR register. This is shown by r9 being re-used immediately without
> doing anything with the value read.
>
> c000e0e0: 60 00 00 00 nop
> c000e0e4: 7d 3a c2 a6 mfmd_ap r9
> c000e0e8: 7d 20 00 a6 mfmsr r9
> c000e0ec: 7c 51 13 a6 mtspr 81,r2
> c000e0f0: 81 3f 00 84 lwz r9,132(r31)
> c000e0f4: 71 29 80 00 andi. r9,r9,32768
>
> This is due to the use of local_irq_save(). The flags read by
> local_irq_save() are never used, use local_irq_disable() instead.
I did have a patch that warns if you do a local_irq_disable() when
irqs are disabled which is why I did this, but it is kind of silly.
You could do 'if (!irqs_disabled()) local_irq_disable()'
Unfortunately that adds another branch but if it is not taken
frequently then maybe avoiding the mtMSR/EID would make it a win?
If you don't change that I might end up doing it if I can get that
warning patch merged (needs a few core kernel changes).
I wonder how much that would help local_irq_save too, if interrupts
are already disabled then avoid the mt? Maybe those things are not
very costly on smaller in-order cores.
Reviewed-by: Nicholas Piggin <[email protected]>
>
> Fixes: 13799748b957 ("powerpc/64: use interrupt restart table to speed up return from interrupt")
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/kernel/interrupt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index e34c72285b4e..f3fc5fe919d9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -368,7 +368,6 @@ void preempt_schedule_irq(void);
>
> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
> {
> - unsigned long flags;
> unsigned long ret = 0;
> unsigned long kuap;
> bool stack_store = read_thread_flags() & _TIF_EMULATE_STACK_STORE;
> @@ -392,7 +391,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>
> kuap = kuap_get_and_assert_locked();
>
> - local_irq_save(flags);
> + local_irq_disable();
>
> if (!arch_irq_disabled_regs(regs)) {
> /* Returning to a kernel context with local irqs enabled. */
> --
> 2.40.1