Re: [Qemu-devel] [PATCH v3 2/9] hw/i386: Adjust nr_dies with configured smp_dies for PCMachine

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:40:57PM +0800, Like Xu wrote:
> To support multiple dies configuration on PCMachine, the best place to
> set CPUX86State->nr_dies with requested PCMachineState->smp_dies is in
> pc_new_cpu() and pc_cpu_pre_plug(). Refactoring pc_new_cpu() is applied
> and redundant parameter "const char *typename" would be removed.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Like Xu 

Reviewed-by: Eduardo Habkost 

> ---
>  hw/i386/pc.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 9e9a42f007..af2e95a1b9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1520,12 +1520,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
> level)
>  }
>  }
>  
> -static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
> +static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
>  {
>  Object *cpu = NULL;
>  Error *local_err = NULL;
> +CPUX86State *env = NULL;
>  
> -cpu = object_new(typename);
> +cpu = object_new(MACHINE(pcms)->cpu_type);
> +
> +env = _CPU(cpu)->env;
> +env->nr_dies = pcms->smp_dies;
>  
>  object_property_set_uint(cpu, apic_id, "apic-id", _err);
>  object_property_set_bool(cpu, true, "realized", _err);
> @@ -1551,7 +1555,7 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, 
> Error **errp)
>  return;
>  }
>  
> -pc_new_cpu(ms->cpu_type, apic_id, _err);
> +pc_new_cpu(PC_MACHINE(ms), apic_id, _err);
>  if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> @@ -1576,8 +1580,7 @@ void pc_cpus_init(PCMachineState *pcms)
>   ms->smp.max_cpus - 1) + 
> 1;
>  possible_cpus = mc->possible_cpu_arch_ids(ms);
>  for (i = 0; i < ms->smp.cpus; i++) {
> -pc_new_cpu(possible_cpus->cpus[i].type, 
> possible_cpus->cpus[i].arch_id,
> -   _fatal);
> +pc_new_cpu(pcms, possible_cpus->cpus[i].arch_id, _fatal);
>  }
>  }
>  
> @@ -2297,6 +2300,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>  CPUArchId *cpu_slot;
>  X86CPUTopoInfo topo;
>  X86CPU *cpu = X86_CPU(dev);
> +CPUX86State *env = >env;
>  MachineState *ms = MACHINE(hotplug_dev);
>  PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  unsigned int smp_cores = ms->smp.cores;
> @@ -2308,6 +2312,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>  return;
>  }
>  
> +env->nr_dies = pcms->smp_dies;
> +
>  /*
>   * If APIC ID is not set,
>   * set it based on socket/die/core/thread properties.
> -- 
> 2.21.0
> 

-- 
Eduardo

[Qemu-devel] [PATCH v3 2/9] hw/i386: Adjust nr_dies with configured smp_dies for PCMachine

[Like Xu]

To support multiple dies configuration on PCMachine, the best place to
set CPUX86State->nr_dies with requested PCMachineState->smp_dies is in
pc_new_cpu() and pc_cpu_pre_plug(). Refactoring pc_new_cpu() is applied
and redundant parameter "const char *typename" would be removed.

Suggested-by: Eduardo Habkost 
Signed-off-by: Like Xu 
---
 hw/i386/pc.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9e9a42f007..af2e95a1b9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1520,12 +1520,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
level)
 }
 }
 
-static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
+static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
 {
 Object *cpu = NULL;
 Error *local_err = NULL;
+CPUX86State *env = NULL;
 
-cpu = object_new(typename);
+cpu = object_new(MACHINE(pcms)->cpu_type);
+
+env = _CPU(cpu)->env;
+env->nr_dies = pcms->smp_dies;
 
 object_property_set_uint(cpu, apic_id, "apic-id", _err);
 object_property_set_bool(cpu, true, "realized", _err);
@@ -1551,7 +1555,7 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, 
Error **errp)
 return;
 }
 
-pc_new_cpu(ms->cpu_type, apic_id, _err);
+pc_new_cpu(PC_MACHINE(ms), apic_id, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
@@ -1576,8 +1580,7 @@ void pc_cpus_init(PCMachineState *pcms)
  ms->smp.max_cpus - 1) + 1;
 possible_cpus = mc->possible_cpu_arch_ids(ms);
 for (i = 0; i < ms->smp.cpus; i++) {
-pc_new_cpu(possible_cpus->cpus[i].type, possible_cpus->cpus[i].arch_id,
-   _fatal);
+pc_new_cpu(pcms, possible_cpus->cpus[i].arch_id, _fatal);
 }
 }
 
@@ -2297,6 +2300,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 CPUArchId *cpu_slot;
 X86CPUTopoInfo topo;
 X86CPU *cpu = X86_CPU(dev);
+CPUX86State *env = >env;
 MachineState *ms = MACHINE(hotplug_dev);
 PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 unsigned int smp_cores = ms->smp.cores;
@@ -2308,6 +2312,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 return;
 }
 
+env->nr_dies = pcms->smp_dies;
+
 /*
  * If APIC ID is not set,
  * set it based on socket/die/core/thread properties.
-- 
2.21.0