Re: [Xen-devel] [PATCH] libx86: Work around GCC bug with ebx output constrants

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.11.18 at 14:11, <andrew.cooper3@xxxxxxxxxx> wrote:
> Some versions of GCC can't compile cpuid.c, and fail with the rather cryptic:
> 
>   In file included from lib/x86/cpuid.c:3:0:
>   lib/x86/cpuid.c: In function ‘x86_cpuid_policy_fill_native’:
>   include/xen/lib/x86/cpuid.h:25:5: error: inconsistent operand constraints 
> in an ‘asm’
>        asm ( "cpuid"
>        ^

And indeed the version I've seen this with (4.3; I wrongly said "newer"
on irc) does slightly better: "can't find a register in class 'BREG' while
reloading 'asm'" (followed again by an "impossible constraint" one).

> In practice, this is a collision between the output constraint and the GOT
> which is held in %ebx when compiling with -fPIC for libraries.
> 
> This affects at least GCC 4.9 as shipped in Debian Jessie, but experimentally
> is fixed in GCC 6 and later.  Curiously, it only affects 32-bit builds.

I don't think there's anything curious here: The GOT (or actually
.rodata here) gets accessed by %rip-relative addressing in small
model 64-bit code, iirc.

> --- a/xen/include/xen/lib/x86/cpuid.h
> +++ b/xen/include/xen/lib/x86/cpuid.h
> @@ -20,21 +20,48 @@ struct cpuid_leaf
>      uint32_t a, b, c, d;
>  };
>  
> +/*
> + * Some versions of GCC are unable to cope with preserving the GOT (held in
> + * %ebx) around an asm() with an %ebx output constraint, and produce a rather
> + * cryptic error:
> + *    error: inconsistent operand constraints in an ‘asm’
> + *
> + * Experimentally, 64-bit builds work correctly, and 32-bit builds on GCC 6 
> or
> + * later work correctly.
> + *
> + * To work around the issue, use a separate register to hold the the ebx
> + * output, and xchg twice to leave %ebx preserved around the asm() statement.
> + */
> +#if __PIC__ && __i386__ && __GNUC__ < 6 && !__clang__

I think you're missing a bunch of defined() here.

> +#define XCHG_BX "xchg %%ebx, %k[bx];"

I don't think the k modifier is actually needed?

> +#define BX_CON [bx] "=&r"
> +#else
> +#define XCHG_BX ""
> +#define BX_CON "=b"
> +#endif
> +
>  static inline void cpuid_leaf(uint32_t leaf, struct cpuid_leaf *l)
>  {
> -    asm ( "cpuid"
> -          : "=a" (l->a), "=b" (l->b), "=c" (l->c), "=d" (l->d)
> +    asm ( XCHG_BX
> +          "cpuid;"
> +          XCHG_BX
> +          : "=a" (l->a), BX_CON (l->b), "=c" (l->c), "=d" (l->d)

Strictly speaking all other outputs also need to use =& in the
32-bit case. But I wouldn't insist on such an adjustment. With
the others done
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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