diff mbox

kvm-unittest: fix build with gcc 4.3.X and older

Message ID 20131016194653.GA10517@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Oct. 16, 2013, 7:46 p.m. UTC
Old GCC didn't let you reference variable by
number if it is listed with a specific register
constraint, on the assumption you can just
use the register name explicitly.

Build fails with errors like this:
a.c:6: error: invalid 'asm': invalid operand code 'd'

To fix, let's just use %eax %al etc.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 lib/x86/pci.c | 4 ++--
 x86/s3.c      | 4 ++--
 x86/vmexit.c  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Paolo Bonzini Oct. 17, 2013, 10:53 a.m. UTC | #1
Il 16/10/2013 21:46, Michael S. Tsirkin ha scritto:
> Old GCC didn't let you reference variable by
> number if it is listed with a specific register
> constraint, on the assumption you can just
> use the register name explicitly.

Tell us the truth, you made this up. :)  Who doesn't do that for invalid
asm error messages...

> Build fails with errors like this:
> a.c:6: error: invalid 'asm': invalid operand code 'd'
> 
> To fix, let's just use %eax %al etc.

The problem is that %d0 is wrong.  The "d" modifier is for "print
duplicated register operand for AVX instruction" (whatever that means)
according to GCC source code (gcc/config/i386/i386.md).

You need to use %k0 according to the same file.

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  lib/x86/pci.c | 4 ++--
>  x86/s3.c      | 4 ++--
>  x86/vmexit.c  | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/x86/pci.c b/lib/x86/pci.c
> index f95cd88..08f7ebf 100644
> --- a/lib/x86/pci.c
> +++ b/lib/x86/pci.c
> @@ -3,13 +3,13 @@
>  
>  static void outl(unsigned short port, unsigned val)
>  {
> -    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
> +    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
>  }
>  
>  static unsigned inl(unsigned short port)
>  {
>      unsigned data;
> -    asm volatile("inl %w1, %d0" : "=a"(data) : "Nd"(port));
> +    asm volatile("inl %w1, %%eax" : "=a"(data) : "Nd"(port));
>      return data;
>  }
>  static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg)
> diff --git a/x86/s3.c b/x86/s3.c
> index 1feb452..eeffa17 100644
> --- a/x86/s3.c
> +++ b/x86/s3.c
> @@ -149,8 +149,8 @@ static inline int rtc_in(u8 reg)
>  
>  static inline void rtc_out(u8 reg, u8 val)
>  {
> -    asm volatile("outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71"
> -		 : "+a"(reg) : "0"(reg), "ri"(val));
> +    asm volatile("outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71"
> +		 : "+a"(reg) : "ri"(val));

Are you sure about this?  The error message here is definitely not
"invalid operand code 'b'".

That said, the patch is correct.  It's only the commit message that is
wrong.

I'm not sure about Gleb's patch.  I'll reply to his message.

Paolo

>  }
>  
>  extern char resume_start, resume_end;
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index 3b945de..29bc582 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val)
>  
>  static void outl(unsigned short port, unsigned val)
>  {
> -    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
> +    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
>  }
>  
>  static unsigned int inb(unsigned short port)
>
Michael S. Tsirkin Oct. 17, 2013, 11:29 a.m. UTC | #2
On Thu, Oct 17, 2013 at 12:53:30PM +0200, Paolo Bonzini wrote:
> Il 16/10/2013 21:46, Michael S. Tsirkin ha scritto:
> > Old GCC didn't let you reference variable by
> > number if it is listed with a specific register
> > constraint, on the assumption you can just
> > use the register name explicitly.
> 
> Tell us the truth, you made this up. :)  Who doesn't do that for invalid
> asm error messages...

I guessed at the reason, yes.

> > Build fails with errors like this:
> > a.c:6: error: invalid 'asm': invalid operand code 'd'
> > 
> > To fix, let's just use %eax %al etc.
> 
> The problem is that %d0 is wrong.  The "d" modifier is for "print
> duplicated register operand for AVX instruction" (whatever that means)
> according to GCC source code (gcc/config/i386/i386.md).
> 
> You need to use %k0 according to the same file.

Aha!

> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  lib/x86/pci.c | 4 ++--
> >  x86/s3.c      | 4 ++--
> >  x86/vmexit.c  | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/x86/pci.c b/lib/x86/pci.c
> > index f95cd88..08f7ebf 100644
> > --- a/lib/x86/pci.c
> > +++ b/lib/x86/pci.c
> > @@ -3,13 +3,13 @@
> >  
> >  static void outl(unsigned short port, unsigned val)
> >  {
> > -    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
> > +    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
> >  }
> >  
> >  static unsigned inl(unsigned short port)
> >  {
> >      unsigned data;
> > -    asm volatile("inl %w1, %d0" : "=a"(data) : "Nd"(port));
> > +    asm volatile("inl %w1, %%eax" : "=a"(data) : "Nd"(port));
> >      return data;
> >  }
> >  static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg)
> > diff --git a/x86/s3.c b/x86/s3.c
> > index 1feb452..eeffa17 100644
> > --- a/x86/s3.c
> > +++ b/x86/s3.c
> > @@ -149,8 +149,8 @@ static inline int rtc_in(u8 reg)
> >  
> >  static inline void rtc_out(u8 reg, u8 val)
> >  {
> > -    asm volatile("outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71"
> > -		 : "+a"(reg) : "0"(reg), "ri"(val));
> > +    asm volatile("outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71"
> > +		 : "+a"(reg) : "ri"(val));
> 
> Are you sure about this?  The error message here is definitely not
> "invalid operand code 'b'".

This produced a different error about conflict in constrains I think,
Gleb saw it too.

> That said, the patch is correct.  It's only the commit message that is
> wrong.
> 
> I'm not sure about Gleb's patch.  I'll reply to his message.
> 
> Paolo
> 
> >  }
> >  
> >  extern char resume_start, resume_end;
> > diff --git a/x86/vmexit.c b/x86/vmexit.c
> > index 3b945de..29bc582 100644
> > --- a/x86/vmexit.c
> > +++ b/x86/vmexit.c
> > @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val)
> >  
> >  static void outl(unsigned short port, unsigned val)
> >  {
> > -    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
> > +    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
> >  }
> >  
> >  static unsigned int inb(unsigned short port)
> >
diff mbox

Patch

diff --git a/lib/x86/pci.c b/lib/x86/pci.c
index f95cd88..08f7ebf 100644
--- a/lib/x86/pci.c
+++ b/lib/x86/pci.c
@@ -3,13 +3,13 @@ 
 
 static void outl(unsigned short port, unsigned val)
 {
-    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
+    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
 }
 
 static unsigned inl(unsigned short port)
 {
     unsigned data;
-    asm volatile("inl %w1, %d0" : "=a"(data) : "Nd"(port));
+    asm volatile("inl %w1, %%eax" : "=a"(data) : "Nd"(port));
     return data;
 }
 static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg)
diff --git a/x86/s3.c b/x86/s3.c
index 1feb452..eeffa17 100644
--- a/x86/s3.c
+++ b/x86/s3.c
@@ -149,8 +149,8 @@  static inline int rtc_in(u8 reg)
 
 static inline void rtc_out(u8 reg, u8 val)
 {
-    asm volatile("outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71"
-		 : "+a"(reg) : "0"(reg), "ri"(val));
+    asm volatile("outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71"
+		 : "+a"(reg) : "ri"(val));
 }
 
 extern char resume_start, resume_end;
diff --git a/x86/vmexit.c b/x86/vmexit.c
index 3b945de..29bc582 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -26,7 +26,7 @@  static void outw(unsigned short port, unsigned val)
 
 static void outl(unsigned short port, unsigned val)
 {
-    asm volatile("outl %d0, %w1" : : "a"(val), "Nd"(port));
+    asm volatile("outl %%eax, %w1" : : "a"(val), "Nd"(port));
 }
 
 static unsigned int inb(unsigned short port)