Re: [Qemu-devel] [PATCH v4 2/9] spapr: Abstract CPU core device and type specific core devices

[David Gibson]

On Fri, Jun 10, 2016 at 06:29:01AM +0530, Bharata B Rao wrote:
> Add sPAPR specific abastract CPU core device that is based on generic
> CPU core device. Use this as base type to create sPAPR CPU specific core
> devices.
> 
> TODO:
> - Add core types for other remaining CPU types
> - Handle CPU model alias correctly
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/ppc/Makefile.objs            |   1 +
>  hw/ppc/spapr.c                  |   3 +-
>  hw/ppc/spapr_cpu_core.c         | 160 
> ++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h          |   1 +
>  include/hw/ppc/spapr_cpu_core.h |  29 ++++++++
>  target-ppc/kvm.c                |  28 +++++++
>  6 files changed, 220 insertions(+), 2 deletions(-)
>  create mode 100644 hw/ppc/spapr_cpu_core.c
>  create mode 100644 include/hw/ppc/spapr_cpu_core.h
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index c1ffc77..5cc6608 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_cpu_core.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o
>  endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0636642..87f4e53 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1606,8 +1606,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
> new file mode 100644
> index 0000000..b8b97c9
> --- /dev/null
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -0,0 +1,160 @@
> +/*
> + * sPAPR CPU core device, acts as container of CPU thread devices.
> + *
> + * Copyright (C) 2016 Bharata B Rao <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "hw/cpu/core.h"
> +#include "hw/ppc/spapr_cpu_core.h"
> +#include "target-ppc/cpu.h"
> +#include "hw/ppc/spapr.h"
> +#include "hw/boards.h"
> +#include "qapi/error.h"
> +#include <sysemu/cpus.h>
> +#include "target-ppc/kvm_ppc.h"
> +
> +static int 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;
> +}
> +
> +static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> +{
> +    sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> +    CPUCore *cc = CPU_CORE(OBJECT(dev));
> +    const char *typename = object_class_get_name(sc->cpu_class);
> +    size_t size = object_type_get_size(typename);
> +    Error *local_err = NULL;
> +    Object *obj;
> +    int i;
> +
> +    sc->threads = g_malloc0(size * cc->nr_threads);
> +    for (i = 0; i < cc->nr_threads; i++) {
> +        char id[32];
> +        void *obj = sc->threads + i * size;
> +
> +        object_initialize(obj, size, typename);
> +        snprintf(id, sizeof(id), "thread[%d]", i);
> +        object_property_add_child(OBJECT(sc), id, obj, &local_err);
> +        if (local_err) {
> +            goto err;
> +        }
> +    }
> +    object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child, 
> &local_err);
> +    if (local_err) {
> +        goto err;
> +    } else {
> +        return;
> +    }
> +
> +err:
> +    while (i >= 0) {
> +        obj = sc->threads + i * size;
> +        object_unparent(obj);
> +        i--;
> +    }
> +    g_free(sc->threads);
> +    error_propagate(errp, local_err);
> +}
> +
> +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    dc->realize = spapr_cpu_core_realize;
> +}
> +
> +/*
> + * instance_init routines from different flavours of sPAPR CPU cores.
> + * TODO: Add support for 'host' core type.
> + */
> +#define SPAPR_CPU_CORE_INITFN(_type, _fname) \
> +static void glue(glue(spapr_cpu_core_, _fname), _initfn(Object *obj)) \
> +{ \
> +    sPAPRCPUCore *core = SPAPR_CPU_CORE(obj); \
> +    char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, stringify(_type)); \
> +    ObjectClass *oc = object_class_by_name(name); \
> +    g_assert(oc); \
> +    g_free((void *)name); \
> +    core->cpu_class = oc; \
> +}

I still think this can be a class_init rather than instance_init
function, but that's a nit we can fix later.

> +
> +SPAPR_CPU_CORE_INITFN(POWER7_v2.3, POWER7);
> +SPAPR_CPU_CORE_INITFN(POWER7+_v2.1, POWER7plus);
> +SPAPR_CPU_CORE_INITFN(POWER8_v2.0, POWER8);
> +SPAPR_CPU_CORE_INITFN(POWER8E_v2.1, POWER8E);
> +
> +typedef struct SPAPRCoreInfo {
> +    const char *name;
> +    void (*initfn)(Object *obj);
> +} SPAPRCoreInfo;
> +
> +static const SPAPRCoreInfo spapr_cores[] = {
> +    /* POWER7 and aliases */
> +    { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn },
> +    { .name = "POWER7", .initfn = spapr_cpu_core_POWER7_initfn },
> +
> +    /* POWER7+ and aliases */
> +    { .name = "POWER7+_v2.1", .initfn = spapr_cpu_core_POWER7plus_initfn },
> +    { .name = "POWER7+", .initfn = spapr_cpu_core_POWER7plus_initfn },
> +
> +    /* POWER8 and aliases */
> +    { .name = "POWER8_v2.0", .initfn = spapr_cpu_core_POWER8_initfn },
> +    { .name = "POWER8", .initfn = spapr_cpu_core_POWER8_initfn },
> +    { .name = "power8", .initfn = spapr_cpu_core_POWER8_initfn },
> +
> +    /* POWER8E and aliases */
> +    { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn },
> +    { .name = "POWER8E", .initfn = spapr_cpu_core_POWER8E_initfn },
> +
> +    { .name = NULL }
> +};
> +
> +static void spapr_cpu_core_register(const SPAPRCoreInfo *info)
> +{
> +    TypeInfo type_info = {
> +        .parent = TYPE_SPAPR_CPU_CORE,
> +        .instance_size = sizeof(sPAPRCPUCore),
> +        .instance_init = info->initfn,
> +    };
> +
> +    type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, info->name);
> +    type_register(&type_info);
> +    g_free((void *)type_info.name);
> +}
> +
> +static const TypeInfo spapr_cpu_core_type_info = {
> +    .name = TYPE_SPAPR_CPU_CORE,
> +    .parent = TYPE_CPU_CORE,
> +    .abstract = true,
> +    .instance_size = sizeof(sPAPRCPUCore),
> +    .class_init = spapr_cpu_core_class_init,
> +};
> +
> +static void spapr_cpu_core_register_types(void)
> +{
> +    const SPAPRCoreInfo *info = spapr_cores;
> +
> +    type_register_static(&spapr_cpu_core_type_info);
> +    while (info->name) {
> +        spapr_cpu_core_register(info);
> +        info++;
> +    }
> +}
> +
> +type_init(spapr_cpu_core_register_types)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 971df3d..4ff14d6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -582,6 +582,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
> new file mode 100644
> index 0000000..424edec
> --- /dev/null
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -0,0 +1,29 @@
> +/*
> + * sPAPR CPU core device.
> + *
> + * Copyright (C) 2016 Bharata B Rao <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef HW_SPAPR_CPU_CORE_H
> +#define HW_SPAPR_CPU_CORE_H
> +
> +#include "hw/qdev.h"
> +#include "hw/cpu/core.h"
> +#include "target-ppc/cpu-qom.h"
> +
> +#define TYPE_SPAPR_CPU_CORE "spapr-cpu-core"
> +#define SPAPR_CPU_CORE(obj) \
> +    OBJECT_CHECK(sPAPRCPUCore, (obj), TYPE_SPAPR_CPU_CORE)
> +
> +typedef struct sPAPRCPUCore {
> +    /*< private >*/
> +    CPUCore parent_obj;
> +
> +    /*< public >*/
> +    void *threads;
> +    ObjectClass *cpu_class;
> +} sPAPRCPUCore;
> +
> +#endif
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 24d6032..20982d0 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -43,6 +43,9 @@
>  #include "exec/memattrs.h"
>  #include "sysemu/hostmem.h"
>  #include "qemu/cutils.h"
> +#if defined(TARGET_PPC64)
> +#include "hw/ppc/spapr_cpu_core.h"
> +#endif
>  
>  //#define DEBUG_KVM
>  
> @@ -2329,6 +2332,19 @@ static PowerPCCPUClass 
> *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
>      return POWERPC_CPU_CLASS(oc);
>  }
>  
> +#if defined(TARGET_PPC64)
> +static void spapr_cpu_core_host_initfn(Object *obj)
> +{
> +    sPAPRCPUCore *core = SPAPR_CPU_CORE(obj);
> +    char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, "host");
> +    ObjectClass *oc = object_class_by_name(name);
> +
> +    g_assert(oc);
> +    g_free((void *)name);
> +    core->cpu_class = oc;
> +}
> +#endif
> +
>  static int kvm_ppc_register_host_cpu_type(void)
>  {
>      TypeInfo type_info = {
> @@ -2350,6 +2366,18 @@ static int kvm_ppc_register_host_cpu_type(void)
>      type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
>      type_register(&type_info);
>  
> +#if defined(TARGET_PPC64)
> +    type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
> +    type_info.parent = TYPE_SPAPR_CPU_CORE,
> +    type_info.instance_size = sizeof(sPAPRCPUCore),
> +    type_info.instance_init = spapr_cpu_core_host_initfn,
> +    type_info.class_init = NULL;
> +    type_register(&type_info);
> +    g_free((void *)type_info.name);
> +    type_info.instance_size = 0;
> +    type_info.instance_init = NULL;
> +#endif

I think this will fail compile if you have a 64bit build which doesn't
have CONFIG_PSERIES.  Since none of the default configs do that, we
can probably get away with it for now.

> +
>      /* Register generic family CPU class for a family */
>      pvr_pcc = ppc_cpu_get_family_class(pvr_pcc);
>      dc = DEVICE_CLASS(pvr_pcc);

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

signature.asc
Description: PGP signature