From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44926) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdOkd-0002l2-UO for qemu-devel@nongnu.org; Tue, 27 Nov 2012 12:15:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TdOkc-0008Iz-KG for qemu-devel@nongnu.org; Tue, 27 Nov 2012 12:15:15 -0500 Received: from cantor2.suse.de ([195.135.220.15]:58813 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdOkc-0008Hw-9r for qemu-devel@nongnu.org; Tue, 27 Nov 2012 12:15:14 -0500 Message-ID: <50B4F51D.5030009@suse.de> Date: Tue, 27 Nov 2012 18:15:09 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1354005233-60955-1-git-send-email-jfrei@linux.vnet.ibm.com> <20121127171346.GA12208@chuck.boeblingen.de.ibm.com> In-Reply-To: <20121127171346.GA12208@chuck.boeblingen.de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] s390: clear registers, psw and prefix at vcpu reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jens Freimann Cc: Cornelia Huck , Christian Borntraeger , Heinz Graalfs , qemu-devel , Einar Lueck On 11/27/2012 06:13 PM, Jens Freimann wrote: > On Tue, Nov 27, 2012 at 10:37:25AM +0100, Alexander Graf wrote: >> >> On 27.11.2012, at 09:33, Jens Freimann wrote: >> >>> When resetting vcpus on s390/kvm we have to clear registers, psw >>> and prefix as described in the z/Architecture PoP, otherwise a >>> reboot won't work. IPL PSW and prefix are set later on by the >>> s390-ipl device reset code. >>> >>> Signed-off-by: Jens Freimann >>> --- >>> >>> changes v1->v2: >>> - moved cpu reset code from kvm.c to cpu.c >>> - only kvm initial_reset ioctl remains in kvm.c >>> - registered reset handler for s390 cpu reset, like x86 does it >>> >>> target-s390x/cpu.c | 29 +++++++++++++++++++++++++++-- >>> target-s390x/kvm.c | 9 ++++++++- >>> 2 files changed, 35 insertions(+), 3 deletions(-) >>> >>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c >>> index 619b202..f74d6f6 100644 >>> --- a/target-s390x/cpu.c >>> +++ b/target-s390x/cpu.c >>> @@ -24,10 +24,21 @@ >>> #include "qemu-common.h" >>> #include "qemu-timer.h" >>> >>> +#ifndef CONFIG_USER_ONLY >>> +#include "hw/s390x/sclp.h" >>> + >>> +/* TODO: remove me, when reset over QOM tree is implemented */ >>> +static void s390_cpu_machine_reset_cb(void *opaque) >>> +{ >>> + S390CPU *cpu = opaque; >>> + cpu_reset(CPU(cpu)); >>> +} >>> +#endif >>> >>> /* CPUClass::reset() */ >>> static void s390_cpu_reset(CPUState *s) >>> { >>> + int i; >>> S390CPU *cpu = S390_CPU(s); >>> S390CPUClass *scc = S390_CPU_GET_CLASS(cpu); >>> CPUS390XState *env =&cpu->env; >>> @@ -40,9 +51,22 @@ static void s390_cpu_reset(CPUState *s) >>> scc->parent_reset(s); >>> >>> memset(env, 0, offsetof(CPUS390XState, breakpoints)); >>> - /* FIXME: reset vector? */ >>> + >>> + env->halted = 1; >> Every cpu would start in halted state? So how does the primary one get rolling? > The first cpu is set to not-halted by the ipl device reset code. Please document this here. >>> + env->exception_index = EXCP_HLT; >>> + for (i = 0; i< 16; i++) { >>> + env->regs[i] = 0; >>> + env->aregs[i] = 0; >>> + env->cregs[i] = 0; >>> + env->fregs[i].ll = 0; >>> + } >> Please make this more self-adjusting. For example using memset(sizeof));. You could also make the clear implicit by ensuring the registers are in the cpu struct before breakpoints. But explicit tends to be more readable ;). > Ok > >>> + /* architectured initial values for CR 0 and 14 */ >>> + env->cregs[0] = 0xE0UL; >>> + env->cregs[14] = 0xC2000000UL; >>> + env->psw.mask = 0; >>> + env->psw.addr = 0; >>> + env->psa = 0; >>> tlb_flush(env, 1); >>> - s390_add_running_cpu(env); >> Why can we remove this one? > Good point. I took a closer look and found that we add an additional > cpu to the counter every time we reboot. Will fix this and send a new > version. Yeah, if anything this should be a separate patch :). Alex