Re: [Qemu-devel] [PATCH qom-cpu 1/7] cpu: Move nr_{cores, threads} fields to CPUState

[Igor Mammedov]

On Wed, 19 Dec 2012 16:31:05 +0100
Andreas Färber <address@hidden> wrote:

> Pass MIPSCPU to malta_mips_config(); avoid that for mips_cpu_map_tc()
> since callers only access MIPS Thread Contexts, inside TCG helpers.
Commit message make me think that only MIPS is affected

> 
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  cpu-defs.h              |    2 --
>  cpus.c                  |    4 ++--
>  hw/mips_malta.c         |    9 ++++++---
>  include/qemu/cpu.h      |    5 +++++
>  target-i386/cpu.c       |   18 +++++++++---------
>  target-mips/op_helper.c |    8 +++++---
>  6 Dateien geändert, 27 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)
> 
> diff --git a/cpu-defs.h b/cpu-defs.h
> index caea72b..548f616 100644
> --- a/cpu-defs.h
> +++ b/cpu-defs.h
> @@ -196,8 +196,6 @@ typedef struct CPUWatchpoint {
>      int cpu_index; /* CPU index (informative) */                        \
>      uint32_t host_tid; /* host thread ID */                             \
>      int numa_node; /* NUMA node this cpu is belonging to  */            \
> -    int nr_cores;  /* number of cores within this CPU package */        \
> -    int nr_threads;/* number of threads within this CPU */              \
>      int running; /* Nonzero if cpu is currently running(usermode).  */  \
>      /* user data */                                                     \
>      void *opaque;                                                       \
> diff --git a/cpus.c b/cpus.c
> index d9c332f..045e501 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1041,8 +1041,8 @@ void qemu_init_vcpu(void *_env)
>      CPUArchState *env = _env;
>      CPUState *cpu = ENV_GET_CPU(env);
>  
> -    env->nr_cores = smp_cores;
> -    env->nr_threads = smp_threads;
> +    cpu->nr_cores = smp_cores;
> +    cpu->nr_threads = smp_threads;
>      cpu->stopped = true;
>      if (kvm_enabled()) {
>          qemu_kvm_start_vcpu(env);
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 4d2464a..9827b8c 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -743,10 +743,13 @@ static int64_t load_kernel (void)
>      return kernel_entry;
>  }
>  
> -static void malta_mips_config(CPUMIPSState *env)
> +static void malta_mips_config(MIPSCPU *cpu)
>  {
> +    CPUMIPSState *env = &cpu->env;
> +    CPUState *cs = CPU(cpu);
> +
>      env->mvp->CP0_MVPConf0 |= ((smp_cpus - 1) << CP0MVPC0_PVPE) |
> -                         ((smp_cpus * env->nr_threads - 1) << CP0MVPC0_PTC);
> +                         ((smp_cpus * cs->nr_threads - 1) << CP0MVPC0_PTC);
>  }
>  
>  static void main_cpu_reset(void *opaque)
> @@ -763,7 +766,7 @@ static void main_cpu_reset(void *opaque)
>          env->CP0_Status &= ~((1 << CP0St_BEV) | (1 << CP0St_ERL));
>      }
>  
> -    malta_mips_config(env);
> +    malta_mips_config(cpu);
>  }
>  
>  static void cpu_request_exit(void *opaque, int irq, int level)
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index fa3ffdb..03faaab 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -57,6 +57,8 @@ struct kvm_run;
>  
>  /**
>   * CPUState:
> + * @nr_cores: Number of cores within this CPU package.
> + * @nr_threads: Number of threads within this CPU.
>   * @created: Indicates whether the CPU thread has been successfully created.
>   * @stop: Indicates a pending stop request.
>   * @stopped: Indicates the CPU has been artificially stopped.
> @@ -69,6 +71,9 @@ struct CPUState {
>      Object parent_obj;
>      /*< public >*/
>  
> +    int nr_cores;
> +    int nr_threads;
> +
>      struct QemuThread *thread;
>  #ifdef _WIN32
>      HANDLE hThread;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 04a90c5..8664ac2 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1668,8 +1668,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad 
> words, Linux wants it. */
>          *ecx = env->cpuid_ext_features;
>          *edx = env->cpuid_features;
> -        if (env->nr_cores * env->nr_threads > 1) {
> -            *ebx |= (env->nr_cores * env->nr_threads) << 16;
> +        if (cs->nr_cores * cs->nr_threads > 1) {
> +            *ebx |= (cs->nr_cores * cs->nr_threads) << 16;
>              *edx |= 1 << 28;    /* HTT bit */
>          }
>          break;
> @@ -1682,8 +1682,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          break;
>      case 4:
>          /* cache info: needed for Core compatibility */
> -        if (env->nr_cores > 1) {
> -            *eax = (env->nr_cores - 1) << 26;
> +        if (cs->nr_cores > 1) {
> +            *eax = (cs->nr_cores - 1) << 26;
>          } else {
>              *eax = 0;
>          }
> @@ -1702,8 +1702,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>                  break;
>              case 2: /* L2 cache info */
>                  *eax |= 0x0000143;
> -                if (env->nr_threads > 1) {
> -                    *eax |= (env->nr_threads - 1) << 14;
> +                if (cs->nr_threads > 1) {
> +                    *eax |= (cs->nr_threads - 1) << 14;
>                  }
>                  *ebx = 0x3c0003f;
>                  *ecx = 0x0000fff;
> @@ -1807,7 +1807,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>           * discards multiple thread information if it is set.
>           * So dont set it here for Intel to make Linux guests happy.
>           */
> -        if (env->nr_cores * env->nr_threads > 1) {
> +        if (cs->nr_cores * cs->nr_threads > 1) {
>              uint32_t tebx, tecx, tedx;
>              get_cpuid_vendor(env, &tebx, &tecx, &tedx);
>              if (tebx != CPUID_VENDOR_INTEL_1 ||
> @@ -1855,8 +1855,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          *ebx = 0;
>          *ecx = 0;
>          *edx = 0;
> -        if (env->nr_cores * env->nr_threads > 1) {
> -            *ecx |= (env->nr_cores * env->nr_threads) - 1;
> +        if (cs->nr_cores * cs->nr_threads > 1) {
> +            *ecx |= (cs->nr_cores * cs->nr_threads) - 1;
>          }
>          break;
>      case 0x8000000A:
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 2972ae3..f9f2b23 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -582,8 +582,9 @@ static inline void mips_tc_sleep(MIPSCPU *cpu, int tc)
>            walking the list of CPUMIPSStates.  */
>  static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc)
>  {
> +    CPUState *cs;
>      CPUMIPSState *other;
> -    int vpe_idx, nr_threads = env->nr_threads;
> +    int vpe_idx;
>      int tc_idx = *tc;
>  
>      if (!(env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP))) {
> @@ -592,8 +593,9 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, 
> int *tc)
>          return env;
>      }
>  
> -    vpe_idx = tc_idx / nr_threads;
> -    *tc = tc_idx % nr_threads;
> +    cs = CPU(mips_env_get_cpu(env));
> +    vpe_idx = tc_idx / cs->nr_threads;
> +    *tc = tc_idx % cs->nr_threads;
>      other = qemu_get_cpu(vpe_idx);
>      return other ? other : env;
>  }
> -- 
> 1.7.10.4
> 
> 


-- 
Regards,
  Igor