Re: [RFC PATCH v2 08/11] powerpc: Add "mask_lvl" paramater to MASKABLE_* macros

[Nicholas Piggin <npiggin@gmail.com>]

On Mon,  1 Aug 2016 00:36:26 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:

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

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


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

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.

Thanks,
Nick