On 18/09/17 21:39, David Gibson wrote:
> On Sun, Sep 17, 2017 at 06:15:48PM +0100, Mark Cave-Ayland wrote:
>> From: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>>
>> Some interrupts get triggered before the OS has setup the
>> right interrupt type. If an edge interrupt is latched that
>> way, not delivered (still masked), then the interrupt is
>> changed to level and isn't asserted anymore, it will be
>> stuck "pending", causing an interrupt flood. This can happen
>> with the PMU GPIO interrupt for example.
>>
>> There are a few other corner cases like this, so let's keep
>> track of the input "level" so we can properly re-evaluate
>> when the trigger type changes.
>>
>> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>> ---
>> hw/intc/openpic.c | 32 +++++++++++++++++++++++++++-----
>> 1 file changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
>> index debfcbf..34749f8 100644
>> --- a/hw/intc/openpic.c
>> +++ b/hw/intc/openpic.c
>> @@ -236,6 +236,7 @@ typedef struct IRQSource {
>> int last_cpu;
>> int output; /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
>> int pending; /* TRUE if IRQ is pending */
>> + int cur_level; /* Cache current level */
>> IRQType type;
>> bool level:1; /* level-triggered */
>> bool nomask:1; /* critical interrupts ignore mask on some FSL MPICs */
>> @@ -552,14 +553,26 @@ static void openpic_set_irq(void *opaque, int n_IRQ,
>> int level)
>> }
>>
>> src = &opp->src[n_IRQ];
>> - DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
>> - n_IRQ, level, src->ivpr);
>> + DPRINTF("openpic: set irq %d = %d ivpr=0x%08x (l=%d,cl=%d)\n",
>> + n_IRQ, level, src->ivpr, src->level, src->cur_level);
>> +
>> + /* Keep track of the current input level in order to properly deal
>> + * with the configuration of the source changing from edge to level
>> + * after it's been latched. Otherwise the interrupt can get stuck.
>> + */
>> + src->cur_level = level;
>> +
>> if (src->level) {
>> - /* level-sensitive irq */
>> src->pending = level;
>> openpic_update_irq(opp, n_IRQ);
>> } else {
>> - /* edge-sensitive irq */
>> + /* edge-sensitive irq
>> + *
>> + * In an ideal world we would only set pending on an "edge", ie
>> + * if level is set and src->cur_level as clear. However that would
>> + * require all the devices to properly send "pulses" rather than
>> + * just "raise" which isn't the case today.
>> + */
>> if (level) {
>> src->pending = 1;
>> openpic_update_irq(opp, n_IRQ);
>> @@ -676,6 +689,13 @@ static inline void write_IRQreg_ivpr(OpenPICState *opp,
>> int n_IRQ, uint32_t val)
>> switch (opp->src[n_IRQ].type) {
>> case IRQ_TYPE_NORMAL:
>> opp->src[n_IRQ].level = !!(opp->src[n_IRQ].ivpr & IVPR_SENSE_MASK);
>> +
>> + /* If we switched to level we need to re-evaluate "pending" based
>> + * on the actual input state.
>> + */
>> + if (opp->src[n_IRQ].level) {
>> + opp->src[n_IRQ].pending = opp->src[n_IRQ].cur_level;
>> + }
>> break;
>>
>> case IRQ_TYPE_FSLINT:
>> @@ -687,6 +707,7 @@ static inline void write_IRQreg_ivpr(OpenPICState *opp,
>> int n_IRQ, uint32_t val)
>> break;
>> }
>>
>> + /* Re-evaluate a level irq */
>> openpic_update_irq(opp, n_IRQ);
>> DPRINTF("Set IVPR %d to 0x%08x -> 0x%08x\n", n_IRQ, val,
>> opp->src[n_IRQ].ivpr);
>> @@ -1232,7 +1253,7 @@ static uint32_t openpic_iack(OpenPICState *opp,
>> IRQDest *dst, int cpu)
>> }
>>
>> if (!src->level) {
>> - /* edge-sensitive IRQ */
>> + /* edge-sensitive IRQ or level dropped */
>> src->ivpr &= ~IVPR_ACTIVITY_MASK;
>> src->pending = 0;
>> IRQ_resetbit(&dst->raised, irq);
>> @@ -1564,6 +1585,7 @@ static const VMStateDescription
>> vmstate_openpic_irqsource = {
>> VMSTATE_UINT32(destmask, IRQSource),
>> VMSTATE_INT32(last_cpu, IRQSource),
>> VMSTATE_INT32(pending, IRQSource),
>> + VMSTATE_INT32(cur_level, IRQSource),
>> VMSTATE_END_OF_LIST()
>
> This alters the migration stream without bumping the version number.
> I suspect it will be best to move this hunk into your other patch
> updating the migration of the openpic.
Yes, that's fine. I'll wait for your reply to the previous openpic patch
before resubmitting though.
ATB,
Mark.
