All of lore.kernel.org
 help / color / mirror / Atom feed
* [x86 setup] Fix assembly constraints
@ 2007-07-13 22:09 H. Peter Anvin
  2007-07-13 22:20 ` Chuck Ebbert
  0 siblings, 1 reply; 2+ messages in thread
From: H. Peter Anvin @ 2007-07-13 22:09 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, H. Peter Anvin, Chuck Ebbert

Fix incorrect assembly constraints.  In particular, fix memory
constraints used inside push..pop, which can cause invalid operation
since gcc may generate %esp-relative references.

Additionally:

outl() should have "dN" not "dn".

query_mca() shouldn't listen 16/32-bit registers in an 8-bit only
context.

has_eflag(): the "mask" is only used well after both the stack pointer
and the output registers have been touched; this requires the output
registers to be earlyclobbers (=&) and the input to exclude memory (so
"ri", not "g").

Thanks to Chuck Ebbert for prompting this review.

Cc: Chuck Ebbert <cebbert@redhat.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>

diff --git a/arch/i386/boot/boot.h b/arch/i386/boot/boot.h
index 0329c4f..dec70c9 100644
--- a/arch/i386/boot/boot.h
+++ b/arch/i386/boot/boot.h
@@ -56,7 +56,7 @@ static inline u16 inw(u16 port)
 
 static inline void outl(u32 v, u16 port)
 {
-	asm volatile("outl %0,%1" : : "a" (v), "dn" (port));
+	asm volatile("outl %0,%1" : : "a" (v), "dN" (port));
 }
 static inline u32 inl(u32 port)
 {
diff --git a/arch/i386/boot/cpucheck.c b/arch/i386/boot/cpucheck.c
index 8b0f447..991e8ce 100644
--- a/arch/i386/boot/cpucheck.c
+++ b/arch/i386/boot/cpucheck.c
@@ -115,8 +115,8 @@ static int has_eflag(u32 mask)
 	    "pushfl ; "
 	    "popl %1 ; "
 	    "popfl"
-	    : "=r" (f0), "=r" (f1)
-	    : "g" (mask));
+	    : "=&r" (f0), "=&r" (f1)
+	    : "ri" (mask));
 
 	return !!((f0^f1) & mask);
 }
diff --git a/arch/i386/boot/mca.c b/arch/i386/boot/mca.c
index 9b68bd1..68222f2 100644
--- a/arch/i386/boot/mca.c
+++ b/arch/i386/boot/mca.c
@@ -26,7 +26,7 @@ int query_mca(void)
 	    "setc %0 ; "
 	    "movw %%es, %1 ; "
 	    "popw %%es"
-	    : "=acdSDm" (err), "=acdSDm" (es), "=b" (bx)
+	    : "=acd" (err), "=acdSD" (es), "=b" (bx)
 	    : "a" (0xc000));
 
 	if (err)
diff --git a/arch/i386/boot/pm.c b/arch/i386/boot/pm.c
index 3fa53e1..1df025c 100644
--- a/arch/i386/boot/pm.c
+++ b/arch/i386/boot/pm.c
@@ -65,7 +65,7 @@ static void move_kernel_around(void)
 			     "popw %%ds ; "
 			     "popw %%es"
 			     : "+c" (dwords)
-			     : "rm" (dst_seg), "rm" (src_seg)
+			     : "r" (dst_seg), "r" (src_seg)
 			     : "esi", "edi");
 
 		syssize -= paras;
diff --git a/arch/i386/boot/video.c b/arch/i386/boot/video.c
index 3bb3573..027a2c9 100644
--- a/arch/i386/boot/video.c
+++ b/arch/i386/boot/video.c
@@ -411,7 +411,7 @@ static void restore_screen(void)
 			     "1: rep;stosl ; "
 			     "popw %%es"
 			     : "+D" (dst), "+c" (npad)
-			     : "bdSm" (video_segment),
+			     : "bdS" (video_segment),
 			       "a" (0x07200720));
 	}
 
diff --git a/arch/i386/boot/voyager.c b/arch/i386/boot/voyager.c
index 9221614..61c8fe0 100644
--- a/arch/i386/boot/voyager.c
+++ b/arch/i386/boot/voyager.c
@@ -32,7 +32,7 @@ int query_voyager(void)
 	    "setc %0 ; "
 	    "movw %%es, %1 ; "
 	    "popw %%es"
-	    : "=qm" (err), "=rm" (es), "=D" (di)
+	    : "=q" (err), "=r" (es), "=D" (di)
 	    : "a" (0xffc0));
 
 	if (err)

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [x86 setup] Fix assembly constraints
  2007-07-13 22:09 [x86 setup] Fix assembly constraints H. Peter Anvin
@ 2007-07-13 22:20 ` Chuck Ebbert
  0 siblings, 0 replies; 2+ messages in thread
From: Chuck Ebbert @ 2007-07-13 22:20 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: torvalds, linux-kernel, Etienne Lorrain

On 07/13/2007 06:09 PM, H. Peter Anvin wrote:
> Fix incorrect assembly constraints.  In particular, fix memory
> constraints used inside push..pop, which can cause invalid operation
> since gcc may generate %esp-relative references.
> 
> Additionally:
> 
> outl() should have "dN" not "dn".
> 
> query_mca() shouldn't listen 16/32-bit registers in an 8-bit only
> context.
> 
> has_eflag(): the "mask" is only used well after both the stack pointer
> and the output registers have been touched; this requires the output
> registers to be earlyclobbers (=&) and the input to exclude memory (so
> "ri", not "g").
> 
> Thanks to Chuck Ebbert for prompting this review.
> 

That was Etienne Lorrain (cc'd), not me. All I did was reply
to Etienne's post and add cc:'s because the original only went
to linux-kernel.

> Cc: Chuck Ebbert <cebbert@redhat.com>
> Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> 
> diff --git a/arch/i386/boot/boot.h b/arch/i386/boot/boot.h
> index 0329c4f..dec70c9 100644
> --- a/arch/i386/boot/boot.h
> +++ b/arch/i386/boot/boot.h
> @@ -56,7 +56,7 @@ static inline u16 inw(u16 port)
>  
>  static inline void outl(u32 v, u16 port)
>  {
> -	asm volatile("outl %0,%1" : : "a" (v), "dn" (port));
> +	asm volatile("outl %0,%1" : : "a" (v), "dN" (port));
>  }
>  static inline u32 inl(u32 port)
>  {
> diff --git a/arch/i386/boot/cpucheck.c b/arch/i386/boot/cpucheck.c
> index 8b0f447..991e8ce 100644
> --- a/arch/i386/boot/cpucheck.c
> +++ b/arch/i386/boot/cpucheck.c
> @@ -115,8 +115,8 @@ static int has_eflag(u32 mask)
>  	    "pushfl ; "
>  	    "popl %1 ; "
>  	    "popfl"
> -	    : "=r" (f0), "=r" (f1)
> -	    : "g" (mask));
> +	    : "=&r" (f0), "=&r" (f1)
> +	    : "ri" (mask));
>  
>  	return !!((f0^f1) & mask);
>  }
> diff --git a/arch/i386/boot/mca.c b/arch/i386/boot/mca.c
> index 9b68bd1..68222f2 100644
> --- a/arch/i386/boot/mca.c
> +++ b/arch/i386/boot/mca.c
> @@ -26,7 +26,7 @@ int query_mca(void)
>  	    "setc %0 ; "
>  	    "movw %%es, %1 ; "
>  	    "popw %%es"
> -	    : "=acdSDm" (err), "=acdSDm" (es), "=b" (bx)
> +	    : "=acd" (err), "=acdSD" (es), "=b" (bx)
>  	    : "a" (0xc000));
>  
>  	if (err)
> diff --git a/arch/i386/boot/pm.c b/arch/i386/boot/pm.c
> index 3fa53e1..1df025c 100644
> --- a/arch/i386/boot/pm.c
> +++ b/arch/i386/boot/pm.c
> @@ -65,7 +65,7 @@ static void move_kernel_around(void)
>  			     "popw %%ds ; "
>  			     "popw %%es"
>  			     : "+c" (dwords)
> -			     : "rm" (dst_seg), "rm" (src_seg)
> +			     : "r" (dst_seg), "r" (src_seg)
>  			     : "esi", "edi");
>  
>  		syssize -= paras;
> diff --git a/arch/i386/boot/video.c b/arch/i386/boot/video.c
> index 3bb3573..027a2c9 100644
> --- a/arch/i386/boot/video.c
> +++ b/arch/i386/boot/video.c
> @@ -411,7 +411,7 @@ static void restore_screen(void)
>  			     "1: rep;stosl ; "
>  			     "popw %%es"
>  			     : "+D" (dst), "+c" (npad)
> -			     : "bdSm" (video_segment),
> +			     : "bdS" (video_segment),
>  			       "a" (0x07200720));
>  	}
>  
> diff --git a/arch/i386/boot/voyager.c b/arch/i386/boot/voyager.c
> index 9221614..61c8fe0 100644
> --- a/arch/i386/boot/voyager.c
> +++ b/arch/i386/boot/voyager.c
> @@ -32,7 +32,7 @@ int query_voyager(void)
>  	    "setc %0 ; "
>  	    "movw %%es, %1 ; "
>  	    "popw %%es"
> -	    : "=qm" (err), "=rm" (es), "=D" (di)
> +	    : "=q" (err), "=r" (es), "=D" (di)
>  	    : "a" (0xffc0));
>  
>  	if (err)
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-07-13 22:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-13 22:09 [x86 setup] Fix assembly constraints H. Peter Anvin
2007-07-13 22:20 ` Chuck Ebbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.