Re: [Qemu-devel] [PATCH 3/5] hw: arm_gic: Keep track of SGI sources

[Peter Maydell]

On 23 August 2013 21:10, Christoffer Dall <address@hidden> wrote:
> Right now the arm gic emulation doesn't keep track of the source of an
> SGI (which apparently Linux guests don't use, or they're fine with
> assuming CPU 0 always).
>
> Add the necessary matrix on the GICState structure and maintain the data
> when setting and clearing the pending state of an IRQ.
>
> Note that we always choose to present the source as the lowest-numbered
> CPU in case multiple cores have signalled the same SGI number to a core
> on the system.

> @@ -525,6 +538,11 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
>              break;
>          }
>          GIC_SET_PENDING(irq, mask);
> +        target_cpu = (unsigned)ffs(mask) - 1;
> +        while (target_cpu < NCPU) {
> +            s->sgi_source[irq][target_cpu] |= (1 << cpu);
> +            target_cpu = (unsigned)ffs(mask) - 1;
> +        }

This is an infinite loop, because you don't do anything
with mask inside the loop, so target_cpu is always
the same each time round. gcc with optimization is
smart enough to notice this:

=> 0x00005555556c1625 <+229>:   jmp    0x5555556c1625 <gic_dist_writel+229>

:-)

Unsurprisingly, my test vexpress-a9 image hangs on startup.

> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -71,6 +71,7 @@ static const VMStateDescription vmstate_gic = {
>          VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, NCPU),
>          VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL),
>          VMSTATE_UINT16_2DARRAY(last_active, GICState, GIC_MAXIRQ, NCPU),
> +        VMSTATE_UINT8_2DARRAY(sgi_source, GICState, GIC_NR_SGIS, NCPU),
>          VMSTATE_UINT16_ARRAY(priority_mask, GICState, NCPU),
>          VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU),
>          VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU),

You need to bump the version_id and minimum_version_id
if you add a new field here.

-- PMM