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

[Peter Maydell]

Looks like nobody cared to review this one, so applying
to target-arm.next.

thanks
-- PMM

On 14 August 2015 at 11:11, Peter Maydell <address@hidden> wrote:
> 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(-)