Re: [PATCH v2 5/7] spapr: TCG allow up to 8-thread SMT on POWER8 and newer CPUs

[Cédric Le Goater]

On 6/22/23 12:06, Cédric Le Goater wrote:
> On 6/22/23 11:33, Nicholas Piggin wrote:
> > PPC TCG supports SMT CPU configurations for non-hypervisor state, so
> > permit POWER8-10 pseries machines to enable SMT.
> >
> > This requires PIR and TIR be set, because that's how sibling thread
> > matching is done by TCG.
> >
> > spapr's nested-HV capability does not currently coexist with SMT, so
> > that combination is prohibited (interestingly somewhat analogous to
> > LPAR-per-core mode on real hardware which also does not support KVM).
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/spapr.c          | 16 ++++++++++++----
> >   hw/ppc/spapr_caps.c     | 14 ++++++++++++++
> >   hw/ppc/spapr_cpu_core.c |  7 +++++--
> >   3 files changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8e7d497f25..677b5eef9d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2525,10 +2525,18 @@ static void spapr_set_vsmt_mode(SpaprMachineState 
> > *spapr, Error **errp)
> >       int ret;
> >       unsigned int smp_threads = ms->smp.threads;
> > -    if (tcg_enabled() && (smp_threads > 1)) {
> > -        error_setg(errp, "TCG cannot support more than 1 thread/core "
> > -                   "on a pseries machine");
> > -        return;
> > +    if (tcg_enabled()) {

I will add :

            if (smp_threads > 1 &&

No need to resend for that.

C.


> > +        if (!ppc_type_check_compat(ms->cpu_type, CPU_POWERPC_LOGICAL_2_07, 0,
> > +                                   spapr->max_compat_pvr)) {
> > +            error_setg(errp, "TCG only supports SMT on POWER8 or newer CPUs");
> > +            return;
> > +        }
>
> So if TCG is enabled and the CPU < P8, QEMU bails out ? You should run
> qemu-ppc-boot :)
>
> Thanks,
>
> C.
>
> > +
> > +        if (smp_threads > 8) {
> > +            error_setg(errp, "TCG cannot support more than 8 threads/core "
> > +                       "on a pseries machine");
> > +            return;
> > +        }
> >       }
> >       if (!is_power_of_2(smp_threads)) {
> >           error_setg(errp, "Cannot support %d threads/core on a pseries "
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index 3fd45a6dec..5a0755d34f 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -473,6 +473,20 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState 
> > *spapr,
> >                   error_append_hint(errp,
> >                                     "Try appending -machine 
> > cap-nested-hv=off\n");
> >           }
> > +    } else if (tcg_enabled()) {
> > +        MachineState *ms = MACHINE(spapr);
> > +        unsigned int smp_threads = ms->smp.threads;
> > +
> > +        /*
> > +         * Nested-HV vCPU env state to L2, so SMT-shared SPR updates, for
> > +         * example, do not necessarily update the correct SPR value on sibling
> > +         * threads that are in a different guest/host context.
> > +         */
> > +        if (smp_threads > 1) {
> > +            error_setg(errp, "TCG does not support nested-HV with SMT");
> > +            error_append_hint(errp, "Try appending -machine cap-nested-hv=off "
> > +                                    "or use threads=1 with -smp\n");
> > +        }
> >       }
> >   }
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 9b88dd549a..a4e3c2fadd 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -255,7 +255,7 @@ static void spapr_cpu_core_unrealize(DeviceState *dev)
> >   }
> >   static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
> > -                               SpaprCpuCore *sc, Error **errp)
> > +                               SpaprCpuCore *sc, int thread_index, Error 
> > **errp)
> >   {
> >       CPUPPCState *env = &cpu->env;
> >       CPUState *cs = CPU(cpu);
> > @@ -267,6 +267,9 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, 
> > SpaprMachineState *spapr,
> >       cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
> >       kvmppc_set_papr(cpu);
> > +    env->spr_cb[SPR_PIR].default_value = cs->cpu_index;
> > +    env->spr_cb[SPR_TIR].default_value = thread_index;
> > +
> >       /* Set time-base frequency to 512 MHz. vhyp must be set first. */
> >       cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
> > @@ -337,7 +340,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
> > **errp)
> >       for (i = 0; i < cc->nr_threads; i++) {
> >           sc->threads[i] = spapr_create_vcpu(sc, i, errp);
> >           if (!sc->threads[i] ||
> > -            !spapr_realize_vcpu(sc->threads[i], spapr, sc, errp)) {
> > +            !spapr_realize_vcpu(sc->threads[i], spapr, sc, i, errp)) {
> >               spapr_cpu_core_unrealize(dev);
> >               return;
> >           }