[Qemu-devel] qemu arm gic assert failed.

[Qemu-devel] qemu arm gic assert failed.

[Johan Karlsson]

Hi,
Enabled DEBUG_GIC in arm_gic.c

Use case:
Core0 wakes up core1 using irq and then core1 acks and runs eoi until it gets 0x3ff.
Note that core1 do not have interrupts enabled and does this in SVC.

>From what I can see s->current_pending[1] isn't updated in gic_update because the following
        if (!s->enabled || !s->cpu_enabled[cpu]) {
            qemu_irq_lower(s->parent_irq[cpu]);
            return;
        }
Is true for core0 after the first ack from core1 causing gic_update to return before updating core1.

Core0 runs the following.
      /* Store the original values */
      iccicr = (*(volatile U32 *)(ICCICR));
      icdipr0 = (*(volatile U32 *)(ICDIPR0));
      icdiser0 = (*(volatile U32 *)(ICDISER0));
      g_icddcr = (*(volatile U32 *)(ICDDCR));   

      /* Enable the distributor and the cpu interface */
      (*(volatile U32 *)(ICDDCR)) = 0x1;
      (*(volatile U32 *)(ICCICR)) = 0x1;
      
      /* Set highest priority on the irq */
      (*(volatile U32 *)(ICDIPR0)) = icdipr0 & 0xFFFFFF00;
      
      /* Enable the irq (id=0) SGI interruppt */
      (*(volatile U32 *)(ICDISER0)) = 1;

      /* Generate irq on destination cpu */
      reg = 1 << (16 + cpu);
      (*(volatile U32 *)(ICDSGIR)) = reg;

      /* Disable the SGI after use so it doesn't cause problem later on. */
      (*(volatile U32 *)(ICDICER0)) = 1;

      /* Restore the original values except for the ICDDCR that is restored 
       * later in smp_init_other_cpu() */
      (*(volatile U32 *)(ICDISER0)) = icdiser0;
      (*(volatile U32 *)(ICDIPR0))  = icdipr0;
      (*(volatile U32 *)(ICCICR))   = iccicr;

Core1 runs the following.
   uint32_t reg;

   /* Ack and EOI the soft irq that released this cpu from the wfi */
   while ( (reg = (*(volatile U32 *)(ICCIAR))) != 0x3ff)
      (*(volatile U32 *)(ICCEOIR)) = reg;


LOG:
arm_gic: ACK no pending IRQ
arm_gic: CPU 0 Disabled
arm_gic: Distribution Enabled
arm_gic: CPU 0 Enabled
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: Raised pending IRQ 0 (cpu 1)
arm_gic: CPU 0 Disabled
arm_gic: ACK 0
arm_gic: EOI 0
qemu-system-arm: hw/intc/arm_gic.c:203: gic_acknowledge_irq: Assertion `s->sgi_pending[irq][cpu] != 0' failed.

I don't understand why 'arm_gic: Raised pending IRQ 0 (cpu 1)' is printed more the once.

Sorry about the ugly formatting.
/Johan Karlsson

Re: [Qemu-devel] qemu arm gic assert failed.

[Christoffer Dall]

On Thu, Nov 13, 2014 at 1:44 PM, Johan Karlsson <Johan.Karlsson@enea.com> wrote:
> Hi,
> Enabled DEBUG_GIC in arm_gic.c
>
> Use case:
> Core0 wakes up core1 using irq and then core1 acks and runs eoi until it gets 0x3ff.
> Note that core1 do not have interrupts enabled and does this in SVC.
>
> From what I can see s->current_pending[1] isn't updated in gic_update because the following
>         if (!s->enabled || !s->cpu_enabled[cpu]) {
>             qemu_irq_lower(s->parent_irq[cpu]);
>             return;
>         }
> Is true for core0 after the first ack from core1 causing gic_update to return before updating core1.
>
It was recently discussed on a previous thread that this is a bug in
the QEMU, the return statement above should be a 'continue;'

Does that solve your issue?

-Christoffer

Re: [Qemu-devel] qemu arm gic assert failed.

[Johan Karlsson]

> -----Original Message-----
> From: Christoffer Dall [mailto:christoffer.dall@linaro.org]
> Sent: den 13 november 2014 15:28 
> On Thu, Nov 13, 2014 at 1:44 PM, Johan Karlsson
> <Johan.Karlsson@enea.com> wrote:
> > Hi,
> > Enabled DEBUG_GIC in arm_gic.c
> >
> > Use case:
> > Core0 wakes up core1 using irq and then core1 acks and runs eoi until it
> gets 0x3ff.
> > Note that core1 do not have interrupts enabled and does this in SVC.
> >
> > From what I can see s->current_pending[1] isn't updated in gic_update
> because the following
> >         if (!s->enabled || !s->cpu_enabled[cpu]) {
> >             qemu_irq_lower(s->parent_irq[cpu]);
> >             return;
> >         }
> > Is true for core0 after the first ack from core1 causing gic_update to return
> before updating core1.
> >
> It was recently discussed on a previous thread that this is a bug in the QEMU,
> the return statement above should be a 'continue;'
> 
> Does that solve your issue?
> 
> -Christoffer

Yes I tried that and it works, however I didn't analyse if it was correct or not.
But it didn't feel right to return from a function that loops over all cores based on a state of one core.

/Johan

Re: [Qemu-devel] qemu arm gic assert failed.

[Christoffer Dall]

On Thu, Nov 13, 2014 at 4:25 PM, Johan Karlsson <Johan.Karlsson@enea.com> wrote:
>> -----Original Message-----
>> From: Christoffer Dall [mailto:christoffer.dall@linaro.org]
>> Sent: den 13 november 2014 15:28
>> On Thu, Nov 13, 2014 at 1:44 PM, Johan Karlsson
>> <Johan.Karlsson@enea.com> wrote:
>> > Hi,
>> > Enabled DEBUG_GIC in arm_gic.c
>> >
>> > Use case:
>> > Core0 wakes up core1 using irq and then core1 acks and runs eoi until it
>> gets 0x3ff.
>> > Note that core1 do not have interrupts enabled and does this in SVC.
>> >
>> > From what I can see s->current_pending[1] isn't updated in gic_update
>> because the following
>> >         if (!s->enabled || !s->cpu_enabled[cpu]) {
>> >             qemu_irq_lower(s->parent_irq[cpu]);
>> >             return;
>> >         }
>> > Is true for core0 after the first ack from core1 causing gic_update to return
>> before updating core1.
>> >
>> It was recently discussed on a previous thread that this is a bug in the QEMU,
>> the return statement above should be a 'continue;'
>>
>> Does that solve your issue?
>>
>> -Christoffer
>
> Yes I tried that and it works, however I didn't analyse if it was correct or not.

as far as I can tell, it is correct.

> But it didn't feel right to return from a function that loops over all cores based on a state of one core.
>
Exactly, care to send a patch to qemu-devel?

-Christoffer