ARM generic timer

So R1=‘WIDE’ (since that can’t be a valid address to call) or R3=‘WIDE’.

‘WIDE’ is &45444957 which is odd and therefore potentially a valid Thumb code address in a dynamic area. Adding meaning to a previously don’t care register is a security risk as existing code could read it from an untrusted source.

If a ‘WIDE’ parameter is to be used, a value between -1 to -256 would be more suitable as it can be constructed with one instruction and points to an area of memory that can’t run code.

Why would an odd address be Thumb code? Don’t you mean 4n+2?

I like Jon’s suggestion more. I was fretting about both the caller and the callee needing to have magic words to supply and compare… but then I though “only I worry about bytes like that”. Glad I’m not alone.

I’m a bit puzzled about the ‘WIDE’ Thumb/security comment too. There’s only 4 possible combinations to consider, and they all look safe, since there’s no vetting on old kernels you’re quite welcome to try executing Thumb mode code in a dynamic area today if you want.

If a ‘WIDE’ parameter is to be used, a value between -1 to -256 would be more suitable as it can be constructed with one instruction and points to an area of memory that can’t run code.

Like the Wimp tends to use ‘TASK’ to add nested operations or ‘TRUE’ for palettes, I noted the kernel already has ‘WIDE’ to extend the OS_ReadUnsigned API to 64b so there was a coherent re-use, in my mind anyway. I’m not concerned about it needing to be an 8 bit ARM immediate constant since we’re not exactly hammering the OS_Call* SWIs, and a literal pool load isn’t the biggest of crimes (or, scary new world post-1997 technology of MOVW/MOVT if you’re worried about hurting the data cache).

Can somebody explain to me what on earth all this Thumb code nonsense is about?

Clearly “WIDE” (or “FAST” or whatever magic keyword) is going to have to be R3; because R0 is the delay, R1 is the address to call, and R2 is the value of R12. This means the only logical place for the magic word is R3, which will have no effect on older systems and is about as unlikely to be “set by accident” as “TASK” and “TRUE” used elsewhere in the API.

There ought to be a ReadSysInfo bit to flag that fast timing is available, because waiting for a million microseconds is a little different to waiting for a million centiseconds. ;-) It’s up to the app to downgrade itself or fail out with an error if the fast mode isn’t available.

I’m not really a fan of using magic values to extend existing calls. Too easy for someone to forget to check that the feature is supported before they try using it, resulting in bad things happening if someone runs the code on an old OS version. Plus of course there’s the risk of code accidentally triggering the new feature (e.g. ‘WIDE’ as a code address isn’t likely to cause any problems, but ‘WIDE’ in a previously unused register could). If I had to choose this option, I’d probably go with Jon’s suggestion of using a code pointer in the range -1 to -256, because then we’d only have to worry about the “forgetting to check the feature is supported” case.

Rather than proliferating similar-but-different SWI names (I already find the various OS_Call* ones confusing!), how about a magic word selector like we have for OS_ReadUnsigned?

So you’re suggesting that a SWI called OS_CallAfter, which implements “call at” behaviour, would be less confusing than adding a new OS_CallAt SWI? I’m sorry, but that sounds like it would make things more confusing.

Why would an odd address be Thumb code?

Address-based interworking branches. If bit 0 of the address is zero, the CPU switches to ARM mode. If it’s 1, it switches to Thumb mode.

https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/branch-and-call-sequences-explained

Since there are several new calls wanted (OS_CallAt, OS_RemoveCallAt, OS_Sleep*, new SWI to allow the RTC + Portable module to communicate with the kernel, and potentially OS_CallEvery64 + OS_RemoveTickerEvent64), perhaps the better solution would be to implement them all as sub-reasons of a new SWI? (e.g. OS_Timer? OS_Time64?)

Too easy for someone to forget to check that the feature is supported before they try using it, resulting in bad things happening if someone runs the code on an old OS version.

Doesn’t this implicate practically every API extension ever?

Tell me, what would COLOUR OF 12 ON 14 (or whatever the syntax is) do on RISC OS 3.1? Or noticing that one is using RISC OS 5 so issuing VFP instructions…on an Iyonix?
Or the fun and frolics of OS_ReadLine being misunderstood?
Or…

Programmer uses new feature without checking it exists.
Program blows up.
Programmer at fault.
Not computer.

FWIW, I like the idea of the new SWI with subreasons. Keep the available SWIs sensible, and I’m not a fan of magic values.

It’s all about how the program blows up. SWI “OS_CallAt” on an old OS version will produce an immediate error, which the calling program can at least do something sensible with (e.g. report the error to the user and then exit the program). SWI “OS_CallAfter”,“WIDE” won’t produce an immediate error from the SWI. Instead it plants a ticking time bomb which at some indeterminate point in the future will blow up, most likely with an abort, which the OS will foolishly attribute to an innocent bystander app.

All the more reason to not recycle SWIs. ;-)

And, also, why a register used as an address should never be repurposed as some sort of flag. Setting R3 as the WIDE flag on an older system would mean that instead of getting called after a second, you’d get called after something like two and a half hours. Maybe. That sounds a little soon for a million centiseconds, but you get the point. It’s all still valid (proper address pointer and all) so shouldn’t literally blow up.

That sounds a little soon for a million centiseconds

2 hours 46 minutes (ish)

Rather than proliferating similar-but-different SWI names (I already find the various OS_Call* ones confusing!), how about a magic word selector like we have for OS_ReadUnsigned?

So you’re suggesting that a SWI called OS_CallAfter, which implements “call at” behaviour, would be less confusing than adding a new OS_CallAt SWI? I’m sorry, but that sounds like it would make things more confusing.

A wise man recently said:

* Allow scheduling & removal of events (similar to OS_CallAfter / OS_RemoveTickerEvent), except you specify a 1MHz 64bit timestamp instead of a centisecond delta time.
* Possibly we’d also want an OS_CallEvery64 for easy scheduling of periodic events

which I read as meaning there would be 3 calls similar to OS_CallAfter / OS_CallEvery / OS_RemoveTickerEvent but with artificially synonymous SWI names when we could just extend the 3 existing ones. If by similar you mean they’re functionally different then new names make sense, of course.

If by similar you mean they’re functionally different then new names make sense, of course.

Yes, sorry – I probably wasn’t clear enough.

OS_CallAt is essentially “call me at 5pm Tuesday” vs. OS_CallAfter “call me after 100 seconds”. The idea being that it’ll result in more accurate event scheduling – if you wanted “5pm Tuesday” with OS_CallAfter then it would be difficult to compensate for the passage of time that occurs between when you read the current time (to calculate the required delta), and when the OS stores the OS_CallAfter request. Not a major problem with the current centisecond OS_CallAfter, but for higher-frequency events (and in a future threaded world) it’ll be more of an issue.

Will these discussions make Acorn’s Timecode module redundant?

I don’t think so – it looks like TimeCode is mainly concerned with synchronising with external clocks, including clocks which don’t run at real time (e.g. the description of the TimeSpeed app mentions the ability to pause & rewind time). Whereas the system here is essentially just a higher-resolution OS_ReadMonotonicTime / OS_CallAfter, with less bugs (e.g. avoiding time slowing down if interrupts are disabled for too long).

Any progress on this topic?

Some weeks ago I modified DigitalCD’s Sonogram plugin to drop the centi-seconds stepping limit for faster sonograms. I also modified DCDUtils to read both OS_ReadMonotonicTime and the the HAL Timer to try to reposition the start of the samples for the FFT to a more time correct part of the SoundDMA buffer’s copy than just the end of the buffer and so avoid displaying consecutive lines with the same values.

For these reasons, SWIs OS_ReadMonotonicTime64 and Wimp_PollIdle64 would help. The first to have an atomic time value instead of having to derive the time from 2 separate calls, the second because below a Wimp_PollIdle with a 1cs delay, it is basically “as fast as the processor allows”, which tends to be to fast for an interesting sonogram unless I really push the FFT size. A reasonable value for 1920×1080 should be a progression of 250-400 lines (=FFTs) per second.

Any progress on this topic?

Not yet, I’m afraid. Updating and testing everything felt a bit tedious, so I put it on hold for a bit while I worked on smaller/more interesting tasks.

I should probably try and get it finished!

So any software which uses HAL_TimerReadCountdown (for timer 0) or HAL_CounterRead to get sub-centisecond timing values is going to get confused when the timer starts to get used for more than just the 100Hz ticker

Yes that would confuse my TimerMod, although it reads the rate and fixes up for RTCadjust, it always assumes it the countdown period corresponds to a cs, so it can add microseconds to the monotonic time. As long as a microsecond returning OS_MonotomicTime64 is available, I’ll make TimerMode use it.

As long as a microsecond returning OS_MonotomicTime64 is available, I’ll make TimerMod use it.

Sounds good.

But to clarify: I’m planning on having the kernel intercept calls to HAL timer 0, so that it can act as if it’s a 1MHz timer which resets every centisecond, i.e. the same as its current behaviour under RISC OS (more info in this thread). So although an updated version of TimerMod would be best, the current version should continue to work as well as it currently does.

Ok, that sounds good.

I am thinking that with the new GHz+ machines, I should be moving to nanosecond timings now. Certainly in !APPstat it should really be accumulating times in at whatever the resolution of the timer is, and only converting for display in the font end. Currently it uses code from TimerMod to get microseconds and accumulates that. I know some machines timer’s have significantly better than microsecond resolution, but I’ve also seen one which was much worse!

Any more progress on this? It seems a bit unfortunate for every platform to have to implement its own timer using platform-specific hardware, when all recent cores have ARM generic timers onboard. It would be nice if the core functionality (one thread, timers, interrupts) worked out of the box.

For the record, this is a blocker on something I’ve been looking at lately.

Any more progress on this?

Nothing significant.

If I was to release the code I currently have, would anyone be interested in finishing it off? Rough todo list is:

• The Iyonix & IOMD implementations have the potential for losing time, more testing will be needed to work out whether this is significant enough to cause problems and/or whether it can be improved upon
• The Cortex-A9 timer will change frequency in response to CPU clock frequency changes, so the timer code needs to be made aware of CPU clock changes (and try and minimise any inaccuracies it introduces)
• Pre-StrongARM needs a fast multiply-divide routine (although my prototype one is probably good enough to start with: https://www.riscosopen.org/forum/forums/5/topics/15437)
• The OS/kernel APIs need implementing: https://www.riscosopen.org/forum/forums/3/topics/11109?page=2#posts-94249
• Titanium HAL interrupt handling needs improving (PPI & SGI support) – OMAP5 HAL should be an adequate reference. At some point I’ve also been meaning to look into FIQ support, but that’s lower priority.
• My test timer driver code (HAL devices implemented in modules) needs moving into the relevant HALs, and the HALs need adjusting to use the new timer instead of the old one where relevant
• Since there are lots of platforms to update (including some which aren’t yet open-source), there’ll need to be some consideration for backwards compatibility. A build time switch in the kernel, or requiring unconverted HALs to use an older kernel version, might be sufficient

Hmm… I think a lot of that is beyond my pay grade. I was wondering… as a stepping stone, is there a simpler way to implement the old API using the generic timer? It will change according to CPU frequency, but that would not seem an insurmountable problem to change the timebase as the frequency changes. That would at least remove the need to write a platform timer driver afresh for each new SoC.

I was wondering… as a stepping stone, is there a simpler way to implement the old API using the generic timer?

Implementing the old API using the ARM generic timer should be trivial. However other than the ability to copy & paste the timer code from one HAL to another, you won’t really gain much – you’ll still be stuck with the flawed/restrictive API.

It will change according to CPU frequency

For all the ARM generic timer implementations I’ve looked at, the timer frequency isn’t affected by the CPU frequency. It’s only the Cortex-A9 where we have that problem (the A9 timer is an older design that predates the ARM generic timer)

At long last I’ve picked up this task again, and made some good progress in the past couple of weeks: The kernel changes are mostly complete, and I’m able to run OMAP3 ROMs where the kernel is using the new 64 bit timer as its time source instead of HAL timer 0.

This merge request has a summary of the status and links to the other changed components: https://gitlab.riscosopen.org/RiscOS/Sources/Kernel/-/merge_requests/63

To keep things simple I’ve decided to not design/implement the OS_Sleep SWI. That can come in a future set of changes.

Open questions:

• To avoid requiring too many kernel SWIs, I’ve squished things down into just two new SWIs: OS_ReadMonotonicTime64 (for getting the time) and OS_TimerControl (for OS_CallAt/OS_CallEvery type functionality, and other misc things like allowing the RTC module to fine-tune the clock rate). How do we feel about that? Is OS_TimerControl a good name?
• Currently the APIs offer microsecond resolution timestamps, but I’m thinking about changing it to nanosecond resolution, because that’s what some modern languages/APIs are designed to work with (e.g. the C timespec struct – available in C11, or as far back as 2001 in POSIX). This would be a trivial change to make, and with 64 bits it’ll still take over 500 years for the monotonic timer to wrap (i.e. “never”). Obviously not all platforms have high enough resolution hardware timers for the extra resolution to make sense (e.g. RPi only has 1MHz timers), and if you’re planning on using it to profile code you still need to make sure the code runs long enough to make the overheads of the SWI insignificant (on a 1GHz BB-xM I’m seeing OS_ReadMonotonicTime take around 147ns, and OS_ReadMonotonicTime64 take around 696ns). But for future-proofing and easier integration with modern languages it seems like a sensible change to make. Any objections?

(i.e. “never”).

Don’t say that, Sveinung thinks ahead…