From: David Gibson <david@gibson.dropbear.id.au>
To: Bharata B Rao <bharata@linux.vnet.ibm.com>
Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com,
pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru,
qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com,
qemu-ppc@nongnu.org, pbonzini@redhat.com, imammedo@redhat.com,
afaerber@suse.de, mdroth@linux.vnet.ibm.com
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 [thread overview]
Message-ID: <20160401050844.GP416@voom.redhat.com> (raw)
In-Reply-To: <1459413561-30745-9-git-send-email-bharata@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 14141 bytes --]
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 <bharata@linux.vnet.ibm.com>
> ---
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2016-04-01 5:17 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-31 8:39 [Qemu-devel] [RFC PATCH v2.1 00/12] Core based CPU hotplug for PowerPC sPAPR Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 01/12] exec: Remove cpu from cpus list during cpu_exec_exit() Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 02/12] exec: Do vmstate unregistration from cpu_exec_exit() Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 03/12] cpu: Reclaim vCPU objects Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 04/12] cpu: Add a sync version of cpu_remove() Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 05/12] qdev: hotplug: Introduce HotplugHandler.pre_plug() callback Bharata B Rao
2016-04-01 3:30 ` David Gibson
2016-04-01 10:38 ` Paolo Bonzini
2016-04-04 0:09 ` David Gibson
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 06/12] cpu: Abstract CPU core type Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 07/12] spapr: Abstract CPU core device Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific core devices Bharata B Rao
2016-04-01 5:08 ` David Gibson [this message]
2016-04-01 6:12 ` Bharata B Rao
2016-04-04 0:13 ` David Gibson
2016-04-09 2:21 ` Michael Roth
2016-04-04 0:16 ` David Gibson
2016-04-08 23:35 ` Michael Roth
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 09/12] spapr: convert boot CPUs into CPU " Bharata B Rao
2016-04-01 5:12 ` David Gibson
2016-04-08 23:35 ` Michael Roth
2016-05-05 9:19 ` Bharata B Rao
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 10/12] spapr: CPU hotplug support Bharata B Rao
2016-04-04 4:23 ` David Gibson
2016-04-05 23:47 ` Michael Roth
2016-05-05 9:22 ` Bharata B Rao
2016-05-06 8:57 ` Igor Mammedov
2016-05-06 10:14 ` Bharata B Rao
2016-05-06 11:01 ` Igor Mammedov
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 11/12] xics, xics_kvm: Handle CPU unplug correctly Bharata B Rao
2016-04-06 0:24 ` Michael Roth
2016-04-06 0:43 ` David Gibson
2016-04-08 23:40 ` Michael Roth
2016-03-31 8:39 ` [Qemu-devel] [RFC PATCH v2.1 12/12] spapr: CPU hot unplug support Bharata B Rao
2016-04-04 4:27 ` David Gibson
2016-05-09 4:24 ` Bharata B Rao
2016-04-04 14:44 ` [Qemu-devel] [RFC PATCH v2.1 00/12] Core based CPU hotplug for PowerPC sPAPR Igor Mammedov
2016-04-05 14:55 ` Bharata B Rao
2016-04-05 18:40 ` Igor Mammedov
2016-04-05 21:58 ` Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160401050844.GP416@voom.redhat.com \
--to=david@gibson.dropbear.id.au \
--cc=afaerber@suse.de \
--cc=agraf@suse.de \
--cc=aik@ozlabs.ru \
--cc=armbru@redhat.com \
--cc=bharata@linux.vnet.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=ehabkost@redhat.com \
--cc=imammedo@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=mjrosato@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=pkrempa@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.