Re: [Qemu-devel] [RFC PATCH v0 3/6] spapr: Represent boot CPUs as spapr-cpu-core devices

[David Gibson]

On Mon, Feb 29, 2016 at 11:05:32AM +0530, Bharata B Rao wrote:
> On Fri, Feb 26, 2016 at 04:18:57PM +0100, Igor Mammedov wrote:
> > On Thu, 25 Feb 2016 21:52:39 +0530
> > Bharata B Rao <address@hidden> wrote:
[snip]
> > > @@ -2209,6 +2251,7 @@ static void 
> > > spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > >                                        DeviceState *dev, Error **errp)
> > >  {
> > >      sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> > > +    sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> > >  
> > >      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> > >          int node;
> > > @@ -2245,6 +2288,11 @@ static void 
> > > spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > >          }
> > >  
> > >          spapr_memory_plug(hotplug_dev, dev, node, errp);
> > > +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> > here probably should be CORE and not TYPE_CPU,
> > then if board needs to set some state for child threads it
> > could ennumerate child of core here and do the required job.
> 
> Hmm, we have things to set for CPUs and cores as well from hotplug
> handler. So the above code is for CPUs and I have spapr_core_plug()
> which is called conditionally when device is of type TYPE_SPAPR_CPU_CORE.
> 
> Given that ->plug() is called during realization of both individual CPU
> threads as well as their parent core, I thought handling the setup for
> them separately like the above is simpler, no ?

So, this bothered me a bit as well.  I think having the hotplug
handler for the threads is misleading, since it suggests you can
individually hotplug them, when in fact the intention is just that
this is part of the hotplug code path for a whole core.

My suggestion earlier was to have the core realize code itself perform
this configuration on the sub-threads.  However, Igor's suggestion of,
essentially, an explicit loop over the threads in the core hotplug
handler would be fine by me also.

-- 
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