| Message ID | 21f20ab5a100e4947d840080114f3f0511aade86.1629799776.git.fthain@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,01/10] hw/mos6522: Remove get_load_time() methods and functions | expand |
On 8/24/21 12:09 PM, Finn Thain wrote: > mos6522_read() and mos6522_write() may call various functions to determine > timer irq state, timer counter value and QEMUTimer deadline. All called > functions must use the same value for the present time. > > Signed-off-by: Finn Thain <fthain@linux-m68k.org> > --- > hw/misc/mos6522.c | 51 +++++++++++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 24 deletions(-) > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) + > ti->load_time; > - > - if (next_time <= current_time) { > - next_time = current_time + 1; > - } Maybe extract as an helper? Otherwise: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > return next_time; > } > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time) > + int64_t now) > { > if (!ti->timer) { > return; > } > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > + ti->next_irq_time = get_next_irq_time(s, ti, now); > + if (ti->next_irq_time <= now) { > + ti->next_irq_time = now + 1; > + } > if ((s->ier & T1_INT) == 0 || > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > timer_del(ti->timer); > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > } > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time) > + int64_t now) > { > if (!ti->timer) { > return; > } > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > + ti->next_irq_time = get_next_irq_time(s, ti, now); > + if (ti->next_irq_time <= now) { > + ti->next_irq_time = now + 1; > + }
On 24/08/2021 11:09, Finn Thain wrote: > mos6522_read() and mos6522_write() may call various functions to determine > timer irq state, timer counter value and QEMUTimer deadline. All called > functions must use the same value for the present time. > > Signed-off-by: Finn Thain <fthain@linux-m68k.org> > --- > hw/misc/mos6522.c | 51 +++++++++++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 24 deletions(-) > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > index 0dd3ccf945..23a440b64f 100644 > --- a/hw/misc/mos6522.c > +++ b/hw/misc/mos6522.c > @@ -39,9 +39,9 @@ > /* XXX: implement all timer modes */ > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time); > + int64_t now); > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time); > + int64_t now); > > static void mos6522_update_irq(MOS6522State *s) > { > @@ -52,12 +52,12 @@ static void mos6522_update_irq(MOS6522State *s) > } > } > > -static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti) > +static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti, int64_t now) > { > int64_t d; > unsigned int counter; > > - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > + d = muldiv64(now - ti->load_time, > ti->frequency, NANOSECONDS_PER_SECOND); > > if (ti->index == 0) { > @@ -89,7 +89,7 @@ static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int val) > } > > static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time) > + int64_t now) > { > int64_t d, next_time; > unsigned int counter; > @@ -99,7 +99,7 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > } > > /* current counter value */ > - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > + d = muldiv64(now - ti->load_time, > ti->frequency, NANOSECONDS_PER_SECOND); > > /* the timer goes down from latch to -1 (period of latch + 2) */ > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) + > ti->load_time; > - > - if (next_time <= current_time) { > - next_time = current_time + 1; > - } > return next_time; > } > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time) > + int64_t now) > { > if (!ti->timer) { > return; > } > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > + ti->next_irq_time = get_next_irq_time(s, ti, now); > + if (ti->next_irq_time <= now) { > + ti->next_irq_time = now + 1; > + } > if ((s->ier & T1_INT) == 0 || > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > timer_del(ti->timer); > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > } > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > - int64_t current_time) > + int64_t now) > { > if (!ti->timer) { > return; > } > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > + ti->next_irq_time = get_next_irq_time(s, ti, now); > + if (ti->next_irq_time <= now) { > + ti->next_irq_time = now + 1; > + } > if ((s->ier & T2_INT) == 0 || (s->acr & T2MODE) || ti->oneshot_fired) { > timer_del(ti->timer); > } else { > @@ -163,9 +165,10 @@ static void mos6522_timer1_expired(void *opaque) > { > MOS6522State *s = opaque; > MOS6522Timer *ti = &s->timers[0]; > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->oneshot_fired = true; > - mos6522_timer1_update(s, ti, ti->next_irq_time); > + mos6522_timer1_update(s, ti, now); Presumably using ti->next_irq_time has already fixed the current time to be that at which the timer routine actually expired, rather than the current executing time. Are you seeing large differences in these numbers that can cause timer drift? If so, I'm wondering if this change should be in a separate patch. > s->ifr |= T1_INT; > mos6522_update_irq(s); > } > @@ -174,9 +177,10 @@ static void mos6522_timer2_expired(void *opaque) > { > MOS6522State *s = opaque; > MOS6522Timer *ti = &s->timers[1]; > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->oneshot_fired = true; > - mos6522_timer2_update(s, ti, ti->next_irq_time); > + mos6522_timer2_update(s, ti, now); And same again here. > s->ifr |= T2_INT; > mos6522_update_irq(s); > } > @@ -233,12 +237,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size) > val = s->dira; > break; > case VIA_REG_T1CL: > - val = get_counter(s, &s->timers[0]) & 0xff; > + val = get_counter(s, &s->timers[0], now) & 0xff; > s->ifr &= ~T1_INT; > mos6522_update_irq(s); > break; > case VIA_REG_T1CH: > - val = get_counter(s, &s->timers[0]) >> 8; > + val = get_counter(s, &s->timers[0], now) >> 8; > break; > case VIA_REG_T1LL: > val = s->timers[0].latch & 0xff; > @@ -247,12 +251,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size) > val = (s->timers[0].latch >> 8) & 0xff; > break; > case VIA_REG_T2CL: > - val = get_counter(s, &s->timers[1]) & 0xff; > + val = get_counter(s, &s->timers[1], now) & 0xff; > s->ifr &= ~T2_INT; > mos6522_update_irq(s); > break; > case VIA_REG_T2CH: > - val = get_counter(s, &s->timers[1]) >> 8; > + val = get_counter(s, &s->timers[1], now) >> 8; > break; > case VIA_REG_SR: > val = s->sr; > @@ -360,10 +364,9 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) > } > mos6522_update_irq(s); > /* if IER is modified starts needed timers */ > - mos6522_timer1_update(s, &s->timers[0], > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > - mos6522_timer2_update(s, &s->timers[1], > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + mos6522_timer1_update(s, &s->timers[0], now); > + mos6522_timer2_update(s, &s->timers[1], now); > break; > default: > g_assert_not_reached(); In terms of functionality it shouldn't really matter (since you have a ns clock compared with a timer that can manage small frequencies in comparison) but I can see how having a constant clock time throughout the entire calculation process could be useful for debugging. ATB, Mark.
On Tue, 24 Aug 2021, Philippe Mathieu-Daudé wrote: > On 8/24/21 12:09 PM, Finn Thain wrote: > > mos6522_read() and mos6522_write() may call various functions to determine > > timer irq state, timer counter value and QEMUTimer deadline. All called > > functions must use the same value for the present time. > > > > Signed-off-by: Finn Thain <fthain@linux-m68k.org> > > --- > > hw/misc/mos6522.c | 51 +++++++++++++++++++++++++---------------------- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) + > > ti->load_time; > > - > > - if (next_time <= current_time) { > > - next_time = current_time + 1; > > - } > > Maybe extract as an helper? There is a small amount of code duplication here but it gets resolved in patch 10/10. > Otherwise: > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > > return next_time; > > } > > > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > + ti->next_irq_time = get_next_irq_time(s, ti, now); > > + if (ti->next_irq_time <= now) { > > + ti->next_irq_time = now + 1; > > + } > > if ((s->ier & T1_INT) == 0 || > > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > > timer_del(ti->timer); > > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > } > > > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > + ti->next_irq_time = get_next_irq_time(s, ti, now); > > + if (ti->next_irq_time <= now) { > > + ti->next_irq_time = now + 1; > > + } > >
On Wed, 25 Aug 2021, Mark Cave-Ayland wrote: > On 24/08/2021 11:09, Finn Thain wrote: > > > mos6522_read() and mos6522_write() may call various functions to determine > > timer irq state, timer counter value and QEMUTimer deadline. All called > > functions must use the same value for the present time. > > > > Signed-off-by: Finn Thain <fthain@linux-m68k.org> > > --- > > hw/misc/mos6522.c | 51 +++++++++++++++++++++++++---------------------- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > > index 0dd3ccf945..23a440b64f 100644 > > --- a/hw/misc/mos6522.c > > +++ b/hw/misc/mos6522.c > > @@ -39,9 +39,9 @@ > > /* XXX: implement all timer modes */ > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time); > > + int64_t now); > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time); > > + int64_t now); > > static void mos6522_update_irq(MOS6522State *s) > > { > > @@ -52,12 +52,12 @@ static void mos6522_update_irq(MOS6522State *s) > > } > > } > > -static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti) > > +static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti, int64_t > > now) > > { > > int64_t d; > > unsigned int counter; > > - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > > + d = muldiv64(now - ti->load_time, > > ti->frequency, NANOSECONDS_PER_SECOND); > > if (ti->index == 0) { > > @@ -89,7 +89,7 @@ static void set_counter(MOS6522State *s, MOS6522Timer *ti, > > unsigned int val) > > } > > static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > int64_t d, next_time; > > unsigned int counter; > > @@ -99,7 +99,7 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > } > > /* current counter value */ > > - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > > + d = muldiv64(now - ti->load_time, > > ti->frequency, NANOSECONDS_PER_SECOND); > > /* the timer goes down from latch to -1 (period of latch + 2) */ > > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) > > + > > ti->load_time; > > - > > - if (next_time <= current_time) { > > - next_time = current_time + 1; > > - } > > return next_time; > > } > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > + ti->next_irq_time = get_next_irq_time(s, ti, now); > > + if (ti->next_irq_time <= now) { > > + ti->next_irq_time = now + 1; > > + } > > if ((s->ier & T1_INT) == 0 || > > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > > timer_del(ti->timer); > > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, > > MOS6522Timer *ti, > > } > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > - ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > + ti->next_irq_time = get_next_irq_time(s, ti, now); > > + if (ti->next_irq_time <= now) { > > + ti->next_irq_time = now + 1; > > + } > > if ((s->ier & T2_INT) == 0 || (s->acr & T2MODE) || ti->oneshot_fired) > > { > > timer_del(ti->timer); > > } else { > > @@ -163,9 +165,10 @@ static void mos6522_timer1_expired(void *opaque) > > { > > MOS6522State *s = opaque; > > MOS6522Timer *ti = &s->timers[0]; > > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->oneshot_fired = true; > > - mos6522_timer1_update(s, ti, ti->next_irq_time); > > + mos6522_timer1_update(s, ti, now); > > Presumably using ti->next_irq_time has already fixed the current time to > be that at which the timer routine actually expired, rather than the > current executing time. Are you seeing large differences in these > numbers that can cause timer drift? If so, I'm wondering if this change > should be in a separate patch. > You're right. This change has more relevance to the synchronization work in the following patch. It's not really covered by the commit log here. > > s->ifr |= T1_INT; > > mos6522_update_irq(s); > > } > > @@ -174,9 +177,10 @@ static void mos6522_timer2_expired(void *opaque) > > { > > MOS6522State *s = opaque; > > MOS6522Timer *ti = &s->timers[1]; > > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->oneshot_fired = true; > > - mos6522_timer2_update(s, ti, ti->next_irq_time); > > + mos6522_timer2_update(s, ti, now); > > And same again here. > I'll find a better way to split up these patches. > > s->ifr |= T2_INT; > > mos6522_update_irq(s); > > } > > @@ -233,12 +237,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, > > unsigned size) > > val = s->dira; > > break; > > case VIA_REG_T1CL: > > - val = get_counter(s, &s->timers[0]) & 0xff; > > + val = get_counter(s, &s->timers[0], now) & 0xff; > > s->ifr &= ~T1_INT; > > mos6522_update_irq(s); > > break; > > case VIA_REG_T1CH: > > - val = get_counter(s, &s->timers[0]) >> 8; > > + val = get_counter(s, &s->timers[0], now) >> 8; > > break; > > case VIA_REG_T1LL: > > val = s->timers[0].latch & 0xff; > > @@ -247,12 +251,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, > > unsigned size) > > val = (s->timers[0].latch >> 8) & 0xff; > > break; > > case VIA_REG_T2CL: > > - val = get_counter(s, &s->timers[1]) & 0xff; > > + val = get_counter(s, &s->timers[1], now) & 0xff; > > s->ifr &= ~T2_INT; > > mos6522_update_irq(s); > > break; > > case VIA_REG_T2CH: > > - val = get_counter(s, &s->timers[1]) >> 8; > > + val = get_counter(s, &s->timers[1], now) >> 8; > > break; > > case VIA_REG_SR: > > val = s->sr; > > @@ -360,10 +364,9 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t > > val, unsigned size) > > } > > mos6522_update_irq(s); > > /* if IER is modified starts needed timers */ > > - mos6522_timer1_update(s, &s->timers[0], > > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > > - mos6522_timer2_update(s, &s->timers[1], > > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > > + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + mos6522_timer1_update(s, &s->timers[0], now); > > + mos6522_timer2_update(s, &s->timers[1], now); > > break; > > default: > > g_assert_not_reached(); > > In terms of functionality it shouldn't really matter (since you have a > ns clock compared with a timer that can manage small frequencies in > comparison) but I can see how having a constant clock time throughout > the entire calculation process could be useful for debugging. > I found it impossible to reason about program behaviour with so many calls to qemu_clock_get_ns(). Thanks for your review.
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c index 0dd3ccf945..23a440b64f 100644 --- a/hw/misc/mos6522.c +++ b/hw/misc/mos6522.c @@ -39,9 +39,9 @@ /* XXX: implement all timer modes */ static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, - int64_t current_time); + int64_t now); static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, - int64_t current_time); + int64_t now); static void mos6522_update_irq(MOS6522State *s) { @@ -52,12 +52,12 @@ static void mos6522_update_irq(MOS6522State *s) } } -static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti) +static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti, int64_t now) { int64_t d; unsigned int counter; - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, + d = muldiv64(now - ti->load_time, ti->frequency, NANOSECONDS_PER_SECOND); if (ti->index == 0) { @@ -89,7 +89,7 @@ static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int val) } static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, - int64_t current_time) + int64_t now) { int64_t d, next_time; unsigned int counter; @@ -99,7 +99,7 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, } /* current counter value */ - d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, + d = muldiv64(now - ti->load_time, ti->frequency, NANOSECONDS_PER_SECOND); /* the timer goes down from latch to -1 (period of latch + 2) */ @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) + ti->load_time; - - if (next_time <= current_time) { - next_time = current_time + 1; - } return next_time; } static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, - int64_t current_time) + int64_t now) { if (!ti->timer) { return; } - ti->next_irq_time = get_next_irq_time(s, ti, current_time); + ti->next_irq_time = get_next_irq_time(s, ti, now); + if (ti->next_irq_time <= now) { + ti->next_irq_time = now + 1; + } if ((s->ier & T1_INT) == 0 || ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { timer_del(ti->timer); @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, } static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, - int64_t current_time) + int64_t now) { if (!ti->timer) { return; } - ti->next_irq_time = get_next_irq_time(s, ti, current_time); + ti->next_irq_time = get_next_irq_time(s, ti, now); + if (ti->next_irq_time <= now) { + ti->next_irq_time = now + 1; + } if ((s->ier & T2_INT) == 0 || (s->acr & T2MODE) || ti->oneshot_fired) { timer_del(ti->timer); } else { @@ -163,9 +165,10 @@ static void mos6522_timer1_expired(void *opaque) { MOS6522State *s = opaque; MOS6522Timer *ti = &s->timers[0]; + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); ti->oneshot_fired = true; - mos6522_timer1_update(s, ti, ti->next_irq_time); + mos6522_timer1_update(s, ti, now); s->ifr |= T1_INT; mos6522_update_irq(s); } @@ -174,9 +177,10 @@ static void mos6522_timer2_expired(void *opaque) { MOS6522State *s = opaque; MOS6522Timer *ti = &s->timers[1]; + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); ti->oneshot_fired = true; - mos6522_timer2_update(s, ti, ti->next_irq_time); + mos6522_timer2_update(s, ti, now); s->ifr |= T2_INT; mos6522_update_irq(s); } @@ -233,12 +237,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size) val = s->dira; break; case VIA_REG_T1CL: - val = get_counter(s, &s->timers[0]) & 0xff; + val = get_counter(s, &s->timers[0], now) & 0xff; s->ifr &= ~T1_INT; mos6522_update_irq(s); break; case VIA_REG_T1CH: - val = get_counter(s, &s->timers[0]) >> 8; + val = get_counter(s, &s->timers[0], now) >> 8; break; case VIA_REG_T1LL: val = s->timers[0].latch & 0xff; @@ -247,12 +251,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size) val = (s->timers[0].latch >> 8) & 0xff; break; case VIA_REG_T2CL: - val = get_counter(s, &s->timers[1]) & 0xff; + val = get_counter(s, &s->timers[1], now) & 0xff; s->ifr &= ~T2_INT; mos6522_update_irq(s); break; case VIA_REG_T2CH: - val = get_counter(s, &s->timers[1]) >> 8; + val = get_counter(s, &s->timers[1], now) >> 8; break; case VIA_REG_SR: val = s->sr; @@ -360,10 +364,9 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) } mos6522_update_irq(s); /* if IER is modified starts needed timers */ - mos6522_timer1_update(s, &s->timers[0], - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); - mos6522_timer2_update(s, &s->timers[1], - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + mos6522_timer1_update(s, &s->timers[0], now); + mos6522_timer2_update(s, &s->timers[1], now); break; default: g_assert_not_reached();
mos6522_read() and mos6522_write() may call various functions to determine timer irq state, timer counter value and QEMUTimer deadline. All called functions must use the same value for the present time. Signed-off-by: Finn Thain <fthain@linux-m68k.org> --- hw/misc/mos6522.c | 51 +++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 24 deletions(-)