From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REdIw-00086x-O6 for qemu-devel@nongnu.org; Fri, 14 Oct 2011 04:39:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1REdIr-0001mQ-Ig for qemu-devel@nongnu.org; Fri, 14 Oct 2011 04:39:46 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:51447) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REdIr-0001m5-5o for qemu-devel@nongnu.org; Fri, 14 Oct 2011 04:39:41 -0400 Received: from moweb001.kundenserver.de (moweb001.kundenserver.de [172.19.20.114]) by fmmailgate02.web.de (Postfix) with ESMTP id 7057F1B1659AA for ; Fri, 14 Oct 2011 10:31:26 +0200 (CEST) Message-ID: <4E97F356.7040700@web.de> Date: Fri, 14 Oct 2011 10:31:18 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20110913093835.GB4265@localhost.localdomain> <20110914093441.e2bb305c.kamezawa.hiroyu@jp.fujitsu.com> <4E705BC3.5000508@cn.fujitsu.com> <20110915164704.9cacd407.kamezawa.hiroyu@jp.fujitsu.com> <4E71B28F.7030201@cn.fujitsu.com> <4E72F3BA.2000603@jp.fujitsu.com> <4E73200A.7040908@jp.fujitsu.com> <4E76C6AA.9080403@cn.fujitsu.com> <4E7B04DC.1030407@cn.fujitsu.com> <4E7B4B8F.507@siemens.com> <4E7C51E4.2000503@cn.fujitsu.com> <4E7F3585.40108@redhat.com> <4E7F635E.6080009@web.de> <4E8035F9.9080908@redhat.com> <4E928B54.1070707@cn.fujitsu.com> <4E92958E.9000509@web.de> <4E9476E2.1070804@cn.fujitsu.com> <4E948842.4030406@web.de> <4E978827.6070008@cn.fujitsu.com> <4E97CE42.9020102@web.de> <4E97D85C.7070107@cn.fujitsu.com> <4E97DB62.9020605@web.de> <4E97E829.5040603@cn.fujitsu.com> In-Reply-To: <4E97E829.5040603@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig36B088AA58156A1FA949C568" Subject: Re: [Qemu-devel] [PATCH 1/1 V4] qemu-kvm: fix improper nmi emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Lai Jiangshan Cc: "kvm@vger.kernel.org" , Avi Kivity , "qemu-devel@nongnu.org" , KAMEZAWA Hiroyuki , Kenji Kaneshige This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig36B088AA58156A1FA949C568 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-10-14 09:43, Lai Jiangshan wrote: > On 10/14/2011 02:49 PM, Jan Kiszka wrote: >> On 2011-10-14 08:36, Lai Jiangshan wrote: >>> On 10/14/2011 01:53 PM, Jan Kiszka wrote: >>>> On 2011-10-14 02:53, Lai Jiangshan wrote: >>>>> >>>>>> >>>>>> As explained in some other mail, we could then emulate the missing= >>>>>> kernel feature by reading out the current in-kernel APIC state, te= sting >>>>>> if LINT1 is unmasked, and then delivering the NMI directly. >>>>>> >>>>> >>>>> Only the thread of the VCPU can safely get the in-kernel LAPIC stat= es, >>>>> so this approach will cause some troubles. >>>> >>>> run_on_cpu() can help. >>>> >>>> Jan >>>> >>> >>> Ah, I forgot it, Thanks. >>> >>> From: Lai Jiangshan >>> >>> Currently, NMI interrupt is blindly sent to all the vCPUs when NMI >>> button event happens. This doesn't properly emulate real hardware on >>> which NMI button event triggers LINT1. Because of this, NMI is sent t= o >>> the processor even when LINT1 is maskied in LVT. For example, this >>> causes the problem that kdump initiated by NMI sometimes doesn't work= >>> on KVM, because kdump assumes NMI is masked on CPUs other than CPU0. >>> >>> With this patch, inject-nmi request is handled as follows. >>> >>> - When in-kernel irqchip is disabled, deliver LINT1 instead of NMI >>> interrupt. >>> - When in-kernel irqchip is enabled, get the in-kernel LAPIC states >>> and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then >>> delivering the NMI directly. (Suggested by Jan Kiszka) >>> >>> Changed from old version: >>> re-implement it by the Jan's suggestion. >>> >>> Signed-off-by: Lai Jiangshan >>> Reported-by: Kenji Kaneshige >>> --- >>> hw/apic.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >>> hw/apic.h | 1 + >>> monitor.c | 6 +++++- >>> 3 files changed, 54 insertions(+), 1 deletions(-) >>> diff --git a/hw/apic.c b/hw/apic.c >>> index 69d6ac5..9a40129 100644 >>> --- a/hw/apic.c >>> +++ b/hw/apic.c >>> @@ -205,6 +205,54 @@ void apic_deliver_pic_intr(DeviceState *d, int l= evel) >>> } >>> } >>> =20 >>> +#ifdef KVM_CAP_IRQCHIP >> >> Again, this is always defined on x86 thus pointless to test. >> >>> +static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int = reg_id); >>> + >>> +struct kvm_get_remote_lapic_params { >>> + CPUState *env; >>> + struct kvm_lapic_state klapic; >>> +}; >>> + >>> +static void kvm_get_remote_lapic(void *p) >>> +{ >>> + struct kvm_get_remote_lapic_params *params =3D p; >>> + >>> + kvm_get_lapic(params->env, ¶ms->klapic); >> >> When you already interrupted that vcpu, why not inject from here? Avoi= ds >> one further ping-pong round. >=20 > get_remote_lapic and inject nmi are two different things, > so I don't inject nmi from here. I didn't notice this ping-pond overhea= d. > Thank you. Actually, it is not performance-critical. But there is a race between obtaining the APIC state and testing for the NMI injection path. So it's better to define an on-vcpu LINT1 NMI injection service. >=20 >> >>> +} >>> + >>> +void apic_deliver_nmi(DeviceState *d) >>> +{ >>> + APICState *s =3D DO_UPCAST(APICState, busdev.qdev, d); >>> + >>> + if (kvm_irqchip_in_kernel()) { >>> + struct kvm_get_remote_lapic_params p =3D {.env =3D s->cpu_en= v,}; >>> + uint32_t lvt; >>> + >>> + run_on_cpu(s->cpu_env, kvm_get_remote_lapic, &p); >>> + lvt =3D kapic_reg(&p.klapic, 0x32 + APIC_LVT_LINT1); >>> + >>> + if (lvt & APIC_LVT_MASKED) { >>> + return; >>> + } >>> + >>> + if (((lvt >> 8) & 7) !=3D APIC_DM_NMI) { >>> + return; >>> + } >>> + >>> + cpu_interrupt(s->cpu_env, CPU_INTERRUPT_NMI); >> >> Err, aren't you introducing KVM_CAP_LAPIC_NMI that allows to test if >> this workaround is needed? Oh, your latest kernel patch is missing thi= s >> again - requires fixing as well. >> >=20 >=20 > Kernel site patch is dropped with this v4 patch. >=20 > Did you mean you want KVM_CAP_SET_LINT1 + KVM_SET_LINT1 patches? > I have made them. OK, so this is going to be applied on top? Then I take this remark back. Jan --------------enig36B088AA58156A1FA949C568 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk6X81kACgkQitSsb3rl5xSyngCeLIE6VD/jctsLaUa7SowDAIaA fWsAnigluV0LmG/8cb2YlNU4zmk0vRgn =+Wdr -----END PGP SIGNATURE----- --------------enig36B088AA58156A1FA949C568--