Re: [Qemu-ppc] [PATCH RFC 2/4] PPC: KVM: Move kvm_cap_smt into sPAPRMachineState

[Greg Kurz]

On Tue, 27 Jun 2017 10:22:37 +1000
Sam Bobroff <address@hidden> wrote:

> PPC QEMU currenlty tracks the host's SMT mode via a KVM capability
> (KVM_CAP_PPC_SMT) and this value is cached in target/ppc/kvm.c.
> 
> This patch moves that tracking into the sPAPRMachineState without
> functional changes (and changes the name of kvmppc_cap_smt() to
> kvmppc_get_cap_smt() to make it clear that it now fetches directly).
> 
> This will facilitate further work introducing virtual SMT modes.
> 
> Signed-off-by: Sam Bobroff <address@hidden>
> ---
>  hw/ppc/spapr.c              | 14 ++++++++------
>  include/hw/ppc/spapr.h      |  1 +
>  target/ppc/kvm.c            |  6 ++----
>  target/ppc/kvm_ppc.h        |  7 +------
>  target/ppc/translate_init.c |  2 +-
>  5 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ede5167bc0..b33dc1b5c0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -285,7 +285,7 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> sPAPRMachineState *spapr)
>      int ret = 0, offset, cpus_offset;
>      CPUState *cs;
>      char cpu_model[32];
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>      uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
>  
>      CPU_FOREACH(cs) {
> @@ -563,7 +563,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, 
> sPAPRMachineState *spapr)
>      CPUState *cs;
>      int cpus_offset;
>      char *nodename;
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>  
>      cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
>      _FDT(cpus_offset);
> @@ -979,7 +979,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      void *fdt;
>      sPAPRPHBState *phb;
>      char *buf;
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>  
>      fdt = g_malloc0(FDT_MAX_SIZE);
>      _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
> @@ -1975,7 +1975,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
>      MachineState *machine = MACHINE(spapr);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      char *type = spapr_get_cpu_core_type(machine->cpu_model);
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>      const CPUArchIdList *possible_cpus;
>      int boot_cores_nr = smp_cpus / smp_threads;
>      int i;
> @@ -2489,6 +2489,7 @@ static void spapr_machine_initfn(Object *obj)
>  
>      spapr->htab_fd = -1;
>      spapr->use_hotplug_event_source = true;
> +    spapr->vsmt = (kvm_enabled() ? kvmppc_get_smt_threads() : 1);

Parenthesitis ? :)

>      object_property_add_str(obj, "kvm-type",
>                              spapr_get_kvm_type, spapr_set_kvm_type, NULL);
>      object_property_set_description(obj, "kvm-type",
> @@ -2827,11 +2828,12 @@ static
>  void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                 Error **errp)
>  {
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
>      int index;
>      sPAPRDRConnector *drc;
>      Error *local_err = NULL;
>      CPUCore *cc = CPU_CORE(dev);
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>  
>      if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
>          error_setg(errp, "Unable to find CPU core with core-id: %d",
> @@ -2867,7 +2869,7 @@ static void spapr_core_plug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>      Error *local_err = NULL;
>      void *fdt = NULL;
>      int fdt_offset = 0;
> -    int smt = kvmppc_smt_threads();
> +    int smt = spapr->vsmt;
>      CPUArchId *core_slot;
>      int index;
>  
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index f973b02845..1ddbe0b81a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -90,6 +90,7 @@ struct sPAPRMachineState {
>      sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
>      bool cas_reboot;
>      bool cas_legacy_guest_workaround;
> +    uint32_t vsmt;                  /* Virtual SMT mode (KVM's "core 
> stride") */
>  
>      Notifier epow_notifier;
>      QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index f2f7c531bc..74dfab95eb 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -72,7 +72,6 @@ static int cap_interrupt_unset = false;
>  static int cap_interrupt_level = false;
>  static int cap_segstate;
>  static int cap_booke_sregs;
> -static int cap_ppc_smt;
>  static int cap_ppc_rma;
>  static int cap_spapr_tce;
>  static int cap_spapr_tce_64;
> @@ -127,7 +126,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_interrupt_level = kvm_check_extension(s, KVM_CAP_PPC_IRQ_LEVEL);
>      cap_segstate = kvm_check_extension(s, KVM_CAP_PPC_SEGSTATE);
>      cap_booke_sregs = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_SREGS);
> -    cap_ppc_smt = kvm_check_extension(s, KVM_CAP_PPC_SMT);
>      cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
>      cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
>      cap_spapr_tce_64 = kvm_check_extension(s, KVM_CAP_SPAPR_TCE_64);
> @@ -2118,9 +2116,9 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int 
> mpic_proxy)
>      }
>  }
>  
> -int kvmppc_smt_threads(void)
> +int kvmppc_get_smt_threads(void)
>  {
> -    return cap_ppc_smt ? cap_ppc_smt : 1;
> +    return kvm_vm_check_extension(kvm_state, KVM_CAP_PPC_SMT);
>  }
>  
>  #ifdef TARGET_PPC64
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index eab7c8fdb3..cdc1407462 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -28,7 +28,7 @@ void kvmppc_enable_clear_ref_mod_hcalls(void);
>  void kvmppc_set_papr(PowerPCCPU *cpu);
>  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> -int kvmppc_smt_threads(void);
> +int kvmppc_get_smt_threads(void);
>  int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
>  int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
>  int kvmppc_set_tcr(PowerPCCPU *cpu);
> @@ -138,11 +138,6 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU 
> *cpu, int mpic_proxy)
>  {
>  }
>  
> -static inline int kvmppc_smt_threads(void)
> -{
> -    return 1;
> -}
> -

Don't we need a stub for kvmppc_get_smt_threads() if !CONFIG_KVM ?

>  static inline int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits)
>  {
>      return 0;
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 56a0ab22cf..0d3da9d746 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -9827,7 +9827,7 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>      Error *local_err = NULL;
>  #if !defined(CONFIG_USER_ONLY)
> -    int max_smt = kvmppc_smt_threads();
> +    int max_smt = kvmppc_get_smt_threads();

And so we will use KVM_CAP_PPC_SMT even in TCG mode ?

Maybe the helper should do the following:

int kvmppc_get_smt_threads(void)
{
    return kvm_enabled() ? kvm_vm_check_extension(kvm_state, KVM_CAP_PPC_SMT) : 
1;
}

>  #endif
>  
>  #if !defined(CONFIG_USER_ONLY)

pgpdFfqwMzvzY.pgp
Description: OpenPGP digital signature