linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e
@ 2013-05-11 23:26 Kevin Hao
  2013-05-11 23:26 ` [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel Kevin Hao
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Kevin Hao @ 2013-05-11 23:26 UTC (permalink / raw)
  To: Kumar Gala, Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc

Hi,

In the current kernel, the external proxy works pretty well with the lazy
EE. But it still treat the external input interrupt as level sensitive
and would hard disable the irq while a interrupt occurs with irq soft
disabled. This patch series enables PACA_IRQ_EE_EDGE support for book3e
and will leave the irq still hard enabled while a external interrupt
occurs with soft irq disabled. This passed the boot test on T4240QDS
board.

---
Kevin Hao (4):
  powerpc/book3e: introduce external_input_edge exception handler for
    64bit kernel
  powerpc: move the patch_exception to a common place
  powerpc: use patch_exception to update the debug exception handler
  powerpc/fsl-book3e: enable the external_input_edge exception handler

 arch/powerpc/include/asm/code-patching.h |  7 +++++++
 arch/powerpc/include/asm/paca.h          |  1 +
 arch/powerpc/kernel/asm-offsets.c        |  1 +
 arch/powerpc/kernel/entry_64.S           |  2 ++
 arch/powerpc/kernel/exceptions-64e.S     | 26 +++++++++++++++++++++++---
 arch/powerpc/kernel/irq.c                | 12 ++++++------
 arch/powerpc/kernel/paca.c               |  3 +++
 arch/powerpc/kernel/setup_64.c           |  6 +-----
 arch/powerpc/lib/code-patching.c         | 15 +++++++++++++++
 arch/powerpc/mm/tlb_nohash.c             | 19 -------------------
 arch/powerpc/platforms/85xx/corenet_ds.c |  9 ++++++++-
 arch/powerpc/sysdev/mpic.c               |  8 ++++++++
 12 files changed, 75 insertions(+), 34 deletions(-)

Thanks,
Kevin
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel
  2013-05-11 23:26 [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e Kevin Hao
@ 2013-05-11 23:26 ` Kevin Hao
  2013-05-13 15:47   ` Scott Wood
  2013-05-11 23:26 ` [PATCH 2/4] powerpc: move the patch_exception to a common place Kevin Hao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Kevin Hao @ 2013-05-11 23:26 UTC (permalink / raw)
  To: Kumar Gala, Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc

In the external proxy facility mode, the interrupt is automatically
acknowledged with the same effect as reading the IACK register. So
this makes external input interrupt more like edge sensitive. That
means we can leave the irq hard enabled when it occurs with irq soft
disabled just like the dec and doorbell interrupt. But the External
Proxy Register(EPR) is only considered valid from the time that the
external interrupt occurs until MSR[EE] is set to 1. So we have to
save the EPR before irq hard enabled.

In general, we would initialize all the interrupt sources to the same
priority. That means if a interrupt is in service and the other
interrupt would never get any chance to be delivered to the cpu.
And also in order not to complicate the implementation, we only support
to save one content of the EPR register and will set PACA_IRQ_EE
and then hard disable the irq if more external interrupt are received.

In order to support to choose the working mode of MPIC at runtime,
we introduce a new external input exception handler. And we will
use this handler if the external proxy is enabled.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/paca.h      |  1 +
 arch/powerpc/kernel/asm-offsets.c    |  1 +
 arch/powerpc/kernel/entry_64.S       |  2 ++
 arch/powerpc/kernel/exceptions-64e.S | 26 +++++++++++++++++++++++---
 arch/powerpc/kernel/irq.c            | 12 ++++++------
 arch/powerpc/kernel/paca.c           |  3 +++
 arch/powerpc/sysdev/mpic.c           |  8 ++++++++
 7 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 77c91e7..9b3649d 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -118,6 +118,7 @@ struct paca_struct {
 	void *mc_kstack;
 	void *crit_kstack;
 	void *dbg_kstack;
+	u32 saved_epr;		/* EPR saved here */
 #endif /* CONFIG_PPC_BOOK3E */
 
 	mm_context_t context;
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index b51a97c..3199708 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -201,6 +201,7 @@ int main(void)
 	DEFINE(PACA_MC_STACK, offsetof(struct paca_struct, mc_kstack));
 	DEFINE(PACA_CRIT_STACK, offsetof(struct paca_struct, crit_kstack));
 	DEFINE(PACA_DBG_STACK, offsetof(struct paca_struct, dbg_kstack));
+	DEFINE(PACASAVEDEPR, offsetof(struct paca_struct, saved_epr));
 #endif /* CONFIG_PPC_BOOK3E */
 
 #ifdef CONFIG_PPC_STD_MMU_64
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 3fe5259..09ac356 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -884,6 +884,8 @@ restore_check_irq_replay:
 	 * still soft-disabled and we keep them that way.
 	*/
 	cmpwi	cr0,r3,0x500
+	cmpwi	cr1,r3,0x510
+	cror	eq,4*cr1+eq,eq
 	bne	1f
 	addi	r3,r1,STACK_FRAME_OVERHEAD;
  	bl	.do_IRQ
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 42a756e..e32bd6c 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -358,6 +358,9 @@ interrupt_end_book3e:
 	MASKABLE_EXCEPTION(0x500, BOOKE_INTERRUPT_EXTERNAL,
 			   external_input, .do_IRQ, ACK_NONE)
 
+	MASKABLE_EXCEPTION(0x510, BOOKE_INTERRUPT_EXTERNAL,
+			   external_input_edge, .do_IRQ, ACK_NONE)
+
 /* Alignment */
 	START_EXCEPTION(alignment);
 	NORMAL_EXCEPTION_PROLOG(0x600, BOOKE_INTERRUPT_ALIGNMENT,
@@ -685,13 +688,25 @@ kernel_dbg_exc:
 	ori	r10,r10,\paca_irq
 	stb	r10,PACAIRQHAPPENED(r13)
 
+	.if \paca_irq == PACA_IRQ_EE_EDGE
+	lwz	r10,PACASAVEDEPR(r13)
+	cmpwi	r10,~0
+	bne-	98f
+	mfspr	r10,SPRN_EPR
+	stw	r10,PACASAVEDEPR(r13)
+	b	99f
+98:	lbz	r10,PACAIRQHAPPENED(r13)
+	ori	r10,r10,PACA_IRQ_EE
+	stb	r10,PACAIRQHAPPENED(r13)
+	.endif
+
 	.if \full_mask == 1
 	rldicl	r10,r11,48,1		/* clear MSR_EE */
 	rotldi	r11,r10,16
 	mtspr	SPRN_SRR1,r11
 	.endif
 
-	lwz	r11,PACA_EXGEN+EX_CR(r13)
+99:	lwz	r11,PACA_EXGEN+EX_CR(r13)
 	mtcr	r11
 	ld	r10,PACA_EXGEN+EX_R10(r13)
 	ld	r11,PACA_EXGEN+EX_R11(r13)
@@ -701,9 +716,11 @@ kernel_dbg_exc:
 .endm
 
 masked_interrupt_book3e_0x500:
-	// XXX When adding support for EPR, use PACA_IRQ_EE_EDGE
 	masked_interrupt_book3e PACA_IRQ_EE 1
 
+masked_interrupt_book3e_0x510:
+	masked_interrupt_book3e PACA_IRQ_EE_EDGE 1
+
 masked_interrupt_book3e_0x900:
 	ACK_DEC(r10);
 	masked_interrupt_book3e PACA_IRQ_DEC 0
@@ -718,7 +735,7 @@ masked_interrupt_book3e_0x2c0:
 
 /*
  * Called from arch_local_irq_enable when an interrupt needs
- * to be resent. r3 contains either 0x500,0x900,0x260 or 0x280
+ * to be resent. r3 contains either 0x500,0x900,0x510 or 0x280
  * to indicate the kind of interrupt. MSR:EE is already off.
  * We generate a stackframe like if a real interrupt had happened.
  *
@@ -745,6 +762,8 @@ _GLOBAL(__replay_interrupt)
 	beq	exc_0x900_common
 	cmpwi	cr0,r3,0x280
 	beq	exc_0x280_common
+	cmpwi	cr0,r3,0x510
+	beq	exc_0x510_common
 	blr
 
 
@@ -859,6 +878,7 @@ BAD_STACK_TRAMPOLINE(0x310)
 BAD_STACK_TRAMPOLINE(0x320)
 BAD_STACK_TRAMPOLINE(0x400)
 BAD_STACK_TRAMPOLINE(0x500)
+BAD_STACK_TRAMPOLINE(0x510)
 BAD_STACK_TRAMPOLINE(0x600)
 BAD_STACK_TRAMPOLINE(0x700)
 BAD_STACK_TRAMPOLINE(0x800)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 5cbcf4d..9349db9 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -165,11 +165,6 @@ notrace unsigned int __check_irq_replay(void)
 	if (decrementer_check_overflow())
 		return 0x900;
 
-	/* Finally check if an external interrupt happened */
-	local_paca->irq_happened &= ~PACA_IRQ_EE;
-	if (happened & PACA_IRQ_EE)
-		return 0x500;
-
 #ifdef CONFIG_PPC_BOOK3E
 	/* Finally check if an EPR external interrupt happened
 	 * this bit is typically set if we need to handle another
@@ -177,7 +172,7 @@ notrace unsigned int __check_irq_replay(void)
 	 */
 	local_paca->irq_happened &= ~PACA_IRQ_EE_EDGE;
 	if (happened & PACA_IRQ_EE_EDGE)
-		return 0x500;
+		return 0x510;
 
 	local_paca->irq_happened &= ~PACA_IRQ_DBELL;
 	if (happened & PACA_IRQ_DBELL)
@@ -191,6 +186,11 @@ notrace unsigned int __check_irq_replay(void)
 	}
 #endif /* CONFIG_PPC_BOOK3E */
 
+	/* Finally check if an external interrupt happened */
+	local_paca->irq_happened &= ~PACA_IRQ_EE;
+	if (happened & PACA_IRQ_EE)
+		return 0x500;
+
 	/* There should be nothing left ! */
 	BUG_ON(local_paca->irq_happened != 0);
 
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index f8f2468..ba370e9 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -144,6 +144,9 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
 #ifdef CONFIG_PPC_STD_MMU_64
 	new_paca->slb_shadow_ptr = &slb_shadow[cpu];
 #endif /* CONFIG_PPC_STD_MMU_64 */
+#ifdef CONFIG_PPC_BOOK3E
+	new_paca->saved_epr = ~0;
+#endif
 }
 
 /* Put the paca pointer into r13 and SPRG_PACA */
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index ee21b5e..4d248d3 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1804,7 +1804,15 @@ unsigned int mpic_get_coreint_irq(void)
 
 	BUG_ON(mpic == NULL);
 
+#ifdef CONFIG_PPC64
+	if (local_paca->saved_epr != ~0) {
+		src = local_paca->saved_epr;
+		local_paca->saved_epr = ~0;
+	} else
+		src = mfspr(SPRN_EPR);
+#else
 	src = mfspr(SPRN_EPR);
+#endif
 
 	if (unlikely(src == mpic->spurious_vec)) {
 		if (mpic->flags & MPIC_SPV_EOI)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] powerpc: move the patch_exception to a common place
  2013-05-11 23:26 [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e Kevin Hao
  2013-05-11 23:26 ` [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel Kevin Hao
@ 2013-05-11 23:26 ` Kevin Hao
  2013-05-11 23:26 ` [PATCH 3/4] powerpc: use patch_exception to update the debug exception handler Kevin Hao
  2013-05-11 23:26 ` [PATCH 4/4] powerpc/fsl-book3e: enable the external_input_edge " Kevin Hao
  3 siblings, 0 replies; 9+ messages in thread
From: Kevin Hao @ 2013-05-11 23:26 UTC (permalink / raw)
  To: Kumar Gala, Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc

So that it can be used by other codes. No function change.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/code-patching.h |  7 +++++++
 arch/powerpc/lib/code-patching.c         | 15 +++++++++++++++
 arch/powerpc/mm/tlb_nohash.c             | 19 -------------------
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index a6f8c7a..97e02f9 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -34,6 +34,13 @@ int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
 unsigned long branch_target(const unsigned int *instr);
 unsigned int translate_branch(const unsigned int *dest,
 			      const unsigned int *src);
+#ifdef CONFIG_PPC_BOOK3E_64
+void __patch_exception(int exc, unsigned long addr);
+#define patch_exception(exc, name) do { \
+	extern unsigned int name; \
+	__patch_exception((exc), (unsigned long)&name); \
+} while (0)
+#endif
 
 static inline unsigned long ppc_function_entry(void *func)
 {
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e5b23..d5edbeb 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -159,6 +159,21 @@ unsigned int translate_branch(const unsigned int *dest, const unsigned int *src)
 	return 0;
 }
 
+#ifdef CONFIG_PPC_BOOK3E_64
+void __patch_exception(int exc, unsigned long addr)
+{
+	extern unsigned int interrupt_base_book3e;
+	unsigned int *ibase = &interrupt_base_book3e;
+
+	/* Our exceptions vectors start with a NOP and -then- a branch
+	 * to deal with single stepping from userspace which stops on
+	 * the second instruction. Thus we need to patch the second
+	 * instruction of the exception, not the first one
+	 */
+
+	patch_branch(ibase + (exc / 4) + 1, addr, 0);
+}
+#endif
 
 #ifdef CONFIG_CODE_PATCHING_SELFTEST
 
diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c
index 6888cad..0658aad 100644
--- a/arch/powerpc/mm/tlb_nohash.c
+++ b/arch/powerpc/mm/tlb_nohash.c
@@ -518,25 +518,6 @@ static void setup_page_sizes(void)
 	}
 }
 
-static void __patch_exception(int exc, unsigned long addr)
-{
-	extern unsigned int interrupt_base_book3e;
- 	unsigned int *ibase = &interrupt_base_book3e;
- 
-	/* Our exceptions vectors start with a NOP and -then- a branch
-	 * to deal with single stepping from userspace which stops on
-	 * the second instruction. Thus we need to patch the second
-	 * instruction of the exception, not the first one
-	 */
-
-	patch_branch(ibase + (exc / 4) + 1, addr, 0);
-}
-
-#define patch_exception(exc, name) do { \
-	extern unsigned int name; \
-	__patch_exception((exc), (unsigned long)&name); \
-} while (0)
-
 static void setup_mmu_htw(void)
 {
 	/* Check if HW tablewalk is present, and if yes, enable it by:
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] powerpc: use patch_exception to update the debug exception handler
  2013-05-11 23:26 [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e Kevin Hao
  2013-05-11 23:26 ` [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel Kevin Hao
  2013-05-11 23:26 ` [PATCH 2/4] powerpc: move the patch_exception to a common place Kevin Hao
@ 2013-05-11 23:26 ` Kevin Hao
  2013-05-11 23:26 ` [PATCH 4/4] powerpc/fsl-book3e: enable the external_input_edge " Kevin Hao
  3 siblings, 0 replies; 9+ messages in thread
From: Kevin Hao @ 2013-05-11 23:26 UTC (permalink / raw)
  To: Kumar Gala, Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/kernel/setup_64.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e379d3f..59dffd0 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -505,9 +505,6 @@ static void __init irqstack_early_init(void)
 #ifdef CONFIG_PPC_BOOK3E
 static void __init exc_lvl_early_init(void)
 {
-	extern unsigned int interrupt_base_book3e;
-	extern unsigned int exc_debug_debug_book3e;
-
 	unsigned int i;
 
 	for_each_possible_cpu(i) {
@@ -520,8 +517,7 @@ static void __init exc_lvl_early_init(void)
 	}
 
 	if (cpu_has_feature(CPU_FTR_DEBUG_LVL_EXC))
-		patch_branch(&interrupt_base_book3e + (0x040 / 4) + 1,
-			     (unsigned long)&exc_debug_debug_book3e, 0);
+		patch_exception(0x040, exc_debug_debug_book3e);
 }
 #else
 #define exc_lvl_early_init()
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] powerpc/fsl-book3e: enable the external_input_edge exception handler
  2013-05-11 23:26 [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e Kevin Hao
                   ` (2 preceding siblings ...)
  2013-05-11 23:26 ` [PATCH 3/4] powerpc: use patch_exception to update the debug exception handler Kevin Hao
@ 2013-05-11 23:26 ` Kevin Hao
  3 siblings, 0 replies; 9+ messages in thread
From: Kevin Hao @ 2013-05-11 23:26 UTC (permalink / raw)
  To: Kumar Gala, Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc

Enable the external_input_edge exception handler for the fsl ppc64
board when external proxy is enabled. This will leave the irq still
hard enabled when a interrupt occurs with irq soft disabled.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/platforms/85xx/corenet_ds.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c
index c59c617..ad688bf 100644
--- a/arch/powerpc/platforms/85xx/corenet_ds.c
+++ b/arch/powerpc/platforms/85xx/corenet_ds.c
@@ -25,6 +25,9 @@
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <asm/mpic.h>
+#ifdef CONFIG_PPC64
+#include <asm/code-patching.h>
+#endif
 
 #include <linux/of_platform.h>
 #include <sysdev/fsl_soc.h>
@@ -37,8 +40,12 @@ void __init corenet_ds_pic_init(void)
 	unsigned int flags = MPIC_BIG_ENDIAN | MPIC_SINGLE_DEST_CPU |
 		MPIC_NO_RESET;
 
-	if (ppc_md.get_irq == mpic_get_coreint_irq)
+	if (ppc_md.get_irq == mpic_get_coreint_irq) {
 		flags |= MPIC_ENABLE_COREINT;
+#ifdef CONFIG_PPC64
+		patch_exception(0x0a0, exc_external_input_edge_book3e);
+#endif
+	}
 
 	mpic = mpic_alloc(NULL, 0, flags, 0, 512, " OpenPIC  ");
 	BUG_ON(mpic == NULL);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel
  2013-05-11 23:26 ` [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel Kevin Hao
@ 2013-05-13 15:47   ` Scott Wood
  2013-05-14  2:03     ` Kevin Hao
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Wood @ 2013-05-13 15:47 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc

On 05/11/2013 06:26:21 PM, Kevin Hao wrote:
> In the external proxy facility mode, the interrupt is automatically
> acknowledged with the same effect as reading the IACK register. So
> this makes external input interrupt more like edge sensitive. That
> means we can leave the irq hard enabled when it occurs with irq soft
> disabled just like the dec and doorbell interrupt. But the External
> Proxy Register(EPR) is only considered valid from the time that the
> external interrupt occurs until MSR[EE] is set to 1. So we have to
> save the EPR before irq hard enabled.

Is it really worth it?  Are you having a real-world problem with =20
profilability as things stand, that this resolves?  We should already =20
be no worse off than non-external-proxy hardware...

-Scott=

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel
  2013-05-13 15:47   ` Scott Wood
@ 2013-05-14  2:03     ` Kevin Hao
  2013-05-15 21:30       ` Scott Wood
  0 siblings, 1 reply; 9+ messages in thread
From: Kevin Hao @ 2013-05-14  2:03 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On Mon, May 13, 2013 at 10:47:17AM -0500, Scott Wood wrote:
> On 05/11/2013 06:26:21 PM, Kevin Hao wrote:
> >In the external proxy facility mode, the interrupt is automatically
> >acknowledged with the same effect as reading the IACK register. So
> >this makes external input interrupt more like edge sensitive. That
> >means we can leave the irq hard enabled when it occurs with irq soft
> >disabled just like the dec and doorbell interrupt. But the External
> >Proxy Register(EPR) is only considered valid from the time that the
> >external interrupt occurs until MSR[EE] is set to 1. So we have to
> >save the EPR before irq hard enabled.
> 
> Is it really worth it?

Maybe. :-)
Compare with the current kernel:
  * The overhead is that we need additional load & store the contents of
    the EPR from/to PACA.
  * The bonus is we keep the irq hard enabled when a external interrupt occurs
    with irq soft-disabled. As I know we should leave the irq hard enabled as
    much as possible. This is also the primary reason that we introduce the
    Lazy EE.

>  Are you having a real-world problem with
> profilability as things stand, that this resolves?

No. 

>  We should
> already be no worse off than non-external-proxy hardware...

Yes, I agree. But we can leverage the advantage of external proxy and
make it even more better.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel
  2013-05-14  2:03     ` Kevin Hao
@ 2013-05-15 21:30       ` Scott Wood
  2013-05-16  8:43         ` Kevin Hao
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Wood @ 2013-05-15 21:30 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc

On 05/13/2013 09:03:17 PM, Kevin Hao wrote:
> On Mon, May 13, 2013 at 10:47:17AM -0500, Scott Wood wrote:
> > On 05/11/2013 06:26:21 PM, Kevin Hao wrote:
> > >In the external proxy facility mode, the interrupt is automatically
> > >acknowledged with the same effect as reading the IACK register. So
> > >this makes external input interrupt more like edge sensitive. That
> > >means we can leave the irq hard enabled when it occurs with irq =20
> soft
> > >disabled just like the dec and doorbell interrupt. But the External
> > >Proxy Register(EPR) is only considered valid from the time that the
> > >external interrupt occurs until MSR[EE] is set to 1. So we have to
> > >save the EPR before irq hard enabled.
> >
> > Is it really worth it?
>=20
> Maybe. :-)
> Compare with the current kernel:
>   * The overhead is that we need additional load & store the contents =20
> of
>     the EPR from/to PACA.

There's also mental overhead of the extra complexity.  The lazy EE =20
stuff is already fiddly enough (e.g. the recent KVM patches).

>   * The bonus is we keep the irq hard enabled when a external =20
> interrupt occurs
>     with irq soft-disabled. As I know we should leave the irq hard =20
> enabled as
>     much as possible. This is also the primary reason that we =20
> introduce the
>     Lazy EE.

I don't think "as much as possible" is a good way to look at it, so =20
much as "as much as is practical", balanced by also wanting to keep the =20
code as simple as is practical.

-Scott=

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel
  2013-05-15 21:30       ` Scott Wood
@ 2013-05-16  8:43         ` Kevin Hao
  0 siblings, 0 replies; 9+ messages in thread
From: Kevin Hao @ 2013-05-16  8:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

On Wed, May 15, 2013 at 04:30:26PM -0500, Scott Wood wrote:
> On 05/13/2013 09:03:17 PM, Kevin Hao wrote:
> >On Mon, May 13, 2013 at 10:47:17AM -0500, Scott Wood wrote:
> >> On 05/11/2013 06:26:21 PM, Kevin Hao wrote:
> >> >In the external proxy facility mode, the interrupt is automatically
> >> >acknowledged with the same effect as reading the IACK register. So
> >> >this makes external input interrupt more like edge sensitive. That
> >> >means we can leave the irq hard enabled when it occurs with irq
> >soft
> >> >disabled just like the dec and doorbell interrupt. But the External
> >> >Proxy Register(EPR) is only considered valid from the time that the
> >> >external interrupt occurs until MSR[EE] is set to 1. So we have to
> >> >save the EPR before irq hard enabled.
> >>
> >> Is it really worth it?
> >
> >Maybe. :-)
> >Compare with the current kernel:
> >  * The overhead is that we need additional load & store the
> >contents of
> >    the EPR from/to PACA.
> 
> There's also mental overhead of the extra complexity.

Yes, I agree. But since we already have the support for the edge sensitive
interrupt such as doorbell, decrementer, adding another one doesn't really
introduce much code complexity in my opinion.

>  The lazy EE
> stuff is already fiddly enough (e.g. the recent KVM patches).

:-)

> 
> >  * The bonus is we keep the irq hard enabled when a external
> >interrupt occurs
> >    with irq soft-disabled. As I know we should leave the irq hard
> >enabled as
> >    much as possible. This is also the primary reason that we
> >introduce the
> >    Lazy EE.
> 
> I don't think "as much as possible" is a good way to look at it, so
> much as "as much as is practical", balanced by also wanting to keep
> the code as simple as is practical.

Yes, I also like simple. That is why I make the following patch first.
http://patchwork.ozlabs.org/patch/235530/

But it seems that Ben doesn't like it. And it also seem not so difficulty
to support the external interrupt as edge sensitive for external proxy,
so I scratch these patches. It seems that you and Ben have different
view about this issue. Anyway I have no strong preference for these two
ways and will leave it to you guys to determine which way we like to adopt.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-05-16  8:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-11 23:26 [PATCH 0/4] enable the PACA_IRQ_EE_EDGE support for book3e Kevin Hao
2013-05-11 23:26 ` [PATCH 1/4] powerpc/book3e: introduce external_input_edge exception handler for 64bit kernel Kevin Hao
2013-05-13 15:47   ` Scott Wood
2013-05-14  2:03     ` Kevin Hao
2013-05-15 21:30       ` Scott Wood
2013-05-16  8:43         ` Kevin Hao
2013-05-11 23:26 ` [PATCH 2/4] powerpc: move the patch_exception to a common place Kevin Hao
2013-05-11 23:26 ` [PATCH 3/4] powerpc: use patch_exception to update the debug exception handler Kevin Hao
2013-05-11 23:26 ` [PATCH 4/4] powerpc/fsl-book3e: enable the external_input_edge " Kevin Hao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).