Re: [BUG] IO-APIC + timer doesn't work!

[Jan Kiszka]

Ingo Molnar wrote:
> * Jeff Mahoney <jeffm@xxxxxxxx> wrote:
> 
>>> Hmmmmm. That somehow reminds me of what I thought I had to fix in the
>>> HPET emulation of QEMU just recently [1] - because of 2.6.30-rc's behavior.
>>>
>>> Could you try if writing 'delta' a second time makes any difference on
>>> that box?
>>>
>>> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
>>> index 648b3a2..523d72b 100644
>>> --- a/arch/x86/kernel/hpet.c
>>> +++ b/arch/x86/kernel/hpet.c
>>> @@ -324,6 +324,7 @@ static void hpet_set_mode(enum clock_event_mode mode,
>>>  		       HPET_TN_SETVAL | HPET_TN_32BIT;
>>>  		hpet_writel(cfg, HPET_Tn_CFG(timer));
>>>  		hpet_writel((unsigned long) delta, HPET_Tn_CMP(timer));
>>> +		hpet_writel((unsigned long) delta, HPET_Tn_CMP(timer));
>>>  		hpet_start_counter();
>>>  		hpet_print_config();
>>>  		break;
>>>
>> Thanks, Jan.
>>
>> That fixed it for me.
> 
> I've queued it up (and i've got a test-system that might be affected 
> by a similar problem - it shows a similar crash very rarely), but it 
> would be nice to know why this duplicate writeout makes a 
> difference. Jan?
> 
> 	Ingo

Well, if you look at the HPET spec [1], you first find the explanation
of the Tn_VAL_SET_CNF bit (HPET_TN_SETVAL):

"[...] By writing this bit to a 1, the software is then allowed to
directly set a periodic timer's accumulator."

That may sound like "you write to the comparator register if 0, and if
1, you set the accumulator". That's also how HPET was emulated in QEMU
so far.

But then you read on about changing the period of a running timer:

"If the software resets the main counter, the value in the comparatorâs
value register needs to reset as well. This can be done by setting the
Tn_VAL_SET_CNF bit. Again, to avoid race conditions, this should be
done with the main counter halted. The following usage model is expected:
1) Software clears the GLOBAL_ENABLE_CNF bit to prevent any interrupts
2) Software Clears the main counter by writing a value of 00000000h to it.
3) Software sets the TIMER0_VAL_SET_CNF bit.
4) Software writes the new value in the TIMER0_COMPARATOR_VAL register
5) Software sets the GLOBAL_ENABLE_CNF bit to enable interrupts."

And that somehow sounds like you only need to write the new period once,
with Tn_VAL_SET_CNF = 1.

I bet now that both interpretations are implemented in silicon somewhere
out there - but I'm all ears to learn the right one (and potentially
re-fix QEMU).

Jan

[1] http://www.intel.com/hardwaredesign/hpetspec_1.pdf

Attachment: signature.asc
Description: OpenPGP digital signature