Re: Help: NVIC, level-triggered interrupts and interrupt pending

[Igor Kotrasiński]

On 30.05.2022 14:02, Igor Kotrasiński wrote:
> 
> 
> On 30.05.2022 13:54, Peter Maydell wrote:
>> On Mon, 30 May 2022 at 12:34, Igor Kotrasiński
>> <i.kotrasinsk@samsung.com> wrote:
>>>
>>> On 30.05.2022 12:49, Peter Maydell wrote:
>>>> On Mon, 30 May 2022 at 10:33, Igor Kotrasiński
>>>> <i.kotrasinsk@samsung.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I've been hacking on QEMU recently, adding support for a custom
>>>>> Cortex-M
>>>>> board. One of the devices that I'm emulating is using level-triggered
>>>>> interrupts, for which I assume qemu_irq_raise/lower is the right tool.
>>>>> However, I'm having trouble receiving interrupts for my device which I
>>>>> *think* are caused by interaction with setting/clearing
>>>>> interrupt-pending status.
>>>>>
>>>>> The way I'm handling my interrupt currently is as follows:
>>>>>
>>>>> 1. The device raises an interrupt with qemu_irq_raise.
>>>>> 2. In interrupt handler, I set the clear-enable nvic register and
>>>>> queue
>>>>> a handler.
>>>>> 3. In handler, I read data from the device and set a register that
>>>>> lowers its interrupt with qemu_irq_lower.
>>>>> 4. Then I set the clear-pending nvic register.
>>>>> 5. Finally I set the set-enable nvic register.
>>>>
>>>> You don't say in this sequence where "return from the interrupt
>>>> exception handler" happens, which is an important part of the
>>>> sequence. (That is, I'm not sure whether your design has only
>>>> step 2 in the CPU exception handler with the "queued" handler
>>>> being run after exception-return, or whether all of 2-5 are
>>>> in the CPU exception handler for the interrupt.)
>>>>
>>>
>>> Sorry, I lied a bit here. I'm running someone else's code and I misread
>>> it a bit. The exact sequence here is:
>>>
>>> 1. The device raises the interrupt.
>>> 2. Interrupt handler sets the set-pending register, then the
>>> clear-enable register, then returns.
>>> 3. A handler running outside exception context reads data from the
>>> device and lowers the irq.
>>> 3.5. The device raises the irq again.
>>> 4. The handler sets the clear-pending register, then the set-enable
>>> register.
>>
>> Does this code work on real hardware?
>>
>> Do you have a test case image you could provide?
>>
>> thanks
>> -- PMM
>>
> 
> The code works on our board which uses a Cortex-M35P, unfortunately I
> don't have a test case image I can share. I'll try to prepare a minimal
> example that runs on mps2-an505 or such.
> 
> Igor

Hi,

I don't have a test image yet, but I'm pretty confident I understand the
bug now.

As you said, an interrupt becomes pending if the input IRQ line is high
and the interrupt is not Active. The first condition in R_CVJS doesn't
mention any transitions, so the "event" mentioned there happens at any
time the condition is valid. So, no matter what writing to pending-clear
register does, pending status should still be set afterwards. This would
also match the explanatory document I found.

This diff would be a matching fix:

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 773e311754..bfd0fafe0a 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2393,6 +2393,14 @@ static MemTxResult nvic_sysreg_write(void
*opaque, hwaddr addr,
         for (i = 0, end = size * 8; i < end && startvec + i <
s->num_irq; i++) {
             if (value & (1 << i) &&
                 (attrs.secure || s->itns[startvec + i])) {
+                /* Stay pending if line is up and interrupt is not active
+                 * (R_CVJS)
+                 */
+                if (!setval &&
+                    !s->vectors[i + NVIC_FIRST_IRQ].active &&
+                    s->vectors[i + NVIC_FIRST_IRQ].level) {
+                    continue;
+                }
                 s->vectors[startvec + i].pending = setval;
             }
         }

Igor