Re: [Qemu-devel] [PATCH v7 1/2] target-ppc: add PowerPCCPU::cpu_dt_id

[Mike Day]

On Sat, Feb 1, 2014 at 9:45 AM, Alexey Kardashevskiy <address@hidden> wrote:
> Normally CPUState::cpu_index is used to pick the right CPU for various
> operations. However default consecutive numbering does not always work
> for POWERPC.
>
> These indexes are reflected in /proc/device-tree/cpus/PowerPC,address@hidden
> and used to call KVM VCPU's ioctls. In order to achieve this,
> kvmppc_fixup_cpu() was introduced. Roughly speaking, it multiplies
> cpu_index by the number of threads per core.
>
> This approach has disadvantages such as:
> 1. NUMA configuration stays broken after the fixup;
> 2. CPU-targeted commands from the QEMU Monitor do not work properly as
> CPU indexes have been fixed and there is no clear way for the user to
> know what the new CPU indexes are.
>
> This introduces a @cpu_dt_id field in the CPUPPCState struct which
> is initialized from @cpu_index by default and can be fixed later
> to meet the device tree requirements.
>
> This adds an API to handle @cpu_dt_id.
>
> This removes kvmppc_fixup_cpu() as it is not more needed, @cpu_dt_id
> is calculated in ppc_cpu_realize().
>
> This will be used later in machine code.
>
Signed-off-by: Alexey Kardashevskiy <address@hidden>
Acked-by: Mike Day <address@hidden>

> ---
> Changes:
> v6: inlined kvmppc_fixup_cpu()
> ---
>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>  target-ppc/cpu-qom.h        |  2 ++
>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>  target-ppc/kvm.c            | 13 -------------
>  target-ppc/kvm_ppc.h        |  6 ------
>  target-ppc/translate_init.c | 10 ++++------
>  6 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 114be64..0e82719 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -26,6 +26,7 @@
>  #include "hw/ppc/ppc_e500.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  #include "hw/timer/m48t59.h"
>  #include "qemu/log.h"
>  #include "hw/loader.h"
> @@ -1362,3 +1363,24 @@ int PPC_NVRAM_set_params (nvram_t *nvram, uint16_t 
> NVRAM_size,
>
>      return 0;
>  }
> +
> +/* CPU device-tree ID helpers */
> +int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
> +{
> +    return cpu->cpu_dt_id;
> +}
> +
> +PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
> +{
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +        if (cpu->cpu_dt_id == cpu_dt_id) {
> +            return cpu;
> +        }
> +    }
> +
> +    return NULL;
> +}
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 72b2232..b17c024 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -79,6 +79,7 @@ typedef struct PowerPCCPUClass {
>  /**
>   * PowerPCCPU:
>   * @env: #CPUPPCState
> + * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
>   *
>   * A PowerPC CPU.
>   */
> @@ -88,6 +89,7 @@ typedef struct PowerPCCPU {
>      /*< public >*/
>
>      CPUPPCState env;
> +    int cpu_dt_id;
>  } PowerPCCPU;
>
>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 51bcd4a..d8577ae 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -2154,4 +2154,22 @@ static inline bool cpu_has_work(CPUState *cpu)
>
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
>
> +/**
> + * ppc_get_vcpu_dt_id:
> + * @cs: a PowerPCCPU struct.
> + *
> + * Returns a device-tree ID for a CPU.
> + */
> +int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
> +
> +/**
> + * ppc_get_vcpu_by_dt_id:
> + * @cpu_dt_id: a device tree id
> + *
> + * Searches for a CPU by @cpu_dt_id.
> + *
> + * Returns: a PowerPCCPU struct
> + */
> +PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
> +
>  #endif /* !defined (__CPU_PPC_H__) */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 781b72f..8bcc5fb 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1766,19 +1766,6 @@ static void kvmppc_host_cpu_class_init(ObjectClass 
> *oc, void *data)
>      }
>  }
>
> -int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> -{
> -    CPUState *cs = CPU(cpu);
> -    int smt;
> -
> -    /* Adjust cpu index for SMT */
> -    smt = kvmppc_smt_threads();
> -    cs->cpu_index = (cs->cpu_index / smp_threads) * smt
> -        + (cs->cpu_index % smp_threads);
> -
> -    return 0;
> -}
> -
>  bool kvmppc_has_cap_epr(void)
>  {
>      return cap_epr;
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 5f78e4b..f3afcdb 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -36,7 +36,6 @@ int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t 
> window_size);
>  int kvmppc_reset_htab(int shift_hint);
>  uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
>  #endif /* !CONFIG_USER_ONLY */
> -int kvmppc_fixup_cpu(PowerPCCPU *cpu);
>  bool kvmppc_has_cap_epr(void);
>  int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);
>  int kvmppc_get_htab_fd(bool write);
> @@ -155,11 +154,6 @@ static inline int kvmppc_update_sdr1(CPUPPCState *env)
>
>  #endif /* !CONFIG_USER_ONLY */
>
> -static inline int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> -{
> -    return -1;
> -}
> -
>  static inline bool kvmppc_has_cap_epr(void)
>  {
>      return false;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 35470d4..6de7126 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7808,14 +7808,12 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>                     max_smt, kvm_enabled() ? "KVM" : "TCG");
>          return;
>      }
> +
> +    cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
> +        + (cs->cpu_index % smp_threads);
>  #endif
>
> -    if (kvm_enabled()) {
> -        if (kvmppc_fixup_cpu(cpu) != 0) {
> -            error_setg(errp, "Unable to virtualize selected CPU with KVM");
> -            return;
> -        }
> -    } else if (tcg_enabled()) {
> +    if (tcg_enabled()) {
>          if (ppc_fixup_cpu(cpu) != 0) {
>              error_setg(errp, "Unable to emulate selected CPU with TCG");
>              return;
> --
> 1.8.4.rc4
>
>