On Monday 01 August 2016 10:51 AM, Nicholas Piggin wrote:

On Mon,  1 Aug 2016 00:36:26 +0530
Madhavan Srinivasan [off-list ref] wrote:

quoted

Make it explicit the interrupt masking level supported
by a gievn interrupt handler. Patch correspondingly
extends the MASKABLE_* macros with an addition's parameter.
"mask_lvl" parameter is passed to SOFTEN_TEST macro to decide
on masking the interrupt.

Hey Madhavan,

It looks like this has worked quite nicely. I think you've
managed to avoid any additional instructions in fastpaths
if I'm reading correctly.

Yes. This avoids condition checking for many cases.

I will do a more comprehensive review, but I wanted to ask:

quoted

@@ -426,79 +426,81 @@ label##_relon_hv:						\
  #define SOFTEN_VALUE_0xe60	PACA_IRQ_HMI
  #define SOFTEN_VALUE_0xe62	PACA_IRQ_HMI
  
-#define __SOFTEN_TEST(h, vec)						\
+#define __SOFTEN_TEST(h, vec, mask_lvl)					\
  	lbz	r10,PACASOFTIRQEN(r13);					\
-	cmpwi	r10,IRQ_DISABLE_LEVEL_LINUX;				\
+	andi.	r10,r10,mask_lvl;					\
  	li	r10,SOFTEN_VALUE_##vec;					\
-	bge	masked_##h##interrupt
-#define _SOFTEN_TEST(h, vec)	__SOFTEN_TEST(h, vec)
+	bne	masked_##h##interrupt
+#define _SOFTEN_TEST(h, vec, mask_lvl)	__SOFTEN_TEST(h, vec, mask_lvl)

We're talking about IRQ masking levels, but here it looks
like you're actually treating it as a mask.

Yes. That is true. I started with "level", but then realized
that I am adding more branch condition checks to
retain the PMI as NMI incase.

I don't have a strong preference. Mask is more flexible, but
potentially constrained in how many interrupt types it can
cope with. That said, I doubt we'll need more than 8 mask bits
considering we've lived with one for years. So perhaps a mask
is a better choice. Ben, others, any preferences?

We should just use either "mask" or "level" everywhere, depending
on what we go with.

Yep. will change it.

Maddy

Thanks,
Nick