| Message ID | 20230531012313.19891-6-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: initial SMT support in TCG | expand |
On 5/31/23 03:23, Nicholas Piggin wrote: > TCG now supports multi-threaded configuration at least enough for > pseries to be functional enough to boot Linux. > > This requires PIR and TIR be set, because that's how sibling thread > matching is done. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/ppc/spapr.c | 4 ++-- > hw/ppc/spapr_cpu_core.c | 7 +++++-- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index dcb7f1c70a..11074cefea 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2524,8 +2524,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) > int ret; > unsigned int smp_threads = ms->smp.threads; > > - if (!kvm_enabled() && (smp_threads > 1)) { > - error_setg(errp, "TCG cannot support more than 1 thread/core " > + if (!kvm_enabled() && (smp_threads > 8)) { > + error_setg(errp, "TCG cannot support more than 8 threads/core " > "on a pseries machine"); I think we should add test on the CPU also. > return; > } > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 9b88dd549a..f35ee600f1 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_nr, Error **errp) thread_index may be ? > { > 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); so, spapr_create_vcpu() set cs->cpu_index : cs->cpu_index = cc->core_id + i; and spapr_realize_vcpu : > + env->spr_cb[SPR_PIR].default_value = cs->cpu_index; > + env->spr_cb[SPR_TIR].default_value = thread_nr; > + it would be cleaner to do the SPR assignment in one place. > /* 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; > }
On Thu Jun 1, 2023 at 5:20 PM AEST, Cédric Le Goater wrote: > On 5/31/23 03:23, Nicholas Piggin wrote: > > TCG now supports multi-threaded configuration at least enough for > > pseries to be functional enough to boot Linux. > > > > This requires PIR and TIR be set, because that's how sibling thread > > matching is done. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > hw/ppc/spapr.c | 4 ++-- > > hw/ppc/spapr_cpu_core.c | 7 +++++-- > > 2 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index dcb7f1c70a..11074cefea 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2524,8 +2524,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) > > int ret; > > unsigned int smp_threads = ms->smp.threads; > > > > - if (!kvm_enabled() && (smp_threads > 1)) { > > - error_setg(errp, "TCG cannot support more than 1 thread/core " > > + if (!kvm_enabled() && (smp_threads > 8)) { > > + error_setg(errp, "TCG cannot support more than 8 threads/core " > > "on a pseries machine"); > > I think we should add test on the CPU also. On the CPU type, POWER7 can have 1/2/4, POWER8 can have 1/2/4/8? POWER9 could also switch PVR between big and small core depending on whether you select SMT8 I suppose. > > return; > > } > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 9b88dd549a..f35ee600f1 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_nr, Error **errp) > > thread_index may be ? Sure. > > { > > 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); > > so, spapr_create_vcpu() set cs->cpu_index : > cs->cpu_index = cc->core_id + i; > > and spapr_realize_vcpu : > > > + env->spr_cb[SPR_PIR].default_value = cs->cpu_index; > > + env->spr_cb[SPR_TIR].default_value = thread_nr; > > + > it would be cleaner to do the SPR assignment in one place. I'll try that, it sounds good. Thanks, Nick
On 6/2/23 08:59, Nicholas Piggin wrote: > On Thu Jun 1, 2023 at 5:20 PM AEST, Cédric Le Goater wrote: >> On 5/31/23 03:23, Nicholas Piggin wrote: >>> TCG now supports multi-threaded configuration at least enough for >>> pseries to be functional enough to boot Linux. >>> >>> This requires PIR and TIR be set, because that's how sibling thread >>> matching is done. >>> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> --- >>> hw/ppc/spapr.c | 4 ++-- >>> hw/ppc/spapr_cpu_core.c | 7 +++++-- >>> 2 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index dcb7f1c70a..11074cefea 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -2524,8 +2524,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) >>> int ret; >>> unsigned int smp_threads = ms->smp.threads; >>> >>> - if (!kvm_enabled() && (smp_threads > 1)) { >>> - error_setg(errp, "TCG cannot support more than 1 thread/core " >>> + if (!kvm_enabled() && (smp_threads > 8)) { >>> + error_setg(errp, "TCG cannot support more than 8 threads/core " >>> "on a pseries machine"); >> >> I think we should add test on the CPU also. > > On the CPU type, POWER7 can have 1/2/4, POWER8 can have 1/2/4/8? > POWER9 could also switch PVR between big and small core depending > on whether you select SMT8 I suppose. What I meant is to limit the support to the CPUs which will be most likely used : POWER8-10. I don't think we care much about the others P7, P5+, 970. Thanks, C.
On Thu Jun 1, 2023 at 5:20 PM AEST, Cédric Le Goater wrote: > On 5/31/23 03:23, Nicholas Piggin wrote: > > @@ -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); > > so, spapr_create_vcpu() set cs->cpu_index : > cs->cpu_index = cc->core_id + i; > > and spapr_realize_vcpu : > > > + env->spr_cb[SPR_PIR].default_value = cs->cpu_index; > > + env->spr_cb[SPR_TIR].default_value = thread_nr; > > + > it would be cleaner to do the SPR assignment in one place. Problem is we can't do it in create because the SPRs have not been registered yet, and can't move cpu_index to realize because it's needed earlier. A nice way to do this might be to have a cpu_index and a thread_index (or require that it is cpu_index % nr_threads), and use those values as the default when registering PIR and TIR SPRs. But I haven't quite looked into it far enough yet. pnv sets PIR in the realize function already so maybe it's okay this way for now and it can be tidied up. Thanks, Nick
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index dcb7f1c70a..11074cefea 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2524,8 +2524,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) int ret; unsigned int smp_threads = ms->smp.threads; - if (!kvm_enabled() && (smp_threads > 1)) { - error_setg(errp, "TCG cannot support more than 1 thread/core " + if (!kvm_enabled() && (smp_threads > 8)) { + error_setg(errp, "TCG cannot support more than 8 threads/core " "on a pseries machine"); return; } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 9b88dd549a..f35ee600f1 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_nr, 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_nr; + /* 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; }
TCG now supports multi-threaded configuration at least enough for pseries to be functional enough to boot Linux. This requires PIR and TIR be set, because that's how sibling thread matching is done. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- hw/ppc/spapr.c | 4 ++-- hw/ppc/spapr_cpu_core.c | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-)