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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/11/2018 13:51, Jan Beulich wrote:
>>>> 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.

Aah - good point, and also explains the other conditionals in GCC's
intrinsic header.

I'll update the logic to cope with the larger 64bit models.

>
>> --- 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.

Am I?  Undefined symbols behave as if they had the value 0 in an #if

>
>> +#define XCHG_BX "xchg %%ebx, %k[bx];"
> I don't think the k modifier is actually needed?

Perhaps not, but the 64bit version will need a q.

~Andrew

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