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

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

[Andre Przywara]

From: Rob Herring <rob.herring@calxeda.com>

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 <rob.herring@calxeda.com>
Signed-off-by: Andre Przywara <andre.przywara@calxeda.com>
---
 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");
+    } 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);
-- 
1.7.12.1

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 <rob.herring@calxeda.com>
> 
> 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 <rob.herring@calxeda.com>
> Signed-off-by: Andre Przywara <andre.przywara@calxeda.com>
> ---
>  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

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

[Andreas Färber]

Am 29.06.2013 14:54, schrieb Andreas Färber:
> Am 28.06.2013 13:59, schrieb Andre Przywara:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> 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 <rob.herring@calxeda.com>
>> Signed-off-by: Andre Przywara <andre.przywara@calxeda.com>
>> ---
>>  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.

Add to that, feature inference is done as part of QOM realize, for which
it is too early to depend on in machine init.

> I would suggest something along these lines:
> 
> if (object_get_class(OBJECT(cpu)) == object_class_by_name("cortex-a15-"
> TYPE_ARM_CPU)) {...}

Andreas

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

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

[Peter Maydell]

On 28 June 2013 12:59, Andre Przywara <andre.przywara@calxeda.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> 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.

I don't think we have any other board models which do "I'm going
to guess which board you actually wanted based on which CPU
you specified", do we? I think it would be nicer just to have
a '-M midway' which gave you the right CPU and peripherals.

thanks
-- PMM