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 <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.

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