On Fri, Sep 02, 2022 at 01:13:00PM -0400, Dave Voutila wrote:
>
> Scott Cheloha <scottchel...@gmail.com> writes:
>
> > The 8254 data sheet [1] says this about the Read-Back command:
> >
> >> The read-back command may be used to latch multi-
> >> ple counter output latches (OL) by setting the
> >> COUNT bit D5 = 0 and selecting the desired coun-
> >> ter(s). This single command is functionally equiva-
> >> lent to several counter latch commands, one for
> >> each counter latched. [...]
> >
> > This is a little ambiguous. But my hunch is that the intent here is
> > "you can latch multiple counters all at once". Simultaneously.
> > Otherwise the utility of the read-back command is suspect.
> >
> > To simulate a simultaneous latch, we should only call clock_gettime(2)
> > once and use that singular timestamp to compute olatch for each
> > counter.
> >
> > ok?
> >
>
> I'm not an expert on the i825{3,4} but have a question below. I did
> quickly test this diff and see no noticeable difference from the point
> of view of my local guests.
>
> > [1] 8254 Programmable Interval Timer, p. 8
> > https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf
> >
> > Index: i8253.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v
> > retrieving revision 1.34
> > diff -u -p -r1.34 i8253.c
> > --- i8253.c 16 Jun 2021 16:55:02 -0000 1.34
> > +++ i8253.c 2 Sep 2022 16:25:02 -0000
> > @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data)
> > int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 };
> > int i;
> >
> > + clock_gettime(CLOCK_MONOTONIC, &now);
> > +
>
> Why make this call to clock_gettime here ^
>
> > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */
> > if (data & ~TIMER_RB_STATUS) {
> > i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0;
> > @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data)
> > if (data & ~TIMER_RB_COUNT) {
>
> ...instead of here where we can save a possible syscall?
Yes, that's better, I'll go with that.
