From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753452AbbENJgi (ORCPT ); Thu, 14 May 2015 05:36:38 -0400 Received: from mail.skyhub.de ([78.46.96.112]:54756 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753155AbbENJgg (ORCPT ); Thu, 14 May 2015 05:36:36 -0400 Date: Thu, 14 May 2015 11:36:30 +0200 From: Borislav Petkov To: Linus Torvalds Cc: Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Andy Lutomirski , Denys Vlasenko , lkml Subject: Re: [RFC PATCH] Drop some asm from copy_user_64.S Message-ID: <20150514093630.GA29125@pd.tnic> References: <20150512205750.GJ3497@pd.tnic> <20150512215320.GK3497@pd.tnic> <20150513095248.GD1517@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 13, 2015 at 09:02:41AM -0700, Linus Torvalds wrote: > The nice thing about using "rep movsb" for the user copy is that not > only is it fairly close to optimal (for non-constant sizes) on newer > Intel CPU's, but the fixup is also trivial. So we really should inline > it. Just look at it: the copy_user_enhanced_fast_string function is > literally just three 2-byte instructions right now: > > mov %edx,%ecx > rep movsb > xor %eax,%eax > > and the rest is just the exception table thing. Yeah, so I thought about it for a while and yeah, those labels there would be a problem. Because you have this: mov %edx,%ecx 1: rep movsb xor %eax,%eax and _ASM_EXTABLE adds the .fixup section entry in the form of relative offsets. So I *think* it would work if we make the REP;STOSB case the default one, i.e. those insns get issued during build. Then the labels will be fine and all is good. When they have to get patched probably with a CALL to the other variants, the label 1: above (or rather the fixup entry) will point to the newly patched instruction which, if it faults, might get fixed up erroneously. Hmm, let me give it a try - I'll have a better idea after I've done it. > (And yes, there's the STAC/CLAC thing around it, but I think that > should just be moved into _copy_from/to_user() too, since *all* of the > copy_user_generic() cases need it). Yeah. > Yeah, yeah, we'd still do the double call thing for the more complex > cases of the unrolled copy loop or the "movsq + tail" cases, but those > are at least big enough that it makes sense. And they are presumably > getting less common anyway. Right, so we can avoid the first CALL if I inline copy_user_generic() which practically inlines the alternative directly. Lemme play with it a little... Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --