[PATCH] libx86: Work around GCC bug with ebx output constrants

[PATCH] libx86: Work around GCC bug with ebx output constrants

[Andrew Cooper]

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"
       ^

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.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/include/xen/lib/x86/cpuid.h | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 266c910..77f63d5 100644
--- 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__
+#define XCHG_BX "xchg %%ebx, %k[bx];"
+#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)
           : "a" (leaf) );
 }
 
 static inline void cpuid_count_leaf(
     uint32_t leaf, uint32_t subleaf, 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)
           : "a" (leaf), "c" (subleaf) );
 }
 
+#undef BX_CON
+#undef XCHG
+
 #define CPUID_GUEST_NR_BASIC      (0xdu + 1)
 #define CPUID_GUEST_NR_FEAT       (0u + 1)
 #define CPUID_GUEST_NR_CACHE      (5u + 1)
-- 
2.1.4


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

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

[Andrew Cooper]

On 19/11/2018 13:11, Andrew Cooper 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"
>        ^
>
> 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.

Actually, having just got GCC 5 working, that is also fine.  I'll adjust
the wording/check, but won't bother posting a v2 if that is the only change.

~Andrew

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

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

[Mihai Donțu]

On Mon, 2018-11-19 at 13:29 +0000, Andrew Cooper wrote:
> On 19/11/2018 13:11, Andrew Cooper 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"
> >        ^
> > 
> > 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.
> 
> Actually, having just got GCC 5 working, that is also fine.  I'll adjust
> the wording/check, but won't bother posting a v2 if that is the only change.

I wonder if it has anything to do with this:

"Reuse of the PIC hard register, instead of using a fixed register, was
implemented on x86/x86-64 targets. This improves generated PIC code
performance as more hard registers can be used. Shared libraries can
significantly benefit from this optimization. Currently it is switched
on only for x86/x86-64 targets. As RA infrastructure is already
implemented for PIC register reuse, other targets might follow this in
the future."

https://gcc.gnu.org/gcc-5/changes.html

-- 
Mihai Donțu


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

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

[Andrew Cooper]

On 19/11/2018 14:52, Mihai Donțu wrote:
> On Mon, 2018-11-19 at 13:29 +0000, Andrew Cooper wrote:
>> On 19/11/2018 13:11, Andrew Cooper 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"
>>>        ^
>>>
>>> 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.
>> Actually, having just got GCC 5 working, that is also fine.  I'll adjust
>> the wording/check, but won't bother posting a v2 if that is the only change.
> I wonder if it has anything to do with this:
>
> "Reuse of the PIC hard register, instead of using a fixed register, was
> implemented on x86/x86-64 targets. This improves generated PIC code
> performance as more hard registers can be used. Shared libraries can
> significantly benefit from this optimization. Currently it is switched
> on only for x86/x86-64 targets. As RA infrastructure is already
> implemented for PIC register reuse, other targets might follow this in
> the future."

That is almost certainly it.  Looks like it is down to the pre 5.0 code
considering %rBX non-spillable

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54232

Thanks for digging this out.  I missed it while scanning the 5
changelog.  I think I'll tweak the text to reflect this.

~Andrew

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

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

[Jan Beulich]

>>> On 19.11.18 at 14:11, <andrew.cooper3@citrix.com> 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@suse.com>

Jan


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

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

[Andrew Cooper]

On 19/11/2018 13:51, Jan Beulich wrote:
>>>> On 19.11.18 at 14:11, <andrew.cooper3@citrix.com> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Jan Beulich]

>>> On 19.11.18 at 15:08, <andrew.cooper3@citrix.com> wrote:
> On 19/11/2018 13:51, Jan Beulich wrote:
>>>>> On 19.11.18 at 14:11, <andrew.cooper3@citrix.com> wrote:
>>> --- 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

But compilers may warn in such cases. Or else there would almost
never be a need to use defined().

Jan


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

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

[Jan Beulich]

>>> On 19.11.18 at 15:08, <andrew.cooper3@citrix.com> wrote:
> On 19/11/2018 13:51, Jan Beulich wrote:
>>>>> On 19.11.18 at 14:11, <andrew.cooper3@citrix.com> wrote:
>>> 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.

Do you need to? The ABI specifies that the GOT pointer lives
in %r15 in that case.

Jan



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

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

[Andrew Cooper]

On 19/11/2018 14:23, Jan Beulich wrote:
>>>> On 19.11.18 at 15:08, <andrew.cooper3@citrix.com> wrote:
>> On 19/11/2018 13:51, Jan Beulich wrote:
>>>>>> On 19.11.18 at 14:11, <andrew.cooper3@citrix.com> wrote:
>>>> 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.
> Do you need to? The ABI specifies that the GOT pointer lives
> in %r15 in that case.

I've checked, and gcc 4.9 complains equally when using -mcmodel=large

~Andrew

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

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

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 838 bytes --]

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

Using =& for the constraints which also have inputs results "operand has
impossible constraints"

It should only matter for the output-only operands, to prevent GCC
allocating %[er]dx for the bx constraint, but that shouldn't matter
anyway because =&r can't be allocated to conflict with =d

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 1493 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

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

[Jan Beulich]

>>> On 19.11.18 at 15:33, <andrew.cooper3@citrix.com> wrote:
> On 19/11/2018 13:51, Jan Beulich wrote:
>>>  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@suse.com>
> 
> Using =& for the constraints which also have inputs results "operand has
> impossible constraints"

Yes, I had (silently) assumed this would be the case.

> It should only matter for the output-only operands, to prevent GCC
> allocating %[er]dx for the bx constraint, but that shouldn't matter
> anyway because =&r can't be allocated to conflict with =d

Hence me having said "strictly speaking".

Jan



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

[PATCH v2] libx86: Work around GCC bug with ebx output constrants

[Andrew Cooper]

Versions of GCC before 5 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"
       ^

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * GCC 5 is fine.  Its cpuid instrinct has none of the PIC workarounds thant 4.9 have.
 * Fix 64bit builds with larger models.
---
 xen/include/xen/lib/x86/cpuid.h | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 266c910..cc30654 100644
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -20,21 +20,50 @@ struct cpuid_leaf
     uint32_t a, b, c, d;
 };
 
+/*
+ * Versions of GCC before 5 are unable to cope with %rBX output constraints
+ * when compiling Position Independent Code, and produce a rather cryptic
+ * error:
+ *    error: inconsistent operand constraints in an ‘asm’
+ *
+ * To work around the issue, use a separate register to hold the the %rBX
+ * output, and xchg twice to leave %rBX preserved around the asm() statement.
+ */
+#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
+# define XCHG_BX "xchg %%ebx, %[bx];"
+# define BX_CON [bx] "=&r"
+#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
+    defined(__x86_64__) && (defined(__code_model_medium__) || \
+                            defined(__code_model_large__))
+# define XCHG_BX "xchg %%rbx, %q[bx];"
+# 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)
           : "a" (leaf) );
 }
 
 static inline void cpuid_count_leaf(
     uint32_t leaf, uint32_t subleaf, 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)
           : "a" (leaf), "c" (subleaf) );
 }
 
+#undef BX_CON
+#undef XCHG
+
 #define CPUID_GUEST_NR_BASIC      (0xdu + 1)
 #define CPUID_GUEST_NR_FEAT       (0u + 1)
 #define CPUID_GUEST_NR_CACHE      (5u + 1)
-- 
2.1.4


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

Re: [PATCH v2] libx86: Work around GCC bug with ebx output constrants

[Jan Beulich]

>>> On 19.11.18 at 15:45, <andrew.cooper3@citrix.com> wrote:
> Versions of GCC before 5 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"
>        ^
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> 
> v2:
>  * GCC 5 is fine.  Its cpuid instrinct has none of the PIC workarounds thant 4.9 have.
>  * Fix 64bit builds with larger models.

It is rather odd that 64-bit is also affected - the error gets raised
even when there's no use of %rbx for GOT accesses. By when
they need a callee-saved register, they indeed use %rbx instead
to the ABI-suggested %r15.

> --- a/xen/include/xen/lib/x86/cpuid.h
> +++ b/xen/include/xen/lib/x86/cpuid.h
> @@ -20,21 +20,50 @@ struct cpuid_leaf
>      uint32_t a, b, c, d;
>  };
>  
> +/*
> + * Versions of GCC before 5 are unable to cope with %rBX output constraints
> + * when compiling Position Independent Code, and produce a rather cryptic
> + * error:
> + *    error: inconsistent operand constraints in an ‘asm’
> + *
> + * To work around the issue, use a separate register to hold the the %rBX
> + * output, and xchg twice to leave %rBX preserved around the asm() statement.
> + */
> +#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
> +# define XCHG_BX "xchg %%ebx, %[bx];"
> +# define BX_CON [bx] "=&r"
> +#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
> +    defined(__x86_64__) && (defined(__code_model_medium__) || \
> +                            defined(__code_model_large__))
> +# define XCHG_BX "xchg %%rbx, %q[bx];"
> +# define BX_CON [bx] "=&r"
> +#else
> +# define XCHG_BX ""
> +# define BX_CON "=&b"

The & is unnecessary here I think. Preferably with it dropped
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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

Re: [PATCH v2] libx86: Work around GCC bug with ebx output constrants

[Andrew Cooper]

On 19/11/2018 15:14, Jan Beulich wrote:
>>>> On 19.11.18 at 15:45, <andrew.cooper3@citrix.com> wrote:
>> Versions of GCC before 5 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"
>>        ^
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>
>> v2:
>>  * GCC 5 is fine.  Its cpuid instrinct has none of the PIC workarounds thant 4.9 have.
>>  * Fix 64bit builds with larger models.
> It is rather odd that 64-bit is also affected - the error gets raised
> even when there's no use of %rbx for GOT accesses. By when
> they need a callee-saved register, they indeed use %rbx instead
> to the ABI-suggested %r15.
>
>> --- a/xen/include/xen/lib/x86/cpuid.h
>> +++ b/xen/include/xen/lib/x86/cpuid.h
>> @@ -20,21 +20,50 @@ struct cpuid_leaf
>>      uint32_t a, b, c, d;
>>  };
>>  
>> +/*
>> + * Versions of GCC before 5 are unable to cope with %rBX output constraints
>> + * when compiling Position Independent Code, and produce a rather cryptic
>> + * error:
>> + *    error: inconsistent operand constraints in an ‘asm’
>> + *
>> + * To work around the issue, use a separate register to hold the the %rBX
>> + * output, and xchg twice to leave %rBX preserved around the asm() statement.
>> + */
>> +#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
>> +# define XCHG_BX "xchg %%ebx, %[bx];"
>> +# define BX_CON [bx] "=&r"
>> +#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
>> +    defined(__x86_64__) && (defined(__code_model_medium__) || \
>> +                            defined(__code_model_large__))
>> +# define XCHG_BX "xchg %%rbx, %q[bx];"
>> +# define BX_CON [bx] "=&r"
>> +#else
>> +# define XCHG_BX ""
>> +# define BX_CON "=&b"
> The & is unnecessary here I think. Preferably with it dropped
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

That was to match the "=&d" and friends.  I'd prefer to be consistent
(one way or the other).

~Andrew

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

Re: [PATCH v2] libx86: Work around GCC bug with ebx output constrants

[Jan Beulich]

>>> On 19.11.18 at 16:19, <andrew.cooper3@citrix.com> wrote:
> On 19/11/2018 15:14, Jan Beulich wrote:
>>>>> On 19.11.18 at 15:45, <andrew.cooper3@citrix.com> wrote:
>>> Versions of GCC before 5 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"
>>>        ^
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>
>>> v2:
>>>  * GCC 5 is fine.  Its cpuid instrinct has none of the PIC workarounds thant 
> 4.9 have.
>>>  * Fix 64bit builds with larger models.
>> It is rather odd that 64-bit is also affected - the error gets raised
>> even when there's no use of %rbx for GOT accesses. By when
>> they need a callee-saved register, they indeed use %rbx instead
>> to the ABI-suggested %r15.
>>
>>> --- a/xen/include/xen/lib/x86/cpuid.h
>>> +++ b/xen/include/xen/lib/x86/cpuid.h
>>> @@ -20,21 +20,50 @@ struct cpuid_leaf
>>>      uint32_t a, b, c, d;
>>>  };
>>>  
>>> +/*
>>> + * Versions of GCC before 5 are unable to cope with %rBX output constraints
>>> + * when compiling Position Independent Code, and produce a rather cryptic
>>> + * error:
>>> + *    error: inconsistent operand constraints in an ‘asm’
>>> + *
>>> + * To work around the issue, use a separate register to hold the the %rBX
>>> + * output, and xchg twice to leave %rBX preserved around the asm() 
> statement.
>>> + */
>>> +#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
>>> +# define XCHG_BX "xchg %%ebx, %[bx];"
>>> +# define BX_CON [bx] "=&r"
>>> +#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
>>> +    defined(__x86_64__) && (defined(__code_model_medium__) || \
>>> +                            defined(__code_model_large__))
>>> +# define XCHG_BX "xchg %%rbx, %q[bx];"
>>> +# define BX_CON [bx] "=&r"
>>> +#else
>>> +# define XCHG_BX ""
>>> +# define BX_CON "=&b"
>> The & is unnecessary here I think. Preferably with it dropped
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> That was to match the "=&d" and friends.  I'd prefer to be consistent
> (one way or the other).

Ah, well, yes - that's fine then.

Jan


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

[PATCH v3] libx86: Work around GCC being unable to spill the PIC hard register

[Andrew Cooper]

Versions of GCC before 5 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"
       ^

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54232 for more details.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * GCC 5 is fine.  Its cpuid instrinct has none of the PIC workarounds thant 4.9 have.
 * Fix 64bit builds with larger models.
v3:
 * Reference the bugzilla entry which fixed this.
---
 xen/include/xen/lib/x86/cpuid.h | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 266c910..22d43ef 100644
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -20,21 +20,51 @@ struct cpuid_leaf
     uint32_t a, b, c, d;
 };
 
+/*
+ * Versions of GCC before 5 unconditionally reserve %rBX as the PIC hard
+ * register, and are unable to cope with spilling it.  This results in a
+ * rather cryptic error:
+ *    error: inconsistent operand constraints in an ‘asm’
+ *
+ * In affected situations, work around the issue by using a separate register
+ * to hold the the %rBX output, and xchg twice to leave %rBX preserved around
+ * the asm() statement.
+ */
+#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
+# define XCHG_BX "xchg %%ebx, %[bx];"
+# define BX_CON [bx] "=&r"
+#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
+    defined(__x86_64__) && (defined(__code_model_medium__) || \
+                            defined(__code_model_large__))
+# define XCHG_BX "xchg %%rbx, %q[bx];"
+# 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)
           : "a" (leaf) );
 }
 
 static inline void cpuid_count_leaf(
     uint32_t leaf, uint32_t subleaf, 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)
           : "a" (leaf), "c" (subleaf) );
 }
 
+#undef BX_CON
+#undef XCHG
+
 #define CPUID_GUEST_NR_BASIC      (0xdu + 1)
 #define CPUID_GUEST_NR_FEAT       (0u + 1)
 #define CPUID_GUEST_NR_CACHE      (5u + 1)
-- 
2.1.4


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