Re: [PATCH 01/14] powerpc/64s: idle move soft interrupt mask logic into C code

On Tue, Jun 13, 2017 at 12:46:02AM +1000, Nicholas Piggin wrote:

Hi Gautham,

Thanks for the reviews.

On Mon, 12 Jun 2017 14:07:27 +0530
Gautham R Shenoy [off-list ref] wrote:

quoted

Hi Nick,

(Added Paul Mackerass to the Cc)
On Mon, Jun 12, 2017 at 09:58:22AM +1000, Nicholas Piggin wrote:

quoted

This simplifies the asm and fixes irq-off tracing over sleep
instructions.

Also move powersave_nap check for POWER8 into C code, and move
PSSCR register value calculation for POWER9 into C.
 

Thanks for doing this. Only one minor comment.

quoted

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
index 98a6d07ecb5c..35cf5bb7daed 100644

--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S  

[..snip..]

quoted

@@ -109,13 +109,9 @@ core_idle_lock_held:
 /*
  * Pass requested state in r3:
  *	r3 - PNV_THREAD_NAP/SLEEP/WINKLE in POWER8
- *	   - Requested STOP state in POWER9
+ *	   - Requested PSSCR value in POWER9
  *
- * To check IRQ_HAPPENED in r4
- * 	0 - don't check
- * 	1 - check
- *
- * Address to 'rfid' to in r5
+ * Address of idle handler to 'rfid' to in r4
  */
 pnv_powersave_common:
 	/* Use r3 to pass state nap/sleep/winkle */

@@ -131,30 +127,7 @@ pnv_powersave_common:
 	std	r0,_LINK(r1)
 	std	r0,_NIP(r1)

-	/* Hard disable interrupts */
-	mfmsr	r9
-	rldicl	r9,r9,48,1
-	rotldi	r9,r9,16
-	mtmsrd	r9,1			/* hard-disable interrupts */
-
-	/* Check if something happened while soft-disabled */
-	lbz	r0,PACAIRQHAPPENED(r13)
-	andi.	r0,r0,~PACA_IRQ_HARD_DIS@l
-	beq	1f
-	cmpwi	cr0,r4,0  

There were callers to power7_nap() in the dynamic-core-split/unsplit
which wanted nap to be forced irrespective of whether an irq was
pending or not. With this new patch, there won't be a way to enforce
that on POWER8.

Actually there are two APIs to sleep now, the low level one which
does not check lazy irq or nap disable is power7_idle_insn(). The
one which sets up the lazy irq state is power7_idle_type(). The
dynamic core split calls the former, so AFAIKS it should be
equivalent.

Ah, I see. Yes, my bad. I didn't check which one we are calling in the
dynamic split case.

Reviewed-by: Gautham R. Shenoy <redacted>

Not the best names perhaps. Open to better suggestions.

Thanks,
Nick

--
Thanks and Regards
gautham.