x86/boot: Avoid using Intel mnemonics in AT&T syntax asm

Peter Zijlstra posted 1 patch 2 years, 8 months ago
There is a newer version of this series
arch/x86/boot/bioscall.S |    4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by Peter Zijlstra 2 years, 8 months ago

With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
build now reports:

  arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
  arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
  arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant

  arch/x86/boot/bioscall.S: Assembler messages:
  arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
  arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant

Which is due to:

  PR gas/29525

  Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
  templates taking operands, mixed IsString/non-IsString template groups
  (with memory operands) cannot occur anymore. With that
  maybe_adjust_templates() becomes unnecessary (and is hence being
  removed).

More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525

Fixes: 7a734e7dd93b ("x86, setup: "glove box" BIOS calls -- infrastructure")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/boot/bioscall.S |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/boot/bioscall.S
+++ b/arch/x86/boot/bioscall.S
@@ -32,7 +32,7 @@
 	movw	%dx, %si
 	movw	%sp, %di
 	movw	$11, %cx
-	rep; movsd
+	rep; movsl
 
 	/* Pop full state from the stack */
 	popal
@@ -67,7 +67,7 @@
 	jz	4f
 	movw	%sp, %si
 	movw	$11, %cx
-	rep; movsd
+	rep; movsl
 4:	addw	$44, %sp
 
 	/* Restore state and return */
Re: x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by Borislav Petkov 2 years, 8 months ago
On Tue, Jan 10, 2023 at 12:15:40PM +0100, Peter Zijlstra wrote:
> 
> With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
> build now reports:
> 
>   arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
>   arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>   arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> 
>   arch/x86/boot/bioscall.S: Assembler messages:
>   arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>   arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> 
> Which is due to:
> 
>   PR gas/29525
> 
>   Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
>   templates taking operands, mixed IsString/non-IsString template groups
>   (with memory operands) cannot occur anymore. With that
>   maybe_adjust_templates() becomes unnecessary (and is hence being
>   removed).
> 
> More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525

Right, I'm being told the particular problem here is is that the 'd' suffix is
"conflicting" in the sense that you can have SSE mnemonics like movsD %xmm...
and the same thing also for string ops (which is the case here) so apparently
the agreement in binutils land is to use the always accepted suffixes 'l' or 'q'
and phase out 'd' slowly...

Which is basically what the PR text says above but more understanable. :-)

Might wanna add that to the commit message.

> Fixes: 7a734e7dd93b ("x86, setup: "glove box" BIOS calls -- infrastructure")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/boot/bioscall.S |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

In any case

Acked-by: Borislav Petkov (AMD) <bp@alien8.de>

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by David Laight 2 years, 8 months ago
From: Borislav Petkov
> Sent: 10 January 2023 11:37
> 
> On Tue, Jan 10, 2023 at 12:15:40PM +0100, Peter Zijlstra wrote:
> >
> > With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
> > build now reports:
> >
> >   arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
> >   arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
> >   arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> >
> >   arch/x86/boot/bioscall.S: Assembler messages:
> >   arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
> >   arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> >
> > Which is due to:
> >
> >   PR gas/29525
> >
> >   Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
> >   templates taking operands, mixed IsString/non-IsString template groups
> >   (with memory operands) cannot occur anymore. With that
> >   maybe_adjust_templates() becomes unnecessary (and is hence being
> >   removed).
> >
> > More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525
> 
> Right, I'm being told the particular problem here is is that the 'd' suffix is
> "conflicting" in the sense that you can have SSE mnemonics like movsD %xmm...
> and the same thing also for string ops (which is the case here) so apparently
> the agreement in binutils land is to use the always accepted suffixes 'l' or 'q'
> and phase out 'd' slowly...
> 
> Which is basically what the PR text says above but more understanable. :-)
> 
> Might wanna add that to the commit message.

Can they be changed to movsq ?
movsl has an implied 32bitness about it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Re: x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by Ingo Molnar 2 years, 8 months ago
* Peter Zijlstra <peterz@infradead.org> wrote:

> 
> With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
> build now reports:
> 
>   arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
>   arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>   arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> 
>   arch/x86/boot/bioscall.S: Assembler messages:
>   arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>   arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
> 
> Which is due to:
> 
>   PR gas/29525
> 
>   Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
>   templates taking operands, mixed IsString/non-IsString template groups
>   (with memory operands) cannot occur anymore. With that
>   maybe_adjust_templates() becomes unnecessary (and is hence being
>   removed).
> 
> More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525
> 
> Fixes: 7a734e7dd93b ("x86, setup: "glove box" BIOS calls -- infrastructure")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/boot/bioscall.S |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/boot/bioscall.S
> +++ b/arch/x86/boot/bioscall.S
> @@ -32,7 +32,7 @@
>  	movw	%dx, %si
>  	movw	%sp, %di
>  	movw	$11, %cx
> -	rep; movsd
> +	rep; movsl
>  
>  	/* Pop full state from the stack */
>  	popal
> @@ -67,7 +67,7 @@
>  	jz	4f
>  	movw	%sp, %si
>  	movw	$11, %cx
> -	rep; movsd
> +	rep; movsl
>  4:	addw	$44, %sp

So I think the GAS change to introduce this warning was probably 
unnecessary - these instructions weren't really causing any trouble and the 
syntax was basically a legacy thing that shouldn't be touched - but I guess 
that argument is water down the bridge now:

   Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo
Re: x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by H. Peter Anvin 2 years, 8 months ago
On January 10, 2023 3:35:50 AM PST, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Peter Zijlstra <peterz@infradead.org> wrote:
>
>> 
>> With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
>> build now reports:
>> 
>>   arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
>>   arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>>   arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
>> 
>>   arch/x86/boot/bioscall.S: Assembler messages:
>>   arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
>>   arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant
>> 
>> Which is due to:
>> 
>>   PR gas/29525
>> 
>>   Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
>>   templates taking operands, mixed IsString/non-IsString template groups
>>   (with memory operands) cannot occur anymore. With that
>>   maybe_adjust_templates() becomes unnecessary (and is hence being
>>   removed).
>> 
>> More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525
>> 
>> Fixes: 7a734e7dd93b ("x86, setup: "glove box" BIOS calls -- infrastructure")
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> ---
>>  arch/x86/boot/bioscall.S |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> --- a/arch/x86/boot/bioscall.S
>> +++ b/arch/x86/boot/bioscall.S
>> @@ -32,7 +32,7 @@
>>  	movw	%dx, %si
>>  	movw	%sp, %di
>>  	movw	$11, %cx
>> -	rep; movsd
>> +	rep; movsl
>>  
>>  	/* Pop full state from the stack */
>>  	popal
>> @@ -67,7 +67,7 @@
>>  	jz	4f
>>  	movw	%sp, %si
>>  	movw	$11, %cx
>> -	rep; movsd
>> +	rep; movsl
>>  4:	addw	$44, %sp
>
>So I think the GAS change to introduce this warning was probably 
>unnecessary - these instructions weren't really causing any trouble and the 
>syntax was basically a legacy thing that shouldn't be touched - but I guess 
>that argument is water down the bridge now:
>
>   Reviewed-by: Ingo Molnar <mingo@kernel.org>
>
>Thanks,
>
>	Ingo

Yeah; looks like a gas bug/regression, but there isn't really any reason not to fix it.

The semicolon after rep isn't needed anymore either ;)

Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com>
[tip: x86/urgent] x86/boot: Avoid using Intel mnemonics in AT&T syntax asm
Posted by tip-bot2 for Peter Zijlstra 2 years, 8 months ago
The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     7c6dd961d0c8e7e8f9fdc65071fb09ece702e18d
Gitweb:        https://git.kernel.org/tip/7c6dd961d0c8e7e8f9fdc65071fb09ece702e18d
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 10 Jan 2023 12:15:40 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 10 Jan 2023 13:03:23 +01:00

x86/boot: Avoid using Intel mnemonics in AT&T syntax asm

With 'GNU assembler (GNU Binutils for Debian) 2.39.90.20221231' the
build now reports:

  arch/x86/realmode/rm/../../boot/bioscall.S: Assembler messages:
  arch/x86/realmode/rm/../../boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
  arch/x86/realmode/rm/../../boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant

  arch/x86/boot/bioscall.S: Assembler messages:
  arch/x86/boot/bioscall.S:35: Warning: found `movsd'; assuming `movsl' was meant
  arch/x86/boot/bioscall.S:70: Warning: found `movsd'; assuming `movsl' was meant

Which is due to:

  PR gas/29525

  Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
  templates taking operands, mixed IsString/non-IsString template groups
  (with memory operands) cannot occur anymore. With that
  maybe_adjust_templates() becomes unnecessary (and is hence being
  removed).

More details: https://sourceware.org/bugzilla/show_bug.cgi?id=29525

Borislav Petkov further explains:

  " the particular problem here is is that the 'd' suffix is
    "conflicting" in the sense that you can have SSE mnemonics like movsD %xmm...
    and the same thing also for string ops (which is the case here) so apparently
    the agreement in binutils land is to use the always accepted suffixes 'l' or 'q'
    and phase out 'd' slowly... "

Fixes: 7a734e7dd93b ("x86, setup: "glove box" BIOS calls -- infrastructure")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/Y71I3Ex2pvIxMpsP@hirez.programming.kicks-ass.net
---
 arch/x86/boot/bioscall.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/bioscall.S b/arch/x86/boot/bioscall.S
index 5521ea1..aa9b964 100644
--- a/arch/x86/boot/bioscall.S
+++ b/arch/x86/boot/bioscall.S
@@ -32,7 +32,7 @@ intcall:
 	movw	%dx, %si
 	movw	%sp, %di
 	movw	$11, %cx
-	rep; movsd
+	rep; movsl
 
 	/* Pop full state from the stack */
 	popal
@@ -67,7 +67,7 @@ intcall:
 	jz	4f
 	movw	%sp, %si
 	movw	$11, %cx
-	rep; movsd
+	rep; movsl
 4:	addw	$44, %sp
 
 	/* Restore state and return */