From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: x86: fix rdrand asm() Date: Wed, 25 Sep 2013 16:52:35 +0100 Message-ID: <524306C3.2090102@citrix.com> References: <5243218C02000078000F66E5@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0164912313002149661==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VOrOM-00074N-RH for xen-devel@lists.xenproject.org; Wed, 25 Sep 2013 15:52:43 +0000 In-Reply-To: <5243218C02000078000F66E5@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============0164912313002149661== Content-Type: multipart/alternative; boundary="------------020302020807050601090903" --------------020302020807050601090903 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 25/09/13 16:46, Jan Beulich wrote: > Just learned the hard way that at least for non-volatile asm()s gcc > indeed does what the documentation says: It may move it across jumps > (i.e. ahead of the cpu_has() check). While the documentation claims > that this can also happen for volatile asm()s, if that was the case > we'd have many more problems in our code (and e,g, Linux would too). > > Signed-off-by: Jan Beulich > > --- a/xen/include/asm-x86/random.h > +++ b/xen/include/asm-x86/random.h > @@ -8,7 +8,7 @@ static inline unsigned int arch_get_rand > unsigned int val = 0; > > if ( cpu_has(¤t_cpu_data, X86_FEATURE_RDRAND) ) > - asm ( ".byte 0x0f,0xc7,0xf0" : "+a" (val) ); > + __asm__ __volatile__ ( ".byte 0x0f,0xc7,0xf0" : "+a" (val) ); Any reason for using the double underscore versions? They have identical meanings, and the prevailing style does appear to be without. Either way, Reviewed-by: Andrew Cooper > > return val; > } > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------020302020807050601090903 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 25/09/13 16:46, Jan Beulich wrote:
Just learned the hard way that at least for non-volatile asm()s gcc
indeed does what the documentation says: It may move it across jumps
(i.e. ahead of the cpu_has() check). While the documentation claims
that this can also happen for volatile asm()s, if that was the case
we'd have many more problems in our code (and e,g, Linux would too).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/random.h
+++ b/xen/include/asm-x86/random.h
@@ -8,7 +8,7 @@ static inline unsigned int arch_get_rand
     unsigned int val = 0;
 
     if ( cpu_has(&current_cpu_data, X86_FEATURE_RDRAND) )
-        asm ( ".byte 0x0f,0xc7,0xf0" : "+a" (val) );
+        __asm__ __volatile__ ( ".byte 0x0f,0xc7,0xf0" : "+a" (val) );

Any reason for using the double underscore versions?  They have identical meanings, and the prevailing style does appear to be without.

Either way,

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

 
     return val;
 }






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------020302020807050601090903-- --===============0164912313002149661== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============0164912313002149661==--