[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 07.03.18 at 16:51, <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)

The construct is right, but the comment still has the old directive name.

> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -207,7 +207,8 @@ void init_or_livepatch apply_alternatives(struct 
> alt_instr *start,
>              base->priv = 1;
>  
>              /* Nothing useful to do? */
> -            if ( a->pad_len <= 1 )
> +            if ( (TOOLCHAIN_P6_NOPS && ideal_nops == p6_nops) ||
> +                 a->pad_len <= 1 )
>                  continue;

I'm sorry, but no - we can't assume all gas versions going forward
will continue to produce the NOPs we want. They may change at
any time, and we may change our mind at any time. If anything
you'd need to actively check that what their .nops produces
matches what our table has.

Additionally such skipping on the vast majority of hardware is
prone to leave bugs undiscovered for quite some time.

Anyway - I continue to fail to see the value of this patch with the
immediately preceding one already doing all we need.

An alternative might be to have a Kconfig option to suppress the
NOP optimization altogether, and rely on what the tool chain
produces.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.