From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aog19-0000Qj-IN for qemu-devel@nongnu.org; Fri, 08 Apr 2016 19:40:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aog16-0001ya-4E for qemu-devel@nongnu.org; Fri, 08 Apr 2016 19:40:47 -0400 Received: from e18.ny.us.ibm.com ([129.33.205.208]:34258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aog16-0001yG-17 for qemu-devel@nongnu.org; Fri, 08 Apr 2016 19:40:44 -0400 Received: from localhost by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 8 Apr 2016 19:40:41 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20160406004346.GW16485@voom.fritz.box> References: <1459413561-30745-1-git-send-email-bharata@linux.vnet.ibm.com> <1459413561-30745-12-git-send-email-bharata@linux.vnet.ibm.com> <20160406002439.589.59245@loki> <20160406004346.GW16485@voom.fritz.box> Message-ID: <20160408234029.30838.55725@loki> Date: Fri, 08 Apr 2016 18:40:29 -0500 Subject: Re: [Qemu-devel] [RFC PATCH v2.1 11/12] xics, xics_kvm: Handle CPU unplug correctly List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Bharata B Rao , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, afaerber@suse.de, imammedo@redhat.com, armbru@redhat.com, thuth@redhat.com, aik@ozlabs.ru, agraf@suse.de, pbonzini@redhat.com, ehabkost@redhat.com, pkrempa@redhat.com, eblake@redhat.com, mjrosato@linux.vnet.ibm.com, borntraeger@de.ibm.com Quoting David Gibson (2016-04-05 19:43:46) > On Tue, Apr 05, 2016 at 07:24:39PM -0500, Michael Roth wrote: > > Quoting Bharata B Rao (2016-03-31 03:39:20) > > > XICS is setup for each CPU during initialization. Provide a routine > > > to undo the same when CPU is unplugged. While here, move ss->cs manag= ement > > > into xics from xics_kvm since there is nothing KVM specific in it. > > > Also ensure xics reset doesn't set irq for CPUs that are already unpl= ugged. > > > = > > > This allows reboot of a VM that has undergone CPU hotplug and unplug > > > to work correctly. > > > = > > > Signed-off-by: Bharata B Rao > > > Reviewed-by: David Gibson > > > --- > > > hw/intc/xics.c | 14 ++++++++++++++ > > > hw/intc/xics_kvm.c | 8 ++++---- > > > include/hw/ppc/xics.h | 1 + > > > 3 files changed, 19 insertions(+), 4 deletions(-) > > > = > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > > index 213a370..9fdb551 100644 > > > --- a/hw/intc/xics.c > > > +++ b/hw/intc/xics.c > > > @@ -45,6 +45,18 @@ static int get_cpu_index_by_dt_id(int cpu_dt_id) > > > return -1; > > > } > > > = > > > +void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu) > > > +{ > > > + CPUState *cs =3D CPU(cpu); > > > + ICPState *ss =3D &icp->ss[cs->cpu_index]; > > > + > > = > > If the following assertion is false, I think you'd get an OOB before you > > reached it due to the assignment above. > = > It should be ok. The assignment above will compute an out of bounds > address, but it doesn't dereference it. Ahh, right. I was thinking there was an actual access for some reason. I might still prefer moving the assignment after the assert from a stylistic standpoint, in case other variables get introduced in the future that rely on *ss. Just a nit though. > = > > > + assert(cs->cpu_index < icp->nr_servers); > > > + assert(cs =3D=3D ss->cs); > > > + > > > + ss->output =3D NULL; > > > + ss->cs =3D NULL; > > > +} > > > + > > = > = > -- = > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _othe= r_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson