qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC qom-cpu v2 03/28] target-arm: Update ARMCPU to QOM


From: Andreas Färber
Subject: Re: [Qemu-devel] [RFC qom-cpu v2 03/28] target-arm: Update ARMCPU to QOM realizefn
Date: Thu, 07 Feb 2013 19:44:24 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2

Am 07.02.2013 17:31, schrieb Eduardo Habkost:
> On Sun, Jan 20, 2013 at 08:22:26AM +0100, Andreas Färber wrote:
>> Turn arm_cpu_realize() into a QOM realize function, no longer called
>> via cpu.h prototype. To maintain the semantics of cpu_init(), set
>> realized = true explicitly in cpu_arm_init().
>>
>> Move GDB coprocessor registration, CPU reset and vCPU initialization
>> into the realizefn.
>>
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>>  target-arm/cpu-qom.h |    3 ++-
>>  target-arm/cpu.c     |   21 ++++++++++++++-------
>>  target-arm/cpu.h     |    1 +
>>  target-arm/helper.c  |   14 ++++++++++----
>>  4 Dateien geändert, 27 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 0f455c4..aff7bf3 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -33,6 +33,7 @@
>>  
>>  /**
>>   * ARMCPUClass:
>> + * @parent_realize: The parent class' realize handler.
>>   * @parent_reset: The parent class' reset handler.
>>   *
>>   * An ARM CPU model.
>> @@ -42,6 +43,7 @@ typedef struct ARMCPUClass {
>>      CPUClass parent_class;
>>      /*< public >*/
>>  
>> +    DeviceRealize parent_realize;
>>      void (*parent_reset)(CPUState *cpu);
>>  } ARMCPUClass;
>>  
>> @@ -107,7 +109,6 @@ static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
>>  
>>  #define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
>>  
>> -void arm_cpu_realize(ARMCPU *cpu);
>>  void register_cp_regs_for_features(ARMCPU *cpu);
>>  
>>  #endif
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index 07588a1..19d5ae4 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -147,15 +147,12 @@ static void arm_cpu_finalizefn(Object *obj)
>>      g_hash_table_destroy(cpu->cp_regs);
>>  }
>>  
>> -void arm_cpu_realize(ARMCPU *cpu)
>> +static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>>  {
>> -    /* This function is called by cpu_arm_init() because it
>> -     * needs to do common actions based on feature bits, etc
>> -     * that have been set by the subclass init functions.
>> -     * When we have QOM realize support it should become
>> -     * a true realize function instead.
>> -     */
>> +    ARMCPU *cpu = ARM_CPU(dev);
>> +    ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
>>      CPUARMState *env = &cpu->env;
>> +
>>      /* Some features automatically imply others: */
>>      if (arm_feature(env, ARM_FEATURE_V7)) {
>>          set_feature(env, ARM_FEATURE_VAPA);
>> @@ -197,6 +194,12 @@ void arm_cpu_realize(ARMCPU *cpu)
>>      }
>>  
>>      register_cp_regs_for_features(cpu);
>> +    arm_cpu_register_gdb_regs_for_features(cpu);
>> +
>> +    cpu_reset(CPU(cpu));
>> +    qemu_init_vcpu(env);
>> +
>> +    acc->parent_realize(dev, errp);
>>  }
>>  
>>  /* CPU models */
>> @@ -763,6 +766,10 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
>> *data)
>>  {
>>      ARMCPUClass *acc = ARM_CPU_CLASS(oc);
>>      CPUClass *cc = CPU_CLASS(acc);
>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>> +
>> +    acc->parent_realize = dc->realize;
>> +    dc->realize = arm_cpu_realizefn;
>>  
>>      acc->parent_reset = cc->reset;
>>      cc->reset = arm_cpu_reset;
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index ffddfcb..2902ba5 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -234,6 +234,7 @@ typedef struct CPUARMState {
>>  
>>  ARMCPU *cpu_arm_init(const char *cpu_model);
>>  void arm_translate_init(void);
>> +void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
>>  int cpu_arm_exec(CPUARMState *s);
>>  void do_interrupt(CPUARMState *);
>>  void switch_mode(CPUARMState *, int);
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 37c34a1..f412143 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -1270,14 +1270,22 @@ ARMCPU *cpu_arm_init(const char *cpu_model)
>>      cpu = ARM_CPU(object_new(cpu_model));
>>      env = &cpu->env;
>>      env->cpu_model_str = cpu_model;
>> -    arm_cpu_realize(cpu);
>> +
>> +    /* TODO this should be set centrally, once possible */
>> +    object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
>>  
>>      if (tcg_enabled() && !inited) {
>>          inited = 1;
>>          arm_translate_init();
>>      }
> 
> My first question I had when I saw this was: are you really sure it is
> safe to call cpu_reset() and qemu_init_vcpu() before
> arm_translate_init()?
> 
> But I see that you change this on commit 092028dbf1. So now I have the
> opposite question: are you really sure it is safe to call
> arm_translate_init() before arm_cpu_realizefn()?

The answer to either is yes. TCG initialization functions themselves
(after this series latest) don't depend on CPU state and only calculate
static field offsets (in one or two cases depending on CPU versions that
I have inlined as part of the series). ARM's doesn't. If you spot any
remaining exception to that rule, please shout.

Andreas

> 
>>  
>> -    cpu_reset(CPU(cpu));
>> +    return cpu;
>> +}
>> +
>> +void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>> +{
>> +    CPUARMState *env = &cpu->env;
>> +
>>      if (arm_feature(env, ARM_FEATURE_NEON)) {
>>          gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
>>                                   51, "arm-neon.xml", 0);
>> @@ -1288,8 +1296,6 @@ ARMCPU *cpu_arm_init(const char *cpu_model)
>>          gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
>>                                   19, "arm-vfp.xml", 0);
>>      }
>> -    qemu_init_vcpu(env);
>> -    return cpu;
>>  }
>>  
>>  /* Sort alphabetically by type name, except for "any". */
>> -- 
>> 1.7.10.4
>>
>>
> 


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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]