From patchwork Fri Jan 8 14:21:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 564896 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3FC541400A0 for ; Sat, 9 Jan 2016 01:48:42 +1100 (AEDT) Received: from localhost ([::1]:36754 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHYLI-0001cD-8i for incoming@patchwork.ozlabs.org; Fri, 08 Jan 2016 09:48:40 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57988) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHXw5-0001vT-R7 for qemu-devel@nongnu.org; Fri, 08 Jan 2016 09:22:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHXw4-0004Is-Q5 for qemu-devel@nongnu.org; Fri, 08 Jan 2016 09:22:37 -0500 Received: from s16892447.onlinehome-server.info ([82.165.15.123]:40906) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHXvw-0004H7-69; Fri, 08 Jan 2016 09:22:28 -0500 Received: from host86-147-229-66.range86-147.btcentralplus.com ([86.147.229.66] helo=[192.168.1.65]) by s16892447.onlinehome-server.info with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1aHXvh-0001Kq-52; Fri, 08 Jan 2016 14:22:14 +0000 To: Alexey Kardashevskiy , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, agraf@suse.de, quintela@redhat.com, amit.shah@redhat.com, David Gibson References: <1452104533-8516-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1452104533-8516-5-git-send-email-mark.cave-ayland@ilande.co.uk> <568F235B.1010506@ozlabs.ru> From: Mark Cave-Ayland X-Enigmail-Draft-Status: N1110 Message-ID: <568FC5FF.9070606@ilande.co.uk> Date: Fri, 8 Jan 2016 14:21:51 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <568F235B.1010506@ozlabs.ru> X-SA-Exim-Connect-IP: 86.147.229.66 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk X-SA-Exim-Version: 4.2.1 (built Sun, 08 Jan 2012 02:45:44 +0000) X-SA-Exim-Scanned: Yes (on s16892447.onlinehome-server.info) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 82.165.15.123 Subject: Re: [Qemu-devel] [PATCH 4/4] target-ppc: ensure we include the decrementer value during migration X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 08/01/16 02:47, Alexey Kardashevskiy wrote: > On 01/07/2016 05:22 AM, Mark Cave-Ayland wrote: >> During local testing with TCG, intermittent errors were found when >> trying to >> migrate Darwin OS images. >> >> The underlying cause was that Darwin resets the decrementer value to >> fairly >> small values on each interrupt. cpu_ppc_set_tb_clk() sets the default >> value >> of the decrementer to 0xffffffff during initialisation which typically >> corresponds to several seconds. Hence when restoring the image, the guest >> would effectively "lose" decrementer interrupts during this time causing >> confusion in the guest. >> >> NOTE: there does seem to be some overlap here with the >> vmstate_ppc_timebase >> code, however it doesn't seem to handle multiple CPUs which is why >> I've gone >> for an independent implementation. > > It does handle multiple CPUs: > > static int timebase_post_load(void *opaque, int version_id) > { > ... > /* Set new offset to all CPUs */ > CPU_FOREACH(cpu) { > PowerPCCPU *pcpu = POWERPC_CPU(cpu); > pcpu->env.tb_env->tb_offset = tb_off_adj; > } > > > It does not transfer DECR though, it transfers the offset instead, one > for all CPUs. > > > The easier solution would be just adding this instead of the whole patch: > > spr_register(env, SPR_DECR, "DECR", > SPR_NOACCESS, SPR_NOACCESS, > &spr_read_decr, &spr_write_decr, > 0x00000000); > > somewhere in target-ppc/translate_init.c for CPUs you want to support, > gen_tbl() seems to be the right place for this. As long as it is just > spr_register() and not spr_register_kvm(), I suppose it should not break > KVM and pseries. I've just tried adding that but it then gives the following error on startup: Error: Trying to register SPR 22 (016) twice ! Based upon this, the existing registration seems fine. I think the problem is that I can't see anything in __cpu_ppc_store_decr() that updates the spr[SPR_DECR] value when the decrementer register is changed, so it needs to be explicitly added to cpu_pre_save()/cpu_post_load(): I've just tried the diff above instead of my original version and repeated my savevm/loadvm pair test with a Darwin installation and it also fixes the random hang I was seeing on loadvm. Seemingly this should work on KVM in that cpu_ppc_load_decr() and cpu_ppc_store_decr() become no-ops as long as KVM maintains env->spr[SPR_DECR], but a second set of eyeballs would be useful here. If you can let me know if this is suitable then I'll update the patchset based upon your feedback and send out a v2. ATB, Mark. diff --git a/target-ppc/machine.c b/target-ppc/machine.c index 251a84b..495e58d 100644 --- a/target-ppc/machine.c +++ b/target-ppc/machine.c @@ -141,6 +141,7 @@ static void cpu_pre_save(void *opaque) env->spr[SPR_CFAR] = env->cfar; #endif env->spr[SPR_BOOKE_SPEFSCR] = env->spe_fscr; + env->spr[SPR_DECR] = cpu_ppc_load_decr(env); for (i = 0; (i < 4) && (i < env->nb_BATs); i++) { env->spr[SPR_DBAT0U + 2*i] = env->DBAT[0][i]; @@ -175,6 +176,7 @@ static int cpu_post_load(void *opaque, int version_id) env->cfar = env->spr[SPR_CFAR]; #endif env->spe_fscr = env->spr[SPR_BOOKE_SPEFSCR]; + cpu_ppc_store_decr(env, env->spr[SPR_DECR]); for (i = 0; (i < 4) && (i < env->nb_BATs); i++) { env->DBAT[0][i] = env->spr[SPR_DBAT0U + 2*i];