Re: [PATCH gnumach] lapic timer: Calibrate via mach timer not PIT

[Samuel Thibault]

Hello,

Thanks for agreeing to switch to that approach :)

Damien Zammit, le dim. 05 mars 2023 02:06:54 +0000, a ecrit:
> diff --git a/i386/i386at/ioapic.c b/i386/i386at/ioapic.c
> index d2ea84ad..95c1b368 100644
> --- a/i386/i386at/ioapic.c
> +++ b/i386/i386at/ioapic.c
> @@ -30,9 +30,11 @@
>  #include <i386/pic.h> /* only for macros */
>  #include <mach/machine.h>
>  #include <kern/printf.h>
> +#include <kern/timer.h>
> 
>  static int has_irq_specific_eoi = 1; /* FIXME: Assume all machines have this 
> */
> -int duplicate_pin;
> +int timer_pin;

> +int timer_gsi;

This does not seem to need to be a global variable?

> 
>  uint32_t lapic_timer_val = 0;
>  uint32_t calibrated_ticks = 0;
> @@ -43,7 +45,7 @@ int iunit[NINTR] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 
> 12, 13, 14, 15,
>                      16, 17, 18, 19, 20, 21, 22, 23};
> 
>  interrupt_handler_fn ivect[NINTR] = {
> -    /* 00 */ intnull,        /* install timer later */
> +    /* 00 */ (interrupt_handler_fn)hardclock,
>      /* 01 */ kdintr,         /* kdintr, ... */
>      /* 02 */ intnull,
>      /* 03 */ intnull,        /* lnpoll, comintr, ... */
> @@ -150,32 +152,58 @@ ioapic_toggle_entry(int apic, int pin, int mask)
>      ioapic_write(apic, APIC_IO_REDIR_LOW(pin), entry.lo);
>  }
> 
> +static void buzz(void *arg)

Rather use a proper name :)

> +{
> +    volatile int *done = (volatile int *)arg;

The cast here is useless in C.

> +    *done = 1;
> +}
> +
>  static uint32_t
> -pit_measure_10x_apic_hz(void)
> +timer_measure_10x_apic_hz(void)
>  {
> -    volatile int i;
> +    volatile int done = 0;
>      uint32_t start = 0xffffffff;
> +    timer_elt_data_t tmp_timer;
> +    tmp_timer.fcn = buzz;
> +    tmp_timer.param = (void *)&done;
> 
> -    /* Prepare accurate delay for 1/hz seconds */
> -    pit_prepare_sleep(hz);
> +    printf("timer calibration...");
> 
>      /* Set APIC timer */
>      lapic->init_count.r = start;
> 
> -    /* zZz */
> -    for (i = 0; i < 10; i++)
> -        pit_sleep();
> +    /* Delay for 10 * 1/hz seconds */
> +    set_timeout(&tmp_timer, hz / 10);
> +    do {
> +        asm volatile("pause" : : : "memory");

Rather use cpu_pause()

> +    } while (!done);
> 
>      /* Stop APIC timer */
>      lapic->lvt_timer.r |= LAPIC_DISABLE;
> 
> +    printf("done\n");

Add a space.

>      return start - lapic->cur_count.r;
>  }