linux-kernel - Re: [PATCH v4 01/10] timekeeper: introduce extended clocksource reading callback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALAqxLW-DnZ0i7F2ve0=S-YYpPsZFAXf473ARV9z7ALq4eGocw@mail.gmail.com>
Date:   Wed, 2 Aug 2017 10:08:54 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Denis Plotnikov <dplotnikov@...tuozzo.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        kvm list <kvm@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        lkml <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>, rkagan@...tuozzo.com,
        den@...tuozzo.com
Subject: Re: [PATCH v4 01/10] timekeeper: introduce extended clocksource
 reading callback

On Wed, Aug 2, 2017 at 7:38 AM, Denis Plotnikov
<dplotnikov@...tuozzo.com> wrote:
> The callback provides extended information about just read
> clocksource value.
>
> It's going to be used in cases when detailed system information
> needed for further time related values calculation, e.g in KVM
> masterclock settings calculation.
>
> Signed-off-by: Denis Plotnikov <dplotnikov@...tuozzo.com>
> ---
>  include/linux/clocksource.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index a78cb18..786a522 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -48,7 +48,14 @@ struct module;
>   *                     400-499: Perfect
>   *                             The ideal clocksource. A must-use where
>   *                             available.
> - * @read:              returns a cycle value, passes clocksource as argument
> + * @read:              returns a cycle value (might be not quite cycles:
> + *                     see pvclock) passes clocksource as argument
> + * @read_with_stamp:   saves cycles value (got from timekeeper) and cycles
> + *                     stamp (got from hardware counter value and used by
> + *                     timekeeper to calculate the cycles value) to
> + *                     corresponding input pointers return true if cycles
> + *                     stamp holds real cycles and false if it holds some
> + *                     time derivative value
>   * @enable:            optional function to enable the clocksource
>   * @disable:           optional function to disable the clocksource
>   * @mask:              bitmask for two's complement
> @@ -78,6 +85,8 @@ struct module;
>   */
>  struct clocksource {
>         u64 (*read)(struct clocksource *cs);
> +       bool (*read_with_stamp)(struct clocksource *cs,
> +                               u64 *cycles, u64 *cycles_stamp);
>         u64 mask;

I'm not really fan of an interface that leaks magic data to users that
know enough.

And its not clear from this if the magic data is standardized or
different clocksources export different data?

What exactly are the attributes you're trying to pull from the
lower-level hardware that you can't get otherwise (without using the
update_pvclock_gtod() since, if I'm understanding that apparently
gives you too much detail to deal with)?

thanks
-john