[PATCH] x86/entry/64: add two more instruction suffixes

[PATCH] x86/entry/64: add two more instruction suffixes

[Jan Beulich]

Sadly, other than claimed in a368d7fd2a ("x86/entry/64: Add instruction
suffix"), there are two more instances which want to be adjusted. As
said there, omitting suffixes from instructions in AT&T mode is bad
practice when operand size cannot be determined by the assembler from
register operands, and is likely going to be warned about by upstream
gas in the future (mine does already). Add the other missing suffixes
here as well.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 arch/x86/entry/entry_64.S |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- 4.18-rc3/arch/x86/entry/entry_64.S
+++ 4.18-rc3-x86_64-entry-insn-suffix/arch/x86/entry/entry_64.S
@@ -92,7 +92,7 @@ END(native_usergs_sysret64)
 .endm
 
 .macro TRACE_IRQS_IRETQ_DEBUG
-	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
+	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
 	jnc	1f
 	TRACE_IRQS_ON_DEBUG
 1:
@@ -701,7 +701,7 @@ retint_kernel:
 #ifdef CONFIG_PREEMPT
 	/* Interrupts are off */
 	/* Check if we need preemption */
-	bt	$9, EFLAGS(%rsp)		/* were interrupts off? */
+	btl	$9, EFLAGS(%rsp)		/* were interrupts off? */
 	jnc	1f
 0:	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	1f

[tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[tip-bot for Jan Beulich]

Commit-ID:  6709812f094d96543b443645c68daaa32d3d3e77
Gitweb:     https://git.kernel.org/tip/6709812f094d96543b443645c68daaa32d3d3e77
Author:     Jan Beulich <JBeulich@suse.com>
AuthorDate: Mon, 2 Jul 2018 04:47:57 -0600
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 3 Jul 2018 09:59:29 +0200

x86/entry/64: Add two more instruction suffixes

Sadly, other than claimed in:

  a368d7fd2a ("x86/entry/64: Add instruction suffix")

... there are two more instances which want to be adjusted.

As said there, omitting suffixes from instructions in AT&T mode is bad
practice when operand size cannot be determined by the assembler from
register operands, and is likely going to be warned about by upstream
gas in the future (mine does already).

Add the other missing suffixes here as well.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/5B3A02DD02000078001CFB78@prv1-mh.provo.novell.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index c6f3677e6105..65aa16d845f6 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -92,7 +92,7 @@ END(native_usergs_sysret64)
 .endm
 
 .macro TRACE_IRQS_IRETQ_DEBUG
-	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
+	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
 	jnc	1f
 	TRACE_IRQS_ON_DEBUG
 1:
@@ -702,7 +702,7 @@ retint_kernel:
 #ifdef CONFIG_PREEMPT
 	/* Interrupts are off */
 	/* Check if we need preemption */
-	bt	$9, EFLAGS(%rsp)		/* were interrupts off? */
+	btl	$9, EFLAGS(%rsp)		/* were interrupts off? */
 	jnc	1f
 0:	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	1f

RE: [tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[David Laight]

From: Jan Beulich
> Sent: 03 July 2018 09:36
...
> As said there, omitting suffixes from instructions in AT&T mode is bad
> practice when operand size cannot be determined by the assembler from
> register operands, and is likely going to be warned about by upstream
> gas in the future (mine does already).
...
> -	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
> +	btl	$9, EFLAGS(%rsp)		/* interrupts off? */

Hmmm....
Does the operand size make any difference at all for the bit instructions?
I'm pretty sure that the cpus (386 onwards) have always done aligned 32bit
transfers (the docs never actually said aligned).
I can't remember whether 64bit mode allows immediates above 31.

So gas accepting 'btb $n,memory' is giving a false impression of
what actually happens.

	David

RE: [tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[Jan Beulich]

>>> On 03.07.18 at 10:46, <David.Laight@ACULAB.COM> wrote:
> From: Jan Beulich
>> Sent: 03 July 2018 09:36
> ...
>> As said there, omitting suffixes from instructions in AT&T mode is bad
>> practice when operand size cannot be determined by the assembler from
>> register operands, and is likely going to be warned about by upstream
>> gas in the future (mine does already).
> ...
>> -	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
>> +	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
> 
> Hmmm....
> Does the operand size make any difference at all for the bit instructions?
> I'm pretty sure that the cpus (386 onwards) have always done aligned 32bit
> transfers (the docs never actually said aligned).
> I can't remember whether 64bit mode allows immediates above 31.
> 
> So gas accepting 'btb $n,memory' is giving a false impression of
> what actually happens.

BTB does not exist at all. BTW and (on 64-bit) BTQ do exist though,
and they have behavior differing from BTL. The only AT&T syntax doc
I have says that L is the default suffix to be used, but there are cases
where this wasn't (and maybe still isn't) the case, so omitting a suffix
when register operands aren't available to size instructions has always
been a risky game.

Jan

RE: [tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[David Laight]

From: Jan Beulich
> Sent: 03 July 2018 11:07
> >>> On 03.07.18 at 10:46, <David.Laight@ACULAB.COM> wrote:
> > From: Jan Beulich
> >> Sent: 03 July 2018 09:36
> > ...
> >> As said there, omitting suffixes from instructions in AT&T mode is bad
> >> practice when operand size cannot be determined by the assembler from
> >> register operands, and is likely going to be warned about by upstream
> >> gas in the future (mine does already).
> > ...
> >> -	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
> >> +	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
> >
> > Hmmm....
> > Does the operand size make any difference at all for the bit instructions?
> > I'm pretty sure that the cpus (386 onwards) have always done aligned 32bit
> > transfers (the docs never actually said aligned).
> > I can't remember whether 64bit mode allows immediates above 31.
> >
> > So gas accepting 'btb $n,memory' is giving a false impression of
> > what actually happens.
> 
> BTB does not exist at all. BTW and (on 64-bit) BTQ do exist though,
> and they have behavior differing from BTL. The only AT&T syntax doc
> I have says that L is the default suffix to be used, but there are cases
> where this wasn't (and maybe still isn't) the case, so omitting a suffix
> when register operands aren't available to size instructions has always
> been a risky game.

I've just looked it up :-)
There are only 16bit, 32bit and 64bit variants.
They mostly affect how much of %cx is used for the bit offset.
With the addition 'may' clauses below - which are more explicit
than they used to be.

"When accessing a bit in memory, the processor may access 4 bytes starting
from the memory address for a 32-bit operand size, using by the following
relationship:
    Effective Address + (4 * (BitOffset DIV 32))

Or, it may access 2 bytes starting from the memory address for a 16-bit
operand, using this relationship:
    Effective Address + (2 * (BitOffset DIV 16))

It may do so even when only a single byte needs to be accessed to reach
the given bit. When using this bit addressing mechanism, software should
avoid referencing areas of memory close to address space holes."
(A strange qualifying clause.)

	David

Re: [tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[Denys Vlasenko]

On 07/03/2018 10:46 AM, David Laight wrote:
> From: Jan Beulich
>> Sent: 03 July 2018 09:36
> ...
>> As said there, omitting suffixes from instructions in AT&T mode is bad
>> practice when operand size cannot be determined by the assembler from
>> register operands, and is likely going to be warned about by upstream
>> gas in the future (mine does already).
> ...
>> -	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
>> +	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
> 
> Hmmm....
> Does the operand size make any difference at all for the bit instructions?
> I'm pretty sure that the cpus (386 onwards) have always done aligned 32bit
> transfers (the docs never actually said aligned).
> I can't remember whether 64bit mode allows immediates above 31.

Immediates up to 63 are allowed in 64 bit mode (IOW: for REX-prefixed form)
(run-tested).

Keep in mind that this instruction is "special" with register bit offset:

Register/memory form (BT REG,[MEM]) does not limit or mask the value of bit offset
in REG, the instruction uses bit REG%8 in byte at address [MEM+REG/8].

This works correctly even for negative values: REG = -1 will access
the most significant bit in the byte immediately before MEM.

Thus, for accesses of standard RAM locations (not memory-mapped IO and such),
the "operand size" concept for this instruction (and BTC, BTR, BTS)
does not make much sense: it accesses one bit. The width of actual memory
access is irrelevant.

I'd say assembler should just use the "natural" width for current mode
(16 or 32-bit), and warn when code tries to use immediate operand which
will be truncated and thus needs a wider operand size.

Intel documentation says that immediate operand in BT IMM,[MEM]
is truncated to operand size. My experiment seems to confirm it:

254:1  <- BT 254,[MEM] actually accesses bit #30, not #254
255:0
254:0
255:0

#include <string.h>
#include <stdio.h>
int main(int argc, char **argv, char **envp)
{
         char buf[256];
         int result;

         memset(buf, 0x55, sizeof(buf)); /* bit pattern: 01010101 */

         buf[255/8] = 0;
         asm("\n"
         "       bt      %1, %2\n"
         "       sbb     %0, %0\n"
         : "=r" (result)
         : "i" (254), "m" (buf)
         );
         printf("254:%x\n", !!result);
         asm("\n"
         "       bt      %1, %2\n"
         "       sbb     %0, %0\n"
         : "=r" (result)
         : "i" (255), "m" (buf)
         );
         printf("255:%x\n", !!result);
         buf[255/8] = 0x55;

         buf[31/8] = 0;
         asm("\n"
         "       bt      %1, %2\n"
         "       sbb     %0, %0\n"
         : "=r" (result)
         : "i" (254), "m" (buf)
         );
         printf("254:%x\n", !!result);
         asm("\n"
         "       bt      %1, %2\n"
         "       sbb     %0, %0\n"
         : "=r" (result)
         : "i" (255), "m" (buf)
         );
         printf("255:%x\n", !!result);

         return 0;
}

When I use "r" instead of "i" to generate REG,[MEM] form,
the instruction does access bit #254:

254:0
255:0
254:1
255:0

RE: [tip:x86/asm] x86/entry/64: Add two more instruction suffixes

[David Laight]

From: Denys Vlasenko
> Sent: 03 July 2018 12:59
> 
> On 07/03/2018 10:46 AM, David Laight wrote:
> > From: Jan Beulich
> >> Sent: 03 July 2018 09:36
> > ...
> >> As said there, omitting suffixes from instructions in AT&T mode is bad
> >> practice when operand size cannot be determined by the assembler from
> >> register operands, and is likely going to be warned about by upstream
> >> gas in the future (mine does already).
> > ...
> >> -	bt	$9, EFLAGS(%rsp)		/* interrupts off? */
> >> +	btl	$9, EFLAGS(%rsp)		/* interrupts off? */
> >
> > Hmmm....
> > Does the operand size make any difference at all for the bit instructions?
> > I'm pretty sure that the cpus (386 onwards) have always done aligned 32bit
> > transfers (the docs never actually said aligned).
> > I can't remember whether 64bit mode allows immediates above 31.
> 
> Immediates up to 63 are allowed in 64 bit mode (IOW: for REX-prefixed form)
> (run-tested).
> 
> Keep in mind that this instruction is "special" with register bit offset:
> 
> Register/memory form (BT REG,[MEM]) does not limit or mask the value of bit offset
> in REG, the instruction uses bit REG%8 in byte at address [MEM+REG/8].
> 
> This works correctly even for negative values: REG = -1 will access
> the most significant bit in the byte immediately before MEM.
> 
> Thus, for accesses of standard RAM locations (not memory-mapped IO and such),
> the "operand size" concept for this instruction (and BTC, BTR, BTS)
> does not make much sense: it accesses one bit. The width of actual memory
> access is irrelevant.
> 
> I'd say assembler should just use the "natural" width for current mode
> (16 or 32-bit), and warn when code tries to use immediate operand which
> will be truncated and thus needs a wider operand size.
...


Indeed.

Truncating the immediate value really ought to be an error.
In 64bit mode the 64bit form should be used for immediates [32..63).
The 16bit form could safely be selected for immediates [0..15).

Requiring the width suffix is just stupid.

	David