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

["Chanho Park" <chanho61.park@samsung.com>]

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Wednesday, January 13, 2021 12:31 AM
> To: Chanho Park <parkch98@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>; linux-arm-
> kernel@lists.infradead.org; Chanho Park <chanho61.park@samsung.com>;
> Daniel Lezcano <daniel.lezcano@linaro.org>; Thomas Gleixner
> <tglx@linutronix.de>
> Subject: Re: [PATCH] clocksource/drivers/arm_arch_timer: export
> arch_timer_get_rate
> 
> 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().

To convert the value in the single place, we may need to use inter-VM
communication so that it makes quite complex design/implementation for me.
Regarding CNTFRQ_EL0, I already tried to read it by arch_timer_get_cntfrq
but I got '0'. It looks like different according to the system.

> 
> >> 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.

Well, I'm not sure the module can be upstream because it's definitely
necessary only to analyze a panic in my hypervisor system. Let me find a
better way to expose this.

Best Regards,
Chanho Park


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