[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 13/16] hw/intc/arm_gic: Change behavior of IA
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 13/16] hw/intc/arm_gic: Change behavior of IAR writes |
Date: |
Tue, 14 Apr 2015 20:31:36 +0100 |
On 30 October 2014 at 22:12, Greg Bellows <address@hidden> wrote:
> From: Fabian Aggeler <address@hidden>
>
> Grouping (GICv2) and Security Extensions change the behavior of IAR
> reads. Acknowledging Group0 interrupts is only allowed from Secure
> state and acknowledging Group1 interrupts from Secure state is only
> allowed if AckCtl bit is set.
Subject says "IAR writes" but it means "IAR reads".
>
> Signed-off-by: Fabian Aggeler <address@hidden>
>
> ---
>
> v1 -> v2
> - Fix issue in gic_acknowledge_irq() where the GICC_CTLR_S_ACK_CTL flag is
> applied without first checking whether the read is secure or non-secure.
> Secure reads of IAR when AckCtl is 0 return a spurious ID of 1022, but
> non-secure ignores the flag.
> ---
> hw/intc/arm_gic.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 2d83225..7eb72df 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -190,11 +190,36 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu)
> int ret, irq, src;
> int cm = 1 << cpu;
> irq = s->current_pending[cpu];
> + bool isGrp0;
> if (irq == 1023
> || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
> DPRINTF("ACK no pending IRQ\n");
> return 1023;
> }
> +
> + if (s->revision >= 2 || s->security_extn) {
> + isGrp0 = GIC_TEST_GROUP0(irq, (1 << cpu));
> + if ((isGrp0 && (!s->enabled_grp[0]
> + || !(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0)))
> + || (!isGrp0 && (!s->enabled_grp[1]
> + || !(s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1)))) {
> + return 1023;
> + }
> +
> + if ((s->revision >= 2 && !s->security_extn)
> + || (s->security_extn && !ns_access())) {
> + if (!isGrp0 && !ns_access() &&
> + !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
> + DPRINTF("Read of IAR ignored for Group1 interrupt %d "
> + "(AckCtl disabled)\n", irq);
> + return 1022;
> + }
> + } else if (s->security_extn && ns_access() && isGrp0) {
> + DPRINTF("Non-secure read of IAR ignored for Group0 interrupt
> %d\n",
> + irq);
> + return 1023;
> + }
> + }
This doesn't quite line up with the pseudocode in the GIC spec.
It's probably going to be easier to read with some utility functions
for 'grouping enabled' etc.
> s->last_active[irq][cpu] = s->running_irq[cpu];
>
> if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> --
> 1.8.3.2
>
-- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 13/16] hw/intc/arm_gic: Change behavior of IAR writes,
Peter Maydell <=