On 04/11/15 03:40, David Gibson wrote: > On Fri, Oct 23, 2015 at 02:56:37PM +0100, Mark Cave-Ayland wrote: >> Fix the counter loading logic and enable the T2 interrupt when the timer >> expires. > > A mention of what uses T2, and therefore why this is useful would be > good.
There is a good chance that nothing has used T2 before MacOS 9 since before this patch, it is impossible for the T2 timer interrupt to fire. It can be seen that MacOS 9 does write to the relevant registers during boot, and if the T2 interrupt is disabled then boot will hang. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> >> --- >> hw/misc/macio/cuda.c | 30 +++++++++++++++++++++--------- >> 1 file changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c >> index 687cb54..d864b24 100644 >> --- a/hw/misc/macio/cuda.c >> +++ b/hw/misc/macio/cuda.c >> @@ -136,7 +136,7 @@ static void cuda_timer_update(CUDAState *s, CUDATimer >> *ti, >> >> static void cuda_update_irq(CUDAState *s) >> { >> - if (s->ifr & s->ier & (SR_INT | T1_INT)) { >> + if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) { >> qemu_irq_raise(s->irq); >> } else { >> qemu_irq_lower(s->irq); >> @@ -175,7 +175,7 @@ static unsigned int get_counter(CUDATimer *ti) >> >> static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val) >> { >> - CUDA_DPRINTF("T%d.counter=%d\n", 1 + (ti->timer == NULL), val); >> + CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val); >> ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), >> s->frequency); >> ti->counter_value = val; >> @@ -220,7 +220,7 @@ static void cuda_timer_update(CUDAState *s, CUDATimer >> *ti, >> { >> if (!ti->timer) >> return; >> - if ((s->acr & T1MODE) != T1MODE_CONT) { >> + if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) { >> timer_del(ti->timer); >> } else { >> ti->next_irq_time = get_next_irq_time(ti, current_time); >> @@ -238,6 +238,16 @@ static void cuda_timer1(void *opaque) >> cuda_update_irq(s); >> } >> >> +static void cuda_timer2(void *opaque) >> +{ >> + CUDAState *s = opaque; >> + CUDATimer *ti = &s->timers[1]; >> + >> + cuda_timer_update(s, ti, ti->next_irq_time); >> + s->ifr |= T2_INT; >> + cuda_update_irq(s); >> +} >> + >> static uint32_t cuda_readb(void *opaque, hwaddr addr) >> { >> CUDAState *s = opaque; >> @@ -276,6 +286,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr) >> case CUDA_REG_T2CL: >> val = get_counter(&s->timers[1]) & 0xff; >> s->ifr &= ~T2_INT; >> + cuda_update_irq(s); >> break; >> case CUDA_REG_T2CH: >> val = get_counter(&s->timers[1]) >> 8; >> @@ -352,11 +363,12 @@ static void cuda_writeb(void *opaque, hwaddr addr, >> uint32_t val) >> cuda_timer_update(s, &s->timers[0], >> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); >> break; >> case CUDA_REG_T2CL: >> - s->timers[1].latch = val; >> - set_counter(s, &s->timers[1], val); >> + s->timers[1].latch = (s->timers[1].latch & 0xff00) | val; >> break; >> case CUDA_REG_T2CH: >> - set_counter(s, &s->timers[1], (val << 8) | s->timers[1].latch); >> + s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8); >> + s->ifr &= ~T2_INT; >> + set_counter(s, &s->timers[1], s->timers[1].latch); > > So the new code appears to be like that for T1CL / T1CH, which makes > sense. However, T1CL has a cuda_timer_update() call. Do you also > need that for T2CL? This is a side-effect of combining the T1 and T2 code. Unlike T1, T2 appears to be free-running from its written value but generating an interrupt just after zero-crossing. So in order to get the correct interval, we write the value to the latch instead of the counter to get the same effect with the shared timer code. >> break; >> case CUDA_REG_SR: >> s->sr = val; >> @@ -719,8 +731,7 @@ static void cuda_reset(DeviceState *dev) >> s->timers[0].latch = 0xffff; >> set_counter(s, &s->timers[0], 0xffff); >> >> - s->timers[1].latch = 0; >> - set_counter(s, &s->timers[1], 0xffff); >> + s->timers[1].latch = 0xffff; >> } >> >> static void cuda_realizefn(DeviceState *dev, Error **errp) >> @@ -730,7 +741,8 @@ static void cuda_realizefn(DeviceState *dev, Error >> **errp) >> >> s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s); >> s->timers[0].frequency = s->frequency; >> - s->timers[1].frequency = s->frequency; >> + s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s); >> + s->timers[1].frequency = (SCALE_US * 6000) / 4700; > > Where does this T2 frequency come from? My understanding of this is that with the shared timer code, the IRQ timing is calculated based upon CUDA_TIMER_FREQ (4.7MHz / 6). Therefore by setting the frequency to the inverse value ((SCALE_US * 6000) / 4700) then this cancels out the effect of the timebase calculation algorithm used in the counters. I believe this came from Alex so he would likely be able to clarify this and give a much better explanation. >> qemu_get_timedate(&tm, 0); >> s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; > ATB, Mark.