On Mon, Feb 26, 2018 at 11:24:55AM +0000, Andrew Cooper wrote: > The correct amount of padding in an origin patch site can be calculated > automatically, based on the relative lengths of the replacements. > > This requires a bit of trickery to calculate correctly, especially in the > ALTENRATIVE_2 case where a branchless max() calculation in needed. The > calculation is further complicated because GAS's idea of true is -1 rather > than 1, which is why the extra negations are required. > > Additionally, have apply_alternatives() attempt to optimise the padding nops. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > --- > CC: Jan Beulich <jbeul...@suse.com> > CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > CC: Roger Pau Monné <roger....@citrix.com> > CC: Wei Liu <wei.l...@citrix.com> > > v2: Fix build with Clang. > --- > xen/arch/x86/Rules.mk | 4 +++ > xen/arch/x86/alternative.c | 32 ++++++++++++++++--- > xen/include/asm-x86/alternative-asm.h | 60 > +++++++++++++++++++++++++++++------ > xen/include/asm-x86/alternative.h | 46 +++++++++++++++++++++------ > 4 files changed, 120 insertions(+), 22 deletions(-) > > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index 9897dea..e169d67 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -24,6 +24,10 @@ $(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \ > -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \ > '-D__OBJECT_LABEL__=$(subst > $(BASEDIR)/,,$(CURDIR))/$$@') > > +# GCC's idea of true is -1. Clang's idea is 1
Nit: that should be GNU as rather than GCC itself? > #define alt_orig_len "(.LXEN%=_orig_e - .LXEN%=_orig_s)" > +#define alt_pad_len "(.LXEN%=_orig_p - .LXEN%=_orig_e)" > +#define alt_total_len "(.LXEN%=_orig_p - .LXEN%=_orig_s)" > #define alt_repl_s(num) ".LXEN%=_repl_s"#num > #define alt_repl_e(num) ".LXEN%=_repl_e"#num > #define alt_repl_len(num) "(" alt_repl_e(num) " - " alt_repl_s(num) ")" > > +/* GCC's idea of true is -1, while Clang's idea is 1. */ > +#ifdef HAVE_AS_NEGATIVE_TRUE > +# define AS_TRUE "-" > +#else > +# define AS_TRUE "" > +#endif > + > +#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE"(("a") < > ("b")))))" > + > +#define OLDINSTR_1(oldinstr, n1) \ > + ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t" \ > + ".LXEN%=_diff = "alt_repl_len(n1)"-"alt_orig_len"\n\t" \ > + ".skip "AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff, 0x90\n\t" \ > + ".LXEN%=_orig_p:\n\t" > + > +#define ALT_PADDING_LEN(n1, n2) \ > + as_max((alt_repl_len(n1), alt_repl_len(n2))"-"alt_orig_len > + > +#define OLDINSTR_2(oldinstr, n1, n2) \ > + ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t" \ > + ".LXEN%=_diff = "ALT_PADDING_LEN(n1, n2)"\n\t" \ > + ".skip "AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff, 0x90\n\t" \ > + ".LXEN%=_orig_p:\n\t" OLDINSTR_1 is mostly the same as OLDINSTR_2, I wonder whether: #define OLDINSTR(oldinstr, pad) \ ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t" \ ".LXEN%=_diff = " pad "\n\t" \ ".skip "AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff, 0x90\n\t" \ ".LXEN%=_orig_p:\n\t" and then: #define OLDINSTR_1(oldinstr, n1) \ OLDINSTR(oldinstr, alt_repl_len(n1)"-"alt_orig_len) #define OLDINSTR_2(oldinstr, n1, n2) \ OLDINSTR(oldinstr, ALT_PADDING_LEN(n1, n2)) Wouldn't work? That seems to avoid some code repetition. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel