HPET race conditions

[nexos]

Hello,

Presently in my OS I am working on multitasking. For context, my OS is kind of a tickless system (or at least I aim for it to become one). However, I noticed today that sometimes timer events will be missed because the comparator in the HPET is not set far ahead enough of the main counter so when the comparator actually gets set, the event has already passed.

In order to fix this, for a while I would have a "minimum delta" and if a requested delta was smaller than this, I would set it to this. This turned problematic as the minimum delta varies from machine to machine and also can race with things like SMI, and also was too inaccurate to use. So I'm not entirely sure what the best way to handle this is.

One idea I have is to loop when arming, checking if the main counter is greater than the comparator. If it is, than we add a continually increasing offset to the base delta. The problem here is this feels error prone and like a hack.

The other idea I have is to artificially decrease the amount of precision available to us. This is similar to my original plan but a bit more elegant. Basically my kernel works on 64-bit nanosec based time deltas, but obviously the max available precision can be less than this depending on hardware. What I would do is if the HPET has a precision greater than a specific amount (e.g., 100 us), I would cap it at that and round all incoming deltas to the nearest 100 us up (or down if its closer and wouldn't cause it to go to 0). This still has the drawback of limiting precision (which isn't a huge deal) and also still being slightly arbitrary so that one unlucky SMI could mess things up.

My final idea (which is admittedly the simplest) i just to not use HPET for timer events and always use the APIC timer. If the system doesn't have an invariant TSC I'll still use the HPET as a clock base (as my kernel separates the abstractions of clock and timer). This is what I'm leaning towards as this would be the most reliable, however I would still like the HPET to be an option for users since it's still not dependent on CPU frequency so things like sleep states and that kind of stuff can't mess up systems without ARAT (which shouldn't be a huge deal for events and not clocks AFAICT).

What do you all think the best option is?

[nullplan]

Time for global variables. You have a global flag "timer interrupt happened". Before setting the comparator, your clear the flag. In the timer interrupt, obviously you set the flag. After setting the comparator, you compare the count against the value you just wrote yourself, and if the main count exceeds the comparator and the timer interrupt flag is not set, you run the timer interrupt yourself. That's how I'd do it, at least. Truthfully I don't use HPET, I just use the LAPIC timer for all my interrupting needs.

[rdos]

Time for global variables. You have a global flag "timer interrupt happened". Before setting the comparator, your clear the flag. In the timer interrupt, obviously you set the flag. After setting the comparator, you compare the count against the value you just wrote yourself, and if the main count exceeds the comparator and the timer interrupt flag is not set, you run the timer interrupt yourself. That's how I'd do it, at least. Truthfully I don't use HPET, I just use the LAPIC timer for all my interrupting needs.

Yes, this is similar to how I handle it. I think the LAPIC timer is better for the event timer, while the continuous tics counter would be a better use of the HPET.

[nexos]

That’s a good idea. I use level triggered ints on the HPET so instead of using a global I’ll check the int status in the registers. I don’t really like handling timer events outside of the interrupt handlers but I can’t forsee it breaking anything so I’ll just do that

[davmac314]

The classical way to avoid a race where you set the timer to an already-expired time is that you read back the main counter after setting the comparator to make sure that it didn't sneak past the comparator value while you were setting it. Apparently there are still some issues with this approach:

https://github.com/torvalds/linux/commi ... 9f7becf47c
https://github.com/torvalds/linux/commi ... 7ed36d64fb
https://github.com/torvalds/linux/commi ... fa954a05c6

Those commits are in chronological order. I'll leave it to you to make full sense of them, but I'll note that the Linux solution in the end seemed to be to just require a really big delta between the current time and the desired timeout (comparator value) and return a failure if that requirement isn't met. In that case I assume the kernel either finds another timer to use (LAPIC?) or it just treats it as an immediate timeout (i.e. loses precision).

I think in theory you could try to measure the approximate latency between setting the comparator and it taking effect, then add a small buffer to that, and use either a main counter readback after setting the comparator or rely on the TSC to detect when setting the comparator has taken longer than it should've (due to SMI/NMI/whatever). But it would be a little bit complicated, so it might be worth doing something more like what Linux does. Or just use the LAPIC timer for short-term timeouts, as has been suggested.

[nullplan]

The classical way to avoid a race where you set the timer to an already-expired time is that you read back the main counter after setting the comparator to make sure that it didn't sneak past the comparator value while you were setting it.

The obvious problem with that approach is that if the counter value started exceeding the comparator value between setting the comparator value and reading the main counter, you now execute the timer callback twice. Which is why I suggested doing that only in conjunction with a flag that tells you the timer interrupt has happened, so you don't execute it twice.

https://github.com/torvalds/linux/commi ... 9f7becf47c
https://github.com/torvalds/linux/commi ... 7ed36d64fb
https://github.com/torvalds/linux/commi ... fa954a05c6

Oh great. So we can see at the time of setting the comparator that the HPET hasn't reached it yet, but the timer interrupt is missed, anyways... You know, I didn't really want to implement HPET anyway, and now I have a great reason.

I use level triggered ints on the HPET so instead of using a global I’ll check the int status in the registers.

When does that bit get reset? The important thing is not whether the interrupt is currently queued or in service, but whether it has happened since setting the comparator. And besides, a hardware reg that exists once per machine is nothing else than a global variable.

I don’t really like handling timer events outside of the interrupt handlers but I can’t forsee it breaking anything so I’ll just do that.

Typically interrupts are the most restricted contexts to run kernel code in, so any function that can serve as interrupt handler can also run in system call or kernel thread context.

[davmac314]

The obvious problem with that approach is that if the counter value started exceeding the comparator value between setting the comparator value and reading the main counter, you now execute the timer callback twice. Which is why I suggested doing that only in conjunction with a flag that tells you the timer interrupt has happened, so you don't execute it twice.

I was not disagreeing with you. You certainly need to make sure you don't execute the timer callback twice. However, there are a bunch of additional concerns with the HPET, as I pointed out.

[rdos]

I use the timer in conjunction with a timer list that includes expire time and a callback. It doesn't matter how many times the list is updated, and as soon as the callback is called the entry is removed from the list.