| Message ID | 146741293285.948.2129430864564241483.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jul 02, 2016 at 12:42:12AM +0200, Greg Kurz wrote: > Now that every supported machine type is able to provide a cpu_index, we > can safely move all the cpu_dt_id bits to the machine code. > > TODO: the cpu_dt_id logic remains the same wannabe generic one as before > because of its target code background: machine types should provide their > own cpu_dt_id logic (it is required by the future powernv machine type for > example). > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/ppc/ppc.c | 28 +++++++++++++++++++++++++++- > target-ppc/translate_init.c | 30 ------------------------------ > 2 files changed, 27 insertions(+), 31 deletions(-) > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index 12de255fb211..506b493bf43b 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -1354,7 +1354,33 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) > > void ppc_set_vcpu_dt_id(PowerPCCPU *cpu, int cpu_index, Error **errp) > { > - ; > + int max_smt = kvmppc_smt_threads(); > + int vcpu_dt_id; > + > + if (smp_threads > max_smt) { > + error_setg(errp, "Cannot support more than %d threads on PPC with %s", > + max_smt, kvm_enabled() ? "KVM" : "TCG"); > + return; > + } > + if (!is_power_of_2(smp_threads)) { > + error_setg(errp, "Cannot support %d threads on PPC with %s, " > + "threads count must be a power of 2.", > + smp_threads, kvm_enabled() ? "KVM" : "TCG"); > + return; > + } Not sure if the above two checks belong here in the routine which set the cpu_dt_id. Regards, Bharata.
On Sat, 2 Jul 2016 13:45:44 +0530 Bharata B Rao <bharata@linux.vnet.ibm.com> wrote: > On Sat, Jul 02, 2016 at 12:42:12AM +0200, Greg Kurz wrote: > > Now that every supported machine type is able to provide a cpu_index, we > > can safely move all the cpu_dt_id bits to the machine code. > > > > TODO: the cpu_dt_id logic remains the same wannabe generic one as before > > because of its target code background: machine types should provide their > > own cpu_dt_id logic (it is required by the future powernv machine type for > > example). > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/ppc/ppc.c | 28 +++++++++++++++++++++++++++- > > target-ppc/translate_init.c | 30 ------------------------------ > > 2 files changed, 27 insertions(+), 31 deletions(-) > > > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > > index 12de255fb211..506b493bf43b 100644 > > --- a/hw/ppc/ppc.c > > +++ b/hw/ppc/ppc.c > > @@ -1354,7 +1354,33 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) > > > > void ppc_set_vcpu_dt_id(PowerPCCPU *cpu, int cpu_index, Error **errp) > > { > > - ; > > + int max_smt = kvmppc_smt_threads(); > > + int vcpu_dt_id; > > + > > + if (smp_threads > max_smt) { > > + error_setg(errp, "Cannot support more than %d threads on PPC with %s", > > + max_smt, kvm_enabled() ? "KVM" : "TCG"); > > + return; > > + } > > + if (!is_power_of_2(smp_threads)) { > > + error_setg(errp, "Cannot support %d threads on PPC with %s, " > > + "threads count must be a power of 2.", > > + smp_threads, kvm_enabled() ? "KVM" : "TCG"); > > + return; > > + } > > Not sure if the above two checks belong here in the routine which > set the cpu_dt_id. > I'm pretty sure they don't belong here :) ! But again, this is the very same code that we currently have in the target and I don't want to change behavior here (maybe I should mention it in the changelog). This can be fixed in a followup patch. > Regards, > Bharata. > Thanks for your feedback Bharata ! Cheers. -- Greg
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index 12de255fb211..506b493bf43b 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -1354,7 +1354,33 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) void ppc_set_vcpu_dt_id(PowerPCCPU *cpu, int cpu_index, Error **errp) { - ; + int max_smt = kvmppc_smt_threads(); + int vcpu_dt_id; + + if (smp_threads > max_smt) { + error_setg(errp, "Cannot support more than %d threads on PPC with %s", + max_smt, kvm_enabled() ? "KVM" : "TCG"); + return; + } + if (!is_power_of_2(smp_threads)) { + error_setg(errp, "Cannot support %d threads on PPC with %s, " + "threads count must be a power of 2.", + smp_threads, kvm_enabled() ? "KVM" : "TCG"); + return; + } + + vcpu_dt_id = (cpu_index / smp_threads) * max_smt + + (cpu_index % smp_threads); + + if (kvm_enabled() && !kvm_vcpu_id_is_valid(vcpu_dt_id)) { + error_setg(errp, "Can't create CPU with id %d in KVM", vcpu_dt_id); + error_append_hint(errp, "Adjust the number of cpus to %d " + "or try to raise the number of threads per core\n", + vcpu_dt_id * smp_threads / max_smt); + return; + } + + cpu->cpu_dt_id = vcpu_dt_id; } PowerPCCPU *ppc_cpu_init(const char *cpu_model, int cpu_index) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 6706787b41a1..a54845a5be8f 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -9515,23 +9515,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) PowerPCCPU *cpu = POWERPC_CPU(dev); PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); Error *local_err = NULL; -#if !defined(CONFIG_USER_ONLY) - int max_smt = kvmppc_smt_threads(); -#endif - -#if !defined(CONFIG_USER_ONLY) - if (smp_threads > max_smt) { - error_setg(errp, "Cannot support more than %d threads on PPC with %s", - max_smt, kvm_enabled() ? "KVM" : "TCG"); - return; - } - if (!is_power_of_2(smp_threads)) { - error_setg(errp, "Cannot support %d threads on PPC with %s, " - "threads count must be a power of 2.", - smp_threads, kvm_enabled() ? "KVM" : "TCG"); - return; - } -#endif cpu_exec_init(cs, &local_err); if (local_err != NULL) { @@ -9539,19 +9522,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) return; } -#if !defined(CONFIG_USER_ONLY) - cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt - + (cs->cpu_index % smp_threads); - - if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) { - error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id); - error_append_hint(errp, "Adjust the number of cpus to %d " - "or try to raise the number of threads per core\n", - cpu->cpu_dt_id * smp_threads / max_smt); - return; - } -#endif - if (tcg_enabled()) { if (ppc_fixup_cpu(cpu) != 0) { error_setg(errp, "Unable to emulate selected CPU with TCG");
Now that every supported machine type is able to provide a cpu_index, we can safely move all the cpu_dt_id bits to the machine code. TODO: the cpu_dt_id logic remains the same wannabe generic one as before because of its target code background: machine types should provide their own cpu_dt_id logic (it is required by the future powernv machine type for example). Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/ppc.c | 28 +++++++++++++++++++++++++++- target-ppc/translate_init.c | 30 ------------------------------ 2 files changed, 27 insertions(+), 31 deletions(-)