Dong, Eddie wrote:
> [EMAIL PROTECTED] wrote:
>
>> Add back pending irqs for apic timer to get precise guest APIC
>> timer interrupt.
>>
>> Signed-off-by: Yaozu (Eddie) Dong <[EMAIL PROTECTED]>
>>
>>
>>
> a typo, please use this one.
>
> With above patches, now guest timer is much accurate! sleep 60
> get exactly 60 seconds in host per my rough test.
>
> BTW, AMD platform is not tested.
>
> thx,eddie
>
>
> diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
> index ed6d20a..8867c82 100644
> --- a/drivers/kvm/irq.h
> +++ b/drivers/kvm/irq.h
> @@ -110,7 +110,7 @@ struct kvm_lapic {
> unsigned long base_address;
> struct kvm_io_device dev;
> struct {
> - unsigned long pending;
> + atomic_t pending;
> s64 period; /* unit: ns */
> u32 divide_count;
> ktime_t last_update;
> @@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec,
> u8 trig);
> void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
> int kvm_ioapic_init(struct kvm *kvm);
> void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
> +void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
>
What does "pt" mean in this context? "pending timer"?
Suggested descriptive names below.
> *----------------------------------------------------------------------
> */
> +
> +/* TODO: make sure __apic_timer_fn runs in current pCPU */
> static int __apic_timer_fn(struct kvm_lapic *apic)
> {
> - u32 vector;
> int result = 0;
>
> - if (unlikely(!apic_enabled(apic) ||
> - !apic_lvt_enabled(apic, APIC_LVTT))) {
> - apic_debug("%s: time interrupt although apic is down\n",
> - __FUNCTION__);
> - return 0;
> - }
> -
> - vector = apic_lvt_vector(apic, APIC_LVTT);
> - apic->timer.last_update = apic->timer.dev.expires;
> - apic->timer.pending++;
> - __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
> -
> + atomic_inc (&apic->timer.pending);
>
Extra space.
> if (apic_lvtt_period(apic)) {
> - u32 offset;
> - u32 tmict = apic_get_reg(apic, APIC_TMICT);
> -
> - offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
> tmict;
> -
> result = 1;
> apic->timer.dev.expires = ktime_add_ns(
> apic->timer.dev.expires,
> apic->timer.period);
> }
> -
> return result;
> }
>
While this improves throughput, doesn't it decrease responsiveness?
Suppose the guest is sleeping and there is no activity except for lapic
interrupts. Won't the wakeup get deferred indefinitely?
>
> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_lapic *apic = vcpu->apic;
> +
> + if (apic && atomic_read(&apic->timer.pending) > 0) {
>
Extra braces. Also this smells like a race. What if timer.pending is
updated just after this?
> + if (inject_apic_timer_irq(apic))
> + atomic_dec(&apic->timer.pending);
> + }
> +}
> +EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
>
How about "kvm_inject_pending_timer_irqs"
> +
> +void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
> +{
> + struct kvm_lapic *apic = vcpu->apic;
> +
> + if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
> + apic->timer.last_update = ktime_add_ns(
> + apic->timer.last_update,
> + apic->timer.period);
> +}
> +EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
>
kvm_update_pending_timer_irq?
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
