From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/3] KVM: x86: Optimize NMI watchdog delivery Date: Fri, 17 Oct 2008 19:23:01 +0200 Message-ID: <48F8C9F5.5070500@web.de> References: <20081015142748.385784583@mchn012c.ww002.siemens.net> <20081015142748.931910158@mchn012c.ww002.siemens.net> <20081017170654.GA22408@yukikaze> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig375D7C7D8963BA2F26B48930" Cc: Jan Kiszka , kvm@vger.kernel.org, avi@redhat.com, jiajun.xu@intel.com To: sheng@linux.intel.com Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:47582 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753072AbYJQRXJ (ORCPT ); Fri, 17 Oct 2008 13:23:09 -0400 In-Reply-To: <20081017170654.GA22408@yukikaze> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig375D7C7D8963BA2F26B48930 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Sheng Yang wrote: > On Wed, Oct 15, 2008 at 04:27:51PM +0200, Jan Kiszka wrote: >> As suggested by Avi, this patch introduces a counter of VCPUs that hav= e >> LVT0 set to NMI mode. Only if the counter > 0, we push the PIT ticks v= ia >> all LAPIC LVT0 lines to enable NMI watchdog support. >> >=20 > I feel a little strange about: if *counter > 0*, we push to *all*. Can = we > only push NMIs to the ones that set NMI for LVT0? We don't do that due to !kvm_apic_accept_pic_intr(). The counter is only about optimizing that case where we don't have to walk the whole chain, asking every vcpu if it would like to receive the IRQ. >=20 > How about add a field in struct kvm_lapic? We can quickly know if we ne= ed to > inject NMI for this vcpu. Well, though kernel mostly enable NMI watchdo= g on > all vcpu, I think this is more precise, and match the logic, and avoid = one > more field in kvm_arch... The point of this patch is to avoid touching vcpu structures AT ALL when there is no interest in the NMI watchdog (normally, OSes will either enable the WD trick for all CPUSs or keep it off). Jan >=20 > -- > regards > Yang, Sheng >=20 >> Signed-off-by: Jan Kiszka >> --- >> arch/x86/kvm/i8254.c | 13 +++++++------ >> arch/x86/kvm/lapic.c | 23 ++++++++++++++++++++--- >> include/asm-x86/kvm_host.h | 1 + >> 3 files changed, 28 insertions(+), 9 deletions(-) >> >> Index: b/arch/x86/kvm/i8254.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- a/arch/x86/kvm/i8254.c >> +++ b/arch/x86/kvm/i8254.c >> @@ -607,12 +607,13 @@ static void __inject_pit_timer_intr(stru >> * The route is: PIT -> PIC -> LVT0 in NMI mode, >> * timer IRQs will continue to flow through the IOAPIC. >> */ >> - for (i =3D 0; i < KVM_MAX_VCPUS; ++i) { >> - vcpu =3D kvm->vcpus[i]; >> - if (!vcpu || !kvm_apic_accept_pic_intr(vcpu)) >> - continue; >> - kvm_apic_local_deliver(vcpu, APIC_LVT0); >> - } >> + if (kvm->arch.vapics_in_nmi_mode > 0) >> + for (i =3D 0; i < KVM_MAX_VCPUS; ++i) { >> + vcpu =3D kvm->vcpus[i]; >> + if (!vcpu || !kvm_apic_accept_pic_intr(vcpu)) >> + continue; >> + kvm_apic_local_deliver(vcpu, APIC_LVT0); >> + } >> } >> =20 >> void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu) >> Index: b/arch/x86/kvm/lapic.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -130,6 +130,11 @@ static inline int apic_lvtt_period(struc >> return apic_get_reg(apic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC; >> } >> =20 >> +static inline int apic_lvt_nmi_mode(u32 lvt_val) >> +{ >> + return (lvt_val & (APIC_MODE_MASK | APIC_LVT_MASKED)) =3D=3D APIC_DM= _NMI; >> +} >> + >> static unsigned int apic_lvt_mask[APIC_LVT_NUM] =3D { >> LVT_MASK | APIC_LVT_TIMER_PERIODIC, /* LVTT */ >> LVT_MASK | APIC_MODE_MASK, /* LVTTHMR */ >> @@ -672,6 +677,20 @@ static void start_apic_timer(struct kvm_ >> apic->timer.period))); >> } >> =20 >> +static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0= _val) >> +{ >> + int nmi_wd_enabled =3D apic_lvt_nmi_mode(apic_get_reg(apic, APIC_LVT= 0)); >> + >> + if (apic_lvt_nmi_mode(lvt0_val)) { >> + if (!nmi_wd_enabled) { >> + apic_debug("Receive NMI setting on APIC_LVT0 " >> + "for cpu %d\n", apic->vcpu->vcpu_id); >> + apic->vcpu->kvm->arch.vapics_in_nmi_mode++; >> + } >> + } else if (nmi_wd_enabled) >> + apic->vcpu->kvm->arch.vapics_in_nmi_mode--; >> +} >> + >> static void apic_mmio_write(struct kvm_io_device *this, >> gpa_t address, int len, const void *data) >> { >> @@ -753,9 +772,7 @@ static void apic_mmio_write(struct kvm_i >> break; >> =20 >> case APIC_LVT0: >> - if (val =3D=3D APIC_DM_NMI) >> - apic_debug("Receive NMI setting on APIC_LVT0 " >> - "for cpu %d\n", apic->vcpu->vcpu_id); >> + apic_manage_nmi_watchdog(apic, val); >> case APIC_LVTT: >> case APIC_LVTTHMR: >> case APIC_LVTPC: >> Index: b/include/asm-x86/kvm_host.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- a/include/asm-x86/kvm_host.h >> +++ b/include/asm-x86/kvm_host.h >> @@ -359,6 +359,7 @@ struct kvm_arch{ >> struct kvm_ioapic *vioapic; >> struct kvm_pit *vpit; >> struct hlist_head irq_ack_notifier_list; >> + int vapics_in_nmi_mode; >> =20 >> int round_robin_prev_vcpu; >> unsigned int tss_addr; >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >=20 --------------enig375D7C7D8963BA2F26B48930 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.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkj4yfoACgkQniDOoMHTA+nTBACfb7mPUK301AG5WkUYbiZURpiv bN8AnRbKKESnoApg4xm9NPoyOATSmwbL =3p1B -----END PGP SIGNATURE----- --------------enig375D7C7D8963BA2F26B48930--