[{"id":1779357,"web_url":"http://patchwork.ozlabs.org/comment/1779357/","msgid":"<20171004013456.GR3260@umbus.fritz.box>","list_archive_url":null,"date":"2017-10-04T01:34:56","subject":"Re: [Qemu-devel] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability","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:21PM +0530, Aravinda Prasad wrote:\n> Enable the KVM capability KVM_CAP_PPC_FWNMI so that\n> the KVM causes guest exit with NMI as exit reason\n> when it encounters a machine check exception on the\n> address belonging to a guest. Without this capability\n> enabled, KVM redirects machine check exceptions to\n> guest's 0x200 vector.\n> \n> Signed-off-by: Aravinda Prasad \n> ---\n> hw/ppc/spapr_rtas.c | 15 +++++++++++++++\n> target/ppc/kvm.c | 13 +++++++++++++\n> target/ppc/kvm_ppc.h | 6 ++++++\n> 3 files changed, 34 insertions(+)\n> \n> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c\n> index 08e9a5e..d017a67 100644\n> --- a/hw/ppc/spapr_rtas.c\n> +++ b/hw/ppc/spapr_rtas.c\n> @@ -46,6 +46,7 @@\n> #include \"qemu/cutils.h\"\n> #include \"trace.h\"\n> #include \"hw/ppc/fdt.h\"\n> +#include \"kvm_ppc.h\"\n> \n> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,\n> uint32_t token, uint32_t nargs,\n> @@ -354,6 +355,20 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,\n> target_ulong args,\n> uint32_t nret, target_ulong rets)\n> {\n> + int ret;\n> +\n> + ret = kvmppc_fwnmi_enable(cpu);\n\nIf you're enabling it here, doesn't that mean you need to disable it\non reset?\n\n> + if (ret == 1) {\n> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);\n> + return;\n> + }\n> +\n> + if (ret < 0) {\n> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);\n> + return;\n> + }\n> +\n> spapr->guest_machine_check_addr = rtas_ld(args, 1);\n> rtas_st(rets, 0, RTAS_OUT_SUCCESS);\n> }\n> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n> index 7e4ce02..59b3322 100644\n> --- a/target/ppc/kvm.c\n> +++ b/target/ppc/kvm.c\n> @@ -92,6 +92,7 @@ static int cap_mmu_radix;\n> static int cap_mmu_hash_v3;\n> static int cap_resize_hpt;\n> static int cap_ppc_pvr_compat;\n> +static int cap_fwnmi;\n> \n> static uint32_t debug_inst_opcode;\n> \n> @@ -150,6 +151,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)\n> cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);\n> cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);\n> cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);\n> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);\n> /*\n> * Note: setting it to false because there is not such capability\n> * in KVM at this moment.\n> @@ -2142,6 +2144,17 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n> }\n> }\n> \n> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n> +{\n> + CPUState *cs = CPU(cpu);\n> +\n> + if (!cap_fwnmi) {\n> + return 1;\n> + }\n> +\n> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);\n\nYeah, this is no good. It means migration from a host that's fwnmi\ncapable to one that isn't will be subtly broken. Instead you need to\nmake fwnmi capability a machine property. If the property is\nrequested and the host kernel doesn't support it, you need to outright\nfail, rather than try to fall back.\n\n> +}\n> +\n> int kvmppc_smt_threads(void)\n> {\n> return cap_ppc_smt ? cap_ppc_smt : 1;\n> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h\n> index 0139dae..55b6df2 100644\n> --- a/target/ppc/kvm_ppc.h\n> +++ b/target/ppc/kvm_ppc.h\n> @@ -28,6 +28,7 @@ void kvmppc_enable_clear_ref_mod_hcalls(void);\n> void kvmppc_set_papr(PowerPCCPU *cpu);\n> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);\n> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);\n> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);\n> int kvmppc_smt_threads(void);\n> void kvmppc_hint_smt_possible(Error **errp);\n> int kvmppc_set_smt_threads(int smt);\n> @@ -157,6 +158,11 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n> {\n> }\n> \n> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n> +{\n> + return 1;\n\nLikewise, this should be available, not banned, on TCG. I think there\nare existing problems with TCG<->KVM migration, but there's no\ninherent reason they shouldn't work, so we don't want to introduce\nextra reasons they don't.\n\nEven if TCG will never generate fwnmis (for now), it should allow the\nguest to register for them.\n\n> +}\n> +\n> static inline int kvmppc_smt_threads(void)\n> {\n> return 1;\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=\"cJvsUL+4\"; \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 3y6JZw215Kz9sNc\n\tfor ;\n\tWed, 4 Oct 2017 12:43:36 +1100 (AEDT)","from localhost ([::1]:60935 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dzYik-0000Ey-D1\n\tfor incoming@patchwork.ozlabs.org; Tue, 03 Oct 2017 21:43:34 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57461)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dzYf3-000727-6k\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-0007BK-5G\n\tfor qemu-devel@nongnu.org; Tue, 03 Oct 2017 21:39:45 -0400","from ozlabs.org ([103.22.144.67]:43225)\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-00077x-Hy; Tue, 03 Oct 2017 21:39:42 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3y6JVJ3cswz9t2c; 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=MmNqMi0qbVoKXgQpZHeVcEclpq4kKRTIYfSKdFpR0XU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cJvsUL+4iYShqrm0EjYJ++zIrwKiaaiB4FZ7CWdssrHKTKaUB5ZJ0wW3qjvQIIfiq\n\tqHb679ihLLBQ6DwyQAIuc/LjtQeLMAOmIZaCMXALpVYgVkZD1MeHf2WJQcY/vFD5ua\n\tyMUIj+lqZPnqtdomDXgLjYVx78YwXjvtzJJmn6Zc=","Date":"Wed, 4 Oct 2017 12:34:56 +1100","From":"David Gibson ","To":"Aravinda Prasad ","Message-ID":"<20171004013456.GR3260@umbus.fritz.box>","References":"<150659494872.25889.2069124544245723984.stgit@aravinda>\n\t<150659510129.25889.17733386928036958909.stgit@aravinda>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"N+qDRRsDvMgizTft\"","Content-Disposition":"inline","In-Reply-To":"<150659510129.25889.17733386928036958909.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 5/6] ppc: spapr: Enable FWNMI capability","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":1782208,"web_url":"http://patchwork.ozlabs.org/comment/1782208/","msgid":"<9b20d8a7-c542-2b57-0b80-1787d7b044b6@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-10-08T08:26:06","subject":"Re: [Qemu-devel] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability","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:04 AM, David Gibson wrote:\n> On Thu, Sep 28, 2017 at 04:08:21PM +0530, Aravinda Prasad wrote:\n>> Enable the KVM capability KVM_CAP_PPC_FWNMI so that\n>> the KVM causes guest exit with NMI as exit reason\n>> when it encounters a machine check exception on the\n>> address belonging to a guest. Without this capability\n>> enabled, KVM redirects machine check exceptions to\n>> guest's 0x200 vector.\n>>\n>> Signed-off-by: Aravinda Prasad \n>> ---\n>> hw/ppc/spapr_rtas.c | 15 +++++++++++++++\n>> target/ppc/kvm.c | 13 +++++++++++++\n>> target/ppc/kvm_ppc.h | 6 ++++++\n>> 3 files changed, 34 insertions(+)\n>>\n>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c\n>> index 08e9a5e..d017a67 100644\n>> --- a/hw/ppc/spapr_rtas.c\n>> +++ b/hw/ppc/spapr_rtas.c\n>> @@ -46,6 +46,7 @@\n>> #include \"qemu/cutils.h\"\n>> #include \"trace.h\"\n>> #include \"hw/ppc/fdt.h\"\n>> +#include \"kvm_ppc.h\"\n>> \n>> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,\n>> uint32_t token, uint32_t nargs,\n>> @@ -354,6 +355,20 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,\n>> target_ulong args,\n>> uint32_t nret, target_ulong rets)\n>> {\n>> + int ret;\n>> +\n>> + ret = kvmppc_fwnmi_enable(cpu);\n> \n> If you're enabling it here, doesn't that mean you need to disable it\n> on reset?\n\nWe don't have a way in KVM to disable it once enabled.\n\n> \n>> + if (ret == 1) {\n>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);\n>> + return;\n>> + }\n>> +\n>> + if (ret < 0) {\n>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);\n>> + return;\n>> + }\n>> +\n>> spapr->guest_machine_check_addr = rtas_ld(args, 1);\n>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);\n>> }\n>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n>> index 7e4ce02..59b3322 100644\n>> --- a/target/ppc/kvm.c\n>> +++ b/target/ppc/kvm.c\n>> @@ -92,6 +92,7 @@ static int cap_mmu_radix;\n>> static int cap_mmu_hash_v3;\n>> static int cap_resize_hpt;\n>> static int cap_ppc_pvr_compat;\n>> +static int cap_fwnmi;\n>> \n>> static uint32_t debug_inst_opcode;\n>> \n>> @@ -150,6 +151,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)\n>> cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);\n>> cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);\n>> cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);\n>> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);\n>> /*\n>> * Note: setting it to false because there is not such capability\n>> * in KVM at this moment.\n>> @@ -2142,6 +2144,17 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n>> }\n>> }\n>> \n>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n>> +{\n>> + CPUState *cs = CPU(cpu);\n>> +\n>> + if (!cap_fwnmi) {\n>> + return 1;\n>> + }\n>> +\n>> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);\n> \n> Yeah, this is no good. It means migration from a host that's fwnmi\n> capable to one that isn't will be subtly broken. Instead you need to\n> make fwnmi capability a machine property. If the property is\n> requested and the host kernel doesn't support it, you need to outright\n> fail, rather than try to fall back.\n\nSure.\n\n> \n>> +}\n>> +\n>> int kvmppc_smt_threads(void)\n>> {\n>> return cap_ppc_smt ? cap_ppc_smt : 1;\n>> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h\n>> index 0139dae..55b6df2 100644\n>> --- a/target/ppc/kvm_ppc.h\n>> +++ b/target/ppc/kvm_ppc.h\n>> @@ -28,6 +28,7 @@ void kvmppc_enable_clear_ref_mod_hcalls(void);\n>> void kvmppc_set_papr(PowerPCCPU *cpu);\n>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);\n>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);\n>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);\n>> int kvmppc_smt_threads(void);\n>> void kvmppc_hint_smt_possible(Error **errp);\n>> int kvmppc_set_smt_threads(int smt);\n>> @@ -157,6 +158,11 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n>> {\n>> }\n>> \n>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n>> +{\n>> + return 1;\n> \n> Likewise, this should be available, not banned, on TCG. I think there\n> are existing problems with TCG<->KVM migration, but there's no\n> inherent reason they shouldn't work, so we don't want to introduce\n> extra reasons they don't.\n> \n> Even if TCG will never generate fwnmis (for now), it should allow the\n> guest to register for them.\n\nShould this be then changed later only when TCG generates fwnmis?\nBecause we don't want to set spapr->guest_machine_check_addr for TCG in\nrtas_ibm_nmi_register(). If set then all machine checks are redirected\nto this address which is not desirable for TCG as we still want machine\nchecks to be passed via 0x200.\n\nRegards,\nAravinda\n\n> \n>> +}\n>> +\n>> static inline int kvmppc_smt_threads(void)\n>> {\n>> return 1;\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 3y8xLR3LB6z9t4B\n\tfor ;\n\tSun, 8 Oct 2017 19:26:55 +1100 (AEDT)","from localhost ([::1]:52893 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e16vF-0006Dr-Ht\n\tfor incoming@patchwork.ozlabs.org; Sun, 08 Oct 2017 04:26:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36464)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e16un-0006C3-7J\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:26:26 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e16uk-0006Bn-2R\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:26:25 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:42416)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1e16uj-0006BG-Qh\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 04:26:22 -0400","from pps.filterd (m0098393.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv988OEPY005008\n\tfor ; Sun, 8 Oct 2017 04:26:17 -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 2dey8evua9-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Sun, 08 Oct 2017 04:26:17 -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:26:16 -0400","from b01cxnp23033.gho.pok.ibm.com (9.57.198.28)\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:26:13 -0400","from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com\n\t[9.57.199.110])\n\tby b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v988QBhn64684052; Sun, 8 Oct 2017 08:26:11 GMT","from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id DD63CAE03B;\n\tSun, 8 Oct 2017 04:26:48 -0400 (EDT)","from [9.79.216.34] (unknown [9.79.216.34])\n\tby b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 42095AE04B;\n\tSun, 8 Oct 2017 04:26:45 -0400 (EDT)"],"To":"David Gibson ","References":"<150659494872.25889.2069124544245723984.stgit@aravinda>\n\t<150659510129.25889.17733386928036958909.stgit@aravinda>\n\t<20171004013456.GR3260@umbus.fritz.box>","From":"Aravinda Prasad ","Date":"Sun, 8 Oct 2017 13:56:06 +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":"<20171004013456.GR3260@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-00000227BBA9","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.00928078; UDB=6.00467026;\n\tIPR=6.00708349; \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:26:14","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17100808-2214-0000-0000-000057C91268","Message-Id":"<9b20d8a7-c542-2b57-0b80-1787d7b044b6@linux.vnet.ibm.com>","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=0\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-1710080125","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability","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":1782364,"web_url":"http://patchwork.ozlabs.org/comment/1782364/","msgid":"<20171008234353.GO10050@umbus.fritz.box>","list_archive_url":null,"date":"2017-10-08T23:43:53","subject":"Re: [Qemu-devel] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Sun, Oct 08, 2017 at 01:56:06PM +0530, Aravinda Prasad wrote:\n> \n> \n> On Wednesday 04 October 2017 07:04 AM, David Gibson wrote:\n> > On Thu, Sep 28, 2017 at 04:08:21PM +0530, Aravinda Prasad wrote:\n> >> Enable the KVM capability KVM_CAP_PPC_FWNMI so that\n> >> the KVM causes guest exit with NMI as exit reason\n> >> when it encounters a machine check exception on the\n> >> address belonging to a guest. Without this capability\n> >> enabled, KVM redirects machine check exceptions to\n> >> guest's 0x200 vector.\n> >>\n> >> Signed-off-by: Aravinda Prasad \n> >> ---\n> >> hw/ppc/spapr_rtas.c | 15 +++++++++++++++\n> >> target/ppc/kvm.c | 13 +++++++++++++\n> >> target/ppc/kvm_ppc.h | 6 ++++++\n> >> 3 files changed, 34 insertions(+)\n> >>\n> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c\n> >> index 08e9a5e..d017a67 100644\n> >> --- a/hw/ppc/spapr_rtas.c\n> >> +++ b/hw/ppc/spapr_rtas.c\n> >> @@ -46,6 +46,7 @@\n> >> #include \"qemu/cutils.h\"\n> >> #include \"trace.h\"\n> >> #include \"hw/ppc/fdt.h\"\n> >> +#include \"kvm_ppc.h\"\n> >> \n> >> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,\n> >> uint32_t token, uint32_t nargs,\n> >> @@ -354,6 +355,20 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,\n> >> target_ulong args,\n> >> uint32_t nret, target_ulong rets)\n> >> {\n> >> + int ret;\n> >> +\n> >> + ret = kvmppc_fwnmi_enable(cpu);\n> > \n> > If you're enabling it here, doesn't that mean you need to disable it\n> > on reset?\n> \n> We don't have a way in KVM to disable it once enabled.\n\nHm, ok. Is it possible to simulate \"old style\" handling in qemu, even\nif KVM is in the new mode? I guess you can catch the NMI exit from\nKVM, but redirect it back to the 0x200 vector in the guest, yes?\n\nIf that's so, you might as well try to enable this in the kernel at\nmachine init time, rather than waiting until nmi-register is called.\nThat way the user gets an error early if the kernel can't handle it\non a VM that's supposed to support it.\n\n> \n> > \n> >> + if (ret == 1) {\n> >> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);\n> >> + return;\n> >> + }\n> >> +\n> >> + if (ret < 0) {\n> >> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);\n> >> + return;\n> >> + }\n> >> +\n> >> spapr->guest_machine_check_addr = rtas_ld(args, 1);\n> >> rtas_st(rets, 0, RTAS_OUT_SUCCESS);\n> >> }\n> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c\n> >> index 7e4ce02..59b3322 100644\n> >> --- a/target/ppc/kvm.c\n> >> +++ b/target/ppc/kvm.c\n> >> @@ -92,6 +92,7 @@ static int cap_mmu_radix;\n> >> static int cap_mmu_hash_v3;\n> >> static int cap_resize_hpt;\n> >> static int cap_ppc_pvr_compat;\n> >> +static int cap_fwnmi;\n> >> \n> >> static uint32_t debug_inst_opcode;\n> >> \n> >> @@ -150,6 +151,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)\n> >> cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);\n> >> cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);\n> >> cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);\n> >> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);\n> >> /*\n> >> * Note: setting it to false because there is not such capability\n> >> * in KVM at this moment.\n> >> @@ -2142,6 +2144,17 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n> >> }\n> >> }\n> >> \n> >> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n> >> +{\n> >> + CPUState *cs = CPU(cpu);\n> >> +\n> >> + if (!cap_fwnmi) {\n> >> + return 1;\n> >> + }\n> >> +\n> >> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);\n> > \n> > Yeah, this is no good. It means migration from a host that's fwnmi\n> > capable to one that isn't will be subtly broken. Instead you need to\n> > make fwnmi capability a machine property. If the property is\n> > requested and the host kernel doesn't support it, you need to outright\n> > fail, rather than try to fall back.\n> \n> Sure.\n> \n> > \n> >> +}\n> >> +\n> >> int kvmppc_smt_threads(void)\n> >> {\n> >> return cap_ppc_smt ? cap_ppc_smt : 1;\n> >> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h\n> >> index 0139dae..55b6df2 100644\n> >> --- a/target/ppc/kvm_ppc.h\n> >> +++ b/target/ppc/kvm_ppc.h\n> >> @@ -28,6 +28,7 @@ void kvmppc_enable_clear_ref_mod_hcalls(void);\n> >> void kvmppc_set_papr(PowerPCCPU *cpu);\n> >> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);\n> >> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);\n> >> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);\n> >> int kvmppc_smt_threads(void);\n> >> void kvmppc_hint_smt_possible(Error **errp);\n> >> int kvmppc_set_smt_threads(int smt);\n> >> @@ -157,6 +158,11 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)\n> >> {\n> >> }\n> >> \n> >> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)\n> >> +{\n> >> + return 1;\n> > \n> > Likewise, this should be available, not banned, on TCG. I think there\n> > are existing problems with TCG<->KVM migration, but there's no\n> > inherent reason they shouldn't work, so we don't want to introduce\n> > extra reasons they don't.\n> > \n> > Even if TCG will never generate fwnmis (for now), it should allow the\n> > guest to register for them.\n> \n> Should this be then changed later only when TCG generates fwnmis?\n> Because we don't want to set spapr->guest_machine_check_addr for TCG in\n> rtas_ibm_nmi_register(). If set then all machine checks are redirected\n> to this address which is not desirable for TCG as we still want machine\n> checks to be passed via 0x200.\n\nUm.. why? The guest has asked for (synchronous) machine checks to go\nto its specified address. That's what it should get, regardless of\nthe accelerator it's running on.\n\n> \n> Regards,\n> Aravinda\n> \n> > \n> >> +}\n> >> +\n> >> static inline int kvmppc_smt_threads(void)\n> >> {\n> >> return 1;\n> >>\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=)","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=\"LG2lddJV\"; \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 3y9L256Rt8z9t2c\n\tfor ;\n\tMon, 9 Oct 2017 10:59:09 +1100 (AEDT)","from localhost ([::1]:55513 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e1LTQ-0005Aj-0Y\n\tfor incoming@patchwork.ozlabs.org; Sun, 08 Oct 2017 19:59:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41330)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e1LSp-00059B-1F\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 19:58:32 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e1LSl-0000Y0-W2\n\tfor qemu-devel@nongnu.org; Sun, 08 Oct 2017 19:58:31 -0400","from ozlabs.org ([103.22.144.67]:56403)\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 1e1LSl-0000X5-C6; Sun, 08 Oct 2017 19:58:27 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3y9L1B6Q4sz9t2c; Mon, 9 Oct 2017 10:58:22 +1100 (AEDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1507507102;\n\tbh=4TD8PyRRqRbeWJpnQuk6KeiWObPQQ2oWtDMULe5pipY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LG2lddJVG/Efro4HUT0W08NAXg+VKVjn+3X9cYuzjkehA6zv66GXjarM5Xxaw1/wG\n\tC2GOPIXlH/77w8ttOcjDAPolcD4fbUxDTeBr3dMy8KjANNNl3uvKigQ8y7d+LFFbSF\n\tNGUc+BLHJ/VxcfsQOrhdMsGN/Oexung/iAK+2/z0=","Date":"Mon, 9 Oct 2017 10:43:53 +1100","From":"David Gibson ","To":"Aravinda Prasad ","Message-ID":"<20171008234353.GO10050@umbus.fritz.box>","References":"<150659494872.25889.2069124544245723984.stgit@aravinda>\n\t<150659510129.25889.17733386928036958909.stgit@aravinda>\n\t<20171004013456.GR3260@umbus.fritz.box>\n\t<9b20d8a7-c542-2b57-0b80-1787d7b044b6@linux.vnet.ibm.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"D3I0HgOdJ5+6n+7I\"","Content-Disposition":"inline","In-Reply-To":"<9b20d8a7-c542-2b57-0b80-1787d7b044b6@linux.vnet.ibm.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","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 5/6] ppc: spapr: Enable FWNMI capability","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"}}]