On Fri, Jan 27, 2023 at 08:07:26AM -0600, Scott Cheloha wrote:
> mlarkin@ noted about a month or so ago that setting the lapic timer
> mode, mask, and divisor every time we rearm it is unnecessary. We
> only need to configure those registers once during
> lapic_timer_trigger(). After that, it is sufficient to set the ICR
> when rearming the timer.
>
> While here, add the missing intr_disable/intr_restore wrapper to
> lapic_timer_trigger(). Writing multiple registers is not atomic, so
> we need to disable interrupts for safety. Setting the ICR during
> lapic_timer_rearm() is atomic, so we don't need to disable interrupts
> there.
>
> ok?
>
ok mlarkin if you verified the mode/mask/divisor reset properly after
un-zzz/un-ZZZ (which I think we do but wanted to point it out just in case).
> Index: amd64/amd64/lapic.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 lapic.c
> --- amd64/amd64/lapic.c 10 Nov 2022 08:26:54 -0000 1.65
> +++ amd64/amd64/lapic.c 27 Jan 2023 13:58:15 -0000
> @@ -431,13 +431,17 @@ lapic_timer_rearm(void *unused, uint64_t
> cycles = (nsecs * lapic_timer_nsec_cycle_ratio) >> 32;
> if (cycles == 0)
> cycles = 1;
> - lapic_timer_oneshot(0, cycles);
> + lapic_writereg(LAPIC_ICR_TIMER, cycles);
> }
>
> void
> lapic_timer_trigger(void *unused)
> {
> + u_long s;
> +
> + s = intr_disable();
> lapic_timer_oneshot(0, 1);
> + intr_restore(s);
> }
>
> /*
> Index: i386/i386/lapic.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 lapic.c
> --- i386/i386/lapic.c 6 Dec 2022 01:56:44 -0000 1.53
> +++ i386/i386/lapic.c 27 Jan 2023 13:58:15 -0000
> @@ -268,13 +268,17 @@ lapic_timer_rearm(void *unused, uint64_t
> cycles = (nsecs * lapic_timer_nsec_cycle_ratio) >> 32;
> if (cycles == 0)
> cycles = 1;
> - lapic_timer_oneshot(0, cycles);
> + i82489_writereg(LAPIC_ICR_TIMER, cycles);
> }
>
> void
> lapic_timer_trigger(void *unused)
> {
> + u_long s;
> +
> + s = intr_disable();
> lapic_timer_oneshot(0, 1);
> + intr_restore(s);
> }
>
> /*
