Re: [Qemu-devel] [PATCH v2 7/7] ppc: move the cpu_dt_id logic to machine code

[Greg Kurz <groug@kaod.org>]

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