2016-10-26 14:35+0800, Wanpeng Li:
> 2016-10-25 19:55 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>:
>> 2016-10-17 16:28+0800, Wanpeng Li:
> [...]
>>> +static void apic_timer_isr(isr_regs_t *regs)
>>> +{
>>> + u64 now = rdtsc();
>>> + ++tdt_count;
>>> +
>>> + if (table_idx < TABLE_SIZE && tdt_count > 1)
>>> + table[table_idx++] = now - exptime;
>>> +
>>> + if (breakmax && tdt_count > 1 && (now - exptime) > breakmax) {
>>> + hitmax = 1;
>>> + apic_write(APIC_EOI, 0);
>>> + return;
>>> + }
>>> +
>>> + exptime = now+delta;
>>
>> Two problems on this line:
>>
>> 1) with periodic timer, the delta is added to last expiration time, not
>> to now().
>> What you are measuring now is the stability of the period -- very
>> important as well, but if we used now() only once, you could also
>> measure the latency.
>>
>> 2) delta is in nanoseconds while exptime is in cycles. Type error.
>
> There needs a method to convert the value which is loaded to
> initial-counter register to TSC.
Yes, you need to know the conversion between APIC timer frequency and
TSC frequency. APIC timer in KVM uses nanoseconds, but TSC is variable.
>>
>>> +
>>> + if (mode == APIC_LVT_TIMER_ONE_SHOT)
>>> + /* Set "Initial Counter Register", which starts the timer */
>>> + apic_write(APIC_TMICT, delta);
>>
>> This is not deadline, so it would be better to read now() for exptime
>> right before the apic_write, so as little time as possible passes in
>> between.
>
> Do you mean something like this?
>
> static void apic_timer_isr(isr_regs_t *regs)
> {
> uint64_t now = rdtsc();
>
> if (breakmax && (now - exptime) > breakmax) {
Note that you must have now and exptime in the same unit to have
meaningful operations on. (especially the "exptime += delta" would bad
without conversion).
> hitmax = 1;
> apic_write(APIC_EOI, 0);
> return;
> }
>
> exptime += delta;
The exptime for oneshot is 'exptime = now + delta', only periodic can do
exptime += delta, so I'd rather read the time again just before writing
the delta
> if (mode == APIC_LVT_TIMER_ONE_SHOT) {
time_t now = now();
> /* Set "Initial Counter Register", which starts the timer */
> apic_write(APIC_TMICT, delta);
exptime = now + delta;
}
> if (table_idx < TABLE_SIZE)
> table[table_idx++] = now - exptime;
>
> apic_write(APIC_EOI, 0);
> }
>
> static void test_apic_timer(int mode)
> {
> handle_irq(APIC_LVT_TIMER_VECTOR, apic_timer_isr);
> irq_enable();
>
> /* Periodic mode */
> apic_write(APIC_LVTT, mode | APIC_LVT_TIMER_VECTOR);
> /* Divider == 1 */
> apic_write(APIC_TDCR, 0x0000000b);
> exptime = rdtsc() + delta;
Yep, if we convert rdtsc() to the same unit that delta uses.
Thanks.
> apic_write(APIC_TMICT, delta);
> }
>
> Regards,
> Wanpeng Li
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
