diff mbox

[Xen-devel] expecting string instruction after `rep' with latest SeaBIOS

Message ID 1404133911.14488.59.camel@kazak.uk.xensource.com
State New
Headers show

Commit Message

Ian Campbell June 30, 2014, 1:11 p.m. UTC 
On Mon, 2014-06-30 at 15:07 +0200, Paolo Bonzini wrote:
> Il 30/06/2014 15:03, Ian Campbell ha scritto:
> > That line is:
> >                 // Acquire lock and take ownership of shared stack
> >         1:      rep nop
> >
> > I've also checked the preprocessed version and the nop isn't being
> > disappeared or anything like that.
> 
> "rep nop" is really just "pause".

Yes, that's how I was translating it in my head, which is why I thought
it was fine to write it like that.

> I think the assembler wants "rep; nop" instead.

That's right, I was just reaching the same conclusion. Build-tested
patch:

8<------------------------

From 0b05042b582a8992c15030406dcb220be82da8ce Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 30 Jun 2014 14:10:02 +0100
Subject: [PATCH] romlayout: Use "rep ; nop" not "rep nop".

Fixes:
  Compiling (16bit) out/romlayout.o
src/romlayout.S: Assembler messages:
src/romlayout.S:285: Error: expecting string instruction after `rep'
make: *** [out/romlayout.o] Error 1

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 src/romlayout.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Bonzini June 30, 2014, 4:18 p.m. UTC | #1 
Il 30/06/2014 15:11, Ian Campbell ha scritto:
> On Mon, 2014-06-30 at 15:07 +0200, Paolo Bonzini wrote:
>> Il 30/06/2014 15:03, Ian Campbell ha scritto:
>>> That line is:
>>>                 // Acquire lock and take ownership of shared stack
>>>         1:      rep nop
>>>
>>> I've also checked the preprocessed version and the nop isn't being
>>> disappeared or anything like that.
>>
>> "rep nop" is really just "pause".
>
> Yes, that's how I was translating it in my head, which is why I thought
> it was fine to write it like that.
>
>> I think the assembler wants "rep; nop" instead.
>
> That's right, I was just reaching the same conclusion. Build-tested
> patch:
>
> 8<------------------------
>
> From 0b05042b582a8992c15030406dcb220be82da8ce Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Mon, 30 Jun 2014 14:10:02 +0100
> Subject: [PATCH] romlayout: Use "rep ; nop" not "rep nop".
>
> Fixes:
>   Compiling (16bit) out/romlayout.o
> src/romlayout.S: Assembler messages:
> src/romlayout.S:285: Error: expecting string instruction after `rep'
> make: *** [out/romlayout.o] Error 1
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  src/romlayout.S |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/romlayout.S b/src/romlayout.S
> index a931b32..a3ba965 100644
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -282,7 +282,7 @@ entry_smp:
>          jmp transition32
>          .code32
>          // Acquire lock and take ownership of shared stack
> -1:      rep nop
> +1:      rep ; nop
>  2:      lock btsl $0, SMPLock
>          jc 1b
>          movl SMPStack, %esp
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Kevin O'Connor July 1, 2014, 4:05 a.m. UTC | #2 
On Mon, Jun 30, 2014 at 02:11:51PM +0100, Ian Campbell wrote:
> > I think the assembler wants "rep; nop" instead.
> 
> That's right, I was just reaching the same conclusion. Build-tested
> patch:

Thanks - I applied this change.

-Kevin
diff mbox

Patch

diff --git a/src/romlayout.S b/src/romlayout.S
index a931b32..a3ba965 100644
--- a/src/romlayout.S
+++ b/src/romlayout.S
@@ -282,7 +282,7 @@  entry_smp:
         jmp transition32
         .code32
         // Acquire lock and take ownership of shared stack
-1:      rep nop
+1:      rep ; nop
 2:      lock btsl $0, SMPLock
         jc 1b
         movl SMPStack, %esp