Re: [Qemu-devel] [RFC PATCH v1 00/10] Core based CPU hotplug for PowerPC sPAPR

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 7204 bytes --]

On Fri, Mar 04, 2016 at 11:57:18AM +0100, Igor Mammedov wrote:
> On Fri,  4 Mar 2016 12:24:11 +0530
> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> 
> > Hi,
> > 
> > This is the next version of "Core based CPU hotplug for PowerPC sPAPR" that
> > was posted at
> > https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00286.html
> > 
> > Here is a quick summary on how this approach is different from the
> > previous approaches that I have been pursuing with the last one being
> > v7 that posted here:
> > https://lists.gnu.org/archive/html/qemu-ppc/2016-01/msg00477.html
> > 
> > - Earlier approaches used an independent PowerPC specific core device while
> >   this approach uses an sPAPR specific core device that is derived from
> >   generic core device.
> > - The earlier approach didn't have the notion of where the boot time as
> >   well as hot-plugged cores would sit. In this approach QOM links are
> >   created from machine object to all possible core objects. While the
> >   links are set for boot time cores during machine init, the same is done
> >   for hotplugged cores at hotplug time. The name of this link property
> >   is standardized as "core[N]" since these links link the machine object
> >   with core devices. The link name ("core[N]") is used with core device's
> >   "slot" property to identify the QOM link to set for this core.
> >   (qemu) device_add spapr-cpu-core,id=core2,nr_threads=8,cpu_model=host,slot=core[2]
> > - The ealier approach created threads from instance_init of the core object
> >   using a modified cpu_generic_init() routine. It used smp_threads and
> >   MachineState->cpu_model globals. In the current approach, nr_threads and
> >   cpu_model are obtained as properties and threads are created from
> >   property setters. The thread objects are allocated as part of core
> >   device and object_initialize() is used to initialize them.
> > 
> > Some open questions remain:
> > 
> > - Does this device_add semantics look ok ?
> >   (qemu) device_add spapr-cpu-core,id=core2,nr_threads=8,cpu_model=host,slot=core[2]
> looks fine to me, only I'd do following changes:
> s/nr_threads/threads/
> s/slot/core/ and make it numeric property
> 
> > - Is there need for machine to core object links ? If not, what would
> >   determine the slot/location of the core device ?
> I'd drop links altogether for this series as they were a part of an attempt
> to implement QOM based introspection interface, which is not must have
> provided QMP interface would provide all necessary for hotplug information
> which links aren't capable of to do.

Yeah, I tend to agree.

> Back in the days Anthony suggested that we 'might' use links to implement
> hotplug because there wasn't any other infrastructure hotplug
> infrastructure for bus-less devices. But now we have HOTPLUG_HANDLER
> interface that obsoleted, what links were supposed to handle at
> link set time hooks.
> 
> But somehow we stuck on links idea, though their initial role in hotplug
> were obsoleted and we still trying to fit them in design where
> they are not really necessary.
> 
> > - How to fit this with CPUSlotProperties HMP interface that Igor is
> >   working on ?
> I'll try to re-factor QMP interface RFC on top of this series
> so it would be easier for you to review it wrt spapr.
> 
> 
> > This version has the following changes:
> > 
> > - Dropped QMP and HMP enumeration patches for now since it isn't clear
> >   if the approach I took would be preferrable by all archs. Will wait
> >   and see how Igor's patches evolve here.
> > - Added the following pre-req patches to support CPU removal:
> >   exec: Remove cpu from cpus list during cpu_exec_exit()
> >   exec: Do vmstate unregistration from cpu_exec_exit()
> >   cpu: Reclaim vCPU objects
> >   cpu: Add a sync version of cpu_remove()
> > - Added sPAPR CPU hot removal support
> >   xics,xics_kvm: Handle CPU unplug correctly
> >   spapr: CPU hot unplug support
> > - Moved up the "slot" property into abstract cpu-core device.
> > - Recover when thread creation fails (spapr_cpu_core_create_threads())
> > - cpu_model property in spapr-cpu-core deivce is now tracked using
> >   ObjectClass pointer instead of string.
> > - Fail topologies with incomplete cores from within sPAPR's machine init.
> > - Fixes in spapr-cpu-core object creation from machine init to create
> >   only boottime CPUs.
> > - Moved board specific wiring code for CPU threads (spapr_cpu_init)
> >   into core's realize method to be called after each thread's realization.
> > - No specific action under TYPE_CPU in ->plug() handler either for hotplug
> >   or hot removal.
> > - Moved all core related CPU hotplug routines into spapr_cpu_core.c where
> >   it truly belongs.
> > - Setting of NUMA node for hotplugged CPUs moved to spapr_cpu_init()).
> > - Compared to previous implementation of hot removal that was part of
> >   different series earlier, this implementation moves all core removal
> >   logic into spapr_cpu_core.c.
> > - Some minor cleanups like use of g_new0 instead of g_malloc0 etc.
> > 
> > This patchset is present at:
> > https://github.com/bharata/qemu/commits/spapr-cpu-core
> > 
> > Bharata B Rao (9):
> >   exec: Remove cpu from cpus list during cpu_exec_exit()
> >   exec: Do vmstate unregistration from cpu_exec_exit()
> >   cpu: Add a sync version of cpu_remove()
> >   cpu: Abstract CPU core type
> >   spapr: CPU core device
> >   spapr: Represent boot CPUs as spapr-cpu-core devices
> >   spapr: CPU hotplug support
> >   xics,xics_kvm: Handle CPU unplug correctly
> >   spapr: CPU hot unplug support
> > 
> > Gu Zheng (1):
> >   cpu: Reclaim vCPU objects
> > 
> >  cpus.c                          |  50 ++++++
> >  exec.c                          |  44 ++++-
> >  hw/cpu/Makefile.objs            |   1 +
> >  hw/cpu/core.c                   |  44 +++++
> >  hw/intc/xics.c                  |  14 ++
> >  hw/intc/xics_kvm.c              |   8 +-
> >  hw/ppc/Makefile.objs            |   1 +
> >  hw/ppc/spapr.c                  | 174 +++++++++++++++++--
> >  hw/ppc/spapr_cpu_core.c         | 361 ++++++++++++++++++++++++++++++++++++++++
> >  hw/ppc/spapr_events.c           |   3 +
> >  hw/ppc/spapr_rtas.c             |  24 +++
> >  include/hw/cpu/core.h           |  30 ++++
> >  include/hw/ppc/spapr.h          |   9 +
> >  include/hw/ppc/spapr_cpu_core.h |  42 +++++
> >  include/hw/ppc/xics.h           |   1 +
> >  include/qom/cpu.h               |  18 ++
> >  include/sysemu/kvm.h            |   1 +
> >  kvm-all.c                       |  57 ++++++-
> >  kvm-stub.c                      |   5 +
> >  19 files changed, 862 insertions(+), 25 deletions(-)
> >  create mode 100644 hw/cpu/core.c
> >  create mode 100644 hw/ppc/spapr_cpu_core.c
> >  create mode 100644 include/hw/cpu/core.h
> >  create mode 100644 include/hw/ppc/spapr_cpu_core.h
> > 
> 
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]