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

[Igor Mammedov]

On Fri,  4 Mar 2016 12:24:11 +0530
Bharata B Rao <address@hidden> 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.

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
>