Re: [Qemu-devel] CPU hotplug, again

[David Gibson]

On Tue, Feb 23, 2016 at 03:10:26PM +0530, Bharata B Rao wrote:
> On Tue, Feb 23, 2016 at 04:24:31PM +1100, David Gibson wrote:
> > Hi Andreas,
> > 
> > I've now found (with Thomas' help) your RFC series for socket/core
> > based cpu hotplug on x86
> > (https://github.com/afaerber/qemu-cpu/compare/qom-cpu-x86).  It seems
> > sensible enough as far as it goes, but doesn't seem to address a bunch
> > of the things that I was attempting to do with the cpu-package
> > proposal - and which we absolutely need for cpu hotplug on Power.
> > 
> > 1) What interface do you envisage beyond cpu_add?
> > 
> > The patches I see just construct extra socket and core objects, but
> > still control hotplug (for x86) through the cpu_add interface.  That
> > interface is absolutely unusable on Power, since it operates on a
> > per-thread basis, whereas the PAPR guest<->host interfaces can only
> > communicate information at a per-core granularity.
> > 
> > 2) When hotplugging at core or socket granularity, where would the
> >    code to construct the individual thread objects sit?
> > 
> > Your series has the construction done in both the machine init path
> > and the hotplug path.  The latter works because hotplug occurs at
> > thread granularity.  If we're hotplugging at core or socket
> > granularity what would do the construct?  The core/socket object
> > itself (in instance_init?  in realize?); the hotplug handler?
> > something else?
> > 
> > 3) How does the management layer determine what is pluggable?
> > 
> > Both the number of pluggable slots, and what it will need to do to
> > populate them.
> > 
> > 4) How do we enforce that toplogies illegal for the platform can't be
> >    constructed?
> 
> 5) QOM-links
> 
> Andreas, You have often talked about setting up links from machine object
> to the CPU objects. Would the below code correctly capture that idea of
> yours ?
> 
> #define SPAPR_MACHINE_CPU_CORE_PROP "core"
> 
> /* MachineClass.init for sPAPR */
> static void ppc_spapr_init(MachineState *machine)
> {
>     sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>     int spapr_smp_cores = smp_cpus / smp_threads;
>     int spapr_max_cores = max_cpus / smp_threads;
> 
>     ...
>     for (i = 0; i < spapr_max_cores; i++) {
>         Object *obj = object_new(TYPE_SPAPR_CPU_CORE);
>         sPAPRCPUCore *core = SPAPR_CPU_CORE(obj);
>         char name[32];
> 
>         snprintf(name, sizeof(name), "%s[%d]", SPAPR_MACHINE_CPU_CORE_PROP, 
> i);
> 
>         /*
>          * Create links from machine objects to all possible cores.
>          */
>         object_property_add_link(OBJECT(spapr), name, TYPE_SPAPR_CPU_CORE,
>                                  (Object **)&spapr->core[i],
>                                  NULL, NULL, &error_abort); 
> 
>         /*
>          * Set the QOM link from machine object to core object for all
>          * boot time CPUs specified with -smp. For rest of the hotpluggable
>          * cores this is done from the core hotplug path.
>          */
>         if (i < spapr_smp_cores) {
>             object_property_set_link(OBJECT(spapr), OBJECT(core),
>                                      SPAPR_MACHINE_CPU_CORE_PROP, 
> &error_abort);

I hope we can at least have a helper function to both construct the
core and create the links, if we can't handle the link creation in the
core object itself.

Having to open-code it in each machine sounds like a recipe for subtle
differences in presentation between platforms, which is exactly what
we want to avoid.

>         }
>     ...
> }
> 

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