Re: [Qemu-devel] [PATCH v7 2/6] arm: make the number of GIC interrupts configurable

["Andreas Färber" <afaerber@suse.de>]

Am 10.01.2012 21:33, schrieb Mark Langsdorf:
> Increase the maximum number of GIC interrupts for a9mp and a11mp to 1020,
> and create a configurable property for each defaulting to 96 and 64
> (respectively) so that device modelers can set the value appropriately
> for their SoC. Other ARM processors also set their maximum number of
> used IRQs appropriately.
> 
> Set the maximum theoretical number of GIC interrupts to 1020 and
> update the save/restore code to only use the appropriate number for
> each SoC.
> 
> Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
> ---
> Changes from v6
> 	Removed trailing whitespace
> 	armv7m_nvic uses num_irq properly
> 	Some comments changed
> Changes from v5
>         Clarify the commit message
>         Rename GIC_NIRQ to GIC_MAXIRQ and change usage slightly
>         Makes num-irq to uint32_t in all cases
>         Clarify the error message
>         Clarify documentation on the num-irq qdev property use in all files
> Changes from v4
>         None
> Changes from v3
>         Increase maximum number of GIC interrupts to 1020
>         Remove SoC/implementation specific GIC_NIRQ #defs
>         Added properties code to arm11mp
>         Changed error handling for too many interrupts
>         Redid save/load handling
> Changes from v2
>         Skipped
> Changes from v1
>         Increase the number of a9mp interrupts to 192
>         Add a property defaulting to 96
>         Add a num_irq member in the gic state structure
>         Use the num_irq value as appropriate
>         Add num_irq argument to gic_init()
>         Add num_irq to various CPU calls to gic_init
> 
>  hw/a9mpcore.c     |   13 +++++++--
>  hw/arm11mpcore.c  |   17 ++++++++----
>  hw/arm_gic.c      |   68 +++++++++++++++++++++++++++++-----------------------
>  hw/armv7m_nvic.c  |   30 ++++++++++++++++++-----
>  hw/realview_gic.c |    7 ++++-
>  5 files changed, 87 insertions(+), 48 deletions(-)

> diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
> index bf8c3c5..befdc16 100644
> --- a/hw/armv7m_nvic.c
> +++ b/hw/armv7m_nvic.c

> @@ -384,16 +382,34 @@ static int armv7m_nvic_init(SysBusDevice *dev)
>  {
>      nvic_state *s= FROM_SYSBUSGIC(nvic_state, dev);
>  
> -    gic_init(&s->gic);
> +   /* note that for the M profile gic_init() takes the number of external
> +    * interrupt lines only.
> +    */
> +    gic_init(&s->gic, s->num_irq);
>      memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->gic.iomem);
>      s->systick.timer = qemu_new_timer_ns(vm_clock, systick_timer_tick, s);

>      vmstate_register(&dev->qdev, -1, &vmstate_nvic, s);

This should probably be removed now that you register it through
sysbus_register_withprop() below.

>      return 0;
>  }
>  
> +static SysBusDeviceInfo armv7m_nvic_priv_info = {
> +    .init = armv7m_nvic_init,
> +    .qdev.name  = "armv7m_nvic",
> +    .qdev.size  = sizeof(nvic_state),
> +    .qdev.vmsd = &vmstate_nvic,

Minor nit: Assignment is inconsistent - add a space here?

> +    .qdev.props = (Property[]) {
> +        /* The ARM v7m may have anything from 0 to 496 external interrupt
> +         * IRQ lines. We default to 64 external and 32 internal
> +         * Other boards may differ and should set this property appropriately.
> +         */
> +        DEFINE_PROP_UINT32("num-irq", nvic_state, num_irq, 64),
> +        DEFINE_PROP_END_OF_LIST(),
> +    }
> +};
> +
>  static void armv7m_nvic_register_devices(void)
>  {
> -    sysbus_register_dev("armv7m_nvic", sizeof(nvic_state), armv7m_nvic_init);
> +    sysbus_register_withprop(&armv7m_nvic_priv_info);
>  }
>  
>  device_init(armv7m_nvic_register_devices)

I'm still wondering whether the num-irq property calls for a version
bump somewhere. My thinking is no, since the SysBus device will not be
user-created from the command line and therefore effectively has the
same values as hardcoded before; but in that case we should assert this
by setting .qdev.no_user = 1.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg