[{"id":1767626,"web_url":"http://patchwork.ozlabs.org/comment/1767626/","msgid":"<20170913071249.GI7550@umbus.fritz.box>","list_archive_url":null,"date":"2017-09-13T07:12:49","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Sun, Sep 10, 2017 at 03:37:35PM +0100, Mark Cave-Ayland wrote:\n> During local testing with TCG, intermittent errors were found when trying to\n> migrate Darwin OS images.\n> \n> The underlying cause was that Darwin resets the decrementer value to fairly\n> small values on each interrupt. cpu_ppc_set_tb_clk() sets the default value\n> of the decrementer to 0xffffffff during initialisation which typically\n> corresponds to several seconds. Hence when restoring the image, the guest\n> would effectively \"lose\" decrementer interrupts during this time causing\n> confusion in the guest.\n> \n> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>\n\nThis is subtly incorrect.  It sets the DECR on load to exactly the\nvalue that was saved.  That effectively means that the DECR is frozen\nfor the migration downtime, which probably isn't what we want.\n\nInstead we need to save the DECR as an offset from the timebase, and\nrestore it relative to the (downtime adjusted) timebase on the\ndestination.\n\nThe complication with that is that the timebase is generally migrated\nat the machine level, not the cpu level: the timebase is generally\nsynchronized between cpus in the machine, and migrating it at the\nindividual cpu could break that.  Which means we probably need a\nmachine level hook to handle the decrementer too, even though it\nlogically *is* per-cpu, because otherwise we'll be trying to restore\nit before the timebase is restored.\n\n> ---\n>  target/ppc/machine.c |    2 ++\n>  1 file changed, 2 insertions(+)\n> \n> diff --git a/target/ppc/machine.c b/target/ppc/machine.c\n> index 10b3c41..a16a856 100644\n> --- a/target/ppc/machine.c\n> +++ b/target/ppc/machine.c\n> @@ -176,6 +176,7 @@ static void cpu_pre_save(void *opaque)\n>      env->spr[SPR_CFAR] = env->cfar;\n>  #endif\n>      env->spr[SPR_BOOKE_SPEFSCR] = env->spe_fscr;\n> +    env->spr[SPR_DECR] = cpu_ppc_load_decr(env);\n>  \n>      for (i = 0; (i < 4) && (i < env->nb_BATs); i++) {\n>          env->spr[SPR_DBAT0U + 2*i] = env->DBAT[0][i];\n> @@ -280,6 +281,7 @@ static int cpu_post_load(void *opaque, int version_id)\n>      env->cfar = env->spr[SPR_CFAR];\n>  #endif\n>      env->spe_fscr = env->spr[SPR_BOOKE_SPEFSCR];\n> +    cpu_ppc_store_decr(env, env->spr[SPR_DECR]);\n>  \n>      for (i = 0; (i < 4) && (i < env->nb_BATs); i++) {\n>          env->DBAT[0][i] = env->spr[SPR_DBAT0U + 2*i];","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=gibson.dropbear.id.au\n\theader.i=@gibson.dropbear.id.au header.b=\"KviB3jkI\"; \n\tdkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsYDy4WZFz9sPs\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 13 Sep 2017 17:28:50 +1000 (AEST)","from localhost ([::1]:40583 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1ds26K-0005h8-Mr\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 03:28:48 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57167)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1ds25i-0005cQ-Ci\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 03:28:11 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1ds25e-0001QR-By\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 03:28:10 -0400","from ozlabs.org ([103.22.144.67]:41023)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgibson@ozlabs.org>)\n\tid 1ds25d-0001OQ-GY; Wed, 13 Sep 2017 03:28:06 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3xsYD113GYz9sRV; Wed, 13 Sep 2017 17:28:00 +1000 (AEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1505287681;\n\tbh=wSLV3d94cyvzMj8uw1WuqFFnvFDcRvVdO4LRSaPTWeA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KviB3jkIjyUQ2Wz4rQi6HW05Va6dL3Xy/KnfJDvtpaFZJzctWlhGelcRkxbEjeuyf\n\tG8Wn24LBENps7ULwpPLTMj9HGH2VwHf2bCsmByIeZowT6VqoFR2t/h2n5UFKy+qW5n\n\tm2oILVPNMeCsOTIz3Z0X08ZpU0KOnK7dRdsfEXqg=","Date":"Wed, 13 Sep 2017 17:12:49 +1000","From":"David Gibson <david@gibson.dropbear.id.au>","To":"Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>","Message-ID":"<20170913071249.GI7550@umbus.fritz.box>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ZY5CS28jBCfb727c\"","Content-Disposition":"inline","In-Reply-To":"<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"103.22.144.67","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"aik@ozlabs.ru, lvivier@redhat.com, qemu-ppc@nongnu.org,\n\tqemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1768039,"web_url":"http://patchwork.ozlabs.org/comment/1768039/","msgid":"<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>","list_archive_url":null,"date":"2017-09-13T17:11:23","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":12451,"url":"http://patchwork.ozlabs.org/api/people/12451/","name":"Mark Cave-Ayland","email":"mark.cave-ayland@ilande.co.uk"},"content":"On 13/09/17 08:12, David Gibson wrote:\n\n> This is subtly incorrect.  It sets the DECR on load to exactly the\n> value that was saved.  That effectively means that the DECR is frozen\n> for the migration downtime, which probably isn't what we want.\n> \n> Instead we need to save the DECR as an offset from the timebase, and\n> restore it relative to the (downtime adjusted) timebase on the\n> destination.\n> \n> The complication with that is that the timebase is generally migrated\n> at the machine level, not the cpu level: the timebase is generally\n> synchronized between cpus in the machine, and migrating it at the\n> individual cpu could break that.  Which means we probably need a\n> machine level hook to handle the decrementer too, even though it\n> logically *is* per-cpu, because otherwise we'll be trying to restore\n> it before the timebase is restored.\n\nI know that we discussed this in-depth last year, however I was working\nalong the lines that Laurent's patch fixed this along the lines of our\nprevious discussion:\nhttps://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00338.html (and\nindeed Laurent's analysis at\nhttps://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01489.html).\n\nHowever looking again at the this patch in the context you mentioned\nabove, I'm starting to wonder if the right solution now is for the\nmachine init function for g3beige/mac99 to do the same as spapr, e.g.\nadd cpu_ppc_clock_vm_state_change() as a vm_change_state_handler and\nthen add VMSTATE_PPC_TIMEBASE_V from the machine PPCTimeBase into my new\nsubsection?\n\nLaurent, do you think that your state change handler would work\ncorrectly in this way?\n\n\nATB,\n\nMark.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xspBf6Knlz9s4s\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 03:12:42 +1000 (AEST)","from localhost ([::1]:43735 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dsBDM-0007Id-Ut\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 13:12:40 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59570)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1dsBCN-00057p-E8\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 13:11:42 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1dsBCH-00013F-Fd\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 13:11:39 -0400","from chuckie.co.uk ([82.165.15.123]:49034\n\thelo=s16892447.onlinehome-server.info)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <mark.cave-ayland@ilande.co.uk>)\n\tid 1dsBCH-00012I-9F; Wed, 13 Sep 2017 13:11:33 -0400","from host109-151-159-252.range109-151.btcentralplus.com\n\t([109.151.159.252] helo=[192.168.1.65])\n\tby s16892447.onlinehome-server.info with esmtpsa\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>)\n\tid 1dsBCH-00087t-CE; Wed, 13 Sep 2017 18:11:34 +0100"],"To":"David Gibson <david@gibson.dropbear.id.au>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<20170913071249.GI7550@umbus.fritz.box>","From":"Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>","Message-ID":"<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>","Date":"Wed, 13 Sep 2017 18:11:23 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170913071249.GI7550@umbus.fritz.box>","Content-Type":"text/plain; charset=windows-1252","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-SA-Exim-Connect-IP":"109.151.159.252","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 [fuzzy]","X-Received-From":"82.165.15.123","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"aik@ozlabs.ru, lvivier@redhat.com, qemu-ppc@nongnu.org,\n\tqemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1768067,"web_url":"http://patchwork.ozlabs.org/comment/1768067/","msgid":"<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>","list_archive_url":null,"date":"2017-09-13T17:58:23","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":66421,"url":"http://patchwork.ozlabs.org/api/people/66421/","name":"Laurent Vivier","email":"lvivier@redhat.com"},"content":"On 13/09/2017 19:11, Mark Cave-Ayland wrote:\n> On 13/09/17 08:12, David Gibson wrote:\n> \n>> This is subtly incorrect.  It sets the DECR on load to exactly the\n>> value that was saved.  That effectively means that the DECR is frozen\n>> for the migration downtime, which probably isn't what we want.\n>>\n>> Instead we need to save the DECR as an offset from the timebase, and\n>> restore it relative to the (downtime adjusted) timebase on the\n>> destination.\n>>\n>> The complication with that is that the timebase is generally migrated\n>> at the machine level, not the cpu level: the timebase is generally\n>> synchronized between cpus in the machine, and migrating it at the\n>> individual cpu could break that.  Which means we probably need a\n>> machine level hook to handle the decrementer too, even though it\n>> logically *is* per-cpu, because otherwise we'll be trying to restore\n>> it before the timebase is restored.\n> \n> I know that we discussed this in-depth last year, however I was working\n> along the lines that Laurent's patch fixed this along the lines of our\n> previous discussion:\n> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00338.html (and\n> indeed Laurent's analysis at\n> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01489.html).\n> \n> However looking again at the this patch in the context you mentioned\n> above, I'm starting to wonder if the right solution now is for the\n> machine init function for g3beige/mac99 to do the same as spapr, e.g.\n> add cpu_ppc_clock_vm_state_change() as a vm_change_state_handler and\n> then add VMSTATE_PPC_TIMEBASE_V from the machine PPCTimeBase into my new\n> subsection?\n> \n> Laurent, do you think that your state change handler would work\n> correctly in this way?\n\nI think all is explained in the second link you have mentioned, it seems \nwe don't need a state handler as KVM DECR will no be updated by the migrated value:\n\nhw/ppc/ppc.c\n\n    736 static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,\n    737                                  QEMUTimer *timer,\n    738                                  void (*raise_excp)(void *),\n    739                                  void (*lower_excp)(PowerPCCPU *),\n    740                                  uint32_t decr, uint32_t value)\n    741 {\n...\n    749     if (kvm_enabled()) {\n    750         /* KVM handles decrementer exceptions, we don't need our own timer */\n    751         return;\n    752     }\n...\n\nBut this allows to migrate it for TCG. And it should be correct because in case of TCG I think [need to check] timebase is stopped too (so offset is 0)\n\nDavid, do you agree with that?\n\nThanks,\nLaurent","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=lvivier@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsqD31pCCz9sNV\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 03:58:59 +1000 (AEST)","from localhost ([::1]:43914 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dsBw9-0004Wq-Bw\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 13:58:57 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57724)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <lvivier@redhat.com>) id 1dsBvj-0004Vo-6P\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 13:58:32 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <lvivier@redhat.com>) id 1dsBvg-0002Yf-3o\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 13:58:31 -0400","from mx1.redhat.com ([209.132.183.28]:56158)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <lvivier@redhat.com>)\n\tid 1dsBvf-0002X0-RI; Wed, 13 Sep 2017 13:58:28 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 63E1581E1E;\n\tWed, 13 Sep 2017 17:58:26 +0000 (UTC)","from [10.36.116.49] (ovpn-116-49.ams2.redhat.com [10.36.116.49])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 693E05D767;\n\tWed, 13 Sep 2017 17:58:24 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 63E1581E1E","To":"Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,\n\tDavid Gibson <david@gibson.dropbear.id.au>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<20170913071249.GI7550@umbus.fritz.box>\n\t<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>","From":"Laurent Vivier <lvivier@redhat.com>","Message-ID":"<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>","Date":"Wed, 13 Sep 2017 19:58:23 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tWed, 13 Sep 2017 17:58:26 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"aik@ozlabs.ru, qemu-ppc@nongnu.org, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1768330,"web_url":"http://patchwork.ozlabs.org/comment/1768330/","msgid":"<20170914035259.GK3972@umbus.fritz.box>","list_archive_url":null,"date":"2017-09-14T03:52:59","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Wed, Sep 13, 2017 at 07:58:23PM +0200, Laurent Vivier wrote:\n> On 13/09/2017 19:11, Mark Cave-Ayland wrote:\n> > On 13/09/17 08:12, David Gibson wrote:\n> > \n> >> This is subtly incorrect.  It sets the DECR on load to exactly the\n> >> value that was saved.  That effectively means that the DECR is frozen\n> >> for the migration downtime, which probably isn't what we want.\n> >>\n> >> Instead we need to save the DECR as an offset from the timebase, and\n> >> restore it relative to the (downtime adjusted) timebase on the\n> >> destination.\n> >>\n> >> The complication with that is that the timebase is generally migrated\n> >> at the machine level, not the cpu level: the timebase is generally\n> >> synchronized between cpus in the machine, and migrating it at the\n> >> individual cpu could break that.  Which means we probably need a\n> >> machine level hook to handle the decrementer too, even though it\n> >> logically *is* per-cpu, because otherwise we'll be trying to restore\n> >> it before the timebase is restored.\n> > \n> > I know that we discussed this in-depth last year, however I was working\n> > along the lines that Laurent's patch fixed this along the lines of our\n> > previous discussion:\n> > https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00338.html (and\n> > indeed Laurent's analysis at\n> > https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01489.html).\n> > \n> > However looking again at the this patch in the context you mentioned\n> > above, I'm starting to wonder if the right solution now is for the\n> > machine init function for g3beige/mac99 to do the same as spapr, e.g.\n> > add cpu_ppc_clock_vm_state_change() as a vm_change_state_handler and\n> > then add VMSTATE_PPC_TIMEBASE_V from the machine PPCTimeBase into my new\n> > subsection?\n> > \n> > Laurent, do you think that your state change handler would work\n> > correctly in this way?\n> \n> I think all is explained in the second link you have mentioned, it seems \n> we don't need a state handler as KVM DECR will no be updated by the migrated value:\n> \n> hw/ppc/ppc.c\n> \n>     736 static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,\n>     737                                  QEMUTimer *timer,\n>     738                                  void (*raise_excp)(void *),\n>     739                                  void (*lower_excp)(PowerPCCPU *),\n>     740                                  uint32_t decr, uint32_t value)\n>     741 {\n> ...\n>     749     if (kvm_enabled()) {\n>     750         /* KVM handles decrementer exceptions, we don't need our own timer */\n>     751         return;\n>     752     }\n> ...\n> \n> But this allows to migrate it for TCG. And it should be correct because in case of TCG I think [need to check] timebase is stopped too (so offset is 0)\n> \n> David, do you agree with that?\n\nYes, I think so.  Some details might be different, but the basic idea\nof migrating the timebase and decrementers at machine level should be\nthe same for pseries and g3beige/whatever.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=gibson.dropbear.id.au\n\theader.i=@gibson.dropbear.id.au header.b=\"kEv/NKF6\"; \n\tdkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xt4XH4dbzz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 13:58:55 +1000 (AEST)","from localhost ([::1]:45619 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dsLIj-0003Z0-Ps\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 23:58:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37309)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1dsLG1-00014m-SW\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 23:56:07 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1dsLFz-0000xz-9P\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 23:56:05 -0400","from ozlabs.org ([103.22.144.67]:59263)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgibson@ozlabs.org>)\n\tid 1dsLFy-0000vE-HU; Wed, 13 Sep 2017 23:56:03 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3xt4Sv4RzFz9sBZ; Thu, 14 Sep 2017 13:55:59 +1000 (AEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1505361359;\n\tbh=TypLfrwNHyUB8FYSV12KOnJI2cCXI9W0a42n3koNQeI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kEv/NKF6hDjNhEuVCLuIj7jtvnqZq5kmXUwhS4UaVJ+A9FMQRufS42HcSPEOrGcAj\n\t4vLLPEQCBtV++xida/LX5GFOWMif7AztynDjHdYKVxo7rYFZQlCKubBAlU8wfZCx7a\n\tYkYlCGcJPC1T3GSkQ3xnIp4YWQwe5Zs8zHTwLQFQ=","Date":"Thu, 14 Sep 2017 13:52:59 +1000","From":"David Gibson <david@gibson.dropbear.id.au>","To":"Laurent Vivier <lvivier@redhat.com>","Message-ID":"<20170914035259.GK3972@umbus.fritz.box>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<20170913071249.GI7550@umbus.fritz.box>\n\t<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>\n\t<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"aqWxf8ydqYKP8htK\"","Content-Disposition":"inline","In-Reply-To":"<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"103.22.144.67","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"aik@ozlabs.ru, qemu-ppc@nongnu.org,\n\tMark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1769159,"web_url":"http://patchwork.ozlabs.org/comment/1769159/","msgid":"<56298dd2-eabe-5075-61b3-c27a0eaf4579@ilande.co.uk>","list_archive_url":null,"date":"2017-09-15T12:45:11","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":12451,"url":"http://patchwork.ozlabs.org/api/people/12451/","name":"Mark Cave-Ayland","email":"mark.cave-ayland@ilande.co.uk"},"content":"On 14/09/17 04:52, David Gibson wrote:\n\n> On Wed, Sep 13, 2017 at 07:58:23PM +0200, Laurent Vivier wrote:\n>> On 13/09/2017 19:11, Mark Cave-Ayland wrote:\n>>> On 13/09/17 08:12, David Gibson wrote:\n>>>\n>>>> This is subtly incorrect.  It sets the DECR on load to exactly the\n>>>> value that was saved.  That effectively means that the DECR is frozen\n>>>> for the migration downtime, which probably isn't what we want.\n>>>>\n>>>> Instead we need to save the DECR as an offset from the timebase, and\n>>>> restore it relative to the (downtime adjusted) timebase on the\n>>>> destination.\n>>>>\n>>>> The complication with that is that the timebase is generally migrated\n>>>> at the machine level, not the cpu level: the timebase is generally\n>>>> synchronized between cpus in the machine, and migrating it at the\n>>>> individual cpu could break that.  Which means we probably need a\n>>>> machine level hook to handle the decrementer too, even though it\n>>>> logically *is* per-cpu, because otherwise we'll be trying to restore\n>>>> it before the timebase is restored.\n>>>\n>>> I know that we discussed this in-depth last year, however I was working\n>>> along the lines that Laurent's patch fixed this along the lines of our\n>>> previous discussion:\n>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00338.html (and\n>>> indeed Laurent's analysis at\n>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01489.html).\n>>>\n>>> However looking again at the this patch in the context you mentioned\n>>> above, I'm starting to wonder if the right solution now is for the\n>>> machine init function for g3beige/mac99 to do the same as spapr, e.g.\n>>> add cpu_ppc_clock_vm_state_change() as a vm_change_state_handler and\n>>> then add VMSTATE_PPC_TIMEBASE_V from the machine PPCTimeBase into my new\n>>> subsection?\n>>>\n>>> Laurent, do you think that your state change handler would work\n>>> correctly in this way?\n>>\n>> I think all is explained in the second link you have mentioned, it seems \n>> we don't need a state handler as KVM DECR will no be updated by the migrated value:\n>>\n>> hw/ppc/ppc.c\n>>\n>>     736 static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,\n>>     737                                  QEMUTimer *timer,\n>>     738                                  void (*raise_excp)(void *),\n>>     739                                  void (*lower_excp)(PowerPCCPU *),\n>>     740                                  uint32_t decr, uint32_t value)\n>>     741 {\n>> ...\n>>     749     if (kvm_enabled()) {\n>>     750         /* KVM handles decrementer exceptions, we don't need our own timer */\n>>     751         return;\n>>     752     }\n>> ...\n>>\n>> But this allows to migrate it for TCG. And it should be correct because in case of TCG I think [need to check] timebase is stopped too (so offset is 0)\n>>\n>> David, do you agree with that?\n> \n> Yes, I think so.  Some details might be different, but the basic idea\n> of migrating the timebase and decrementers at machine level should be\n> the same for pseries and g3beige/whatever.\n\nFrom my perspective, I'd really like to bury all of the timebase\nmigration logic into timebase_load()/timebase_save() so then as you\nsuggest above, everything will work regardless of the machine type and host.\n\nPlaying around with these functions I can see that they are very\nPPC-specific - for example cpu_get_host_ticks() can only ever work\ncorrectly migrating between 2 PPC hosts because here on Intel the value\nreturned is the TSC register which is completely unrelated to the\ntimebase frequency. Hence the calculated values are nonsense and the\nguest inevitably ends up freezing.\n\nI've had a go at converting this back to using the host clock to\ncalculate the required adjustment to tb_offset (see\nhttps://github.com/mcayland/qemu/commit/68e31f0455a79d41b2ab703364c74ceb4d513154)\nbut then even that struck me as incorrect since unless there are other\nhost CPUs with an equivalent of tb_offset, this whole section of code\nshould only ever run if the host CPU is PPC, and maybe even only under KVM.\n\nMy current thoughts are now as follows:\n\n- The existing timebase_load() should never adjust tb_offset unless the\nhost CPU is also PPC (how do we actually determine this?). Otherwise it\nshould always be 0.\n\n- The per-CPU decrementer values should be *stored* in the normal\nvmstate SPR array as per my latest patch at\nhttps://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02346.html.\n\nBUT:\n\n- The per-CPU decrementer values should be *restored* in timebase_load()\nbut again should not be adjusted by tb_offset unless the host CPU is\nalso PPC.\n\nThe real question is whether tb_offset is used for a TCG PPC guest on a\nPPC host or whether it also remains at 0 as it does here on an Intel\nhost? If it does remain at 0 for a TCG PPC guest on a PPC host then we\ndon't need to work out whether we are running on a PPC host at all: we\ncan just skip the timebase adjustments made by timebase_load() if\n!kvm_enabled() and everything should just work.\n\nIf this all seems reasonable then what I need to do for the PPC\ng3beige/mac99 machines is add the relevant machine state, include the\nexisting vmstate_change_handler and then add the code to timebase_load()\nto set the decrementer. I'm happy to do this as long as everyone agrees\nthis is a sensible approach?\n\n\nATB,\n\nMark.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtwBG247Jz9s82\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 15 Sep 2017 22:46:14 +1000 (AEST)","from localhost ([::1]:53237 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dsq0a-0003M6-A5\n\tfor incoming@patchwork.ozlabs.org; Fri, 15 Sep 2017 08:46:12 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:34275)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1dsq06-0003LY-8k\n\tfor qemu-devel@nongnu.org; Fri, 15 Sep 2017 08:45:47 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1dsq02-0004qC-U8\n\tfor qemu-devel@nongnu.org; Fri, 15 Sep 2017 08:45:42 -0400","from chuckie.co.uk ([82.165.15.123]:58153\n\thelo=s16892447.onlinehome-server.info)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <mark.cave-ayland@ilande.co.uk>)\n\tid 1dsq02-0004mH-Mz; Fri, 15 Sep 2017 08:45:38 -0400","from host109-151-159-252.range109-151.btcentralplus.com\n\t([109.151.159.252] helo=[192.168.1.65])\n\tby s16892447.onlinehome-server.info with esmtpsa\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76)\n\t(envelope-from <mark.cave-ayland@ilande.co.uk>)\n\tid 1dspzq-0002MS-1w; Fri, 15 Sep 2017 13:45:30 +0100"],"From":"Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>","To":"David Gibson <david@gibson.dropbear.id.au>,\n\tLaurent Vivier <lvivier@redhat.com>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<20170913071249.GI7550@umbus.fritz.box>\n\t<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>\n\t<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>\n\t<20170914035259.GK3972@umbus.fritz.box>","Message-ID":"<56298dd2-eabe-5075-61b3-c27a0eaf4579@ilande.co.uk>","Date":"Fri, 15 Sep 2017 13:45:11 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170914035259.GK3972@umbus.fritz.box>","Content-Type":"text/plain; charset=windows-1252","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-SA-Exim-Connect-IP":"109.151.159.252","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 [fuzzy]","X-Received-From":"82.165.15.123","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"aik@ozlabs.ru, qemu-ppc@nongnu.org, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1770849,"web_url":"http://patchwork.ozlabs.org/comment/1770849/","msgid":"<20170919083630.GV27153@umbus>","list_archive_url":null,"date":"2017-09-19T08:36:30","subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Fri, Sep 15, 2017 at 01:45:11PM +0100, Mark Cave-Ayland wrote:\n> On 14/09/17 04:52, David Gibson wrote:\n> \n> > On Wed, Sep 13, 2017 at 07:58:23PM +0200, Laurent Vivier wrote:\n> >> On 13/09/2017 19:11, Mark Cave-Ayland wrote:\n> >>> On 13/09/17 08:12, David Gibson wrote:\n> >>>\n> >>>> This is subtly incorrect.  It sets the DECR on load to exactly the\n> >>>> value that was saved.  That effectively means that the DECR is frozen\n> >>>> for the migration downtime, which probably isn't what we want.\n> >>>>\n> >>>> Instead we need to save the DECR as an offset from the timebase, and\n> >>>> restore it relative to the (downtime adjusted) timebase on the\n> >>>> destination.\n> >>>>\n> >>>> The complication with that is that the timebase is generally migrated\n> >>>> at the machine level, not the cpu level: the timebase is generally\n> >>>> synchronized between cpus in the machine, and migrating it at the\n> >>>> individual cpu could break that.  Which means we probably need a\n> >>>> machine level hook to handle the decrementer too, even though it\n> >>>> logically *is* per-cpu, because otherwise we'll be trying to restore\n> >>>> it before the timebase is restored.\n> >>>\n> >>> I know that we discussed this in-depth last year, however I was working\n> >>> along the lines that Laurent's patch fixed this along the lines of our\n> >>> previous discussion:\n> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00338.html (and\n> >>> indeed Laurent's analysis at\n> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01489.html).\n> >>>\n> >>> However looking again at the this patch in the context you mentioned\n> >>> above, I'm starting to wonder if the right solution now is for the\n> >>> machine init function for g3beige/mac99 to do the same as spapr, e.g.\n> >>> add cpu_ppc_clock_vm_state_change() as a vm_change_state_handler and\n> >>> then add VMSTATE_PPC_TIMEBASE_V from the machine PPCTimeBase into my new\n> >>> subsection?\n> >>>\n> >>> Laurent, do you think that your state change handler would work\n> >>> correctly in this way?\n> >>\n> >> I think all is explained in the second link you have mentioned, it seems \n> >> we don't need a state handler as KVM DECR will no be updated by the migrated value:\n> >>\n> >> hw/ppc/ppc.c\n> >>\n> >>     736 static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,\n> >>     737                                  QEMUTimer *timer,\n> >>     738                                  void (*raise_excp)(void *),\n> >>     739                                  void (*lower_excp)(PowerPCCPU *),\n> >>     740                                  uint32_t decr, uint32_t value)\n> >>     741 {\n> >> ...\n> >>     749     if (kvm_enabled()) {\n> >>     750         /* KVM handles decrementer exceptions, we don't need our own timer */\n> >>     751         return;\n> >>     752     }\n> >> ...\n> >>\n> >> But this allows to migrate it for TCG. And it should be correct because in case of TCG I think [need to check] timebase is stopped too (so offset is 0)\n> >>\n> >> David, do you agree with that?\n> > \n> > Yes, I think so.  Some details might be different, but the basic idea\n> > of migrating the timebase and decrementers at machine level should be\n> > the same for pseries and g3beige/whatever.\n> \n> >From my perspective, I'd really like to bury all of the timebase\n> migration logic into timebase_load()/timebase_save() so then as you\n> suggest above, everything will work regardless of the machine type and host.\n\nThat would certainly be ideal if we can manage it.\n\n> Playing around with these functions I can see that they are very\n> PPC-specific - for example cpu_get_host_ticks() can only ever work\n> correctly migrating between 2 PPC hosts because here on Intel the value\n> returned is the TSC register which is completely unrelated to the\n> timebase frequency. Hence the calculated values are nonsense and the\n> guest inevitably ends up freezing.\n\nUgh.  That sounds like it needs fixing.\n\n> \n> I've had a go at converting this back to using the host clock to\n> calculate the required adjustment to tb_offset (see\n> https://github.com/mcayland/qemu/commit/68e31f0455a79d41b2ab703364c74ceb4d513154)\n> but then even that struck me as incorrect since unless there are other\n> host CPUs with an equivalent of tb_offset, this whole section of code\n> should only ever run if the host CPU is PPC, and maybe even only under KVM.\n\nHm, maybe.  It certainly should be possible to do something\nequivalent, recalculating guest TB as necessary from guest generic\nclock, frequency and offsets in real time units.  It might end up\nbeing desirable to have a special KVM path which works with raw TB\noffsets for simplicity, though.\n\n> My current thoughts are now as follows:\n> \n> - The existing timebase_load() should never adjust tb_offset unless the\n> host CPU is also PPC (how do we actually determine this?). Otherwise it\n> should always be 0.\n\nThat doesn't sound right.  But to work out what does makes sense, we\nneed to figure out what \"tb_offset\" means in the non-KVM case, which\nisn't immediately obvious to me.\n\n> - The per-CPU decrementer values should be *stored* in the normal\n> vmstate SPR array as per my latest patch at\n> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02346.html.\n\nUh.. maybe.  It kind of makes sense, but the lack of symmetry with the\nload path is a bit messy, and may not play well with the way vmstate\ndescriptions work.\n\n> BUT:\n> \n> - The per-CPU decrementer values should be *restored* in timebase_load()\n> but again should not be adjusted by tb_offset unless the host CPU is\n> also PPC.\n\nThat doesn't seem right.  Regardless of whether we're KVM or TCG, we\nshould recalculate the DECR values based on the stored value adjusted\nby how many guest timebase ticks have passed since then (which we'll\nneed to calculate based on qemu_clock_ns() combined with various other\nbits of info (basically the same stuff we already need to compute the\ncorrect timebase on load).\n\nOr to put it another way timebase_load() should calculate (and/or\nload) both the guest TB at save time and the guest TB at load time,\nthen adjust all the DECRs based on the delta between them.\n\nFor extra fun, to cover some of the embedded cpus, we'll need to\nconsider how to handle the PIT, which is sort-of-but-not-quite like\nthe DECR.\n\n> The real question is whether tb_offset is used for a TCG PPC guest on a\n> PPC host or whether it also remains at 0 as it does here on an Intel\n> host? If it does remain at 0 for a TCG PPC guest on a PPC host then we\n> don't need to work out whether we are running on a PPC host at all: we\n> can just skip the timebase adjustments made by timebase_load() if\n> !kvm_enabled() and everything should just work.\n\nIt kind of sounds like we need to rework that value entirely - or at\nleast rename it to something less misleading.  Basically to represent\nthe guest TB we want a tuple of a snapshot TB value with the qemu\nclock time at which that snapshot was taken.  Based on that, plus the\ntb frequency we can calculate a future TB value - and it's a\nmigratable quantity, which raw tb_offset is not (even between ppc\nhosts).\n\nObviously there are several equivalent ways of representing that.\n\n> If this all seems reasonable then what I need to do for the PPC\n> g3beige/mac99 machines is add the relevant machine state, include the\n> existing vmstate_change_handler and then add the code to timebase_load()\n> to set the decrementer. I'm happy to do this as long as everyone agrees\n> this is a sensible approach?\n> \n> \n> ATB,\n> \n> Mark.\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gibson.dropbear.id.au\n\theader.i=@gibson.dropbear.id.au header.b=\"IxapgQNR\"; \n\tdkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxKMs6j4Jz9s7m\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 20:47:53 +1000 (AEST)","from localhost ([::1]:41411 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duG4F-0001oZ-UX\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 06:47:52 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51444)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1duFtd-0001M1-8h\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:57 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1duFtZ-00031w-5b\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:53 -0400","from ozlabs.org ([103.22.144.67]:34991)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgibson@ozlabs.org>)\n\tid 1duFtY-0002xB-2B; Tue, 19 Sep 2017 06:36:48 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3xxK6r2qGsz9t5R; Tue, 19 Sep 2017 20:36:34 +1000 (AEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1505817396;\n\tbh=B6MEU0g2taF2k0w5cdsstnUI4Z6vc3PaDcfwILPfdM0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IxapgQNRr71sIavZ3wnoPp7LlDNzD/Jwuml9OWxuGA+wJ0ci686fc+ykTCRT35YNR\n\tOkR/CgvjKuJZldn1i1tn+Y/d7A8JIFzJkI5VfRWc2sToWJ+ZpbTPRpP0qhvOUZW0Qa\n\tilA9Lqd+kxhofp99Tm71mTbfuObr9wxNaUClwPOY=","Date":"Tue, 19 Sep 2017 18:36:30 +1000","From":"David Gibson <david@gibson.dropbear.id.au>","To":"Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>","Message-ID":"<20170919083630.GV27153@umbus>","References":"<1505054255-2990-1-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<1505054255-2990-5-git-send-email-mark.cave-ayland@ilande.co.uk>\n\t<20170913071249.GI7550@umbus.fritz.box>\n\t<03027eb0-6528-22c6-dcc1-d20323399ec9@ilande.co.uk>\n\t<ccacf611-c2e2-9cad-d927-67c0fcaa3ad6@redhat.com>\n\t<20170914035259.GK3972@umbus.fritz.box>\n\t<56298dd2-eabe-5075-61b3-c27a0eaf4579@ilande.co.uk>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"3jK+0sHr6j/jwA0V\"","Content-Disposition":"inline","In-Reply-To":"<56298dd2-eabe-5075-61b3-c27a0eaf4579@ilande.co.uk>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"103.22.144.67","Subject":"Re: [Qemu-devel] [PATCH 4/4] ppc: ensure we update the decrementer\n\tvalue during migration","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Laurent Vivier <lvivier@redhat.com>, aik@ozlabs.ru, qemu-ppc@nongnu.org, \n\tqemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]