Re: [PATCH v7] KVM: LAPIC: Apply change to TDCR right away to the timer

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 06/10/2017 16:01, Wanpeng Li wrote:
> +	if (!apic->lapic_timer.period)
> +		return;
> +
> +	now = ktime_get();
> +	remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
> +	if (ktime_to_ns(remaining) < 0)
> +		remaining = 0;
> +	delta = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period);

I think this shouldn't be happening.  If it does, I'm not sure the mod 
is the right thing to do, so I'd just use ktime_to_ns(remaining).

So perhaps let's simplify all this to:

	ns_remaining_old = ktime_to_ns(remaining);
	ns_remaining_new = mul_u64_u32_div(ns_remaining_old,
                                           apic->divide_count, old_divisor);

because below you're calling nsec_to_cycles but remaining is not expressed
in nanoseconds.

> +	if (!delta)
> +		return;
> +
> +	delta = delta * apic->divide_count / old_divisor;
> +
> +	limit_periodic_timer_frequency(apic);

This should be done before all the "if"s (which should not be there in v8,
but you should still call it before "now = ktime_get();").

Paolo

> +	apic->lapic_timer.tscdeadline += nsec_to_cycles(apic->vcpu, delta) -
> +                                    nsec_to_cycles(apic->vcpu, remaining);
> +	apic->lapic_timer.target_expiration = ktime_add_ns(now, delta);
> +}