From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yps2j-0000x7-RD for qemu-devel@nongnu.org; Wed, 06 May 2015 01:38:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yps2e-0003ui-QS for qemu-devel@nongnu.org; Wed, 06 May 2015 01:38:49 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:34235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yps2e-0003tT-6K for qemu-devel@nongnu.org; Wed, 06 May 2015 01:38:44 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 6 May 2015 15:38:40 +1000 Date: Wed, 6 May 2015 11:07:42 +0530 From: Bharata B Rao Message-ID: <20150506053742.GM18380@in.ibm.com> References: <1429858066-12088-1-git-send-email-bharata@linux.vnet.ibm.com> <1429858066-12088-13-git-send-email-bharata@linux.vnet.ibm.com> <20150504175323.2bdf71b0@thh440s> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150504175323.2bdf71b0@thh440s> Subject: Re: [Qemu-devel] [RFC PATCH v3 12/24] spapr: CPU hotplug support Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: mdroth@linux.vnet.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, imammedo@redhat.com, nfont@linux.vnet.ibm.com, afaerber@suse.de, david@gibson.dropbear.id.au On Mon, May 04, 2015 at 05:53:23PM +0200, Thomas Huth wrote: > On Fri, 24 Apr 2015 12:17:34 +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. > > > > Also support cold plugged CPUs that are specified by -device option > > on cmdline. > > > > Signed-off-by: Bharata B Rao > > --- > > hw/ppc/spapr.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/ppc/spapr_events.c | 8 ++-- > > hw/ppc/spapr_rtas.c | 11 +++++ > > 3 files changed, 145 insertions(+), 3 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index b526b7d..9b0701c 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > [...] > > +static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > + Error **errp) > > +{ > > + CPUState *cs = CPU(dev); > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + int id = ppc_get_vcpu_dt_id(cpu); > > + sPAPRDRConnector *drc = > > + spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); > > + sPAPRDRConnectorClass *drck; > > + int smt = kvmppc_smt_threads(); > > + Error *local_err = NULL; > > + void *fdt = NULL; > > + int i, fdt_offset = 0; > > + > > + /* Set NUMA node for the added CPUs */ > > + for (i = 0; i < nb_numa_nodes; i++) { > > + if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) { > > + cs->numa_node = i; > > + break; > > + } > > + } > > + > > + /* > > + * SMT threads return from here, only main thread (core) will > > + * continue and signal hotplug event to the guest. > > + */ > > + if ((id % smt) != 0) { > > + return; > > + } > > + > > + if (!spapr->dr_cpu_enabled) { > > + /* > > + * This is a cold plugged CPU but the machine doesn't support > > + * DR. So skip the hotplug path ensuring that the CPU is brought > > + * up online with out an associated DR connector. > > + */ > > + return; > > + } > > + > > + g_assert(drc); > > + > > + /* > > + * Setup CPU DT entries only for hotplugged CPUs. For boot time or > > + * coldplugged CPUs DT entries are setup in spapr_finalize_fdt(). > > + */ > > + if (dev->hotplugged) { > > + fdt = spapr_populate_hotplug_cpu_dt(dev, cs, &fdt_offset); > > + } > > + > > + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > + drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err); > > + if (local_err) { > > + g_free(fdt); > > + error_propagate(errp, local_err); > > + return; > > + } > > + > > + /* > > + * We send hotplug notification interrupt to the guest only in case > > + * of hotplugged CPUs. > > + */ > > + if (dev->hotplugged) { > > + spapr_hotplug_req_add_event(drc); > > + } else { > > + /* > > + * HACK to support removal of hotplugged CPU after VM migration: > > + * > > + * Since we want to be able to hot-remove those coldplugged CPUs > > + * started at boot time using -device option at the target VM, we set > > + * the right allocation_state and isolation_state for them, which for > > + * the hotplugged CPUs would be set via RTAS calls done from the > > + * guest during hotplug. > > + * > > + * This allows the coldplugged CPUs started using -device option to > > + * have the right isolation and allocation states as expected by the > > + * CPU hot removal code. > > + * > > + * This hack will be removed once we have DRC states migrated as part > > + * of VM migration. > > + */ > > + drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE); > > + drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED); > > + } > > + > > + return; > > Cosmetic nit: Superfluous return statement > > > +} > > + > [...] > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > > index 57ec97a..48aeb86 100644 > > --- a/hw/ppc/spapr_rtas.c > > +++ b/hw/ppc/spapr_rtas.c > > @@ -121,6 +121,16 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_, > > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > > } > > > > +static void spapr_cpu_set_endianness(PowerPCCPU *cpu) > > +{ > > + PowerPCCPU *fcpu = POWERPC_CPU(first_cpu); > > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(fcpu); > > + > > + if (!(*pcc->interrupts_big_endian)(fcpu)) { > > Functions pointers are sometimes still confusing to me, but can't you > simplify that to: > > if (!pcc->interrupts_big_endian(fcpu)) { That should work too. Thanks. Regards, Bharata.