[{"id":1779356,"web_url":"http://patchwork.ozlabs.org/comment/1779356/","msgid":"<20171004013931.GS3260@umbus.fritz.box>","list_archive_url":null,"date":"2017-10-04T01:39:32","subject":"Re: [Qemu-devel] [PATCH v5 6/6] migration: Block migration while\n\thandling machine check","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Thu, Sep 28, 2017 at 04:08:31PM +0530, Aravinda Prasad wrote:\n> Block VM migration requests until the machine check\n> error handling is complete as (i) these errors are\n> specific to the source hardware and is irrelevant on\n> the target hardware, (ii) these errors cause data\n> corruption and should be handled before migration.\n> \n> Signed-off-by: Aravinda Prasad \n> ---\n> hw/ppc/spapr_rtas.c | 3 +++\n> include/hw/ppc/spapr.h | 2 ++\n> target/ppc/kvm.c | 17 +++++++++++++++++\n> 3 files changed, 22 insertions(+)\n> \n> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c\n> index d017a67..17f6567 100644\n> --- a/hw/ppc/spapr_rtas.c\n> +++ b/hw/ppc/spapr_rtas.c\n> @@ -47,6 +47,7 @@\n> #include \"trace.h\"\n> #include \"hw/ppc/fdt.h\"\n> #include \"kvm_ppc.h\"\n> +#include \"migration/blocker.h\"\n> \n> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,\n> uint32_t token, uint32_t nargs,\n> @@ -390,6 +391,8 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,\n> spapr->mc_status = -1;\n> qemu_cond_signal(&spapr->mc_delivery_cond);\n> rtas_st(rets, 0, RTAS_OUT_SUCCESS);\n> + migrate_del_blocker(spapr->migration_blocker);\n> + error_free(spapr->migration_blocker);\n> }\n> }\n> \n> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h\n> index a75e9cf..0890a44 100644\n> --- a/include/hw/ppc/spapr.h\n> +++ b/include/hw/ppc/spapr.h\n> @@ -7,6 +7,7 @@\n> #include \"hw/ppc/spapr_drc.h\"\n> #include \"hw/mem/pc-dimm.h\"\n> #include \"hw/ppc/spapr_ovec.h\"\n> +#include \"qapi/error.h\"\n> \n> struct VIOsPAPRBus;\n> struct sPAPRPHBState;\n> @@ -136,6 +137,7 @@ struct sPAPRMachineState {\n> MemoryHotplugState hotplug_memory;\n> \n> const char *icp_type;\n> + Error *migration_blocker;\n\nThis isn't a good name, because it's _specifically_ the fwnmi as a\nmigration blocker - trying to put another migration blocker in here\nwould break horribly, because nmi-interlock would clear it regardless.\n\n> };\n> \n> #define H_SUCCESS 0\n> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n> index 59b3322..58de7ea 100644\n> --- a/target/ppc/kvm.c\n> +++ b/target/ppc/kvm.c\n> @@ -52,6 +52,7 @@\n> #endif\n> #include \"elf.h\"\n> #include \"sysemu/kvm_int.h\"\n> +#include \"migration/blocker.h\"\n> \n> //#define DEBUG_KVM\n> \n> @@ -2770,10 +2771,26 @@ int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)\n> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());\n> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);\n> target_ulong msr = 0;\n> + Error *local_err = NULL;\n> + int ret;\n> bool type, le;\n> \n> cpu_synchronize_state(CPU(cpu));\n> \n> + error_setg(&spapr->migration_blocker,\n> + \"Live migration not supported during machine check error handling\");\n\nIn fact, there's no real reason to generate the error here. The\nerror's always the same so you could just create it at startup as a\nglobal and just add/remove it to the block list.\n\n> + ret = migrate_add_blocker(spapr->migration_blocker, &local_err);\n> + if (ret < 0) {\n> + /*\n> + * We don't want to abort and let the migration to continue. In a\n> + * rare case, the machine check handler will run on the target\n> + * hardware. Though this is not preferable, it is better than aborting\n\nWhy is it not preferable? I mean it's an edge case, but AFAICT it's\nstill the correct behaviour.\n\n> + * the migration or killing the VM.\n> + */\n> + error_free(spapr->migration_blocker);\n> + fprintf(stderr, \"Warning: Machine check during VM migration\\n\");\n\nUse error_report(), not fprintf().\n\n> + }\n> +\n> /*\n> * Properly set bits in MSR before we invoke the handler.\n> * SRR0/1, DAR and DSISR are properly set by KVM\n>","headers":{"Return-Path":"","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=)","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=\"gxVhY3GL\"; \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 3y6JY74p7Vz9sNc\n\tfor ;\n\tWed, 4 Oct 2017 12:42:03 +1100 (AEDT)","from localhost ([::1]:60932 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dzYhF-00082B-PD\n\tfor incoming@patchwork.ozlabs.org; Tue, 03 Oct 2017 21:42:01 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57462)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dzYf3-000728-6y\n\tfor qemu-devel@nongnu.org; Tue, 03 Oct 2017 21:39:46 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dzYf0-0007BE-4w\n\tfor qemu-devel@nongnu.org; Tue, 03 Oct 2017 21:39:45 -0400","from ozlabs.org ([103.22.144.67]:47995)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dzYez-00077w-Hn; Tue, 03 Oct 2017 21:39:42 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3y6JVJ2tB3z9s7B; Wed, 4 Oct 2017 12:39:36 +1100 (AEDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1507081176;\n\tbh=cO7ZjY7MXWvrA8AlGgLZh/FtauQDrqhBCI9HSIGsExs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gxVhY3GLPlFFaOW3heURjUm836912/SxIJt5dG/A56zzj8oDeUiRKQte/eoifNqTN\n\tP/dlZgxThYctrowgLx1yjV+w8O5WMdRJLd6GbjMOBbU0d78TtUuJy4M9KgWDRjrdQm\n\tPyEHNzca2Yjh8yboJGECIlVso3tx66aBt+nzLvGw=","Date":"Wed, 4 Oct 2017 12:39:32 +1100","From":"David Gibson ","To":"Aravinda Prasad ","Message-ID":"<20171004013931.GS3260@umbus.fritz.box>","References":"<150659494872.25889.2069124544245723984.stgit@aravinda>\n\t<150659511184.25889.10868411111377268218.stgit@aravinda>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"1Sa0i77dkqaUxqYg\"","Content-Disposition":"inline","In-Reply-To":"<150659511184.25889.10868411111377268218.stgit@aravinda>","User-Agent":"Mutt/1.9.0 (2017-09-02)","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 v5 6/6] migration: Block migration while\n\thandling machine check","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"benh@au1.ibm.com, aik@ozlabs.ru, qemu-devel@nongnu.org,\n\tqemu-ppc@nongnu.org, paulus@samba.org, sam.bobroff@au1.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1782207,"web_url":"http://patchwork.ozlabs.org/comment/1782207/","msgid":"","list_archive_url":null,"date":"2017-10-08T08:07:19","subject":"Re: [Qemu-devel] [PATCH v5 6/6] migration: Block migration while\n\thandling machine check","submitter":{"id":18580,"url":"http://patchwork.ozlabs.org/api/people/18580/","name":"Aravinda Prasad","email":"aravinda@linux.vnet.ibm.com"},"content":"On Wednesday 04 October 2017 07:09 AM, David Gibson wrote:\n> On Thu, Sep 28, 2017 at 04:08:31PM +0530, Aravinda Prasad wrote:\n>> Block VM migration requests until the machine check\n>> error handling is complete as (i) these errors are\n>> specific to the source hardware and is irrelevant on\n>> the target hardware, (ii) these errors cause data\n>> corruption and should be handled before migration.\n>>\n>> Signed-off-by: Aravinda Prasad \n>> ---\n>> hw/ppc/spapr_rtas.c | 3 +++\n>> include/hw/ppc/spapr.h | 2 ++\n>> target/ppc/kvm.c | 17 +++++++++++++++++\n>> 3 files changed, 22 insertions(+)\n>>\n>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c\n>> index d017a67..17f6567 100644\n>> --- a/hw/ppc/spapr_rtas.c\n>> +++ b/hw/ppc/spapr_rtas.c\n>> @@ -47,6 +47,7 @@\n>> #include \"trace.h\"\n>> #include \"hw/ppc/fdt.h\"\n>> #include \"kvm_ppc.h\"\n>> +#include \"migration/blocker.h\"\n>> \n>> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,\n>> uint32_t token, uint32_t nargs,\n>> @@ -390,6 +391,8 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,\n>> spapr->mc_status = -1;\n>> qemu_cond_signal(&spapr->mc_delivery_cond);\n>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);\n>> + migrate_del_blocker(spapr->migration_blocker);\n>> + error_free(spapr->migration_blocker);\n>> }\n>> }\n>> \n>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h\n>> index a75e9cf..0890a44 100644\n>> --- a/include/hw/ppc/spapr.h\n>> +++ b/include/hw/ppc/spapr.h\n>> @@ -7,6 +7,7 @@\n>> #include \"hw/ppc/spapr_drc.h\"\n>> #include \"hw/mem/pc-dimm.h\"\n>> #include \"hw/ppc/spapr_ovec.h\"\n>> +#include \"qapi/error.h\"\n>> \n>> struct VIOsPAPRBus;\n>> struct sPAPRPHBState;\n>> @@ -136,6 +137,7 @@ struct sPAPRMachineState {\n>> MemoryHotplugState hotplug_memory;\n>> \n>> const char *icp_type;\n>> + Error *migration_blocker;\n> \n> This isn't a good name, because it's _specifically_ the fwnmi as a\n> migration blocker - trying to put another migration blocker in here\n> would break horribly, because nmi-interlock would clear it regardless.\n\nWill rename it.\n\n> \n>> };\n>> \n>> #define H_SUCCESS 0\n>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n>> index 59b3322..58de7ea 100644\n>> --- a/target/ppc/kvm.c\n>> +++ b/target/ppc/kvm.c\n>> @@ -52,6 +52,7 @@\n>> #endif\n>> #include \"elf.h\"\n>> #include \"sysemu/kvm_int.h\"\n>> +#include \"migration/blocker.h\"\n>> \n>> //#define DEBUG_KVM\n>> \n>> @@ -2770,10 +2771,26 @@ int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)\n>> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());\n>> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);\n>> target_ulong msr = 0;\n>> + Error *local_err = NULL;\n>> + int ret;\n>> bool type, le;\n>> \n>> cpu_synchronize_state(CPU(cpu));\n>> \n>> + error_setg(&spapr->migration_blocker,\n>> + \"Live migration not supported during machine check error handling\");\n> \n> In fact, there's no real reason to generate the error here. The\n> error's always the same so you could just create it at startup as a\n> global and just add/remove it to the block list.\n\nok.\n\n> \n>> + ret = migrate_add_blocker(spapr->migration_blocker, &local_err);\n>> + if (ret < 0) {\n>> + /*\n>> + * We don't want to abort and let the migration to continue. In a\n>> + * rare case, the machine check handler will run on the target\n>> + * hardware. Though this is not preferable, it is better than aborting\n> \n> Why is it not preferable? I mean it's an edge case, but AFAICT it's\n> still the correct behaviour.\n\nWill remove that line in the comment.\n\n> \n>> + * the migration or killing the VM.\n>> + */\n>> + error_free(spapr->migration_blocker);\n>> + fprintf(stderr, \"Warning: Machine check during VM migration\\n\");\n> \n> Use error_report(), not fprintf().\n\nok.\n\nRegards,\nAravinda\n\n> \n>> + }\n>> +\n>> /*\n>> * Properly set bits in MSR before we invoke the handler.\n>> * SRR0/1, DAR and DSISR are properly set by KVM\n>>\n>","headers":{"Return-Path":"","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=)","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 3y8wwl4cjbz9t45\n\tfor ;\n\tSun, 8 Oct 2017 19:08:07 +1100 (AEDT)","from localhost ([::1]:52848 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e16d3-0002Mc-My\n\tfor incoming@patchwork.ozlabs.org; Sun, 08 Oct 2017 04:08:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33328)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e16cZ-0002LY-Mp\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:07:36 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e16cW-0003nO-IJ\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:07:35 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:57346\n\thelo=mx0a-001b2d01.pphosted.com)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1e16cW-0003n6-CY\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:07:32 -0400","from pps.filterd (m0098421.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv9884Bxn108304\n\tfor ; Sun, 8 Oct 2017 04:07:28 -0400","from e11.ny.us.ibm.com (e11.ny.us.ibm.com [129.33.205.201])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2dew4aqmsv-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Sun, 08 Oct 2017 04:07:28 -0400","from localhost\n\tby e11.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tSun, 8 Oct 2017 04:07:27 -0400","from b01cxnp23032.gho.pok.ibm.com (9.57.198.27)\n\tby e11.ny.us.ibm.com (146.89.104.198) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tSun, 8 Oct 2017 04:07:25 -0400","from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com\n\t[9.57.199.110])\n\tby b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v9887NSK36700194; Sun, 8 Oct 2017 08:07:23 GMT","from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 394E4AE043;\n\tSun, 8 Oct 2017 04:08:01 -0400 (EDT)","from [9.79.216.34] (unknown [9.79.216.34])\n\tby b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 69024AE03B;\n\tSun, 8 Oct 2017 04:07:58 -0400 (EDT)"],"To":"David Gibson ","References":"<150659494872.25889.2069124544245723984.stgit@aravinda>\n\t<150659511184.25889.10868411111377268218.stgit@aravinda>\n\t<20171004013931.GS3260@umbus.fritz.box>","From":"Aravinda Prasad ","Date":"Sun, 8 Oct 2017 13:37:19 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.7.0","MIME-Version":"1.0","In-Reply-To":"<20171004013931.GS3260@umbus.fritz.box>","Content-Type":"text/plain; charset=windows-1252","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17100808-2213-0000-0000-00000227BA6C","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007860; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000235; SDB=6.00928072; UDB=6.00467022;\n\tIPR=6.00708343; \n\tBA=6.00005626; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017445;\n\tXFM=3.00000015; UTC=2017-10-08 08:07:27","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17100808-2214-0000-0000-000057C90A46","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-10-08_02:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=2\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1710080119","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.158.5","Subject":"Re: [Qemu-devel] [PATCH v5 6/6] migration: Block migration while\n\thandling machine check","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"benh@au1.ibm.com, aik@ozlabs.ru, qemu-devel@nongnu.org,\n\tqemu-ppc@nongnu.org, paulus@samba.org, sam.bobroff@au1.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]