Re: [Minios-devel] [UNIKRAFT PATCHv4 23/43] plat/kvm: Add Arm64 virtual timer library to provide ticks

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Sharan,

> -----Original Message-----
> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> Sent: 2018年7月24日 20:03
> To: minios-devel@xxxxxxxxxxxxx
> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 23/43] plat/kvm: Add Arm64
> virtual timer library to provide ticks
>
> Hello Wei Chen,
>
> Please find my comment inline:
>
> On 07/06/2018 11:03 AM, Wei Chen wrote:
> > On KVM platform, print debug message will use ukplat_monotonic_clock
> > to provide timestamp. So we implement this simple virtual timer
> > library for timestamp.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   plat/kvm/arm/time.c | 127 ++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 127 insertions(+)
> >   create mode 100644 plat/kvm/arm/time.c
> >
> > diff --git a/plat/kvm/arm/time.c b/plat/kvm/arm/time.c
> > new file mode 100644
> > index 0000000..ab4968f
> > --- /dev/null
> > +++ b/plat/kvm/arm/time.c
> > @@ -0,0 +1,127 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause */
> > +/*
> > + * Authors: Wei Chen <Wei.Chen@xxxxxxx>
> > + *
> > + * Copyright (c) 2018, Arm Ltd. All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of the copyright holder nor the names of its
> > + *    contributors may be used to endorse or promote products derived from
> > + *    this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS
> BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
> THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> > + */
> > +#include <stdlib.h>
> > +#include <uk/assert.h>
> > +#include <uk/plat/time.h>
> > +
> > +static uint64_t cntvct_at_init;
> > +static uint32_t counter_freq;
> > +/*
> > + * Shift factor for TSC scaling multiplier; referred to as S in the
> following
> > + * comments.
> > + */
> > +static uint8_t tsc_shift;
> > +
> > +/* Multiplier for converting TSC ticks to nsecs. (0.S) fixed point. */
> > +static uint32_t ns_per_tick;
> > +
> > +/*
> > + * System Time
> > + * 64 bit value containing the nanoseconds elapsed since boot time.
> > + * This value is adjusted by frequency drift.
> > + * NOW() returns the current time.
> > + * The other macros are for convenience to approximate short intervals
> > + * of real time into system time
> > + */
> > +#define NSEC_PER_SEC 1000000000ULL
> > +
> > +static inline uint64_t ticks_to_ns(uint64_t ticks)
> > +{
> > +return (ns_per_tick * ticks) >> tsc_shift;
> > +}
> > +
> > +static inline uint64_t get_counter_frequency(void)
> > +{
> > +uint64_t frq;
> > +
> > +__asm__ __volatile__("mrs %0, cntfrq_el0" : "=r" (frq):: "memory");
> > +
> > +return frq;
> > +}
> > +
> > +static inline uint64_t read_virtual_count(void)
> > +{
> > +uint64_t val;
> > +
>
> Cortex-A73 counter returns a wrong value if read while crossing a 32bit
> boundary
>
> Please refer to errata #858921, in document[1]. For instance, Linux[2]
> work around the problem by using
>
>          do {
>                  _old = read_sysreg(reg);
>                  _new = read_sysreg(reg);
>                  _retries--;
>          } while (unlikely((_new - _old) >> 5) && _retries);
>
>

Thanks for giving so detailed documents : )
While I was doing Arm64 porting work, I had considered whether or not to
support errata. Because we don't have an errata framework. For this
specific errata, we can support it easily by providing an option and
#ifdef/else for users to enable or disable. But if we want to avoid
users to enable this errata workaround for CPUs without this errata,
we have to check the CPU IDs and features further.

I can implement this errata workaround for counter in next version,
but we'd better have a framework : )

> > +__asm__ __volatile__("mrs %0, cntvct_el0" : "=r" (val)::);
> > +return val;
> > +}
> > +
> > +/* monotonic_clock(): returns # of nanoseconds passed since time_init()
> > + * Note: This function is required to return accurate
> > + *       time even in the absence of multiple timer ticks.
> > + */
> > +__nsec ukplat_monotonic_clock(void)
> > +{
> > +return (__nsec) ticks_to_ns(read_virtual_count() - cntvct_at_init);
> > +}
> > +
> > +void ukplat_time_init(void)
> > +{
> > +/*
> > + * Calculate TSC shift factor and scaling multiplier.
> > + *
> > + * tsc_shift (S) needs to be the largest (<=32) shift factor where the
> > + * result of the tsc_mult calculcation below fits into uint32_t without
> > + * truncation. Note that we disallow an S of zero to ensure the loop
> always
> > + * terminates.
> > + *
> > + * (0.S) tsc_mult = NSEC_PER_SEC (S.S) / tsc_freq (S.0)
> > + */
> > +uint64_t tmp;
> > +
> > +counter_freq = get_counter_frequency();
> > +tsc_shift = 32;
> > +do {
> > +tmp = (NSEC_PER_SEC << tsc_shift) / counter_freq;
> > +if ((tmp & 0xFFFFFFFF00000000L) == 0L)
> > +ns_per_tick = (uint32_t)tmp;
> > +else
> > +tsc_shift--;
> > +} while (tsc_shift > 0 && ns_per_tick == 0L);
> > +#if 0 /* assert has been implemented for kvm */
> > +assert(ns_per_tick != 0L);
> > +#endif
> > +
> > +/*
> > + * Monotonic time begins at tsc_base (first read of TSC before
> > + * calibration).
> > + */
> > +cntvct_at_init = read_virtual_count();
> > +}
> > +
> > +void ukplat_time_fini(void)
> > +{
> > +/* TODO */
> > +}
> >
>
>
> [1]https://elixir.bootlin.com/linux/latest/source/Documentation/arm64/silicon-
> errata.txt
> [2] https://lkml.org/lkml/2017/4/7/369
>
>
> Thanks & Regards
> Sharan
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel