Re: [Qemu-devel] [RFC PATCH v2 12/23] spapr: CPU hotplug support

From: David Gibson <david@gibson.dropbear.id.au>
To: Bharata B Rao <bharata@linux.vnet.ibm.com>
Cc: mdroth@linux.vnet.ibm.com, agraf@suse.de, qemu-devel@nongnu.org,
	qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com,
	nfont@linux.vnet.ibm.com, imammedo@redhat.com, afaerber@suse.de
Subject: Re: [Qemu-devel] [RFC PATCH v2 12/23] spapr: CPU hotplug support
Date: Thu, 26 Mar 2015 12:42:45 +1100	[thread overview]
Message-ID: <20150326014245.GJ28039@voom.redhat.com> (raw)
In-Reply-To: <20150325083629.GD32581@in.ibm.com>

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

On Wed, Mar 25, 2015 at 02:06:29PM +0530, Bharata B Rao wrote:
> On Wed, Mar 25, 2015 at 02:03:45PM +1100, David Gibson wrote:
> > On Mon, Mar 23, 2015 at 07:05:53PM +0530, Bharata B Rao wrote:
> > > Support CPU hotplug via device-add command. Set up device tree
> > > entries for the hotplugged CPU core and use the exising EPOW event
> > > infrastructure to send CPU hotplug notification to the guest.
> > > 
> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > ---
> > >  hw/ppc/spapr.c        | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  hw/ppc/spapr_events.c |  8 +++---
> > >  hw/ppc/spapr_rtas.c   | 11 ++++++++
> > >  3 files changed, 91 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index f52d38f..b48994b 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -33,6 +33,7 @@
> > >  #include "sysemu/block-backend.h"
> > >  #include "sysemu/cpus.h"
> > >  #include "sysemu/kvm.h"
> > > +#include "sysemu/device_tree.h"
> > >  #include "kvm_ppc.h"
> > >  #include "mmu-hash64.h"
> > >  #include "qom/cpu.h"
> > > @@ -660,6 +661,10 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset)
> > >      QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
> > >      unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
> > >      uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
> > > +    sPAPRDRConnector *drc =
> > > +        spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
> > > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > > +    int drc_index = drck->get_index(drc);
> > >  
> > >      _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
> > >      _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu")));
> > > @@ -728,6 +733,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset)
> > >  
> > >      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
> > >                              cs->cpu_index / cpus_per_socket)));
> > > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
> > > +
> > 
> > What effect will this have when running with DR disabled?
> 
> I realize now that I shouldn't probably populate this at all in that case.
> This routine is now common for both bootpath and hotplug path. Will take
> care of this in next post.

Ok.

[snip]
> > > +static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
> > > +{
> > > +    PowerPCCPU *fcpu = POWERPC_CPU(first_cpu);
> > > +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(fcpu);
> > 
> > In some ways it might be nicer to store the global endian mode in
> > sPAPRMachineState, updating it on H_SET_MODE, rather than copying the
> > first cpu, although I guess that works.
> 
> I can give it a stab at that if you feel strongly about it.

Not that strongly.  It's a small cleanup we can consider later.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]