Re: [Qemu-devel] [PATCH] highbank: add initial Calxeda Midway A15 support

[Andreas Färber]

Am 28.06.2013 13:59, schrieb Andre Przywara:
> From: Rob Herring <address@hidden>
> 
> While the Calxeda Midway part is actually a bit more than a "Highbank
> with A15s", for QEMU's purposes this view is sufficient. So to allow
> both emulation with that chip as well as KVM guests using that model
> add an A15 CPU and it's peripherals as an option. The use of:
> "-M highbank -cpu cortex-a15" simply gives the new chip without the
> need for a new model.
> 
> Signed-off-by: Rob Herring <address@hidden>
> Signed-off-by: Andre Przywara <address@hidden>
> ---
>  hw/arm/highbank.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 4405dbd..ed864c6 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -196,6 +196,7 @@ static void highbank_init(QEMUMachineInitArgs *args)
>      const char *kernel_filename = args->kernel_filename;
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
> +    CPUARMState *env = NULL;
>      DeviceState *dev;
>      SysBusDevice *busdev;
>      qemu_irq *irqp;
> @@ -223,6 +224,8 @@ static void highbank_init(QEMUMachineInitArgs *args)
>          cpu->reset_cbar = GIC_BASE_ADDR;
>          irqp = arm_pic_init_cpu(cpu);
>          cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
> +
> +        env = &cpu->env;
>      }
>  
>      sysmem = get_system_memory();
> @@ -246,7 +249,16 @@ static void highbank_init(QEMUMachineInitArgs *args)
>          }
>      }
>  
> -    dev = qdev_create(NULL, "a9mpcore_priv");
> +    if (arm_feature(env, ARM_FEATURE_LPAE)) {
> +        dev = qdev_create(NULL, "a15mpcore_priv");

This feels a bit fragile to me... Cortex-A7 or other cores might grow
support for LPAE, too. I would suggest something along these lines:

if (object_get_class(OBJECT(cpu)) == object_class_by_name("cortex-a15-"
TYPE_ARM_CPU)) {...}

In a QOM context Peter, the Samsung guys and me had been discussing how
to improve CPU modelling, which I have been experimenting with some more
on my Tegra branch:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra

Transferred to Highbank this would result in something like this:

/machine/highbank-soc - name board-specific, a DeviceState
/machine/highbank-soc/cortex - a Container for ARM standard peripherals
/machine/highbank-soc/cortex/cpu[0..3] - an ARMCPU core
/machine/highbank-soc/foo - anything that's specific to this SoC

I now wonder whether it would make sense to turn the "cortex" node into
a DeviceState type "cortex-a15-arm-soc" rather than an empty Container
filled by each SoC? DeviceState would help with recursive QOM
realization, and we could then better embed what is now a15mpcore_priv.

Certainly outside the scope of this patch, but long-term I would like to
get away from the board-level CPU peripheral fiddling that is being done
here and clearly separate SoC from machine via different files.

Regards,
Andreas

> +    } else {
> +        dev = qdev_create(NULL, "l2x0");
> +        qdev_init_nofail(dev);
> +        busdev = SYS_BUS_DEVICE(dev);
> +        sysbus_mmio_map(busdev, 0, 0xfff12000);
> +
> +        dev = qdev_create(NULL, "a9mpcore_priv");
> +    }
>      qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
>      qdev_prop_set_uint32(dev, "num-irq", NIRQ_GIC);
>      qdev_init_nofail(dev);
> @@ -260,11 +272,6 @@ static void highbank_init(QEMUMachineInitArgs *args)
>          pic[n] = qdev_get_gpio_in(dev, n);
>      }
>  
> -    dev = qdev_create(NULL, "l2x0");
> -    qdev_init_nofail(dev);
> -    busdev = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(busdev, 0, 0xfff12000);
> -
>      dev = qdev_create(NULL, "sp804");
>      qdev_prop_set_uint32(dev, "freq0", 150000000);
>      qdev_prop_set_uint32(dev, "freq1", 150000000);
> 


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