From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752249AbbDDHhC (ORCPT ); Sat, 4 Apr 2015 03:37:02 -0400 Received: from mail.skyhub.de ([78.46.96.112]:58626 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751733AbbDDHhA (ORCPT ); Sat, 4 Apr 2015 03:37:00 -0400 Date: Sat, 4 Apr 2015 09:34:54 +0200 From: Borislav Petkov To: Quentin Casasnovas Cc: X86 ML , LKML , "H. Peter Anvin" , Ingo Molnar , Thomas Gleixner , Oleg Nesterov , Andy Lutomirski Subject: Re: [PATCH] x86/xsave: Robustify and merge macros Message-ID: <20150404073454.GA21152@pd.tnic> References: <20150402155210.GB6703@chrystal.uk.oracle.com> <20150402161259.GE3483@pd.tnic> <20150403140630.GD14902@chrystal.uk.oracle.com> <20150403141426.GE14902@chrystal.uk.oracle.com> <20150403152324.GG3418@pd.tnic> <20150403154055.GF14902@chrystal.uk.oracle.com> <20150403170625.GJ3418@pd.tnic> <20150403173306.GG14902@chrystal.uk.oracle.com> <20150403174824.GL3418@pd.tnic> <20150403204217.GH14902@chrystal.uk.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150403204217.GH14902@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 Fri, Apr 03, 2015 at 10:42:17PM +0200, Quentin Casasnovas wrote: > If you're happy with the extra padding in such cases then your second > approach looks okay to me. But IMO, even if taking the '.if' directive > approach is certainly bigger LOC-wise, it should be much easier to review > in a rush than some other .skip trickery. .if needs absolute expressions and I can't get it to even compile with the experiments I've done so far. How about this instead? It basically computes the padding length by doing max(len(repl1), len(repl2)) - len(orig) and without conditionals. The macros all do string expansion so that the strings can get parsed by gas. Initial smoke testing in kvm seems to work, I need to test it on real metal: --- diff --git a/arch/x86/include/asm/alternative-asm.h b/arch/x86/include/asm/alternative-asm.h index 524bddce0b76..44a1fc5439d3 100644 --- a/arch/x86/include/asm/alternative-asm.h +++ b/arch/x86/include/asm/alternative-asm.h @@ -45,12 +45,22 @@ .popsection .endm +#define old_len 141b-140b +#define new_len1 144f-143f +#define new_len2 145f-144f + +/* + * Shamelessly stolen and adapted from: + * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax + */ +#define alt_max_short(a,b) (((a) - (((a) - (b)) & (((a) - (b)) >> 15))) & 0xffff) + .macro ALTERNATIVE_2 oldinstr, newinstr1, feature1, newinstr2, feature2 140: \oldinstr 141: - .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 - .skip -(((145f-144f)-(144f-143f)-(141b-140b)) > 0) * ((145f-144f)-(144f-143f)-(141b-140b)),0x90 + .skip -((alt_max_short(new_len1, new_len2) - old_len) > 0) * \ + (alt_max_short(new_len1, new_len2) - old_len),0x90 142: .pushsection .altinstructions,"a" diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 5aef6a97d80e..2c515ebcc767 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -96,13 +96,19 @@ static inline int alternatives_text_reserved(void *start, void *end) alt_end_marker ":\n" /* + * max without conditionals. Shamelessly stolen and adapted from: + * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax + */ +#define alt_max_short(a, b) "(((" a ") - (((" a ") - (" b ")) & (((" a ") - (" b ")) >> 15))) & 0xffff)" + +/* * Pad the second replacement alternative with additional NOPs if it is * additionally longer than the first replacement alternative. */ -#define OLDINSTR_2(oldinstr, num1, num2) \ - __OLDINSTR(oldinstr, num1) \ - ".skip -(((" alt_rlen(num2) ")-(" alt_rlen(num1) ")-(662b-661b)) > 0) * " \ - "((" alt_rlen(num2) ")-(" alt_rlen(num1) ")-(662b-661b)),0x90\n" \ +#define OLDINSTR_2(oldinstr, num1, num2) \ + "661:\n\t" oldinstr "\n662:\n" \ + ".skip -((" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")) > 0) * " \ + "(" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")), 0x90\n" \ alt_end_marker ":\n" #define ALTINSTR_ENTRY(feature, num) \ diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 5c993c94255e..7c4ad005d7a0 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -369,11 +369,11 @@ void __init_or_module apply_alternatives(struct alt_instr *start, continue; } - DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d)", + DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d), pad: %d", a->cpuid >> 5, a->cpuid & 0x1f, instr, a->instrlen, - replacement, a->replacementlen); + replacement, a->replacementlen, a->padlen); DUMP_BYTES(instr, a->instrlen, "%p: old_insn: ", instr); DUMP_BYTES(replacement, a->replacementlen, "%p: rpl_insn: ", replacement); -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --