linux-kernel - Re: [RFC 2/2] x86, tsc: Enable clock for ealry printk timestamp

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: <CAHp75Vcii2=TDxBWeOve05peyiYiPeMu_ocBvj5SJ8UB+94eNQ@mail.gmail.com>
Date:   Wed, 30 May 2018 16:25:07 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Feng Tang <feng.tang@...el.com>
Cc:     Petr Mladek <pmladek@...e.com>, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Peter Zijlstra <peterz@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        alek.du@...el.com
Subject: Re: [RFC 2/2] x86, tsc: Enable clock for ealry printk timestamp

On Wed, May 30, 2018 at 12:20 PM, Feng Tang <feng.tang@...el.com> wrote:
> To show time info in kernel log earlier and help optimizing kernel
> boot time, printk adds a debug hook "boot_printk_clock_fn()"  for
> capable platform which has accurate clock in early boot phase.
>
> This patch will add early param setup option, so that user can
> chose to provide a tsc based early printk clock simply by adding
> in command line: "boot_tsc=xxxxM" (xxxxM is the stable TSC freq).

> +

This line is not needed.

> +       cur_tsc = rdtsc();
> +       cur_tsc -= boot_tsc_offset;

In one expression?

> +       if (!p)
> +               return -EINVAL;
> +

I'm not sure it's needed at all.

> +       boot_tsc_mhz = div64_u64(tsc_hz, 1024 * 1024);

Hmm... 1024*1024 != 1000 * 1000. So, hz -> mhz here (as by suffixes)
looks weird.

> +       if (boot_tsc_mhz == 0)
> +               return -EINVAL;

> +       pr_info("TSC has run for %lld us\n",
> +               div64_u64(boot_tsc_offset, boot_tsc_mhz));
> +
> +       /* Setup the early printk clock */
> +       boot_printk_clock_fn = boot_tsc_clock;

> +       pr_info("TSC: Setup early printk timestamp with %lldM TSC.",
> +               boot_tsc_mhz);

Perhaps remove period (above doesn't have it) and move this to one line?

-- 
With Best Regards,
Andy Shevchenko