Re: [Qemu-devel] [PATCH 0/5] arm_gic: Drop running_irq and last_active arrays

[Peter Maydell]

Ping?

thanks
-- PMM

On 28 July 2015 at 14:22, Peter Maydell <address@hidden> wrote:
> This patchset is a bit of cleanup to our GIC implementation that
> I've wanted to do for ages.
>
> Our current GIC code uses a couple of arrays (running_irq and
> last_active) to track currently active interrupts so that
> it can correctly determine the running priority as potentially
> nested interrupts are taken and then dismissed. This does
> work, but:
>  * the effectively-a-linked-list is not very hardware-ish,
>    which is usually a bit of a red flag when doing modelling
>  * the GICv2 spec adds the Active Priority Registers which are
>    for use for saving and restoring this sort of state, and
>    implementing these properly effectively constrains you to
>    an implementation that doesn't look like what we have now
>  * it doesn't really fit with the GIC grouping and security
>    extensions, where a guest can say "dismiss the last group 1
>    interrupt" rather than just "dismiss the last interrupt".
>
> This series gets rid of those arrays and instead uses the
> Active Priority Registers to do the job. The APRs have one
> bit per "preemption level" (ie per distinct group priority).
> When we take an interrupt we set the appropriate bit to 1
> (and this will always be the lowest set bit in the register,
> because low-numbered priorities are higher and we wouldn't
> have taken the interrupt unless it was higher than our current
> priority). Similarly, when we dismiss an interrupt this just
> clears the lowest set bit in the register, which must be the
> current active interrupt. (It's important not to try to look
> at the current configured priority of the interrupt number,
> because the guest might have reconfigured it while it was
> active.) The new running priority is then calculable by
> looking at the new lowest set bit.
>
> The new code also takes a step in the direction of
> separating the idea of "priority drop" from "deactivate interrupt",
> which we will need to implement the GICv2 feature which allows
> guests to do these two things as separate operations. There's
> more work to do in this area though.
>
> Patch series structure:
>  * patch 1 disentangles the v7M NVIC from some of the internal
>    state we're about to rewrite
>  * patch 2 fixes a bug that would have meant we could have multiple
>    active interrupts at the same group priority, which (a) isn't
>    permitted and (b) would break the redesign we're about to do
>  * patch 3 is fixing the guest accessors for the APR registers
>  * patch 4 is the meat of the change
>  * patch 5 is a bonus bugfix
>
>
> Peter Maydell (5):
>   armv7m_nvic: Implement ICSR without using internal GIC state
>   hw/intc/arm_gic: Running priority is group priority, not full priority
>   hw/intc/arm_gic: Fix handling of GICC_APR<n>, GICC_NSAPR<n> registers
>   hw/intc/arm_gic: Drop running_irq and last_active arrays
>   hw/intc/arm_gic: Actually set the active bits for active interrupts
>
>  hw/intc/arm_gic.c                | 245 
> ++++++++++++++++++++++++++++++++++-----
>  hw/intc/arm_gic_common.c         |   8 +-
>  hw/intc/armv7m_nvic.c            |  13 +--
>  include/hw/intc/arm_gic_common.h |  11 +-
>  4 files changed, 224 insertions(+), 53 deletions(-)