Re: [Qemu-devel] Questions about timer

[Alex Bligh]

On 23 Feb 2014, at 03:46, Xuebing Wang wrote:

> Hi Community,
> 
> I am new to qemu, I am trying to familiarize myself with aarch64 
> timer/interrupt support (maybe someone already did this, but not merged into 
> mainline yet).
> 
> As shown in include/qemu/timer.h, there are 6 categories of APIs:
> - QEMUClockType
> - QEMUTimerList
> - QEMUTimerListGroup
> - QEMUTimer
> - General utility functions
> - Low level clock functions
> 
> I have a few questions:

If you have not stumbled across this yet, try:
   http://blog.alex.org.uk/2013/08/24/changes-to-qemus-timer-system/

> Q1) All QEMUTimerList functions are private to qemu-timer.c. Can we make them 
> static and remove them from include/qemu/timer.h because they are NOT APIs?

In the general case, just because an API is not currently used, that doesn't 
mean it will never be used. The QEMUTimerList stuff is relatively new.

Currently they are not used outside qemu-timer.c. I suspect the reason they 
ended up that way was the the timer stuff was spread a bit more between files 
and I didn't make everything static at the end. I see no harm in general in 
this save for (i) the fact code entropy causes bugs, and (ii) someone needs to 
think through whether a future tlg user might need them (aio is really simple). 
What would be the advantage?

> Q2) The implementation of other categories of API functions are kind of 
> inter-wined, does it make sense to re-organize them in qemu-timer.c, and make 
> them strictly follow the sequence in include/qemu/timer.h?

You may need some forward declarations.

I see problem with this, but please make sure the patch(es) that move code 
touch nothing else.

> Q3) Below functions are NOT implemented in qemu-timer.c, but declared in 
> include/qemu/timer.h, should we address this?
> - qemu_clock_warp
> - timer_get and timer_put
> - cpu_get_ticks, cpu_enable_ticks, cpu_disable_ticks

They are there so that they inline. Why change that?

> Q4) Do we need this API qemu_clock_get_main_loop_timerlist?


It's not currently used. It currently uses the mainloop tpg stuff instead, 
which was less than 100% popular. I see no problem with it going.

-- 
Alex Bligh