qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific cor


From: David Gibson
Subject: Re: [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific core devices
Date: Fri, 1 Apr 2016 16:08:44 +1100
User-agent: Mutt/1.5.24 (2015-08-30)

On Thu, Mar 31, 2016 at 02:09:17PM +0530, Bharata B Rao wrote:
> Introduce core devices for each CPU type supported by sPAPR. These
> core devices are derived from the base spapr-cpu-core device type.
> 
> TODO:
> - Add core types for other remaining CPU types
> - Handle CPU model alias correctly
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/ppc/spapr.c                  |   3 +-
>  hw/ppc/spapr_cpu_core.c         | 118 
> ++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h          |   1 +
>  include/hw/ppc/spapr_cpu_core.h |  36 ++++++++++++
>  4 files changed, 156 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 64c4acc..45ac5dc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1614,8 +1614,7 @@ static void spapr_boot_set(void *opaque, const char 
> *boot_device,
>      machine->boot_order = g_strdup(boot_device);
>  }
>  
> -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> -                           Error **errp)
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
>  {
>      CPUPPCState *env = &cpu->env;
>  
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 8cbe2a5..3751a54 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -22,9 +22,127 @@ static const TypeInfo spapr_cpu_core_type_info = {
>      .instance_size = sizeof(sPAPRCPUCore),
>  };
>  
> +#define DEFINE_SPAPR_CPU_CORE(_name)                                         
>   \
> +static void                                                                  
>   \
> +glue(_name, _spapr_cpu_core_create_threads)(DeviceState *dev, int threads,   
>   \
> +    Error **errp)                                                            
>   \
> +{                                                                            
>   \
> +    int i;                                                                   
>   \
> +    Error *local_err = NULL;                                                 
>   \
> +    sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));                          
>   \
> +    glue(_name, sPAPRCPUCore) * core =                                       
>   \
> +         glue(_name, _SPAPR_CPU_CORE)(OBJECT(dev));                          
>   \
> +                                                                             
>   \
> +    for (i = 0; i < threads; i++) {                                          
>   \
> +        char id[32];                                                         
>   \
> +                                                                             
>   \
> +        object_initialize(&sc->threads[i], sizeof(sc->threads[i]),           
>   \
> +                          object_class_get_name(core->cpu));                 
>   \
> +        snprintf(id, sizeof(id), "thread[%d]", i);                           
>   \
> +        object_property_add_child(OBJECT(core), id, OBJECT(&sc->threads[i]), 
>   \
> +                                  &local_err);                               
>   \
> +        if (local_err) {                                                     
>   \
> +            goto err;                                                        
>   \
> +        }                                                                    
>   \
> +    }                                                                        
>   \
> +    return;                                                                  
>   \
> +                                                                             
>   \
> +err:                                                                         
>   \
> +    while (--i) {                                                            
>   \
> +        object_unparent(OBJECT(&sc->threads[i]));                            
>   \
> +    }                                                                        
>   \
> +    error_propagate(errp, local_err);                                        
>   \
> +}                                                                            
>   \
> +                                                                             
>   \
> +static int                                                                   
>   \
> +glue(_name, _spapr_cpu_core_realize_child)(Object *child, void *opaque)      
>   \
> +{                                                                            
>   \
> +    Error **errp = opaque;                                                   
>   \
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());            
>   \
> +    CPUState *cs = CPU(child);                                               
>   \
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);                                       
>   \
> +                                                                             
>   \
> +    object_property_set_bool(child, true, "realized", errp);                 
>   \
> +    if (*errp) {                                                             
>   \
> +        return 1;                                                            
>   \
> +    }                                                                        
>   \
> +                                                                             
>   \
> +    spapr_cpu_init(spapr, cpu, errp);                                        
>   \
> +    if (*errp) {                                                             
>   \
> +        return 1;                                                            
>   \
> +    }                                                                        
>   \
> +    return 0;                                                                
>   \
> +}                                                                            
>   \

If you put the ObjectClass * for the threads in the base abstract
class's class structure, then you can move most of this logic to the
abstract class as well and make the macro-ized stuff much smaller.

The realize_child stuff doesn't even need the ObjectClass* in the base
class to factor out.

> +static void                                                                  
>   \
> +glue(_name, _spapr_cpu_core_realize)(DeviceState *dev, Error **errp)         
>   \
> +{                                                                            
>   \
> +    sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));                          
>   \
> +    CPUCore *cc = CPU_CORE(OBJECT(dev));                                     
>   \
> +    Error *local_err = NULL;                                                 
>   \
> +                                                                             
>   \
> +    /*                                                                       
>   \
> +     * TODO: This is CPU model specific CPU core's realize routine.          
>   \
> +     * However I am initializing "threads" field of the parent type          
>   \
> +     * sPAPRCPUCore here. Is this ok ? If not I will have make "threads"     
>   \
> +     * part of CPU model specific CPU core type and have different plug()    
>   \
> +     * handlers for each type instead of having a common plug() handler      
>   \
> +     * for all core types.                                                   
>   \
> +     */                                                                      
>   \
> +    sc->threads = g_new0(PowerPCCPU, cc->threads);                           
>   \
> +    glue(_name, _spapr_cpu_core_create_threads)(dev, cc->threads, 
> &local_err); \
> +    if (local_err) {                                                         
>   \
> +        goto out;                                                            
>   \
> +    }                                                                        
>   \
> +                                                                             
>   \
> +    object_child_foreach(OBJECT(dev),                                        
>   \
> +                         glue(_name, _spapr_cpu_core_realize_child),         
>   \
> +                         &local_err);                                        
>   \
> +                                                                             
>   \
> +out:                                                                         
>   \
> +    if (local_err) {                                                         
>   \
> +        g_free(sc->threads);                                                 
>   \
> +        error_propagate(errp, local_err);                                    
>   \
> +    }                                                                        
>   \
> +}                                                                            
>   \
> +                                                                             
>   \
> +static void                                                                  
>   \
> +glue(_name, _spapr_cpu_core_instance_init)(Object *obj)                      
>   \
> +{                                                                            
>   \
> +    glue(_name, sPAPRCPUCore) * core = glue(_name, _SPAPR_CPU_CORE)(obj);    
>   \
> +    const char *type = stringify(_name) "-" TYPE_POWERPC_CPU;                
>   \
> +    ObjectClass *oc = object_class_by_name(type);                            
>   \
> +                                                                             
>   \
> +    core->cpu = oc;                                                          
>   \
> +}                                                                            
>   \
> +                                                                             
>   \
> +static void                                                                  
>   \
> +glue(_name, _spapr_cpu_core_class_init)(ObjectClass *oc, void *data)         
>   \
> +{                                                                            
>   \
> +                                                                             
>   \
> +    DeviceClass *dc = DEVICE_CLASS(oc);                                      
>   \
> +    dc->realize = glue(_name, _spapr_cpu_core_realize);                      
>   \

I think the only callback you should need to construct in the macro is
class_init to initialize the ObjectClass* field.

> +}                                                                            
>   \
> +                                                                             
>   \
> +static const TypeInfo glue(_name, _spapr_cpu_core_type_info) =               
>   \
> +{                                                                            
>   \
> +    .name = stringify(_name) "-" TYPE_SPAPR_CPU_CORE,                        
>   \
> +    .parent = TYPE_SPAPR_CPU_CORE,                                           
>   \
> +    .instance_size = sizeof(glue(_name, sPAPRCPUCore)),                      
>   \
> +    .instance_init = glue(_name, _spapr_cpu_core_instance_init),             
>   \
> +    .class_init = glue(_name, _spapr_cpu_core_class_init),                   
>   \
> +};
> +
> +DEFINE_SPAPR_CPU_CORE(host);
> +DEFINE_SPAPR_CPU_CORE(POWER7);
> +DEFINE_SPAPR_CPU_CORE(POWER8);
> +
>  static void spapr_cpu_core_register_types(void)
>  {
>      type_register_static(&spapr_cpu_core_type_info);
> +    type_register_static(&host_spapr_cpu_core_type_info);
> +    type_register_static(&POWER7_spapr_cpu_core_type_info);
> +    type_register_static(&POWER8_spapr_cpu_core_type_info);
>  }
>  
>  type_init(spapr_cpu_core_register_types)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 098d85d..0fdf448 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -585,6 +585,7 @@ void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType 
> drc_type,
>                                         uint32_t count);
>  void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
>                                            uint32_t count);
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp);
>  
>  /* rtas-configure-connector state */
>  struct sPAPRConfigureConnectorState {
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index e3340ea..71e69c0 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -24,4 +24,40 @@ typedef struct sPAPRCPUCore {
>      PowerPCCPU *threads;
>  } sPAPRCPUCore;
>  
> +#define TYPE_host_SPAPR_CPU_CORE "host-spapr-cpu-core"
> +#define host_SPAPR_CPU_CORE(obj) \
> +    OBJECT_CHECK(hostsPAPRCPUCore, (obj), TYPE_host_SPAPR_CPU_CORE)
> +
> +typedef struct hostsPAPRCPUCore {
> +    /*< private >*/
> +    sPAPRCPUCore parent_obj;
> +
> +    /*< public >*/
> +    ObjectClass *cpu;
> +} hostsPAPRCPUCore;
> +#define TYPE_POWER7_SPAPR_CPU_CORE "POWER7-spapr-cpu-core"
> +#define POWER7_SPAPR_CPU_CORE(obj) \
> +    OBJECT_CHECK(POWER7sPAPRCPUCore, (obj), TYPE_POWER7_SPAPR_CPU_CORE)
> +
> +typedef struct POWER7sPAPRCPUCore {
> +    /*< private >*/
> +    sPAPRCPUCore parent_obj;
> +
> +    /*< public >*/
> +    ObjectClass *cpu;
> +} POWER7sPAPRCPUCore;
> +
> +#define TYPE_POWER8_SPAPR_CPU_CORE "POWER8-spapr-cpu-core"
> +#define POWER8_SPAPR_CPU_CORE(obj) \
> +    OBJECT_CHECK(POWER8sPAPRCPUCore, (obj), TYPE_POWER8_SPAPR_CPU_CORE)
> +
> +typedef struct POWER8sPAPRCPUCore {
> +    /*< private >*/
> +    sPAPRCPUCore parent_obj;
> +
> +    /*< public >*/
> +    ObjectClass *cpu;
> +} POWER8sPAPRCPUCore;

These are all identical so should also be macro constructed as well.
I don't think there's actually any need for the structures to be
exposed in a header file either, so you should be able to do it in the
same macro that constructs the implementation.

Uh.. except if you move the ObjectClass* to the base class you won't
even need these.

>  #endif

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


reply via email to

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