Re: [Qemu-devel] [RFC 08/17] ppc: Rewrite ppc_get_compat_smt_threads()

[David Gibson]

On Fri, Nov 04, 2016 at 02:37:18PM +1100, Alexey Kardashevskiy wrote:
> On 30/10/16 22:11, David Gibson wrote:
> > To continue consolidation of compatibility mode information, this rewrites
> > the ppc_get_compat_smt_threads() function using the table of compatiblity
> > modes in target-ppc/compat.c.
> > 
> > It's not a direct replacement, the new ppc_compat_max_threads() function
> > has simpler semantics - it just returns the number of threads the cpu
> > model has, taking into account any compatiblity mode it is in.
> > 
> > This no longer takes into account kvmppc_smt_threads() as the previous
> > version did.  That check wasn't useful because we check elsewhere that
> 
> Nit: s/elsewhere/in ppc_cpu_realizefn()/

Changed.

> 
> 
> Reviewed-by: Alexey Kardashevskiy <address@hidden>
> 
> 
> 
> 
> > CPUs aren't instantiated with more threads than kvm allows (or if we didn't
> > things will already be broken and this won't make it any worse).
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> >  hw/ppc/spapr.c              |  8 ++++----
> >  target-ppc/compat.c         | 18 ++++++++++++++++++
> >  target-ppc/cpu.h            |  2 +-
> >  target-ppc/translate_init.c | 20 --------------------
> >  4 files changed, 23 insertions(+), 25 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 276cefa..6c78889 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -207,6 +207,7 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> > sPAPRMachineState *spapr)
> >          PowerPCCPU *cpu = POWERPC_CPU(cs);
> >          DeviceClass *dc = DEVICE_GET_CLASS(cs);
> >          int index = ppc_get_vcpu_dt_id(cpu);
> > +        int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
> >  
> >          if ((index % smt) != 0) {
> >              continue;
> > @@ -241,8 +242,7 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> > sPAPRMachineState *spapr)
> >              return ret;
> >          }
> >  
> > -        ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu,
> > -                                     ppc_get_compat_smt_threads(cpu));
> > +        ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt);
> >          if (ret < 0) {
> >              return ret;
> >          }
> > @@ -408,6 +408,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> > *fdt, int offset,
> >      size_t page_sizes_prop_size;
> >      uint32_t vcpus_per_socket = smp_threads * smp_cores;
> >      uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
> > +    int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
> >      sPAPRDRConnector *drc;
> >      sPAPRDRConnectorClass *drck;
> >      int drc_index;
> > @@ -495,8 +496,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> > *fdt, int offset,
> >  
> >      _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cs));
> >  
> > -    _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu,
> > -                                ppc_get_compat_smt_threads(cpu)));
> > +    _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
> >  }
> >  
> >  static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState 
> > *spapr)
> > diff --git a/target-ppc/compat.c b/target-ppc/compat.c
> > index f3fd9c6..66529a6 100644
> > --- a/target-ppc/compat.c
> > +++ b/target-ppc/compat.c
> > @@ -28,6 +28,7 @@
> >  typedef struct {
> >      uint32_t pvr;
> >      uint64_t pcr;
> > +    int max_threads;
> >  } CompatInfo;
> >  
> >  static const CompatInfo compat_table[] = {
> > @@ -35,18 +36,22 @@ static const CompatInfo compat_table[] = {
> >          .pvr = CPU_POWERPC_LOGICAL_2_05,
> >          .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
> >                 | PCR_TM_DIS | PCR_VSX_DIS,
> > +        .max_threads = 2,
> >      },
> >      { /* POWER7, ISA2.06 */
> >          .pvr = CPU_POWERPC_LOGICAL_2_06,
> >          .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
> > +        .max_threads = 4,
> >      },
> >      {
> >          .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
> >          .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
> > +        .max_threads = 4,
> >      },
> >      { /* POWER8, ISA2.07 */
> >          .pvr = CPU_POWERPC_LOGICAL_2_07,
> >          .pcr = PCR_COMPAT_2_07,
> > +        .max_threads = 8,
> >      },
> >  };
> >  
> > @@ -89,3 +94,16 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t 
> > compat_pvr, Error **errp)
> >          }
> >      }
> >  }
> > +
> > +int ppc_compat_max_threads(PowerPCCPU *cpu)
> > +{
> > +    const CompatInfo *compat = compat_by_pvr(cpu->compat_pvr);
> > +    int n_threads = CPU(cpu)->nr_threads;
> > +
> > +    if (cpu->compat_pvr) {
> > +        g_assert(compat);
> > +        n_threads = MIN(n_threads, compat->max_threads);
> > +    }
> > +
> > +    return n_threads;
> > +}
> > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> > index 15d5e4b..cfda7b2 100644
> > --- a/target-ppc/cpu.h
> > +++ b/target-ppc/cpu.h
> > @@ -1241,7 +1241,6 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong 
> > value);
> >  void ppc_store_msr (CPUPPCState *env, target_ulong value);
> >  
> >  void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
> > -int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
> >  #if defined(TARGET_PPC64)
> >  #endif
> >  
> > @@ -1316,6 +1315,7 @@ static inline int cpu_mmu_index (CPUPPCState *env, 
> > bool ifetch)
> >  /* Compatibility modes */
> >  #if defined(TARGET_PPC64)
> >  void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
> > +int ppc_compat_max_threads(PowerPCCPU *cpu);
> >  #endif /* defined(TARGET_PPC64) */
> >  
> >  #include "exec/cpu-all.h"
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index a70eafb..ba48242 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -9952,26 +9952,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, 
> > Error **errp)
> >      }
> >  }
> >  
> > -int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
> > -{
> > -    CPUState *cs = CPU(cpu);
> > -    int ret = MIN(cs->nr_threads, kvmppc_smt_threads());
> > -
> > -    switch (cpu->compat_pvr) {
> > -    case CPU_POWERPC_LOGICAL_2_05:
> > -        ret = MIN(ret, 2);
> > -        break;
> > -    case CPU_POWERPC_LOGICAL_2_06:
> > -        ret = MIN(ret, 4);
> > -        break;
> > -    case CPU_POWERPC_LOGICAL_2_07:
> > -        ret = MIN(ret, 8);
> > -        break;
> > -    }
> > -
> > -    return ret;
> > -}
> > -
> >  static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
> >  {
> >      ObjectClass *oc = (ObjectClass *)a;
> > 
> 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature