[PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

CentOS 7 gets into trouble when compiling Xen citing:

  flushtlb.c: Assembler messages:
  flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1

The line number is wrong, and the error message not helpful.  It turns out
that the intermediate generated assembly was

  # 139 "arch/x86/flushtlb.c" 1
      661:
      rex clflush (%r15)
  662:
  .pushsection .altinstructions,"a"

and it was having trouble combining the explicit REX prefix with the REX.B
required for the use of %r15.

Follow what Linux does and use a redundant %ds prefix instead, for a final
generated instruction of `3e 41 0f ae 3f`

While modifying this line, fix the indentation which was out by one space.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/flushtlb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 90a004f..727434e 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int flags)
         {
             alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
             for ( i = 0; i < sz; i += c->x86_clflush_size )
-                 alternative_input("rex clflush %0",
-                                   "data16 clflush %0",
-                                   X86_FEATURE_CLFLUSHOPT,
-                                   "m" (((const char *)va)[i]));
+                alternative_input(".byte 0x3e; clflush %0", /* %ds override. */
+                                  "data16 clflush %0",      /* clflushopt.   */
+                                  X86_FEATURE_CLFLUSHOPT,
+                                  "m" (((const char *)va)[i]));
         }
         else
         {
-- 
2.1.4

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 11/02/16 19:25, Andrew Cooper wrote:
> CentOS 7 gets into trouble when compiling Xen citing:
>
>   flushtlb.c: Assembler messages:
>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>
> The line number is wrong, and the error message not helpful.  It turns out
> that the intermediate generated assembly was
>
>   # 139 "arch/x86/flushtlb.c" 1
>       661:
>       rex clflush (%r15)
>   662:
>   .pushsection .altinstructions,"a"
>
> and it was having trouble combining the explicit REX prefix with the REX.B
> required for the use of %r15.
>
> Follow what Linux does and use a redundant %ds prefix instead, for a final
> generated instruction of `3e 41 0f ae 3f`
>
> While modifying this line, fix the indentation which was out by one space.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> ---
>  xen/arch/x86/flushtlb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
> index 90a004f..727434e 100644
> --- a/xen/arch/x86/flushtlb.c
> +++ b/xen/arch/x86/flushtlb.c
> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int flags)
>          {
>              alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>              for ( i = 0; i < sz; i += c->x86_clflush_size )
> -                 alternative_input("rex clflush %0",
> -                                   "data16 clflush %0",
> -                                   X86_FEATURE_CLFLUSHOPT,
> -                                   "m" (((const char *)va)[i]));
> +                alternative_input(".byte 0x3e; clflush %0", /* %ds override. */
> +                                  "data16 clflush %0",      /* clflushopt.   */
> +                                  X86_FEATURE_CLFLUSHOPT,
> +                                  "m" (((const char *)va)[i]));
>          }
>          else
>          {

It turns out that Clang is far more useful at diagnosing this issue than
GCC.

flushtlb.c:144:18: error: invalid instruction mnemonic 'rex'
                 alternative_input("rex clflush %0",
                 ^
/local/xen.git/xen/include/asm/alternative.h:98:16: note: expanded from
macro 'alternative_input'
        asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)          \
                      ^
/local/xen.git/xen/include/asm/alternative.h:60:2: note: expanded from
macro 'ALTERNATIVE'
        OLDINSTR(oldinstr)                                                \
        ^
/local/xen.git/xen/include/asm/alternative.h:28:40: note: expanded from
macro 'OLDINSTR'
#define OLDINSTR(oldinstr)      "661:\n\t" oldinstr "\n662:\n"
                                       ^
<inline asm>:2:2: note: instantiated into assembly here
        rex clflush (%r15,%rdx)
        ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

~Andrew

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Doug Goldstein]

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

On 2/11/16 1:25 PM, Andrew Cooper wrote:
> CentOS 7 gets into trouble when compiling Xen citing:
> 
>   flushtlb.c: Assembler messages:
>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
> 
> The line number is wrong, and the error message not helpful.  It turns out
> that the intermediate generated assembly was
> 
>   # 139 "arch/x86/flushtlb.c" 1
>       661:
>       rex clflush (%r15)
>   662:
>   .pushsection .altinstructions,"a"
> 
> and it was having trouble combining the explicit REX prefix with the REX.B
> required for the use of %r15.
> 
> Follow what Linux does and use a redundant %ds prefix instead, for a final
> generated instruction of `3e 41 0f ae 3f`
> 
> While modifying this line, fix the indentation which was out by one space.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Tested-by: Doug Goldstein <cardoe@cardoe.com>

Confirmed to fix the build issue. Results:
https://travis-ci.org/cardoe/xen/builds/108653604

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> ---
>  xen/arch/x86/flushtlb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
> index 90a004f..727434e 100644
> --- a/xen/arch/x86/flushtlb.c
> +++ b/xen/arch/x86/flushtlb.c
> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int flags)
>          {
>              alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>              for ( i = 0; i < sz; i += c->x86_clflush_size )
> -                 alternative_input("rex clflush %0",
> -                                   "data16 clflush %0",
> -                                   X86_FEATURE_CLFLUSHOPT,
> -                                   "m" (((const char *)va)[i]));
> +                alternative_input(".byte 0x3e; clflush %0", /* %ds override. */
> +                                  "data16 clflush %0",      /* clflushopt.   */
> +                                  X86_FEATURE_CLFLUSHOPT,
> +                                  "m" (((const char *)va)[i]));
>          }
>          else
>          {
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Jan Beulich]

>>> On 11.02.16 at 20:41, <andrew.cooper3@citrix.com> wrote:
> On 11/02/16 19:25, Andrew Cooper wrote:
>> --- a/xen/arch/x86/flushtlb.c
>> +++ b/xen/arch/x86/flushtlb.c
>> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int flags)
>>          {
>>              alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>>              for ( i = 0; i < sz; i += c->x86_clflush_size )
>> -                 alternative_input("rex clflush %0",
>> -                                   "data16 clflush %0",
>> -                                   X86_FEATURE_CLFLUSHOPT,
>> -                                   "m" (((const char *)va)[i]));
>> +                alternative_input(".byte 0x3e; clflush %0", /* %ds override. */
>> +                                  "data16 clflush %0",      /* clflushopt.  
>  */
>> +                                  X86_FEATURE_CLFLUSHOPT,
>> +                                  "m" (((const char *)va)[i]));
>>          }
>>          else
>>          {
> 
> It turns out that Clang is far more useful at diagnosing this issue than
> GCC.
> 
> flushtlb.c:144:18: error: invalid instruction mnemonic 'rex'
>                  alternative_input("rex clflush %0",
>                  ^

Except that 'rex' is by no means invalid. If anything Clang's internal
assembler doesn't support it (and hence is not gas compatible).

Jan

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Jan Beulich]

>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
> CentOS 7 gets into trouble when compiling Xen citing:
> 
>   flushtlb.c: Assembler messages:
>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
> 
> The line number is wrong, and the error message not helpful.  It turns out
> that the intermediate generated assembly was
> 
>   # 139 "arch/x86/flushtlb.c" 1
>       661:
>       rex clflush (%r15)
>   662:
>   .pushsection .altinstructions,"a"
> 
> and it was having trouble combining the explicit REX prefix with the REX.B
> required for the use of %r15.

What gas version is this? I just checked with 2.20, which has no
problem combining an explicit with a generated REX prefix. Or
wait, no, your description of the issue is wrong: It actually is the
folding of the two REX prefixes which causes the problem, since
that results in the replacement instruction being one byte longer
than the to be replaced one.

> Follow what Linux does and use a redundant %ds prefix instead, for a final
> generated instruction of `3e 41 0f ae 3f`
> 
> While modifying this line, fix the indentation which was out by one space.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I think before committing I'll extend your patch by introducing
and using Linux'es NOP_DS_PREFIX.

Jan

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 12/02/16 08:13, Jan Beulich wrote:
>>>> On 11.02.16 at 20:41, <andrew.cooper3@citrix.com> wrote:
>> On 11/02/16 19:25, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/flushtlb.c
>>> +++ b/xen/arch/x86/flushtlb.c
>>> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int flags)
>>>          {
>>>              alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>>>              for ( i = 0; i < sz; i += c->x86_clflush_size )
>>> -                 alternative_input("rex clflush %0",
>>> -                                   "data16 clflush %0",
>>> -                                   X86_FEATURE_CLFLUSHOPT,
>>> -                                   "m" (((const char *)va)[i]));
>>> +                alternative_input(".byte 0x3e; clflush %0", /* %ds override. */
>>> +                                  "data16 clflush %0",      /* clflushopt.  
>>  */
>>> +                                  X86_FEATURE_CLFLUSHOPT,
>>> +                                  "m" (((const char *)va)[i]));
>>>          }
>>>          else
>>>          {
>> It turns out that Clang is far more useful at diagnosing this issue than
>> GCC.
>>
>> flushtlb.c:144:18: error: invalid instruction mnemonic 'rex'
>>                  alternative_input("rex clflush %0",
>>                  ^
> Except that 'rex' is by no means invalid. If anything Clang's internal
> assembler doesn't support it (and hence is not gas compatible).

That is tangential to the point.  The useful part is the end message:

> <inline asm>:2:2: note: instantiated into assembly here
>         rex clflush (%r15,%rdx)
>         ^~~~~~~~~~~~~~~~~~~~~~~

which makes it substantially more clear that there is both a REX prefix
and REX.B required.

~Andrew

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 12/02/16 08:23, Jan Beulich wrote:
>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>> CentOS 7 gets into trouble when compiling Xen citing:
>>
>>   flushtlb.c: Assembler messages:
>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>
>> The line number is wrong, and the error message not helpful.  It turns out
>> that the intermediate generated assembly was
>>
>>   # 139 "arch/x86/flushtlb.c" 1
>>       661:
>>       rex clflush (%r15)
>>   662:
>>   .pushsection .altinstructions,"a"
>>
>> and it was having trouble combining the explicit REX prefix with the REX.B
>> required for the use of %r15.
> What gas version is this? I just checked with 2.20, which has no
> problem combining an explicit with a generated REX prefix.

bash-4.2$ as --version
GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226


>  Or
> wait, no, your description of the issue is wrong: It actually is the
> folding of the two REX prefixes which causes the problem,

This is what I said.  What do you think I meant with "trouble combining
the" ?

>  since
> that results in the replacement instruction being one byte longer
> than the to be replaced one.

But that is still the case with an explicit %ds override.  The assembler
still has to insert an extra byte somehow.

>
>> Follow what Linux does and use a redundant %ds prefix instead, for a final
>> generated instruction of `3e 41 0f ae 3f`
>>
>> While modifying this line, fix the indentation which was out by one space.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> I think before committing I'll extend your patch by introducing
> and using Linux'es NOP_DS_PREFIX.

Ok.

~Andrew

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Jan Beulich]

>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
> On 12/02/16 08:23, Jan Beulich wrote:
>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>
>>>   flushtlb.c: Assembler messages:
>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>
>>> The line number is wrong, and the error message not helpful.  It turns out
>>> that the intermediate generated assembly was
>>>
>>>   # 139 "arch/x86/flushtlb.c" 1
>>>       661:
>>>       rex clflush (%r15)
>>>   662:
>>>   .pushsection .altinstructions,"a"
>>>
>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>> required for the use of %r15.
>> What gas version is this? I just checked with 2.20, which has no
>> problem combining an explicit with a generated REX prefix.
> 
> bash-4.2$ as --version
> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
> 
> 
>>  Or
>> wait, no, your description of the issue is wrong: It actually is the
>> folding of the two REX prefixes which causes the problem,
> 
> This is what I said.  What do you think I meant with "trouble combining
> the" ?

Argh - I meant to say "It actually isn't ...".

>>  since
>> that results in the replacement instruction being one byte longer
>> than the to be replaced one.
> 
> But that is still the case with an explicit %ds override.  The assembler
> still has to insert an extra byte somehow.

No. We now always have one non-REX prefix, and both instructions
will have the same REX/ModRM/SIB encoding.

Jan

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 12/02/16 10:00, Jan Beulich wrote:
>>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
>> On 12/02/16 08:23, Jan Beulich wrote:
>>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>>
>>>>   flushtlb.c: Assembler messages:
>>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>>
>>>> The line number is wrong, and the error message not helpful.  It turns out
>>>> that the intermediate generated assembly was
>>>>
>>>>   # 139 "arch/x86/flushtlb.c" 1
>>>>       661:
>>>>       rex clflush (%r15)
>>>>   662:
>>>>   .pushsection .altinstructions,"a"
>>>>
>>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>>> required for the use of %r15.
>>> What gas version is this? I just checked with 2.20, which has no
>>> problem combining an explicit with a generated REX prefix.
>> bash-4.2$ as --version
>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
>>
>>
>>>  Or
>>> wait, no, your description of the issue is wrong: It actually is the
>>> folding of the two REX prefixes which causes the problem,
>> This is what I said.  What do you think I meant with "trouble combining
>> the" ?
> Argh - I meant to say "It actually isn't ...".
>
>>>  since
>>> that results in the replacement instruction being one byte longer
>>> than the to be replaced one.
>> But that is still the case with an explicit %ds override.  The assembler
>> still has to insert an extra byte somehow.
> No. We now always have one non-REX prefix, and both instructions
> will have the same REX/ModRM/SIB encoding.

I see now, given your wording on the patch committed.

In hindsight this should have been obvious, but GCCs error message was
particularly unhelpful at diagnosing the issue.

~Andrew

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Jan Beulich]

>>> On 12.02.16 at 11:02, <andrew.cooper3@citrix.com> wrote:
> On 12/02/16 10:00, Jan Beulich wrote:
>>>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
>>> On 12/02/16 08:23, Jan Beulich wrote:
>>>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>>>
>>>>>   flushtlb.c: Assembler messages:
>>>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>>>
>>>>> The line number is wrong, and the error message not helpful.  It turns out
>>>>> that the intermediate generated assembly was
>>>>>
>>>>>   # 139 "arch/x86/flushtlb.c" 1
>>>>>       661:
>>>>>       rex clflush (%r15)
>>>>>   662:
>>>>>   .pushsection .altinstructions,"a"
>>>>>
>>>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>>>> required for the use of %r15.
>>>> What gas version is this? I just checked with 2.20, which has no
>>>> problem combining an explicit with a generated REX prefix.
>>> bash-4.2$ as --version
>>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
>>>
>>>
>>>>  Or
>>>> wait, no, your description of the issue is wrong: It actually is the
>>>> folding of the two REX prefixes which causes the problem,
>>> This is what I said.  What do you think I meant with "trouble combining
>>> the" ?
>> Argh - I meant to say "It actually isn't ...".
>>
>>>>  since
>>>> that results in the replacement instruction being one byte longer
>>>> than the to be replaced one.
>>> But that is still the case with an explicit %ds override.  The assembler
>>> still has to insert an extra byte somehow.
>> No. We now always have one non-REX prefix, and both instructions
>> will have the same REX/ModRM/SIB encoding.
> 
> I see now, given your wording on the patch committed.
> 
> In hindsight this should have been obvious, but GCCs error message was
> particularly unhelpful at diagnosing the issue.

It was actually an assembler error message, and I can't really see
how we could improve that (since afaict the intended checking can
only be done at assembly time).

Jan

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 12/02/16 10:12, Jan Beulich wrote:
>>>> On 12.02.16 at 11:02, <andrew.cooper3@citrix.com> wrote:
>> On 12/02/16 10:00, Jan Beulich wrote:
>>>>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
>>>> On 12/02/16 08:23, Jan Beulich wrote:
>>>>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>>>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>>>>
>>>>>>   flushtlb.c: Assembler messages:
>>>>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>>>>
>>>>>> The line number is wrong, and the error message not helpful.  It turns out
>>>>>> that the intermediate generated assembly was
>>>>>>
>>>>>>   # 139 "arch/x86/flushtlb.c" 1
>>>>>>       661:
>>>>>>       rex clflush (%r15)
>>>>>>   662:
>>>>>>   .pushsection .altinstructions,"a"
>>>>>>
>>>>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>>>>> required for the use of %r15.
>>>>> What gas version is this? I just checked with 2.20, which has no
>>>>> problem combining an explicit with a generated REX prefix.
>>>> bash-4.2$ as --version
>>>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
>>>>
>>>>
>>>>>  Or
>>>>> wait, no, your description of the issue is wrong: It actually is the
>>>>> folding of the two REX prefixes which causes the problem,
>>>> This is what I said.  What do you think I meant with "trouble combining
>>>> the" ?
>>> Argh - I meant to say "It actually isn't ...".
>>>
>>>>>  since
>>>>> that results in the replacement instruction being one byte longer
>>>>> than the to be replaced one.
>>>> But that is still the case with an explicit %ds override.  The assembler
>>>> still has to insert an extra byte somehow.
>>> No. We now always have one non-REX prefix, and both instructions
>>> will have the same REX/ModRM/SIB encoding.
>> I see now, given your wording on the patch committed.
>>
>> In hindsight this should have been obvious, but GCCs error message was
>> particularly unhelpful at diagnosing the issue.
> It was actually an assembler error message, and I can't really see
> how we could improve that (since afaict the intended checking can
> only be done at assembly time).

Oh right.  It is an assembler BUILD_BUG_ON().

Is there anything useful we can do to get the error message to properly
point at alternative.h: DISCARD_ENTRY()?  That would at least have helped.

As it stood, the actual reported error line was a closing brace after
the wbinvd() call.

~Andrew

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Jan Beulich]

>>> On 12.02.16 at 11:50, <andrew.cooper3@citrix.com> wrote:
> On 12/02/16 10:12, Jan Beulich wrote:
>>>>> On 12.02.16 at 11:02, <andrew.cooper3@citrix.com> wrote:
>>> On 12/02/16 10:00, Jan Beulich wrote:
>>>>>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
>>>>> On 12/02/16 08:23, Jan Beulich wrote:
>>>>>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>>>>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>>>>>
>>>>>>>   flushtlb.c: Assembler messages:
>>>>>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>>>>>
>>>>>>> The line number is wrong, and the error message not helpful.  It turns out
>>>>>>> that the intermediate generated assembly was
>>>>>>>
>>>>>>>   # 139 "arch/x86/flushtlb.c" 1
>>>>>>>       661:
>>>>>>>       rex clflush (%r15)
>>>>>>>   662:
>>>>>>>   .pushsection .altinstructions,"a"
>>>>>>>
>>>>>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>>>>>> required for the use of %r15.
>>>>>> What gas version is this? I just checked with 2.20, which has no
>>>>>> problem combining an explicit with a generated REX prefix.
>>>>> bash-4.2$ as --version
>>>>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
>>>>>
>>>>>
>>>>>>  Or
>>>>>> wait, no, your description of the issue is wrong: It actually is the
>>>>>> folding of the two REX prefixes which causes the problem,
>>>>> This is what I said.  What do you think I meant with "trouble combining
>>>>> the" ?
>>>> Argh - I meant to say "It actually isn't ...".
>>>>
>>>>>>  since
>>>>>> that results in the replacement instruction being one byte longer
>>>>>> than the to be replaced one.
>>>>> But that is still the case with an explicit %ds override.  The assembler
>>>>> still has to insert an extra byte somehow.
>>>> No. We now always have one non-REX prefix, and both instructions
>>>> will have the same REX/ModRM/SIB encoding.
>>> I see now, given your wording on the patch committed.
>>>
>>> In hindsight this should have been obvious, but GCCs error message was
>>> particularly unhelpful at diagnosing the issue.
>> It was actually an assembler error message, and I can't really see
>> how we could improve that (since afaict the intended checking can
>> only be done at assembly time).
> 
> Oh right.  It is an assembler BUILD_BUG_ON().
> 
> Is there anything useful we can do to get the error message to properly
> point at alternative.h: DISCARD_ENTRY()?  That would at least have helped.

I'd like to, but I can't see how, not the least because ...

> As it stood, the actual reported error line was a closing brace after
> the wbinvd() call.

... I've also noticed that, but the assembler would just use whatever
line directive the compiler had put in the assembly file.

Jan

Re: [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"

[Andrew Cooper]

On 12/02/16 10:57, Jan Beulich wrote:
>>>> On 12.02.16 at 11:50, <andrew.cooper3@citrix.com> wrote:
>> On 12/02/16 10:12, Jan Beulich wrote:
>>>>>> On 12.02.16 at 11:02, <andrew.cooper3@citrix.com> wrote:
>>>> On 12/02/16 10:00, Jan Beulich wrote:
>>>>>>>> On 12.02.16 at 10:51, <andrew.cooper3@citrix.com> wrote:
>>>>>> On 12/02/16 08:23, Jan Beulich wrote:
>>>>>>>>>> On 11.02.16 at 20:25, <andrew.cooper3@citrix.com> wrote:
>>>>>>>> CentOS 7 gets into trouble when compiling Xen citing:
>>>>>>>>
>>>>>>>>   flushtlb.c: Assembler messages:
>>>>>>>>   flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
>>>>>>>>
>>>>>>>> The line number is wrong, and the error message not helpful.  It turns out
>>>>>>>> that the intermediate generated assembly was
>>>>>>>>
>>>>>>>>   # 139 "arch/x86/flushtlb.c" 1
>>>>>>>>       661:
>>>>>>>>       rex clflush (%r15)
>>>>>>>>   662:
>>>>>>>>   .pushsection .altinstructions,"a"
>>>>>>>>
>>>>>>>> and it was having trouble combining the explicit REX prefix with the REX.B
>>>>>>>> required for the use of %r15.
>>>>>>> What gas version is this? I just checked with 2.20, which has no
>>>>>>> problem combining an explicit with a generated REX prefix.
>>>>>> bash-4.2$ as --version
>>>>>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226
>>>>>>
>>>>>>
>>>>>>>  Or
>>>>>>> wait, no, your description of the issue is wrong: It actually is the
>>>>>>> folding of the two REX prefixes which causes the problem,
>>>>>> This is what I said.  What do you think I meant with "trouble combining
>>>>>> the" ?
>>>>> Argh - I meant to say "It actually isn't ...".
>>>>>
>>>>>>>  since
>>>>>>> that results in the replacement instruction being one byte longer
>>>>>>> than the to be replaced one.
>>>>>> But that is still the case with an explicit %ds override.  The assembler
>>>>>> still has to insert an extra byte somehow.
>>>>> No. We now always have one non-REX prefix, and both instructions
>>>>> will have the same REX/ModRM/SIB encoding.
>>>> I see now, given your wording on the patch committed.
>>>>
>>>> In hindsight this should have been obvious, but GCCs error message was
>>>> particularly unhelpful at diagnosing the issue.
>>> It was actually an assembler error message, and I can't really see
>>> how we could improve that (since afaict the intended checking can
>>> only be done at assembly time).
>> Oh right.  It is an assembler BUILD_BUG_ON().
>>
>> Is there anything useful we can do to get the error message to properly
>> point at alternative.h: DISCARD_ENTRY()?  That would at least have helped.
> I'd like to, but I can't see how, not the least because ...
>
>> As it stood, the actual reported error line was a closing brace after
>> the wbinvd() call.
> ... I've also noticed that, but the assembler would just use whatever
> line directive the compiler had put in the assembly file.

Hmm.

Well, the one saving grace is that if you google the error message, you
now get to this thread.  Hopefully this might help someone else out of a
similar situation in the future.

~Andrew