linux-kernel - RE: Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0d104f0e26134b1ea9c530be895e65d1@hisilicon.com>
Date:   Fri, 12 Feb 2021 00:03:52 +0000
From:   "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
To:     Finn Thain <fthain@...egraphics.com.au>
CC:     tanxiaofei <tanxiaofei@...wei.com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxarm@...neuler.org" <linuxarm@...neuler.org>,
        "linux-m68k@...r.kernel.org" <linux-m68k@...r.kernel.org>
Subject: RE: Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI
 drivers



> -----Original Message-----
> From: Finn Thain [mailto:fthain@...egraphics.com.au]
> Sent: Friday, February 12, 2021 12:57 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@...ilicon.com>
> Cc: tanxiaofei <tanxiaofei@...wei.com>; jejb@...ux.ibm.com;
> martin.petersen@...cle.com; linux-scsi@...r.kernel.org;
> linux-kernel@...r.kernel.org; linuxarm@...neuler.org;
> linux-m68k@...r.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.

> 
> 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