Re: CVS commit: src/sys/arch/aarch64/aarch64

[Ryo Shimizu <ryo%nerv.org@localhost>]

>Module Name:	src
>Committed By:	jmcneill
>Date:		Sun Feb 21 23:37:10 UTC 2021
>
>Modified Files:
>	src/sys/arch/aarch64/aarch64: idle_machdep.S
>
>Log Message:
>When waking from cpu_idle(), only call dosoftints if ci_intr_depth == 0
>
>
>To generate a diff of this commit:
>cvs rdiff -u -r1.7 -r1.8 src/sys/arch/aarch64/aarch64/idle_machdep.S

I think this condition is not necessary since cpu_idle() is just called from idle_loop(),
and ci_intr_depth is always zero at this time.

After thinking about it, I realized that there is no need to even increment intr_depth.

  curcpu()->ci_ntr_depth = 1;
  ARM_IRQ_HANDLER();
  curcpu()->ci_ntr_depth = 0;


In addition, because of the possibility of kpreemption (but aarch64 has no KPREEMPT yet),
the acquisition of curcpu() is moved to after DISABLE_INTERRUPT and got the following.

cvs -q diff -aU10 -a -p idle_machdep.S
Index: idle_machdep.S
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/aarch64/idle_machdep.S,v
retrieving revision 1.8
diff -a -U 10 -a -p -r1.8 idle_machdep.S
--- idle_machdep.S	21 Feb 2021 23:37:09 -0000	1.8
+++ idle_machdep.S	22 Feb 2021 10:16:25 -0000
@@ -67,40 +67,36 @@ ENTRY(cpu_idle)
 	stp	x29, x30, [sp, #TF_X29]		/* save x29,x30 */
 #ifdef DDB
 	add	x29, sp, #TF_X29		/* link frame for backtrace */
 #endif
 
 	/* fill the minimum required trapframe */
 	mov	x2, #SPSR_M_EL1H		/* what our spsr should be */
 	str	x2, [sp, #TF_SPSR]
 	adr	x0, 1f
 	str	x0, [sp, #TF_PC]		/* CLKF_PC refer to tf_pc */
-
-	mrs	x1, tpidr_el1			/* get curlwp */
-	ldr	x1, [x1, #L_CPU]		/* get curcpu */
-	ldr	w28, [x1, #CI_INTR_DEPTH]	/* w28 = ci->ci_intr_depth */
-	add	w2, w28, #1			/* w2 = intr_depth + 1 */
-
 	mov	x0, sp				/* get pointer to trapframe */
+	mrs	x1, tpidr_el1			/* get curlwp */
 
 	DISABLE_INTERRUPT
-	wfi
+	ldr	x1, [x1, #L_CPU]		/* get curcpu */
+	mov	w2, #1
+	str	w2, [x1, #CI_INTR_DEPTH]	/* ci->ci_intr_depth = 1 */
 
-	str	w2, [x1, #CI_INTR_DEPTH]	/* ci->ci_intr_depth++ */
+	wfi
 	bl	ARM_IRQ_HANDLER			/* irqhandler(trapframe) */
 1:
 	mrs	x1, tpidr_el1			/* get curlwp */
 	ldr	x1, [x1, #L_CPU]		/* get curcpu */
-	str	w28, [x1, #CI_INTR_DEPTH]	/* ci->ci_intr_depth = old */
+	str	wzr, [x1, #CI_INTR_DEPTH]	/* ci->ci_intr_depth = 0 */
 
 #if defined(__HAVE_FAST_SOFTINTS) && !defined(__HAVE_PIC_FAST_SOFTINTS)
-	cbnz	w28, 1f				/* Skip if intr_depth > 0 */
 	ldr	w3, [x1, #CI_SOFTINTS]		/* Get pending softint mask */
 	/* CPL should be 0 */
 	ldr	w2, [x1, #CI_CPL]		/* Get current priority level */
 	lsr	w3, w3, w2			/* shift mask by cpl */
 	cbz	w3, 1f
 	bl	_C_LABEL(dosoftints)		/* dosoftints() */
 1:
 #endif /* __HAVE_FAST_SOFTINTS && !__HAVE_PIC_FAST_SOFTINTS */
 
 	ldr	x28, [sp, #TF_X28]		/* restore x28 */


Is this ok?
-- 
ryo shimizu