Hi folks,
We can see that in a single timer interrupt, there will be two writes to EOIR, one in timer interrupt handler, the other in GIC IRQ handler.
The question is: does two writes to EOIR have any side effect? From GIC specification (ARM IHI 0069B (ID121715)), we can see below text about writing EOIR (page 205):
A write to this register must correspond to the most recent valid read from an Interrupt Acknowledge Register for which there has not been a priority drop and that this identifier was read from ICC_IAR1_EL1 while operating in the same Security state as that in which the write occurs, otherwise the system behavior is UNPREDICTABLE. A valid read is a read that returns a valid interrupt ID, that is not a special INTID.
For GICv2, we can also see below text about EOIR:
For nested interrupts, the order of writes to this register must be the reverse of the order of interrupt acknowledgement. Behavior is UNPREDICTABLE if: • This ordering constraint is not maintained. *• The value written to this register does not match an active interrupt, or the ID of a spurious interrupt. * • The value written to this register does not match the last valid interrupt value read from GICC_IAR.
When we issue the 1st EOIR, the interrupt will be deactivated, so it is not in active state anymore for the 2nd write of EOIR.
Could anyone help to confirm whether it is OK to keep the 2nd write of EOIR? And how to explain the above paragraphs from GIC specification?
Thanks and regards.
Heyi
On Thu, Jul 28, 2016 at 08:43:46AM +0800, Heyi Guo wrote:
Hi folks,
We can see that in a single timer interrupt, there will be two writes to EOIR, one in timer interrupt handler, the other in GIC IRQ handler.
Without looking at the code: that sounds like a bug.
The question is: does two writes to EOIR have any side effect? From GIC specification (ARM IHI 0069B (ID121715)), we can see below text about writing EOIR (page 205):
A write to this register must correspond to the most recent valid read from an Interrupt Acknowledge Register for which there has not been a priority drop and that this identifier was read from ICC_IAR1_EL1 while operating in the same Security state as that in which the write occurs, otherwise the system behavior is UNPREDICTABLE. A valid read is a read that returns a valid interrupt ID, that is not a special INTID.
For GICv2, we can also see below text about EOIR:
For nested interrupts, the order of writes to this register must be the reverse of the order of interrupt acknowledgement. Behavior is UNPREDICTABLE if: • This ordering constraint is not maintained. *• The value written to this register does not match an active interrupt, or the ID of a spurious interrupt. * • The value written to this register does not match the last valid interrupt value read from GICC_IAR.
When we issue the 1st EOIR, the interrupt will be deactivated, so it is not in active state anymore for the 2nd write of EOIR.
Could anyone help to confirm whether it is OK to keep the 2nd write of EOIR? And how to explain the above paragraphs from GIC specification?
I don't think keeping the 2nd write to EOIR is OK. In the best possible scenario, there remains a race condition in which this could cause interrupts to be dropped.
Regards,
Leif
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler (move it to unregistered interrupt branch only) and keep the one in timer interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Regards.
Heyi
在 7/28/2016 10:19 PM, Leif Lindholm 写道:
On Thu, Jul 28, 2016 at 08:43:46AM +0800, Heyi Guo wrote:
Hi folks,
We can see that in a single timer interrupt, there will be two writes to EOIR, one in timer interrupt handler, the other in GIC IRQ handler.
Without looking at the code: that sounds like a bug.
The question is: does two writes to EOIR have any side effect? From GIC specification (ARM IHI 0069B (ID121715)), we can see below text about writing EOIR (page 205):
A write to this register must correspond to the most recent valid read from an Interrupt Acknowledge Register for which there has not been a priority drop and that this identifier was read from ICC_IAR1_EL1 while operating in the same Security state as that in which the write occurs, otherwise the system behavior is UNPREDICTABLE. A valid read is a read that returns a valid interrupt ID, that is not a special INTID.
For GICv2, we can also see below text about EOIR:
For nested interrupts, the order of writes to this register must be the reverse of the order of interrupt acknowledgement. Behavior is UNPREDICTABLE if: • This ordering constraint is not maintained. *• The value written to this register does not match an active interrupt, or the ID of a spurious interrupt. * • The value written to this register does not match the last valid interrupt value read from GICC_IAR.
When we issue the 1st EOIR, the interrupt will be deactivated, so it is not in active state anymore for the 2nd write of EOIR.
Could anyone help to confirm whether it is OK to keep the 2nd write of EOIR? And how to explain the above paragraphs from GIC specification?
I don't think keeping the 2nd write to EOIR is OK. In the best possible scenario, there remains a race condition in which this could cause interrupts to be dropped.
Regards,
Leif
On 29 July 2016 at 03:23, Heyi Guo heyi.guo@linaro.org wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler (move it to unregistered interrupt branch only) and keep the one in timer interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
On 29 July 2016 at 09:09, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 29 July 2016 at 03:23, Heyi Guo heyi.guo@linaro.org wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler (move it to unregistered interrupt branch only) and keep the one in timer interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts.
In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though.
Hi Leif and Ard,
There might be another issue in timer interrupt handler. Timer interrupt seems to be level triggered, so is it OK to write EOIR before clearing the interrupt source, i.e. updating compare value register?
Heyi
On 29 July 2016 at 15:11, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 29 July 2016 at 09:09, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 29 July 2016 at 03:23, Heyi Guo heyi.guo@linaro.org wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ
handler
(move it to unregistered interrupt branch only) and keep the one in
timer
interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts.
In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though.
Hi Heyi,
The write to GICC_EOIR clears corresponding bit in GICD_ISACTIVER0 and the following update of CNTP_CVAL will clear bit in CICD_ISPENDR0 register, so there shouldn’t be any issue with this order. By the way call to ArmGenericTimerEnableTimer() is obsolete because timer is enabled and doesn’t get disabled in TimerInterruptHandler(). Alexei.
From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Heyi Guo Sent: 02 August 2016 02:28 To: Ard Biesheuvel Cc: Linaro UEFI Mailman List Subject: Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt
Hi Leif and Ard,
There might be another issue in timer interrupt handler. Timer interrupt seems to be level triggered, so is it OK to write EOIR before clearing the interrupt source, i.e. updating compare value register?
Heyi
On 29 July 2016 at 15:11, Ard Biesheuvel <ard.biesheuvel@linaro.orgmailto:ard.biesheuvel@linaro.org> wrote: On 29 July 2016 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.orgmailto:ard.biesheuvel@linaro.org> wrote:
On 29 July 2016 at 03:23, Heyi Guo <heyi.guo@linaro.orgmailto:heyi.guo@linaro.org> wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler (move it to unregistered interrupt branch only) and keep the one in timer interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts.
In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Alexei,
Thanks for your explanation. However, when I tested it on D02, it didn't act as expected, i.e. we did see a subsequent interrupt triggered immediately when gBS->RestoreTPL was called in which IRQ is enabled, and I had set timer interrupt period to some fairly large value (e.g. 5 seconds).
So I'd like to confirm, is it declared in GIC specification that clearing interrupt source will also clear pending status in GICD?
Thanks and regards.
Heyi
On 08/03/2016 12:27 AM, Alexei Fedorov wrote:
Hi Heyi,
The write to GICC_EOIR clears corresponding bit in GICD_ISACTIVER0 and the following update of CNTP_CVAL will clear bit in CICD_ISPENDR0 register, so there shouldn’t be any issue with this order.
By the way call to ArmGenericTimerEnableTimer() is obsolete because timer is enabled and doesn’t get disabled in TimerInterruptHandler().
Alexei.
*From:*Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] *On Behalf Of *Heyi Guo *Sent:* 02 August 2016 02:28 *To:* Ard Biesheuvel *Cc:* Linaro UEFI Mailman List *Subject:* Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt
Hi Leif and Ard,
There might be another issue in timer interrupt handler. Timer interrupt seems to be level triggered, so is it OK to write EOIR before clearing the interrupt source, i.e. updating compare value register?
Heyi
On 29 July 2016 at 15:11, Ard Biesheuvel <ard.biesheuvel@linaro.org mailto:ard.biesheuvel@linaro.org> wrote:
On 29 July 2016 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org mailto:ard.biesheuvel@linaro.org> wrote:
On 29 July 2016 at 03:23, Heyi Guo <heyi.guo@linaro.org
mailto:heyi.guo@linaro.org> wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long
duration
handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ
handler
(move it to unregistered interrupt branch only) and keep the one in
timer
interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts.
In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Heyi,
is it declared in GIC specification that clearing interrupt source will also clear pending status in GICD?
No. Pending status will be cleared when timer level interrupt is de-asserted, which according to ARM ARM:
This means that, to deassert the timer output signal, software must do one of the following: • Reprogram the timer registers so that neither of the timer conditions is met. • Mask the timer output signal, in the timer control register. • Disable the timer, in the timer control register.
we did see a subsequent interrupt triggered immediately when gBS->RestoreTPL
Could you print GICD_ISACTIVER0 and CICD_ISPENDR0 values before calling RestoreTPL()?
Regards. Alexei.
From: Heyi Guo [mailto:heyi.guo@linaro.org] Sent: 10 August 2016 14:22 To: Alexei Fedorov; Ard Biesheuvel Cc: Linaro UEFI Mailman List; Evan Lloyd; Sami Mujawar; Mitch Ishihara Subject: Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt
Hi Alexei,
Thanks for your explanation. However, when I tested it on D02, it didn't act as expected, i.e. we did see a subsequent interrupt triggered immediately when gBS->RestoreTPL was called in which IRQ is enabled, and I had set timer interrupt period to some fairly large value (e.g. 5 seconds).
So I'd like to confirm, is it declared in GIC specification that clearing interrupt source will also clear pending status in GICD?
Thanks and regards.
Heyi
On 08/03/2016 12:27 AM, Alexei Fedorov wrote: Hi Heyi,
The write to GICC_EOIR clears corresponding bit in GICD_ISACTIVER0 and the following update of CNTP_CVAL will clear bit in CICD_ISPENDR0 register, so there shouldn’t be any issue with this order. By the way call to ArmGenericTimerEnableTimer() is obsolete because timer is enabled and doesn’t get disabled in TimerInterruptHandler(). Alexei.
From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Heyi Guo Sent: 02 August 2016 02:28 To: Ard Biesheuvel Cc: Linaro UEFI Mailman List Subject: Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt
Hi Leif and Ard,
There might be another issue in timer interrupt handler. Timer interrupt seems to be level triggered, so is it OK to write EOIR before clearing the interrupt source, i.e. updating compare value register?
Heyi
On 29 July 2016 at 15:11, Ard Biesheuvel <ard.biesheuvel@linaro.orgmailto:ard.biesheuvel@linaro.org> wrote: On 29 July 2016 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.orgmailto:ard.biesheuvel@linaro.org> wrote:
On 29 July 2016 at 03:23, Heyi Guo <heyi.guo@linaro.orgmailto:heyi.guo@linaro.org> wrote:
In timer interrupt handler, we can see the comment that "Signal end of interrupt early to help avoid losing subsequent ticks from long duration handlers".
So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler (move it to unregistered interrupt branch only) and keep the one in timer interrupt handler, and declare that EOIR should be taken care by each interrupt handler itself, rather than by GIC driver?
Can we use priority drop instead?
Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts.
In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Alexei,
Yes you are right. Below is the print result:
At the head of timer interrupt handler: ACT = 1, PENDING = 1
After EOIR write:
ACT = 0, PENDING = 1
After timer compare register updating:
ACT = 0, PENDING = 0
So I need to investigate more on why we have a second interrupt even when GICD pending status is cleared.
Thanks for your help.
Heyi
On 08/10/2016 10:56 PM, Alexei Fedorov wrote:
Hi Heyi,
is it declared in GIC specification that clearing interrupt source
will also clear pending status in GICD?
No. Pending status will be cleared when timer level interrupt is de-asserted, which according to ARM ARM:
This means that, to deassert the timer output signal, software must do one of the following:
• Reprogram the timer registers so that neither of the timer conditions is met.
• Mask the timer output signal, in the timer control register.
• Disable the timer, in the timer control register.
we did see a subsequent interrupt triggered immediately when
gBS->RestoreTPL
Could you print GICD_ISACTIVER0 and CICD_ISPENDR0 values before calling RestoreTPL()?
Regards.
Alexei.
*From:*Heyi Guo [mailto:heyi.guo@linaro.org] *Sent:* 10 August 2016 14:22 *To:* Alexei Fedorov; Ard Biesheuvel *Cc:* Linaro UEFI Mailman List; Evan Lloyd; Sami Mujawar; Mitch Ishihara *Subject:* Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt
Hi Alexei,
Thanks for your explanation. However, when I tested it on D02, it didn't act as expected, i.e. we did see a subsequent interrupt triggered immediately when gBS->RestoreTPL was called in which IRQ is enabled, and I had set timer interrupt period to some fairly large value (e.g. 5 seconds).
So I'd like to confirm, is it declared in GIC specification that clearing interrupt source will also clear pending status in GICD?
Thanks and regards.
Heyi
On 08/03/2016 12:27 AM, Alexei Fedorov wrote:
Hi Heyi, The write to GICC_EOIR clears corresponding bit in GICD_ISACTIVER0 and the following update of CNTP_CVAL will clear bit in CICD_ISPENDR0 register, so there shouldn’t be any issue with this order. By the way call to ArmGenericTimerEnableTimer() is obsolete because timer is enabled and doesn’t get disabled in TimerInterruptHandler(). Alexei. *From:*Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] *On Behalf Of *Heyi Guo *Sent:* 02 August 2016 02:28 *To:* Ard Biesheuvel *Cc:* Linaro UEFI Mailman List *Subject:* Re: [Linaro-uefi] [linaro-uefi] Two write to EOIR in a single interrupt Hi Leif and Ard, There might be another issue in timer interrupt handler. Timer interrupt seems to be level triggered, so is it OK to write EOIR before clearing the interrupt source, i.e. updating compare value register? Heyi On 29 July 2016 at 15:11, Ard Biesheuvel <ard.biesheuvel@linaro.org <mailto:ard.biesheuvel@linaro.org>> wrote: On 29 July 2016 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org <mailto:ard.biesheuvel@linaro.org>> wrote: > On 29 July 2016 at 03:23, Heyi Guo <heyi.guo@linaro.org <mailto:heyi.guo@linaro.org>> wrote: >> In timer interrupt handler, we can see the comment that "Signal end of >> interrupt early to help avoid losing subsequent ticks from long duration >> handlers". >> >> So, if we want to fix this, shall we remove EOIR writing in GIC IRQ handler >> (move it to unregistered interrupt branch only) and keep the one in timer >> interrupt handler, and declare that EOIR should be taken care by each >> interrupt handler itself, rather than by GIC driver? >> > > Can we use priority drop instead? Reply to self: probably not. I suppose the whole point is to allow nested timer interrupts. In any case, we need to get rid of the double EOIR write. I would prefer not to change the contract with existing interrupt handlers, though. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-
Alexei Fedorov -
Ard Biesheuvel -
Heyi Guo -
Leif Lindholm