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 <[EMAIL PROTECTED]>
> ---
> 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 [EMAIL PROTECTED]
> 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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
