On Mon, Oct 20, 2008 at 10:20:02AM +0200, Jan Kiszka wrote:
> This patch refactors the NMI watchdog delivery patch, consolidating
> tests and providing a proper API for delivering watchdog events.
>
> An included micro-optimization is to check only for apic_hw_enabled in
> kvm_apic_local_deliver (the test for LVT mask is covering the
> soft-disabled case already).
Oh... Please ignore my comments on the other patch for software enable bit.
:) (But I think this kind of optimization is reducing the readability a
little as well... It's easy to get curious about why only hardware enable is
OK)
Others seems OK to me.
>
> Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]>
Acked-by: Sheng Yang <[EMAIL PROTECTED]>
--
regards
Yang, Sheng
> ---
> arch/x86/kvm/i8254.c | 15 +++++++++------
> arch/x86/kvm/irq.h | 2 +-
> arch/x86/kvm/lapic.c | 20 ++++++++++----------
> 3 files changed, 20 insertions(+), 17 deletions(-)
>
> Index: b/arch/x86/kvm/i8254.c
> ===================================================================
> --- a/arch/x86/kvm/i8254.c
> +++ b/arch/x86/kvm/i8254.c
> @@ -610,15 +610,18 @@ static void __inject_pit_timer_intr(stru
> mutex_unlock(&kvm->lock);
>
> /*
> - * Provides NMI watchdog support in IOAPIC mode.
> - * The route is: PIT -> PIC -> LVT0 in NMI mode,
> - * timer IRQs will continue to flow through the IOAPIC.
> + * Provides NMI watchdog support via Virtual Wire mode.
> + * The route is: PIT -> PIC -> LVT0 in NMI mode.
> + *
> + * Note: Our Virtual Wire implementation is simplified, only
> + * propagating PIT interrupts to all VCPUs when they have set
> + * LVT0 to NMI delivery. Other PIC interrupts are just sent to
> + * VCPU0, and only if its LVT0 is in EXTINT mode.
> */
> for (i = 0; i < KVM_MAX_VCPUS; ++i) {
> vcpu = kvm->vcpus[i];
> - if (!vcpu)
> - continue;
> - kvm_apic_local_deliver(vcpu, APIC_LVT0);
> + if (vcpu)
> + kvm_apic_nmi_wd_deliver(vcpu);
> }
> }
>
> Index: b/arch/x86/kvm/irq.h
> ===================================================================
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -87,7 +87,7 @@ void kvm_pic_reset(struct kvm_kpic_state
> void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
> void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
> void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
> -int kvm_apic_local_deliver(struct kvm_vcpu *vcpu, int lvt_type);
> +void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
> void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
> void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
> void __kvm_migrate_timers(struct kvm_vcpu *vcpu);
> Index: b/arch/x86/kvm/lapic.c
> ===================================================================
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -975,14 +975,12 @@ int apic_has_pending_timer(struct kvm_vc
> return 0;
> }
>
> -int kvm_apic_local_deliver(struct kvm_vcpu *vcpu, int lvt_type)
> +static int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
> {
> - struct kvm_lapic *apic = vcpu->arch.apic;
> + u32 reg = apic_get_reg(apic, lvt_type);
> int vector, mode, trig_mode;
> - u32 reg;
>
> - if (apic && apic_enabled(apic)) {
> - reg = apic_get_reg(apic, lvt_type);
> + if (apic_hw_enabled(apic) && !(reg & APIC_LVT_MASKED)) {
> vector = reg & APIC_VECTOR_MASK;
> mode = reg & APIC_MODE_MASK;
> trig_mode = reg & APIC_LVT_LEVEL_TRIGGER;
> @@ -991,9 +989,12 @@ int kvm_apic_local_deliver(struct kvm_vc
> return 0;
> }
>
> -static inline int __inject_apic_timer_irq(struct kvm_lapic *apic)
> +void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
> {
> - return kvm_apic_local_deliver(apic->vcpu, APIC_LVTT);
> + struct kvm_lapic *apic = vcpu->arch.apic;
> +
> + if (apic)
> + kvm_apic_local_deliver(apic, APIC_LVT0);
> }
>
> static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
> @@ -1088,9 +1089,8 @@ void kvm_inject_apic_timer_irqs(struct k
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
>
> - if (apic && apic_lvt_enabled(apic, APIC_LVTT) &&
> - atomic_read(&apic->timer.pending) > 0) {
> - if (__inject_apic_timer_irq(apic))
> + if (apic && atomic_read(&apic->timer.pending) > 0) {
> + if (kvm_apic_local_deliver(apic, APIC_LVTT))
> atomic_dec(&apic->timer.pending);
> }
> }
>
> --
> 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
