[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 3/3] plat/kvm: Add KVM (x86_64) timer support


  • To: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>, minios-devel@xxxxxxxxxxxxx
  • From: Costin Lupu <costin.lupu@xxxxxxxxx>
  • Date: Tue, 1 May 2018 02:54:30 +0300
  • Delivery-date: Mon, 30 Apr 2018 23:54:45 +0000
  • Ironport-phdr: 9a23:Y62LwBMMJumBGLLQxTEl6mtUPXoX/o7sNwtQ0KIMzox0I//zrarrMEGX3/hxlliBBdydt6ofzbKO+4nbGkU4qa6bt34DdJEeHzQksu4x2zIaPcieFEfgJ+TrZSFpVO5LVVti4m3peRMNQJW2aFLduGC94iAPERvjKwV1Ov71GonPhMiryuy+4ZLebxlGiTanfb9+MAi9oBnMuMURnYZsMLs6xAHTontPdeRWxGdoKkyWkh3h+Mq+/4Nt/jpJtf45+MFOTav1f6IjTbxFFzsmKHw65NfqtRbYUwSC4GYXX3gMnRpJBwjF6wz6Xov0vyDnuOdxxDWWMMvrRr0vRz+s87lkRwPpiCcfNj427mfXitBrjKlGpB6tvgFzz5LIbI2QMvd1Y6HTcs4ARWdZQ8hfSSJBDIO/YYUBAeUOMuRXoJXyqVYVsRu+HBOhCP/zxjJGhHL727Ax3eQ7EQHB2QwtB8wDsHPPrNXpNacSV/2+wq/VzTXbcvNdxDDw55TPchA6vfGMXLRwfdDVyUkyDwPFk06dppD+Pz+PzuQNrnOU4/B6VeKokmMqrRx6rDaoxscpkIbJh4QVx0jL9CpnxoY1Pce4SEl5YdG6DJRQqzuWN4xsQsMtRWxjpSU0yqUetJKmcyUG1Y4ryh3fZvCdbYSE/BDuWPyfLDtgmX5oe7Gyiwys/UWg0OHwS8u53VZQoiZYk9TBsG0G2QbJ5cidUPR9+1+s2TOI1w/O9O5JOVs0la/HK545xb4wi4YTvVzDHiDonEX2i7ebdlsh+uey6uTnZq/qqYOHN4NukgH+L78hltalAeQ/KgQOXm6b9vqg1LD74EH0T6hGguc1n6TZqpzWO9oXq6yjDwJbyooj7gywDzai0NQWh3kHK1dFdQqbgIjuIFHOPPH4DfGlj1SojTdr3+3GM6b9DZXWNHTDiKrhcq1n505Gzwo/1cpf6I5MCrEdPPLzXVf8tNnZDh8/Mgy0xP3nBMxg2YwAR2KAHKuZPbjWsV+J/eIvP/KMaJUauTnjLfgp/fnujWU2mVUFZ6mmwYMXaGykHvRhO0iZenvsgtIGEWcMpAY+T/Hqh0OEUT9SeXmyRbkx5jclB426CYfMXJuijKaf0yemTdVqYTV9C1WLFz/LaoiCSfoWIHaJI8pmlHoHSLWnWYI7/RejvwvzwfxqM7yQsigZs5Pkz5156vPekTk29CdoFIKN3mfLSHt7zU0SQDpj96dkvU17gnOeybUw1/dfDsBS4bVNTx8nHZXHifRnAZboXVSSLZ+yVF+6T4D+UnkKRdUrzopLOh4lFg==
  • List-id: Mini-os development list <minios-devel.lists.xenproject.org>

On 04/30/2018 03:44 PM, Simon Kuenzer wrote:
> See my comments inline.
> 
> On 05.04.2018 17:21, Costin Lupu wrote:
>> We are using TSC clock as main timer on KVM.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>> ---
>>   plat/kvm/Makefile.uk              |   3 +
>>   plat/kvm/clock_subr.c             | 226 ++++++++++++++++++++++++
>>   plat/kvm/include/kvm/clock_subr.h |  83 +++++++++
>>   plat/kvm/include/kvm/tscclock.h   |  42 +++++
>>   plat/kvm/irq.c                    |  10 ++
>>   plat/kvm/time.c                   |  62 +++++++
>>   plat/kvm/tscclock.c               | 356
>> ++++++++++++++++++++++++++++++++++++++
>>   7 files changed, 782 insertions(+)
>>   create mode 100644 plat/kvm/clock_subr.c
>>   create mode 100644 plat/kvm/include/kvm/clock_subr.h
>>   create mode 100644 plat/kvm/include/kvm/tscclock.h
>>   create mode 100644 plat/kvm/time.c
>>   create mode 100644 plat/kvm/tscclock.c
>>
>> diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
>> index 46258ff..76c40f1 100644
>> --- a/plat/kvm/Makefile.uk
>> +++ b/plat/kvm/Makefile.uk
>> @@ -34,4 +34,7 @@ LIBKVMPLAT_SRCS-$(ARCH_X86_64) +=
>> $(LIBKVMPLAT_BASE)/x86/intctrl.c
>>   LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/shutdown.c
>>   LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/memory.c
>>   LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/irq.c
>> +LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/time.c
>> +LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/tscclock.c
>> +LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/clock_subr.c
>>   LIBKVMPLAT_SRCS-y              += $(UK_PLAT_COMMON_BASE)/lcpu.c|common
>> diff --git a/plat/kvm/clock_subr.c b/plat/kvm/clock_subr.c
>> new file mode 100644
>> index 0000000..6d7388b
>> --- /dev/null
>> +++ b/plat/kvm/clock_subr.c
>> @@ -0,0 +1,226 @@
>> +/* SPDX-License-Identifier: ISC */
> 
> I think this should be
> /* SPDX-License-Identifier: ISC AND BSD-2-Clause-NetBSD AND BSD-3-Clause */

Right.

>> +/*
>> + * Authors: Martin Lucina
>> + *          Ricardo Koller
>> + *          Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2015-2017 IBM
>> + * Copyright (c) 2016-2017 Docker, Inc.
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> + * for any purpose with or without fee is hereby granted, provided
>> + * that the above copyright notice and this permission notice appear
>> + * in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
>> + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
>> + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
>> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
>> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +/* Taken from solo5 clock_subr.c */
>> +
>> +/*    $NetBSD: clock_subr.c,v 1.26 2014/12/22 18:09:20 christos Exp
>> $    */
>> +
>> +/*-
>> + * Copyright (c) 1996 The NetBSD Foundation, Inc.
>> + * All rights reserved.
>> + *
>> + * This code is derived from software contributed to The NetBSD
>> Foundation
>> + * by Gordon W. Ross
>> + *
>> + * 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.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. 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 FOUNDATION 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.
>> + */
>> +
>> +/*
>> + * Copyright (c) 1988 University of Utah.
>> + * Copyright (c) 1982, 1990, 1993
>> + *    The Regents of the University of California.  All rights reserved.
>> + *
>> + * This code is derived from software contributed to Berkeley by
>> + * the Systems Programming Group of the University of Utah Computer
>> + * Science Department.
>> + *
>> + * 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 University 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 REGENTS 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 REGENTS 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.
>> + *
>> + * from: Utah $Hdr: clock.c 1.18 91/01/21$
>> + *
>> + *    @(#)clock.c    8.2 (Berkeley) 1/12/94
>> + */
>> +
>> +#include <kvm/clock_subr.h>
>> +
>> +/* Some handy constants. */
>> +#define SECS_PER_MINUTE         60
>> +#define SECS_PER_HOUR           3600
>> +#define SECS_PER_DAY            86400
>> +#define DAYS_PER_COMMON_YEAR    365
>> +#define DAYS_PER_LEAP_YEAR      366
>> +#define SECS_PER_COMMON_YEAR    (SECS_PER_DAY * DAYS_PER_COMMON_YEAR)
>> +#define SECS_PER_LEAP_YEAR      (SECS_PER_DAY * DAYS_PER_LEAP_YEAR)
>> +
>> +/* Traditional POSIX base year */
>> +#define    POSIX_BASE_YEAR    1970
>> +
>> +/* Some handy functions */
>> +static int days_in_month(int m)
>> +{
>> +    switch (m) {
>> +    case 2:
>> +        return 28;
>> +    case 4: case 6: case 9: case 11:
>> +        return 30;
>> +    case 1: case 3: case 5: case 7: case 8: case 10: case 12:
>> +        return 31;
>> +    default:
>> +        return -1;
>> +    }
>> +}
>> +
>> +/*
>> + * This inline avoids some unnecessary modulo operations
>> + * as compared with the usual macro:
>> + *   ( ((year % 4) == 0 &&
>> + *      (year % 100) != 0) ||
>> + *     ((year % 400) == 0) )
>> + * It is otherwise equivalent.
>> + */
>> +static int is_leap_year(__u64 year)
>> +{
>> +    if ((year & 3) != 0)
>> +        return 0;
>> +
>> +    if ((year % 100) != 0)
>> +        return 1;
>> +
>> +    return (year % 400) == 0;
>> +}
>> +
>> +static int days_per_year(__u64 year)
>> +{
>> +    return is_leap_year(year) ? DAYS_PER_LEAP_YEAR :
>> DAYS_PER_COMMON_YEAR;
>> +}
>> +
>> +/*
>> + * Generic routines to convert between a POSIX date
>> + * (seconds since 1/1/1970) and yr/mo/day/hr/min/sec
>> + * Derived from arch/hp300/hp300/clock.c
>> + */
>> +
>> +#define FEBRUARY  2
>> +
>> +/* for easier alignment:
>> + * time from the epoch to 2000 (there were 7 leap years):
>> + */
>> +#define    DAYSTO2000         (365 * 30 + 7)
>> +
>> +/* 4 year intervals include 1 leap year */
>> +#define    DAYS4YEARS         (365 * 4 + 1)
>> +
>> +/* 100 year intervals include 24 leap years */
>> +#define    DAYS100YEARS       (365 * 100 + 24)
>> +
>> +/* 400 year intervals include 97 leap years */
>> +#define    DAYS400YEARS       (365 * 400 + 97)
>> +
> 
> In general, we should revisit the naming of functions that are
> non-static. In order to avoid naming conflicts with application code
> later, I would prefer adding the prefix _libkvmplat_ in order to mark
> those functions as library-internal. Since, these are new files, can you
> call it _libkvmplat_clock_ymdhms_to_secs()? You may also adopt this
> scheme to the other functions that are non-static and not part of the
> ukplat API.
> 
> I know that much more cleaning-up work of this sort has to be done but
> we should start somewhere. I think doing this with new files and
> functions is good start.
> 
> As we discussed off-line, I agree that we should introduce a design
> principle document that explains considerations for Unikraft and library
> APIs.

Actually this is an util function, not a KVM specific one. I'd prefer
moving it to a common area, which we don't have now and which should
contain util functions. Please advise where it should go in that case.

Now regarding the name conflicts with application code, it's kind of
expected to meet name conflicts at one point sooner or later. And this
would require a more elegant and generic approach, such as name
mangling, prefixing or whatever to alter the Unikraft function name.
What if I want in my application to have a function named
_libkvmplat_clock_ymdhms_to_secs? If you want to just keep prefixes for
all Unikraft functions maybe we should get rid of the 80 characters line
length limit.
>> +__u64 clock_ymdhms_to_secs(struct bmk_clock_ymdhms *dt)
>> +{
>> +    __u64 secs, i, year, days;
>> +
>> +    year = dt->dt_year;
>> +
>> +    /*
>> +     * Compute days since start of time
>> +     * First from years, then from months.
>> +     */
>> +    if (year < POSIX_BASE_YEAR)
>> +        return 0;
>> +
>> +    days = 0;
>> +    if (is_leap_year(year) && dt->dt_mon > FEBRUARY)
>> +        days++;
>> +
>> +    if (year < 2000) {
>> +        /* simple way for early years */
>> +        for (i = POSIX_BASE_YEAR; i < year; i++)
>> +            days += days_per_year(i);
>> +
>> +    } else {
>> +        /* years are properly aligned */
>> +        days += DAYSTO2000;
>> +        year -= 2000;
>> +
>> +        i = year / 400;
>> +        days += i * DAYS400YEARS;
>> +        year -= i * 400;
>> +
>> +        i = year / 100;
>> +        days += i * DAYS100YEARS;
>> +        year -= i * 100;
>> +
>> +        i = year / 4;
>> +        days += i * DAYS4YEARS;
>> +        year -= i * 4;
>> +
>> +        for (i = dt->dt_year - year; i < dt->dt_year; i++)
>> +            days += days_per_year(i);
>> +    }
>> +
>> +    /* Months */
>> +    for (i = 1; i < dt->dt_mon; i++)
>> +        days += days_in_month(i);
>> +    days += (dt->dt_day - 1);
>> +
>> +    /* Add hours, minutes, seconds. */
>> +    secs = (((__u64) days
>> +        * 24 + dt->dt_hour)
>> +        * 60 + dt->dt_min)
>> +        * 60 + dt->dt_sec;
>> +
>> +    return secs;
>> +}
>> diff --git a/plat/kvm/include/kvm/clock_subr.h
>> b/plat/kvm/include/kvm/clock_subr.h
>> new file mode 100644
>> index 0000000..66f829b
>> --- /dev/null
>> +++ b/plat/kvm/include/kvm/clock_subr.h
>> @@ -0,0 +1,83 @@
> 
> /* SPDX-License-Identifier: ISC AND BSD-2-Clause-NetBSD */

Right.

>> +/* SPDX-License-Identifier: ISC */
>> +/*
>> + * Authors: Martin Lucina
>> + *          Ricardo Koller
>> + *          Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2015-2017 IBM
>> + * Copyright (c) 2016-2017 Docker, Inc.
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> + * for any purpose with or without fee is hereby granted, provided
>> + * that the above copyright notice and this permission notice appear
>> + * in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
>> + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
>> + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
>> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
>> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +/* Taken from solo5 clock_subr.h */
>> +
>> +/*-
>> + * Copyright (c) 1996 The NetBSD Foundation, Inc.
>> + * All rights reserved.
>> + *
>> + * This code is derived from software contributed to The NetBSD
>> Foundation
>> + * by Gordon W. Ross
>> + *
>> + * 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.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. 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 FOUNDATION 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.
>> + */
>> +
>> +#ifndef _BMK_CLOCK_SUBR_H_
>> +#define _BMK_CLOCK_SUBR_H_
>> +
>> +#include <uk/arch/types.h>
>> +
>> +/*
>> + * "POSIX time" to/from "YY/MM/DD/hh/mm/ss"
>> + */
>> +struct bmk_clock_ymdhms {
>> +    __u64 dt_year;
>> +    __u8 dt_mon;
>> +    __u8 dt_day;
>> +    __u8 dt_hour;
>> +    __u8 dt_min;
>> +    __u8 dt_sec;
>> +};
>> +
>> +__u64 clock_ymdhms_to_secs(struct bmk_clock_ymdhms *dt);
>> +
>> +/*
>> + * BCD to binary.
>> + */
>> +static inline unsigned int bcdtobin(unsigned int bcd)
>> +{
>> +    return ((bcd >> 4) & 0x0f) * 10 + (bcd & 0x0f);
>> +}
>> +
>> +#endif /* _BMK_CLOCK_SUBR_H_ */
>> diff --git a/plat/kvm/include/kvm/tscclock.h
>> b/plat/kvm/include/kvm/tscclock.h
>> new file mode 100644
>> index 0000000..27d0e02
>> --- /dev/null
>> +++ b/plat/kvm/include/kvm/tscclock.h
>> @@ -0,0 +1,42 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. 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.
>> + */
>> +
>> +#ifndef __KVM_TSCCLOCK_H__
>> +#define __KVM_TSCCLOCK_H__
>> +
> 
> Same prefixing here? "_libkvmplat_" for kvmplat internal, or
> "libkvmplat_" for KVM plat only provided functionality.

It's not clear for me yet if this is only KVM specific. AFAIK it can be
used on bare-metal as well. When that will be available, I expect to be
moved in a common area.

>> +int tscclock_init(void);
>> +__u64 tscclock_monotonic(void);
>> +__u64 tscclock_epochoffset(void);
>> +
>> +#endif /* __KVM_TSCCLOCK_H__ */
>> diff --git a/plat/kvm/irq.c b/plat/kvm/irq.c
>> index 55f8e67..a3b2121 100644
>> --- a/plat/kvm/irq.c
>> +++ b/plat/kvm/irq.c
>> @@ -68,12 +68,22 @@ void irq_register(unsigned long irq,
>> irq_handler_func_t func, void *arg)
>>       intctrl_clear_irq(irq);
>>   }
>>   +/*
>> + * TODO This is a temporary solution used to identify non TSC clock
>> + * interrupts in order to stop waiting for interrupts with deadline.
>> + */
>> +extern long nontsc_interrupt_assert;
>> +
>>   void irq_handle(unsigned long irq)
>>   {
>>       struct irq_handler *h;
>>       int handled = 0;
>>         UK_SLIST_FOREACH(h, &irq_handlers[irq], entries) {
>> +        /* TODO define platform wise macro for timer IRQ number */
>> +        if (irq != 0)
>> +            nontsc_interrupt_assert = 1;
>> +
>>           if (h->func(h->arg) == 1) {
>>               handled = 1;
>>               break;
>> diff --git a/plat/kvm/time.c b/plat/kvm/time.c
>> new file mode 100644
>> index 0000000..1276997
>> --- /dev/null
>> +++ b/plat/kvm/time.c
>> @@ -0,0 +1,62 @@
>> +/* SPDX-License-Identifier: ISC */
>> +/*
>> + * Authors: Dan Williams
>> + *          Martin Lucina
>> + *          Ricardo Koller
>> + *          Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2015-2017 IBM
>> + * Copyright (c) 2016-2017 Docker, Inc.
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> + * for any purpose with or without fee is hereby granted, provided
>> + * that the above copyright notice and this permission notice appear
>> + * in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
>> + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
>> + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
>> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
>> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +/* Taken from solo5 time.c */
>> +
>> +#include <stdlib.h>
>> +#include <uk/plat/time.h>
>> +#include <kvm/irq.h>
>> +#include <kvm/tscclock.h>
>> +#include <uk/assert.h>
>> +
>> +
>> +/* return ns since time_init() */
>> +__nsec ukplat_monotonic_clock(void)
>> +{
>> +    return tscclock_monotonic();
>> +}
>> +
>> +/* return wall time in nsecs */
>> +__nsec ukplat_clock_wall(void)
>> +{
>> +    return tscclock_monotonic() + tscclock_epochoffset();
>> +}
>> +
>> +static int timer_handler(void *arg __unused)
>> +{
>> +    /* Yes, we handled the irq. */
>> +    return 1;
>> +}
>> +
>> +/* must be called before interrupts are enabled */
>> +void ukplat_time_init(void)
>> +{
>> +    int rc;
>> +
>> +    irq_register(0, timer_handler, NULL);
>> +
>> +    rc = tscclock_init();
>> +    UK_ASSERT(rc == 0);
>> +}
>> diff --git a/plat/kvm/tscclock.c b/plat/kvm/tscclock.c
>> new file mode 100644
>> index 0000000..1199f18
>> --- /dev/null
>> +++ b/plat/kvm/tscclock.c
>> @@ -0,0 +1,356 @@
>> +/* SPDX-License-Identifier: ISC */
>> +/*
>> + * Authors: Dan Williams
>> + *          Martin Lucina
>> + *          Ricardo Koller
>> + *          Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2015-2017 IBM
>> + * Copyright (c) 2016-2017 Docker, Inc.
>> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software
>> + * for any purpose with or without fee is hereby granted, provided
>> + * that the above copyright notice and this permission notice appear
>> + * in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
>> + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
>> + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
>> + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
>> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
>> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +/* Taken from solo5 tscclock.c */
>> +
>> +/*-
>> + * Copyright (c) 2014, 2015 Antti Kantee.  All Rights Reserved.
>> + * Copyright (c) 2015 Martin Lucina.  All Rights Reserved.
>> + * Modified for solo5 by Ricardo Koller <kollerr@xxxxxxxxxx>
>> + *
>> + * 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.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``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 AUTHOR 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.
>> + */
>> +
>> +#include <uk/plat/lcpu.h>
>> +#include <uk/plat/time.h>
>> +#include <x86/cpu.h>
>> +#include <kvm/clock_subr.h>
>> +#include <uk/print.h>
>> +#include <uk/assert.h>
>> +
>> +#define NSEC_PER_SEC         1000000000ULL
>> +
>> +#define TIMER_CNTR           0x40
>> +#define TIMER_MODE           0x43
>> +#define TIMER_SEL0           0x00
>> +#define TIMER_LATCH          0x00
>> +#define TIMER_RATEGEN        0x04
>> +#define TIMER_ONESHOT        0x08
>> +#define TIMER_16BIT          0x30
>> +#define TIMER_HZ             1193182
>> +
>> +#define    RTC_COMMAND          0x70
>> +#define    RTC_DATA             0x71
>> +#define RTC_NMI_DISABLE      (1<<8)
>> +#define RTC_NMI_ENABLE       0
>> +#define    RTC_SEC              0x00
>> +#define    RTC_MIN              0x02
>> +#define    RTC_HOUR             0x04
>> +#define    RTC_DAY              0x07
>> +#define    RTC_MONTH            0x08
>> +#define    RTC_YEAR             0x09
>> +#define    RTC_STATUS_A         0x0a
>> +#define    RTC_UIP              (1<<7)
>> +
>> +/* RTC wall time offset at monotonic time base. */
>> +static __u64 rtc_epochoffset;
>> +
>> +/*
>> + * TSC clock specific.
>> + */
>> +
>> +/* Base time values at the last call to tscclock_monotonic(). */
>> +static __u64 time_base;
>> +static __u64 tsc_base;
>> +
>> +/* Multiplier for converting TSC ticks to nsecs. (0.32) fixed point. */
>> +static __u32 tsc_mult;
>> +
>> +/*
>> + * Multiplier for converting nsecs to PIT ticks. (1.32) fixed point.
>> + *
>> + * Calculated as:
>> + *
>> + *     f = NSEC_PER_SEC / TIMER_HZ   (0.31) fixed point.
>> + *     pit_mult = 1 / f              (1.32) fixed point.
>> + */
>> +static const __u32 pit_mult =
>> +    (1ULL << 63) / ((NSEC_PER_SEC << 31) / TIMER_HZ);
>> +
>> +
>> +/*
>> + * Read the current i8254 channel 0 tick count.
>> + */
>> +static unsigned int i8254_gettick(void)
>> +{
>> +    __u16 rdval;
>> +
>> +    outb(TIMER_MODE, TIMER_SEL0 | TIMER_LATCH);
>> +    rdval  = inb(TIMER_CNTR);
>> +    rdval |= (inb(TIMER_CNTR) << 8);
>> +    return rdval;
>> +}
>> +
>> +/*
>> + * Delay for approximately n microseconds using the i8254 channel 0
>> counter.
>> + * Timer must be programmed appropriately before calling this function.
>> + */
>> +static void i8254_delay(unsigned int n)
>> +{
>> +    unsigned int cur_tick, initial_tick;
>> +    int remaining;
>> +    const unsigned long timer_rval = TIMER_HZ / 100;
>> +
>> +    initial_tick = i8254_gettick();
>> +
>> +    remaining = (unsigned long long) n * TIMER_HZ / 1000000;
>> +
>> +    while (remaining > 1) {
>> +        cur_tick = i8254_gettick();
>> +        if (cur_tick > initial_tick)
>> +            remaining -= timer_rval - (cur_tick - initial_tick);
>> +        else
>> +            remaining -= initial_tick - cur_tick;
>> +        initial_tick = cur_tick;
>> +    }
>> +}
>> +
>> +/*
>> + * Read a RTC register. Due to PC platform braindead-ness also
>> disables NMI.
>> + */
>> +static inline __u8 rtc_read(__u8 reg)
>> +{
>> +    outb(RTC_COMMAND, reg | RTC_NMI_DISABLE);
>> +    return inb(RTC_DATA);
>> +}
>> +
>> +/*
>> + * Return current RTC time. Note that due to waiting for the update
>> cycle to
>> + * complete, this call may take some time.
>> + */
>> +static __u64 rtc_gettimeofday(void)
>> +{
>> +    struct bmk_clock_ymdhms dt;
>> +    unsigned long flags;
>> +
>> +    flags = ukplat_lcpu_save_irqf();
>> +
>> +    /*
>> +     * If RTC_UIP is down, we have at least 244us to obtain a
>> +     * consistent reading before an update can occur.
>> +     */
>> +    while (rtc_read(RTC_STATUS_A) & RTC_UIP)
>> +        continue;
>> +
>> +    dt.dt_sec = bcdtobin(rtc_read(RTC_SEC));
>> +    dt.dt_min = bcdtobin(rtc_read(RTC_MIN));
>> +    dt.dt_hour = bcdtobin(rtc_read(RTC_HOUR));
>> +    dt.dt_day = bcdtobin(rtc_read(RTC_DAY));
>> +    dt.dt_mon = bcdtobin(rtc_read(RTC_MONTH));
>> +    dt.dt_year = bcdtobin(rtc_read(RTC_YEAR)) + 2000;
>> +
>> +    ukplat_lcpu_restore_irqf(flags);
>> +
>> +    return ukarch_time_sec_to_nsec(clock_ymdhms_to_secs(&dt));
>> +}
>> +
>> +/*
>> + * Beturn monotonic time using TSC clock.
>> + */
>> +__u64 tscclock_monotonic(void)
>> +{
>> +    __u64 tsc_now, tsc_delta;
>> +
>> +    /*
>> +     * Update time_base (monotonic time) and tsc_base (TSC time).
>> +     */
>> +    tsc_now = rdtsc();
>> +    tsc_delta = tsc_now - tsc_base;
>> +    time_base += mul64_32(tsc_delta, tsc_mult);
>> +    tsc_base = tsc_now;
>> +
>> +    return time_base;
>> +}
>> +
>> +/*
>> + * Calibrate TSC and initialise TSC clock.
>> + */
>> +int tscclock_init(void)
>> +{
>> +    __u64 tsc_freq, rtc_boot;
>> +
>> +    /* Initialise i8254 timer channel 0 to mode 2 at 100 Hz */
>> +    outb(TIMER_MODE, TIMER_SEL0 | TIMER_RATEGEN | TIMER_16BIT);
>> +    outb(TIMER_CNTR, (TIMER_HZ / 100) & 0xff);
>> +    outb(TIMER_CNTR, (TIMER_HZ / 100) >> 8);
>> +
>> +    /*
>> +     * Read RTC "time at boot". This must be done just before
>> tsc_base is
>> +     * initialised in order to get a correct offset below.
>> +     */
>> +    rtc_boot = rtc_gettimeofday();
>> +
>> +    /*
>> +     * Calculate TSC frequency by calibrating against an 0.1s delay
>> +     * using the i8254 timer.
>> +     */
> 
> Wow, this is adds a 100ms boot delay to the Unikernels on KVM. Can you
> put an TODO comment for revisiting this later? Maybe we can find a
> different method to get the correct value for the TSC frequency.

Right.

>> +    tsc_base = rdtsc();
>> +    i8254_delay(100000);
>> +    tsc_freq = (rdtsc() - tsc_base) * 10;
>> +    uk_printd(DLVL_INFO,
>> +        "Clock source: TSC, frequency estimate is %llu Hz\n",
>> +        (unsigned long long) tsc_freq); //TODO
>> +
>> +    /*
>> +     * Calculate TSC scaling multiplier.
>> +     *
>> +     * (0.32) tsc_mult = NSEC_PER_SEC (32.32) / tsc_freq (32.0)
>> +     */
>> +    tsc_mult = (NSEC_PER_SEC << 32) / tsc_freq;
>> +
>> +    /*
>> +     * Monotonic time begins at tsc_base (first read of TSC before
>> +     * calibration).
>> +     */
>> +    time_base = mul64_32(tsc_base, tsc_mult);
>> +
>> +    /*
>> +     * Compute RTC epoch offset by subtracting monotonic time_base
>> from RTC
>> +     * time at boot.
>> +     */
>> +    rtc_epochoffset = rtc_boot - time_base;
>> +
>> +    /*
>> +     * Initialise i8254 timer channel 0 to mode 4 (one shot).
>> +     */
>> +    outb(TIMER_MODE, TIMER_SEL0 | TIMER_ONESHOT | TIMER_16BIT);
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * Return epoch offset (wall time offset to monotonic clock start).
>> + */
>> +__u64 tscclock_epochoffset(void)
>> +{
>> +    return rtc_epochoffset;
>> +}
>> +
>> +/*
>> + * Minimum delta to sleep using PIT. Programming seems to have an
>> overhead of
>> + * 3-4us, but play it safe here.
>> + */
>> +#define PIT_MIN_DELTA    16
>> +
>> +/*
>> + * Returns early if any interrupts are serviced, or if the requested
>> delay is
>> + * too short. Must be called with interrupts disabled, will enable
>> interrupts
>> + * "atomically" during idle loop.
>> + */
>> +static void tscclock_cpu_block(__u64 until)
>> +{
>> +    __u64 now, delta_ns;
>> +    __u64 delta_ticks;
>> +    unsigned int ticks;
>> +
>> +    UK_ASSERT(ukplat_lcpu_irqs_disabled());
>> +
>> +    now = ukplat_monotonic_clock();
>> +
>> +    /*
>> +     * Compute delta in PIT ticks. Return if it is less than minimum
>> safe
>> +     * amount of ticks.  Essentially this will cause us to spin until
>> +     * the timeout.
>> +     */
>> +    delta_ns = until - now;
>> +    delta_ticks = mul64_32(delta_ns, pit_mult);
>> +    if (delta_ticks < PIT_MIN_DELTA) {
>> +        /*
>> +         * Since we are "spinning", quickly enable interrupts in
>> +         * the hopes that we might get new work and can do something
>> +         * else than spin.
>> +         */
>> +        ukplat_lcpu_enable_irq();
>> +        nop(); /* ints are enabled 1 instr after sti */
>> +        ukplat_lcpu_disable_irq();
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * Program the timer to interrupt the CPU after the delay has
>> expired.
>> +     * Maximum timer delay is 65535 ticks.
>> +     */
>> +    if (delta_ticks > 65535)
>> +        ticks = 65535;
>> +    else
>> +        ticks = delta_ticks;
>> +
>> +    /*
>> +     * Note that according to the Intel 82C54 datasheet, p12 the
>> +     * interrupt is actually delivered in N + 1 ticks.
>> +     */
>> +    ticks -= 1;
>> +    outb(TIMER_CNTR, ticks & 0xff);
>> +    outb(TIMER_CNTR, ticks >> 8);
>> +
>> +    /*
>> +     * Wait for any interrupt. If we got an interrupt then
>> +     * just return into the scheduler which will check if there is
>> +     * work to do and send us back here if not.
>> +     *
>> +     * TODO: It would be more efficient for longer sleeps to be
>> +     * able to distinguish if the interrupt was the PIT interrupt
>> +     * and no other, but this will do for now.
>> +     */
>> +    ukplat_lcpu_halt_irq();
>> +}
>> +
>> +long nontsc_interrupt_assert;
>> +
>> +void time_block_until(__snsec until)
>> +{
>> +    volatile long *pnontsc_interrupt_assert = &nontsc_interrupt_assert;
>> +
>> +    while ((__snsec) ukplat_monotonic_clock() < until) {
>> +        tscclock_cpu_block(until);
>> +
>> +        /* who triggered the interrupt? */
>> +        if (*pnontsc_interrupt_assert) {
>> +            /* it was another device, stop blocking */
>> +            nontsc_interrupt_assert = 0;
>> +            break;
>> +        }
>> +        /* it was us */
>> +    }
>> +}
>>
> 
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.