From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933170AbbCRJ4D (ORCPT ); Wed, 18 Mar 2015 05:56:03 -0400 Received: from cantor2.suse.de ([195.135.220.15]:38986 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755760AbbCRJzd (ORCPT ); Wed, 18 Mar 2015 05:55:33 -0400 Date: Wed, 18 Mar 2015 10:53:56 +0100 From: Borislav Petkov To: Quentin Casasnovas Cc: Oleg Nesterov , Dave Hansen , Ingo Molnar , Andy Lutomirski , Linus Torvalds , Pekka Riikonen , Rik van Riel , Suresh Siddha , LKML , "Yu, Fenghua" , "H. Peter Anvin" Subject: Re: [PATCH RFC 0/2] x86/fpu: avoid "xstate_fault" in xsave_user/xrestore_user Message-ID: <20150318095356.GC3411@pd.tnic> References: <54F74F59.5070107@intel.com> <20150315164948.GA28149@redhat.com> <20150316223743.GA14575@chrystal.uk.oracle.com> <20150317094750.GD18917@pd.tnic> <20150317100046.GA19131@chrystal.uk.oracle.com> <20150317112014.GG18917@pd.tnic> <20150317113658.GC19131@chrystal.uk.oracle.com> <20150317120739.GH18917@pd.tnic> <20150318090632.GF19131@chrystal.uk.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150318090632.GF19131@chrystal.uk.oracle.com> 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, Mar 18, 2015 at 10:06:32AM +0100, Quentin Casasnovas wrote: > What if we renamed it to check_asm()/check_user_asm() and have the first > argument be a string, like an asm statement? So basically check_asm() > would be exactly like an asm() statement except that it'll use a comma to > separate the input, output and clobber operands instead of a colon, and > would protect the first instruction of the assembler template. > > if (config_enabled(CONFIG_X86_32)) > return check_user_asm("fxrstor %[fx]", [fx] "=m" (*fx),,); > > Then we can move that macro up the headers so it can be used elsewhere. Actually, I don't like the variable arguments thing and am not sure at all that there's a wide need for a check* thing across the tree. Maybe there is but I haven't seen it yet. So I'd much prefer macros of the sort: fxsave() xsave() xsaves() xrstor() ... (no need for the "check" thing) which are self contained and get passed the needed operands. I.e., fxsave(fx) and fx is "struct i387_fxsave_struct __user *fx". We can wrap it in inline functions for arguments checking too. Also: xsave(state, lmask, hmask) and the macro definition does the exception table thing. And we can have a lower level __save_state() macro which is getting called by all those so that we can save us the code duplication. This is much cleaner IMO than the check_insn() things. > Readability will be a tough one since gcc extended asm isn't readable > (IMO) and we need to deal with the input/output/clobber operands > syntax. That's why I'm saying we wrap all that inline asm syntax in macros and not pass inline asm-like but not really arguments to our macros. > I do agree with all your above points, which is why I drafted that > proposal rework of check_insn() in my first e-mail :) AFAICT, you were > giving arguments against the current macros, not against my previous > proposal. All I'm saying is, it should be done cleanly instead of improving an already not so optimal design. Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --