Re: [PATCH] ArmPkg: Fix double GIC EIOR write per interrupt

["Cohen, Eugene" <eugene@hp.com>]

> My description was not very clear, and the point is academic if you are
> happy with the solution.

I think it's important that we're aligned on how the GIC works so thanks for humoring me.

> However:
> The GIC spec has a state machine diagram (Figure 4.3), where:
> Transition D, pending to active and pending
> This transition occurs on acknowledgement of the interrupt by the PE
> for level-sensitive SPIs, SGIs,
> and PPIs.
> Transition B1 or B2, remove pending state
> This transition occurs when the interrupt has been deasserted by the
> peripheral, if the interrupt is a
> level-sensitive interrupt, or when software has changed the pending
> state.
> Transition E1 or E2, remove active state
> This transition occurs when software deactivates an interrupt for SPIs,
> SGIs, and PPIs.
> 
> I suspect that because we EOI ("deactivate" for E1/E2) without
> "deasserting" the peripheral interrupt then the GIC may restore the
> pending state (transition E2 instead of B2 then E1), which will look
> remarkably like a latch.

But no latching will occur because, for a level sensitive interrupt, the EOI-before deassert should manifest as transition E2 (caused by EOI) followed by transition B1 (caused by clearing the source).  This is per the text that transition B1 occurs if "the level-sensitive interrupt is pending only because of the assertion of an input signal, and that signal is deasserted".  So the transition is Active+Pending -> Pending -> Inactive for this odd ordering versus the more sensible Active+Pending -> Active -> Inactive.

> That looks like a pretty sensible way forward.  I'll vote for EFI_SUCCESS,
> white lies are sometimes needed.

Okay, what's the worst that can happen? :)

Perhaps the real test would be that a driver that uses HwInterrupt is shown to work equally well on a fake-EOI interface system as well as a real-EOI interface system.  I doubt if we have any common peripherals (timer block or serial port or whatever) to really test this.

Eugene