Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.08.18 at 19:57, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid 
> (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
>  $(call as-option-add,CFLAGS,CC,\
>      ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
>  
> +# Check to see whether the assmbler supports the .nop directive.
> +$(call as-option-add,CFLAGS,CC,\
> +    ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE)

Can this please be HAVE_AS_NOPS_DIRECTIVE?

> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -84,6 +84,19 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1] 
> init_or_livepatch_cons
>  
>  static const unsigned char * const *ideal_nops init_or_livepatch_data = 
> p6_nops;
>  
> +#ifdef HAVE_AS_NOP_DIRECTIVE
> +
> +/* Nops in .init.rodata to compare against the runtime ideal nops. */
> +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t"
> +      "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t"
> +      ".popsection\n\t");

Any particular reason not to put them in .init.text?

> @@ -27,6 +26,14 @@ extern void add_nops(void *insns, unsigned int len);
>  extern void apply_alternatives(struct alt_instr *start, struct alt_instr 
> *end);
>  extern void alternative_instructions(void);
>  
> +asm ( ".macro mknops nr_bytes\n\t"
> +#ifdef HAVE_AS_NOP_DIRECTIVE
> +      ".nops \\nr_bytes, " __stringify(ASM_NOP_MAX) "\n\t"
> +#else
> +      ".skip \\nr_bytes, 0x90\n\t"
> +#endif
> +      ".endm\n\t" );

Did you take a look at "x86/alternatives: allow using assembler macros
in favor of C ones" yet? Perhaps this redundant macro definition could
be avoided that way? I'm not going to exclude though that some more
work might be needed to get there.

Anyway, I'm in principle fine with the change, irrespective of the other
discussion we've had, so with the name change and possibly the other
two remarks taken care of
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel