Re: [PATCH] x86: apic: add APIC Timer periodic/oneshot mode latency test

[Wanpeng Li <kernellwp@xxxxxxxxx>]

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.

>
>> +
>> +    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) {
        hitmax = 1;
        apic_write(APIC_EOI, 0);
        return;
    }

    exptime += delta;

    if (mode == APIC_LVT_TIMER_ONE_SHOT)
        /* Set "Initial Counter Register", which starts the timer */
        apic_write(APIC_TMICT, 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;
    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