[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 12/13] cuda.c: fix T2 timer and enabl
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 12/13] cuda.c: fix T2 timer and enable its interrupt |
Date: |
Wed, 04 Nov 2015 23:25:43 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 |
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 <address@hidden>
>> ---
>> 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.