Re: [RFC PATCH v2 08/11] powerpc: Add "mask_lvl" paramater to MASKABLE_* macros
flat view
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2016-08-01 05:21:13
On Mon, 1 Aug 2016 00:36:26 +0530 Madhavan Srinivasan [off-list ref] 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:
quoted hunk ↗ jump to hunk
@@ -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