asm-x86/msr.h for sanitized headers: clean it or punt it

asm-x86/msr.h for sanitized headers: clean it or punt it

[Mike Frysinger]

The current asm-x86/msr.h does not actually define anything for (!__KERNEL__
&& __i386__).  For x86_64, it fails to build due to u32/u64 types being used.
Simply not installing the header seems easiest to me.  Otherwise, x86_64 will
need sanitizing and i386 should have things added back, otherwise it's just a
pointless empty header.

Signed-Off-By: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/include/asm-x86/Kbuild b/include/asm-x86/Kbuild
index 12db5a1..6453804 100644
--- a/include/asm-x86/Kbuild
+++ b/include/asm-x86/Kbuild
@@ -4,7 +4,6 @@ header-y += boot.h
 header-y += bootparam.h
 header-y += debugreg.h
 header-y += ldt.h
-header-y += msr-index.h
 header-y += prctl.h
 header-y += ptrace-abi.h
 header-y += sigcontext32.h
@@ -14,7 +13,6 @@ header-y += vsyscall32.h
 unifdef-y += e820.h
 unifdef-y += ist.h
 unifdef-y += mce.h
-unifdef-y += msr.h
 unifdef-y += mtrr.h
 unifdef-y += page_32.h
 unifdef-y += page_64.h

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[H. Peter Anvin]

Mike Frysinger wrote:
> The current asm-x86/msr.h does not actually define anything for (!__KERNEL__
> && __i386__).  For x86_64, it fails to build due to u32/u64 types being used.
> Simply not installing the header seems easiest to me.  Otherwise, x86_64 will
> need sanitizing and i386 should have things added back, otherwise it's just a
> pointless empty header.
> 
> Signed-Off-By: Mike Frysinger <vapier@gentoo.org>

<asm/msr-index.h> is usable from userspace, <asm/msr.h> includes it for 
backwards compatibility.

Nothing else is wanted or needed.

	-hpa

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Andi Kleen]

Mike Frysinger <vapier@gentoo.org> writes:

> The current asm-x86/msr.h does not actually define anything for (!__KERNEL__
> && __i386__).  For x86_64, it fails to build due to u32/u64 types being used.
> Simply not installing the header seems easiest to me.  Otherwise, x86_64 will
> need sanitizing and i386 should have things added back, otherwise it's just a
> pointless empty header.

Please don't -- asm/msr.h is very useful in user space for rdtscll et.al.
I use it all the time in test programs.

-Andi

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On Monday 31 December 2007, Andi Kleen wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > The current asm-x86/msr.h does not actually define anything for
> > (!__KERNEL__ && __i386__).  For x86_64, it fails to build due to u32/u64
> > types being used. Simply not installing the header seems easiest to me. 
> > Otherwise, x86_64 will need sanitizing and i386 should have things added
> > back, otherwise it's just a pointless empty header.
>
> Please don't -- asm/msr.h is very useful in user space for rdtscll et.al.
> I use it all the time in test programs.

can you enumerate which functions need to actually go to userspace then ?  the 
__KERNEL__ markings are a crapshoot at the moment.  you must also be jumping 
through hoops in order to use the header in the first place since the vanilla 
one wont compile out of the box.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Andi Kleen]

> can you enumerate which functions need to actually go to userspace then ?  the 
rdtscl/rdtscll/rdpmc primarily

> __KERNEL__ markings are a crapshoot at the moment.  you must also be jumping 
> through hoops in order to use the header in the first place since the vanilla 
> one wont compile out of the box.

At least the 64bit one or older 32bit one works without hoops. Agreed that the 
paravirt changes destroyed it on newer 32bit though. That's an unfortunate
regression.

-Andi

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Mike Frysinger]

On Monday 31 December 2007, Andi Kleen wrote:
> > can you enumerate which functions need to actually go to userspace then ?
> >  the
>
> rdtscl/rdtscll/rdpmc primarily

i'll just mark out the useless C funcs then

> > __KERNEL__ markings are a crapshoot at the moment.  you must also be
> > jumping through hoops in order to use the header in the first place since
> > the vanilla one wont compile out of the box.
>
> At least the 64bit one or older 32bit one works without hoops. Agreed that
> the paravirt changes destroyed it on newer 32bit though. That's an
> unfortunate regression.

i dont see how when the exported code has prototypes with "u32" types in them.
but at any rate ...
---
Use __asm__ and __volatile__ in code that is exported to userspace.  Wrap
kernel functions with __KERNEL__ so they get scrubbed.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/include/asm-x86/msr.h b/include/asm-x86/msr.h
index ba4b314..ffb9319 100644
--- a/include/asm-x86/msr.h
+++ b/include/asm-x86/msr.h
@@ -193,7 +193,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 
 /* wrmsr with exception handling */
 #define wrmsr_safe(msr,a,b) ({ int ret__;			\
-	asm volatile("2: wrmsr ; xorl %0,%0\n"			\
+	__asm__ __volatile__("2: wrmsr ; xorl %0,%0\n"			\
 		     "1:\n\t"					\
 		     ".section .fixup,\"ax\"\n\t"		\
 		     "3:  movl %4,%0 ; jmp 1b\n\t"		\
@@ -210,7 +210,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 
 #define rdmsr_safe(msr,a,b) \
 	({ int ret__;						\
-	  asm volatile ("1:       rdmsr\n"			\
+	  __asm__ __volatile__ ("1:       rdmsr\n"			\
 			"2:\n"					\
 			".section .fixup,\"ax\"\n"		\
 			"3:       movl %4,%0\n"			\
@@ -230,17 +230,17 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
      __asm__ __volatile__ ("rdtsc" : "=a" (low) : : "edx")
 
 #define rdtscp(low,high,aux) \
-     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
+     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
 
 #define rdtscll(val) do { \
      unsigned int __a,__d; \
-     asm volatile("rdtsc" : "=a" (__a), "=d" (__d)); \
+     __asm__ __volatile__("rdtsc" : "=a" (__a), "=d" (__d)); \
      (val) = ((unsigned long)__a) | (((unsigned long)__d)<<32); \
 } while(0)
 
 #define rdtscpll(val, aux) do { \
      unsigned long __a, __d; \
-     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
+     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
      (val) = (__d << 32) | __a; \
 } while (0)
 
@@ -320,6 +320,7 @@ static inline unsigned int cpuid_edx(unsigned int op)
 	return edx;
 }
 
+#ifdef __KERNEL__
 #ifdef CONFIG_SMP
 void rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);
 void wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h);
@@ -343,6 +344,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 	return wrmsr_safe(msr_no, l, h);
 }
 #endif  /* CONFIG_SMP */
+#endif  /* __KERNEL__ */
 #endif  /* __ASSEMBLY__ */
 
 #endif  /* !__i386__ */

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[H. Peter Anvin]

Mike Frysinger wrote:
>> ---
> Use __asm__ and __volatile__ in code that is exported to userspace.  Wrap
> kernel functions with __KERNEL__ so they get scrubbed.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> diff --git a/include/asm-x86/msr.h b/include/asm-x86/msr.h
> index ba4b314..ffb9319 100644
> --- a/include/asm-x86/msr.h
> +++ b/include/asm-x86/msr.h
> @@ -193,7 +193,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
>  
>  /* wrmsr with exception handling */
>  #define wrmsr_safe(msr,a,b) ({ int ret__;			\
> -	asm volatile("2: wrmsr ; xorl %0,%0\n"			\
> +	__asm__ __volatile__("2: wrmsr ; xorl %0,%0\n"			\
>  		     "1:\n\t"					\
>  		     ".section .fixup,\"ax\"\n\t"		\
>  		     "3:  movl %4,%0 ; jmp 1b\n\t"		\

rdmsr_safe/wrmsr_safe are definitely kernel-only.

	-hpa

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Mike Frysinger]

Use __asm__ and __volatile__ in code that is exported to userspace.  Wrap
kernel functions with __KERNEL__ so they get scrubbed.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/include/asm-x86/msr.h b/include/asm-x86/msr.h
index ba4b314..664a2fa 100644
--- a/include/asm-x86/msr.h
+++ b/include/asm-x86/msr.h
@@ -191,38 +191,6 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 
 #define wrmsrl(msr,val) wrmsr(msr,(__u32)((__u64)(val)),((__u64)(val))>>32)
 
-/* wrmsr with exception handling */
-#define wrmsr_safe(msr,a,b) ({ int ret__;			\
-	asm volatile("2: wrmsr ; xorl %0,%0\n"			\
-		     "1:\n\t"					\
-		     ".section .fixup,\"ax\"\n\t"		\
-		     "3:  movl %4,%0 ; jmp 1b\n\t"		\
-		     ".previous\n\t"				\
-		     ".section __ex_table,\"a\"\n"		\
-		     "   .align 8\n\t"				\
-		     "   .quad	2b,3b\n\t"			\
-		     ".previous"				\
-		     : "=a" (ret__)				\
-		     : "c" (msr), "0" (a), "d" (b), "i" (-EFAULT)); \
-	ret__; })
-
-#define checking_wrmsrl(msr,val) wrmsr_safe(msr,(u32)(val),(u32)((val)>>32))
-
-#define rdmsr_safe(msr,a,b) \
-	({ int ret__;						\
-	  asm volatile ("1:       rdmsr\n"			\
-			"2:\n"					\
-			".section .fixup,\"ax\"\n"		\
-			"3:       movl %4,%0\n"			\
-			" jmp 2b\n"				\
-			".previous\n"				\
-			".section __ex_table,\"a\"\n"		\
-			" .align 8\n"				\
-			" .quad 1b,3b\n"				\
-			".previous":"=&bDS" (ret__), "=a"(*(a)), "=d"(*(b)) \
-			:"c"(msr), "i"(-EIO), "0"(0));			\
-	  ret__; })
-
 #define rdtsc(low,high) \
      __asm__ __volatile__("rdtsc" : "=a" (low), "=d" (high))
 
@@ -230,17 +198,17 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
      __asm__ __volatile__ ("rdtsc" : "=a" (low) : : "edx")
 
 #define rdtscp(low,high,aux) \
-     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
+     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
 
 #define rdtscll(val) do { \
      unsigned int __a,__d; \
-     asm volatile("rdtsc" : "=a" (__a), "=d" (__d)); \
+     __asm__ __volatile__("rdtsc" : "=a" (__a), "=d" (__d)); \
      (val) = ((unsigned long)__a) | (((unsigned long)__d)<<32); \
 } while(0)
 
 #define rdtscpll(val, aux) do { \
      unsigned long __a, __d; \
-     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
+     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
      (val) = (__d << 32) | __a; \
 } while (0)
 
@@ -253,6 +221,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 			  : "=a" (low), "=d" (high) \
 			  : "c" (counter))
 
+
 static inline void cpuid(int op, unsigned int *eax, unsigned int *ebx,
 			 unsigned int *ecx, unsigned int *edx)
 {
@@ -320,6 +289,40 @@ static inline unsigned int cpuid_edx(unsigned int op)
 	return edx;
 }
 
+#ifdef __KERNEL__
+
+/* wrmsr with exception handling */
+#define wrmsr_safe(msr,a,b) ({ int ret__;			\
+	asm volatile("2: wrmsr ; xorl %0,%0\n"			\
+		     "1:\n\t"					\
+		     ".section .fixup,\"ax\"\n\t"		\
+		     "3:  movl %4,%0 ; jmp 1b\n\t"		\
+		     ".previous\n\t"				\
+		     ".section __ex_table,\"a\"\n"		\
+		     "   .align 8\n\t"				\
+		     "   .quad	2b,3b\n\t"			\
+		     ".previous"				\
+		     : "=a" (ret__)				\
+		     : "c" (msr), "0" (a), "d" (b), "i" (-EFAULT)); \
+	ret__; })
+
+#define checking_wrmsrl(msr,val) wrmsr_safe(msr,(u32)(val),(u32)((val)>>32))
+
+#define rdmsr_safe(msr,a,b) \
+	({ int ret__;						\
+	  asm volatile ("1:       rdmsr\n"			\
+			"2:\n"					\
+			".section .fixup,\"ax\"\n"		\
+			"3:       movl %4,%0\n"			\
+			" jmp 2b\n"				\
+			".previous\n"				\
+			".section __ex_table,\"a\"\n"		\
+			" .align 8\n"				\
+			" .quad 1b,3b\n"				\
+			".previous":"=&bDS" (ret__), "=a"(*(a)), "=d"(*(b)) \
+			:"c"(msr), "i"(-EIO), "0"(0));			\
+	  ret__; })
+
 #ifdef CONFIG_SMP
 void rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);
 void wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h);
@@ -343,6 +346,7 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 	return wrmsr_safe(msr_no, l, h);
 }
 #endif  /* CONFIG_SMP */
+#endif  /* __KERNEL__ */
 #endif  /* __ASSEMBLY__ */
 
 #endif  /* !__i386__ */

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Ingo Molnar]

* Mike Frysinger <vapier@gentoo.org> wrote:

> Use __asm__ and __volatile__ in code that is exported to userspace.  
> Wrap kernel functions with __KERNEL__ so they get scrubbed.

thanks Mike, i have applied it to x86.git. (with a v2.6.24 merge target)

	Ingo

Re: asm-x86/msr.h for sanitized headers: clean it or punt it

[Adrian Bunk]

On Mon, Dec 31, 2007 at 01:49:27PM -0500, Mike Frysinger wrote:
> Use __asm__ and __volatile__ in code that is exported to userspace.  Wrap
> kernel functions with __KERNEL__ so they get scrubbed.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> diff --git a/include/asm-x86/msr.h b/include/asm-x86/msr.h
> index ba4b314..664a2fa 100644
> --- a/include/asm-x86/msr.h
> +++ b/include/asm-x86/msr.h
>...
>  #define rdtscp(low,high,aux) \
> -     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
> +     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (low), "=d" (high), "=c" (aux))
>  
>  #define rdtscll(val) do { \
>       unsigned int __a,__d; \
> -     asm volatile("rdtsc" : "=a" (__a), "=d" (__d)); \
> +     __asm__ __volatile__("rdtsc" : "=a" (__a), "=d" (__d)); \
>       (val) = ((unsigned long)__a) | (((unsigned long)__d)<<32); \
>  } while(0)
>  
>  #define rdtscpll(val, aux) do { \
>       unsigned long __a, __d; \
> -     asm volatile (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
> +     __asm__ __volatile__ (".byte 0x0f,0x01,0xf9" : "=a" (__a), "=d" (__d), "=c" (aux)); \
>       (val) = (__d << 32) | __a; \
>  } while (0)
>...

How is this part of the kernel<->userspace interface?

Unless I miss anything this sounds more like userspace abusing kernel 
headers as a utility library?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed