linux-kernel - Re: [RFC PATCH 1/5] x86: tsc: add tsc to art helpers

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]

Date:   Thu, 15 Aug 2019 08:57:11 +0300
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org,
        "Christopher S . Hall" <christopher.s.hall@...el.com>
Subject: Re: [RFC PATCH 1/5] x86: tsc: add tsc to art helpers


Hi,


Thomas Gleixner <tglx@...utronix.de> writes:

> Felipe,
>
> On Tue, 16 Jul 2019, Felipe Balbi wrote:
>
> -ENOCHANGELOG
>
> As you said in the cover letter:
>
>>  (3) The change in arch/x86/kernel/tsc.c needs to be reviewed at length
>>      before going in.
>
> So some information what those interfaces are used for and why they are
> needed would be really helpful.

Okay, I have some more details about this. The TGPIO device itself uses
ART since TSC is not directly available to anything other than the
CPU. The 'problem' here is that reading ART incurs extra latency which
we would like to avoid. Therefore, we use TSC and scale it to
nanoseconds which, would be the same as ART to ns.

>> +void get_tsc_ns(struct system_counterval_t *tsc_counterval, u64 *tsc_ns)
>> +{
>> +	u64 tmp, res, rem;
>> +	u64 cycles;
>> +
>> +	tsc_counterval->cycles = clocksource_tsc.read(NULL);
>> +	cycles = tsc_counterval->cycles;
>> +	tsc_counterval->cs = art_related_clocksource;
>> +
>> +	rem = do_div(cycles, tsc_khz);
>> +
>> +	res = cycles * USEC_PER_SEC;
>> +	tmp = rem * USEC_PER_SEC;
>> +
>> +	do_div(tmp, tsc_khz);
>> +	res += tmp;
>> +
>> +	*tsc_ns = res;
>> +}
>> +EXPORT_SYMBOL(get_tsc_ns);
>> +
>> +u64 get_art_ns_now(void)
>> +{
>> +	struct system_counterval_t tsc_cycles;
>> +	u64 tsc_ns;
>> +
>> +	get_tsc_ns(&tsc_cycles, &tsc_ns);
>> +
>> +	return tsc_ns;
>> +}
>> +EXPORT_SYMBOL(get_art_ns_now);
>
> While the changes look innocuous I'm missing the big picture why this needs
> to emulate ART instead of simply using TSC directly.

i don't think we're emulating ART here (other than the name in the
function). We're just reading TSC and converting to nanoseconds, right?

Cheers

-- 
balbi