Re: [Xen-devel] [PATCH v2] x86/alt: Support for automatic padding calculations

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel