[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 1/2] i386: introduce "struct X86TopoInfo" for s
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [RFC v2 1/2] i386: introduce "struct X86TopoInfo" for saving cpu topology information |
Date: |
Tue, 4 Mar 2014 16:35:13 -0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Mar 04, 2014 at 06:50:24PM +0800, Chen Fan wrote:
> Signed-off-by: Chen Fan <address@hidden>
> ---
> hw/i386/pc.c | 13 +++++++++----
> target-i386/cpu.c | 16 ++++++++++++++++
> target-i386/cpu.h | 4 ++++
> target-i386/topology.h | 7 +++++++
> 4 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 348b15f..a4d539e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -927,11 +927,12 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> }
> }
>
> -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> +static X86CPU *pc_new_cpu(const char *cpu_model, X86TopoInfo *topo,
> DeviceState *icc_bridge, Error **errp)
> {
> X86CPU *cpu;
> Error *local_err = NULL;
> + int64_t apic_id = x86_cpu_apic_id_from_index(topo->cpu_index);
You can rewrite apicid_from_topo_ids() to be:
void x86_apicid_from_topo(X86TopoInfo *topo)
then x86_cpu_apic_id_from_index() won't need to exist anymore.
>
> cpu = cpu_x86_create(cpu_model, icc_bridge, &local_err);
> if (local_err != NULL) {
> @@ -956,6 +957,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> {
> DeviceState *icc_bridge;
> int64_t apic_id = x86_cpu_apic_id_from_index(id);
> + X86TopoInfo *topo;
>
> if (id < 0) {
> error_setg(errp, "Invalid CPU id: %" PRIi64, id);
> @@ -976,7 +978,9 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>
> icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
> TYPE_ICC_BRIDGE, NULL));
> - pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
> + topo = x86_cpu_topo_from_index(id);
> + pc_new_cpu(current_cpu_model, topo, icc_bridge, errp);
> + g_free(topo);
See comment below about simplifying x86_cpu_topo_from_index() to not
require malloc/free. The code would be much simpler if it looked like
this:
X86TopoInfo topo;
x86_cpu_topo_from_index(cpu_index, &topo)
pc_new_cpu(current_cpu_model, &topo, icc_bridge, errp)
> }
>
> void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> @@ -996,8 +1000,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState
> *icc_bridge)
> current_cpu_model = cpu_model;
>
> for (i = 0; i < smp_cpus; i++) {
> - cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> - icc_bridge, &error);
> + X86TopoInfo *topo = x86_cpu_topo_from_index(i);
> + cpu = pc_new_cpu(cpu_model, topo, icc_bridge, &error);
> + g_free(topo);
> if (error) {
> error_report("%s", error_get_pretty(error));
> error_free(error);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 0e8812a..1858a66 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2609,6 +2609,22 @@ uint32_t x86_cpu_apic_id_from_index(unsigned int
> cpu_index)
> }
> }
>
> +X86TopoInfo *x86_cpu_topo_from_index(unsigned int cpu_index)
What about simply making it:
void x86_cpu_topo_from_index(unsigned int cpu_index, X86TopoInfo *topo)
so code can use a stack variable for X86TopoInfo. Much simpler than
having to manually allocate/free X86TopoInfo.
What about rewriting x86_topo_ids_from_idx() to get X86TopoInfo* as
argument, instead of creating this new function? Both functions have
exactly the same purpose.
> +{
> + unsigned pkg_id, core_id, smt_id;
> + X86TopoInfo *x86TopoInfo;
> +
> + x86TopoInfo = g_malloc0(sizeof(X86TopoInfo));
> + x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index,
> + &pkg_id, &core_id, &smt_id);
> + x86TopoInfo->pkg_id = pkg_id;
> + x86TopoInfo->core_id = core_id;
> + x86TopoInfo->smt_id = smt_id;
> + x86TopoInfo->cpu_index = cpu_index;
> +
> + return x86TopoInfo;
> +}
[...]
--
Eduardo
- [Qemu-devel] [RFC v2 0/2] prebuild cpu QOM tree /machine/node/socket/core/thread/.., Chen Fan, 2014/03/04
- [Qemu-devel] [RFC v2 1/2] i386: introduce "struct X86TopoInfo" for saving cpu topology information, Chen Fan, 2014/03/04
- Re: [Qemu-devel] [RFC v2 1/2] i386: introduce "struct X86TopoInfo" for saving cpu topology information,
Eduardo Habkost <=
- [Qemu-devel] [RFC v3 2/3] i386: use CpuTopoInfo instead apic_id as argument for pc_new_cpu(), chen.fan.fnst, 2014/03/11
- Re: [Qemu-devel] [RFC v3 2/3] i386: use CpuTopoInfo instead apic_id as argument for pc_new_cpu(), Eduardo Habkost, 2014/03/11
- Re: [Qemu-devel] [RFC v3 2/3] i386: use CpuTopoInfo instead apic_id as argument for pc_new_cpu(), Chen Fan, 2014/03/12
- [Qemu-devel] [RFC v4 0/3] prebuild cpu QOM tree /machine/node/socket/core ->link-cpu, Chen Fan, 2014/03/12
- [Qemu-devel] [RFC v4 1/3] cpu: introduce CpuTopoInfo structure for argument simplification, Chen Fan, 2014/03/12
- Re: [Qemu-devel] [RFC v4 1/3] cpu: introduce CpuTopoInfo structure for argument simplification, Eduardo Habkost, 2014/03/12