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

Message ID Y71I3Ex2pvIxMpsP@hirez.programming.kicks-ass.net
State New
Headers
Series x86/boot: Avoid using Intel mnemonics in AT&T syntax asm |

Commit Message

Peter Zijlstra Jan. 10, 2023, 11:15 a.m. UTC
  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(-)
  

Comments

Ingo Molnar Jan. 10, 2023, 11:35 a.m. UTC | #1
* 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
  
Borislav Petkov Jan. 10, 2023, 11:36 a.m. UTC | #2
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.
  
David Laight Jan. 10, 2023, 2:01 p.m. UTC | #3
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)
  
H. Peter Anvin Jan. 11, 2023, 10:59 p.m. UTC | #4
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>
  

Patch

--- 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 */