Re: interrupt_exit_kernel_prepare() on booke/32 has a useless 'mfmsr' and two 'wrteei 0'

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Christophe Leroy's message of February 10, 2021 7:21 pm:
> 44x/bamboo_defconfig
> 
> For the mfmsr, that's because mfmsr is defined as 'asm volatile'. Is that correct ? Reading MSR 
> doesn't have any side effects as far as I know, should we remove the volatile ?

Well I'm not really sure. It depends on the MSR value, so it must not 
assume it's unchanging.

If you just have asm ("mfmsr %0" : "=r"(msr)) then that's wrong because 
it will omit the second mfmsr in a mfmsr() ; mtmsr() ; mfmsr() sequence.

Adding a "memory" clobber there makes gcc produce the second mfmsr, but 
I don't know if that's really the right thing to do.

> 
> For the wrteei, that's because we are calling __hard_EE_RI_disable() after local_irq_save(). On 
> booke those two fonctions do exactly the same because RI doesn't exist. Could we replace that by a 
> __hard_RI_disable() that would be a nop on booke ?

Not on 64-bit because local_irq_disable() doesn't disable EE there.
You could have something like __hard_EE_RI_disable_irqoff() that is to 
be called only in irq disabled region. But is that just adding too much 
complexity to try to keep 32 and 64 bit unified?

Maybe just making different code paths for 32 and 64 would be best. 
32-bit seems fairly simple

	if (!arch_irq_disabled_regs(regs)) {
                /* Returning to a kernel context with local irqs enabled. */
                WARN_ON_ONCE(!(regs->msr & MSR_EE));

       		local_irq_disable();
                if (IS_ENABLED(CONFIG_PREEMPT)) {
                        /* Return to preemptible kernel context */
                        if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
                                if (preempt_count() == 0)
                                        preempt_schedule_irq();
                        }
                }
		trace_hardirqs_on();
		__hard_RI_disable();
        } else {
		__hard_EE_RI_disable();
	}

You could get rid of that entirely if no preempt or irq tracing and just
have __hard_EE_RI_disable even AFAIKS.

Thanks,
Nick