From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46216) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWnlM-0005qJ-HD for qemu-devel@nongnu.org; Fri, 19 Feb 2016 11:18:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWnlI-0005lr-Cr for qemu-devel@nongnu.org; Fri, 19 Feb 2016 11:18:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWnlI-0005lT-4A for qemu-devel@nongnu.org; Fri, 19 Feb 2016 11:18:32 -0500 Date: Fri, 19 Feb 2016 17:18:27 +0100 From: Igor Mammedov Message-ID: <20160219171827.340ff471@nial.brq.redhat.com> In-Reply-To: <56C7294D.6090506@linux.vnet.ibm.com> References: <1455854617-30964-1-git-send-email-mjrosato@linux.vnet.ibm.com> <1455854617-30964-7-git-send-email-mjrosato@linux.vnet.ibm.com> <20160219152835.69d75f7a@nial.brq.redhat.com> <56C7294D.6090506@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 6/6] s390x/cpu: Allow hotplug of CPUs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Matthew Rosato , afaerber@suse.de Cc: dahi@linux.vnet.ibm.com, qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com, bharata@linux.vnet.ibm.com, cornelia.huck@de.ibm.com, pbonzini@redhat.com, rth@twiddle.net On Fri, 19 Feb 2016 09:40:13 -0500 Matthew Rosato wrote: > On 02/19/2016 09:28 AM, Igor Mammedov wrote: > > On Thu, 18 Feb 2016 23:03:37 -0500 > > Matthew Rosato wrote: > > > >> Implement cpu hotplug routine and add the machine hook. > >> > >> Signed-off-by: Matthew Rosato > >> --- > >> hw/s390x/s390-virtio-ccw.c | 1 + > >> target-s390x/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > >> target-s390x/cpu.h | 1 + > >> 3 files changed, 47 insertions(+) > >> > >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > >> index 31c1082..3be41ee 100644 > >> --- a/hw/s390x/s390-virtio-ccw.c > >> +++ b/hw/s390x/s390-virtio-ccw.c > >> @@ -199,6 +199,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > >> > >> mc->init = ccw_init; > >> mc->reset = s390_machine_reset; > >> + mc->hot_add_cpu = s390_hot_add_cpu; > >> mc->block_default_type = IF_VIRTIO; > >> mc->no_cdrom = 1; > >> mc->no_floppy = 1; > >> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > >> index 8dfd063..99394ec 100644 > >> --- a/target-s390x/cpu.c > >> +++ b/target-s390x/cpu.c > >> @@ -32,6 +32,12 @@ > >> #include "trace.h" > >> #ifndef CONFIG_USER_ONLY > >> #include "sysemu/arch_init.h" > >> +#include "sysemu/sysemu.h" > >> +#include "hw/boards.h" > >> +#include "hw/s390x/sclp.h" > >> +#include "qom/cpu.h" > >> + > >> +#define last_cpu QTAILQ_LAST(&cpus, CPUTailQ) > > this should be in qom/cpu.h, maybe a separate patch > > > > I did that originally, but trips up due to usage of the phrase > 'last_cpu' elsewhere in the code. Looking now, hw/intc/openpic.c is the > culprit. > > I could split this out into a patch that adds last_cpu to qom/cpu.h + > changes the variable name in openpic.c. I'd change variable name in openpic.c. > > Or use a different name for the macro. (cpu_last for example). > > Preference? typically macros should be UPPER_CASED, I guess first_cpu is lower cased due historical reasons to avoid renaming it across a bunch of files. Andreas, do you have any preference in which way fix last_cpu issue? > > >> #endif > >> > >> #define CR0_RESET 0xE0UL > >> @@ -211,6 +217,12 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp) > >> #endif > >> > >> scc->parent_realize(dev, errp); > >> + > >> +#if !defined(CONFIG_USER_ONLY) > >> + if (dev->hotplugged) { > >> + raise_irq_cpu_hotplug(); > >> + } > >> +#endif > >> } > >> > >> static void s390_cpu_initfn(Object *obj) > >> @@ -254,6 +266,39 @@ static void s390_cpu_finalize(Object *obj) > >> } > >> > >> #if !defined(CONFIG_USER_ONLY) > >> +void s390_hot_add_cpu(const int64_t id, Error **errp) > >> +{ > > this one looks like a func that belongs to board and should > > be a static in hw/s390x/s390-virtio-ccw.c > > > > OK, sure. > > Matt >