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 ?
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 ?
00000364 <interrupt_exit_kernel_prepare>:
364: 81 23 00 84 lwz r9,132(r3)
368: 55 29 04 62 rlwinm r9,r9,0,17,17
36c: 0f 09 00 00 twnei r9,0
370: 81 22 00 4c lwz r9,76(r2)
374: 75 23 00 01 andis. r3,r9,1
378: 40 82 00 14 bne 38c <interrupt_exit_kernel_prepare+0x28>
37c: 7d 20 00 a6 mfmsr r9
380: 7c 00 01 46 wrteei 0
384: 7c 00 01 46 wrteei 0
388: 4e 80 00 20 blr
38c: 38 e2 00 4c addi r7,r2,76
390: 3d 20 00 01 lis r9,1
394: 7c c0 38 28 lwarx r6,0,r7
398: 7c c6 48 78 andc r6,r6,r9
39c: 7c c0 39 2d stwcx. r6,0,r7
3a0: 40 a2 ff f4 bne 394 <interrupt_exit_kernel_prepare+0x30>
3a4: 38 60 00 01 li r3,1
3a8: 4b ff ff d4 b 37c <interrupt_exit_kernel_prepare+0x18>
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