[PATCH v2 1/2] ARM: arch_timers: enable the use of the virtual timer

[Marc Zyngier]

On 11/08/12 15:28, Cyril Chemparathy wrote:
> On 8/11/2012 6:31 AM, Marc Zyngier wrote:
>> At the moment, the arch_timer driver only uses the physical timer,
>> which can cause problem if PL2 hasn't enabled PL1 access in CNTHCTL,
>> which is likely in a virtualized environment. Instead, the virtual
>> timer is always available.
>>
>> This patch enables the use of both the virtual timer, unless no
>> interrupt is provided in the DT for it, in which case is falls
>> back to the physical timer.
>>
> 
> I'm curious about the cost of the added pointer chasing introduced by 
> this patch.
> 
> The original code gets nicely inlined by the compiler, and this hides 
> all the switch-case register index stuff.  For instance:
> 
>   10797 c001288c <arch_timer_set_next_event>:
>   10798 c001288c:       ee1e3f32        mrc     15, 0, r3, cr14, cr2, {1}
>   10799 c0012890:       e3c33002        bic     r3, r3, #2
>   10800 c0012894:       ee0e0f12        mcr     15, 0, r0, cr14, cr2, {0}
>   10801 c0012898:       f57ff06f        isb     sy
>   10802 c001289c:       e3833001        orr     r3, r3, #1
>   10803 c00128a0:       ee0e3f32        mcr     15, 0, r3, cr14, cr2, {1}
>   10804 c00128a4:       f57ff06f        isb     sy
>   10805 c00128a8:       e3a00000        mov     r0, #0
>   10806 c00128ac:       e12fff1e        bx      lr
> 
> With the added pointer chasing, we unfortunately lose out on all the 
> work that the compiler used to do for us.  We now end up having to snake 
> our way through the following:

[...]

Yup, that doesn't look too good.

> I think we'd be better off separating between these (virt, phys, ...) 
> implementations at a higher level of operations (set_mode, 
> set_next_event, ...) rather than separating at a register operations 
> level as you have in this patch.

This approach leads to a lot of code duplication, unless we do some ugly
preprocessor hackery (see patch attached). I then get much better code
(smaller, faster), but I'm not sure we want to live with this...

	M.
-- 
Jazz is not dead. It just smells funny...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-arch_timers-enable-the-use-of-the-virtual-timer.patch
Type: text/x-diff
Size: 14514 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120813/b73ddd01/attachment-0001.bin>