On Fri, 12 Feb 2021, Song Bao Hua (Barry Song) wrote:
-----Original Message----- From: Finn Thain [mailto:fthain@telegraphics.com.au] Sent: Friday, February 12, 2021 12:57 PM To: Song Bao Hua (Barry Song) song.bao.hua@hisilicon.com Cc: tanxiaofei tanxiaofei@huawei.com; jejb@linux.ibm.com; martin.petersen@oracle.com; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; linuxarm@openeuler.org; linux-m68k@vger.kernel.org Subject: RE: Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers
On Thu, 11 Feb 2021, Song Bao Hua (Barry Song) wrote:
Actually in m68k, I also saw its IRQ entry disabled interrupts by ' move #0x2700,%sr /* disable intrs */'
arch/m68k/include/asm/entry.h:
.macro SAVE_ALL_SYS move #0x2700,%sr /* disable intrs */ btst #5,%sp@(2) /* from user? */ bnes 6f /* no, skip */ movel %sp,sw_usp /* save user sp */ ...
.macro SAVE_ALL_INT SAVE_ALL_SYS moveq #-1,%d0 /* not system call entry */ movel %d0,%sp@(PT_OFF_ORIG_D0) .endm
arch/m68k/kernel/entry.S:
/* This is the main interrupt handler for autovector interrupts */
ENTRY(auto_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0
movel %sp,%sp@- movel %d0,%sp@- | put vector # on stack auto_irqhandler_fixup = . + 2 jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack jra ret_from_exception
So my question is that " move #0x2700,%sr" is actually disabling all interrupts? And is m68k actually running irq handlers with interrupts disabled?
When sonic_interrupt() executes, the IPL is 2 or 3 (since either IRQ may be involved). That is, SR & 0x700 is 0x200 or 0x300. The level 3 interrupt may interrupt execution of the level 2 handler so an irq lock is used to avoid re-entrance.
This patch,
diff --git a/drivers/net/ethernet/natsemi/sonic.c b/drivers/net/ethernet/natsemi/sonic.c index d17d1b4f2585..041354647bad 100644 --- a/drivers/net/ethernet/natsemi/sonic.c +++ b/drivers/net/ethernet/natsemi/sonic.c @@ -355,6 +355,8 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id) */ spin_lock_irqsave(&lp->lock, flags);
printk_once(KERN_INFO "%s: %08lx\n", __func__, flags);status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT; if (!status) { spin_unlock_irqrestore(&lp->lock, flags);produces this output,
[ 3.800000] sonic_interrupt: 00002300
I actually hope you can directly read the register rather than reading a flag which might be a software one not from register.
Again, the implementation of arch_local_irq_save() may be found in arch/m68k/include/asm/irqflags.h
I ran that code in QEMU, but experience shows that Apple hardware works exactly the same. Please do confirm this for yourself, if you still think the code and comments in sonic_interrupt are wrong.
Best Regards Barry
Thanks Barry