Re: [RFC PATCH 5/9] powerpc: reverse the soft_enable logic

On Tuesday 26 July 2016 11:01 AM, Nicholas Piggin wrote:

On Mon, 25 Jul 2016 20:22:18 +0530
Madhavan Srinivasan [off-list ref] wrote:

quoted

"paca->soft_enabled" is used as a flag to mask some of interrupts.
Currently supported flags values and their details:

soft_enabled		MSR[EE]

0			0	Disabled (PMI and HMI not masked)
1			1	Enabled

"paca->soft_enabled" is initialed to 1 to make the interripts as
enabled. arch_local_irq_disable() will toggle the value when
interrupts needs to disbled. At this point, the interrupts are not
actually disabled, instead, interrupt vector has code to check for
the flag and mask it when it occurs. By "mask it", it updated
interrupt paca->irq_happened and return. arch_local_irq_restore() is
called to re-enable interrupts, which checks and replays interrupts
if any occured.

Now, as mentioned, current logic doesnot mask "performance monitoring
interrupts" and PMIs are implemented as NMI. But this patchset
depends on local_irq_* for a successful local_* update. Meaning, mask
all possible interrupts during local_* update and replay them after
the update.

So the idea here is to reserve the "paca->soft_enabled" logic. New
values and details:

soft_enabled		MSR[EE]

1			0	Disabled  (PMI and HMI not masked)
0			1	Enabled

Reason for the this change is to create foundation for a third flag
value "2" for "soft_enabled" to add support to mask PMIs. When
arch_irq_disable_* is called with a value "2", PMI interrupts are
mask. But when called with a value of "1", PMI are not mask.

With new flag value for "soft_enabled", states looks like:

soft_enabled		MSR[EE]

2			0	Disbaled PMIs also
1			0	Disabled  (PMI and HMI not masked)
0			1	Enabled

And interrupt handler code for checking has been modified to check for
for "greater than or equal" to 1 condition instead.

This bit of the patch seems to have been moved into other part
of the series. Ideally (unless there is a good reason), it is nice
to have each individual patch result in a working kernel before
and after.

Agreed. But I need to reason out the change and hence add
all info here. But will edit the info in the next version.

Maddy

Nice way to avoid adding more branches though.

Thanks,
Nick