linux-kernel - Re: [patch] x86: convert rdtscll() to use __native_read

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: <49477452.6030307@goop.org>
Date:	Tue, 16 Dec 2008 01:26:42 -0800
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Ken Chen <kenchen@...gle.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [patch] x86: convert rdtscll() to use __native_read_tsc

Ingo Molnar wrote:
> The reason for the __native_read_tsc() / native_read_tsc() distinction is 
> and obscure problem with paravirt function pointers. Such constructs:
>
>   ./xen/enlighten.c:	.read_tsc = native_read_tsc,
>
> do not always work fine with all versions of gcc, if native_read_tsc() is 
> a simple static inline (as it should be) - the build would fail with 
> certain gcc flags.

I don't think that's true.  We rely on taking function pointers of 
static inlines pretty extensively; native_read_tsc is hardly unique in 
this respect.  I don't remember seeing any problems of the sort you 
describe.  (I can well believe this may have been a problem at some 
point, but not during the pv-ops development timeframe.)

> Perhaps the real fix is to do this rename as well:
>
>     native_read_tsc => native_read_tsc_paravirt
>   __native_read_tsc => native_read_tsc
>
> as this makes the native_read_tsc_paravirt() a pure technical variant, to 
> be used in paravirt_ops function pointer assignments. People would thus 
> just use the obvious native_read_tsc() inline function most of the time 
> and could forget about native_read_tsc_paravirt().
>
> Jeremy?
>   

I'm trying to remember the real reason for 
__native_read_tsc/native_read_tsc.  At least part of it is that 
__native_read_tsc is used in a vdso, and so *must* be inlined to avoid a 
bogus call from user to kernel space.  But I don't know why you wouldn't 
want to inline native_read_tsc everywhere.  I have a feeling it may be a 
relic from unification - possibly because x86-64 was late to the 
clocksource party - but I don't remember anything specific.

I think we can probably make do with a single native_read_tsc, so long 
as its always inlined.

    J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/