From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 3/3] KVM: x86: Optimize NMI watchdog delivery Date: Sat, 18 Oct 2008 01:06:54 +0800 Message-ID: <20081017170654.GA22408@yukikaze> References: <20081015142748.385784583@mchn012c.ww002.siemens.net> <20081015142748.931910158@mchn012c.ww002.siemens.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, avi@redhat.com, jiajun.xu@intel.com, sheng@linux.intel.com To: Jan Kiszka Return-path: Received: from ti-out-0910.google.com ([209.85.142.189]:52006 "EHLO ti-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757340AbYJQRHF (ORCPT ); Fri, 17 Oct 2008 13:07:05 -0400 Received: by ti-out-0910.google.com with SMTP id b6so333208tic.23 for ; Fri, 17 Oct 2008 10:07:03 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20081015142748.931910158@mchn012c.ww002.siemens.net> Sender: kvm-owner@vger.kernel.org List-ID: 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 have > LVT0 set to NMI mode. Only if the counter > 0, we push the PIT ticks via > all LAPIC LVT0 lines to enable NMI watchdog support. > 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? How about add a field in struct kvm_lapic? We can quickly know if we need to inject NMI for this vcpu. Well, though kernel mostly enable NMI watchdog on all vcpu, I think this is more precise, and match the logic, and avoid one more field in kvm_arch... -- regards Yang, Sheng > 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 > =================================================================== > --- 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 = 0; i < KVM_MAX_VCPUS; ++i) { > - vcpu = 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 = 0; i < KVM_MAX_VCPUS; ++i) { > + vcpu = kvm->vcpus[i]; > + if (!vcpu || !kvm_apic_accept_pic_intr(vcpu)) > + continue; > + kvm_apic_local_deliver(vcpu, APIC_LVT0); > + } > } > > void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu) > Index: b/arch/x86/kvm/lapic.c > =================================================================== > --- 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; > } > > +static inline int apic_lvt_nmi_mode(u32 lvt_val) > +{ > + return (lvt_val & (APIC_MODE_MASK | APIC_LVT_MASKED)) == APIC_DM_NMI; > +} > + > static unsigned int apic_lvt_mask[APIC_LVT_NUM] = { > 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))); > } > > +static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val) > +{ > + int nmi_wd_enabled = apic_lvt_nmi_mode(apic_get_reg(apic, APIC_LVT0)); > + > + 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; > > case APIC_LVT0: > - if (val == 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 > =================================================================== > --- 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; > > 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