Re: [PATCH] clocksource/drivers/arm_arch_timer: export arch_timer_get_rate

[Marc Zyngier <maz@kernel.org>]

Chanho,

On 2021-01-12 15:14, Chanho Park wrote:
> Hi Marc,
> 
> On Tue, Jan 12, 2021 at 11:45 PM Marc Zyngier <maz@kernel.org> wrote:
>> 
>> On 2021-01-12 13:39, Chanho Park wrote:
>> > Hi,
>> >
>> >> On Tue, Jan 12, 2021 at 10:31:40AM +0900, Chanho Park wrote:
>> >> > This patch adds to export arch_timer_get_rate function for calculating
>> >> > absolute timestamp which is based on arch timer like below.
>> >> > arch_timer_read_counter was already exported but arch_timer_get_rate
>> >> > wasn't. Thus, this patch tries to export this to use this function from
>> >> > loadable kernel module.
>> >>
>> >> Can you please explain /where/ this would be used? i.e. which module?
>> >>
>> >> Generally we try to avoid drivers depending on the specific
>> >> clocksource,
>> >> so I think there needs to be stronger rationale for exposing this.
>> >
>> > I need a system-wide timestamp which can be available from bootloader
>> > and kernel stages including virtual machines.
>> > Actually, it's necessary to record a timestamp of each log message for
>> > system-wide debugging on type-1 hypervisor.
>> > RTC can be used for this purpose but we should make it to hypervisor
>> > awareness.
>> > |---------------|-------------------------|
>> >  Bootloader        VM1 (Guest)
>> >                    |-------------------------|
>> >                           VM2 (Guest)
>> >
>> > So, the easiest way is using the arm architect timer's timestamp
>> > because it's already supported on each VM by the hypervisor.
>> 
>> This doesn't make much sense. The hypervisor and the VMs are
>> independent software entities, and they don't use symbols from
>> each other.
> 
> I meant that each VM needs to be synchronized by the ARM arch timer's
> timestamp not the symbol itself.
> The symbol is necessary to calculate the system-wide time by the
> timestamp(counter) value.
> 
> The counter of the arm architect timer will be increasing from the 
> bootloader.
> The hypervisor will not reset the counter and each VMs as well. So, we
> can use this timestamp for comparing _real_ time :)

Well, you can compare the raw counter values, and do the conversion
in a single place. Also, if your system is correctly configured,
you have access to CNTFRQ_EL0, which contains the same thing as
arch_timer_get_rate().

>> So this symbol is probably used by a module *inside* the VMs,
>> and Mark's question still stands.
>> 
> 
> Yes. Actually, we use this timestamp for our internal module which is
> not yet upstreamed.

If the module code is not upstream, I don't see the point in exporting
this symbol. I suggest you post both as a series so that we can decide
whether that is a good idea or not.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel