public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] 1/5 more-asm-cleanup
@ 2005-08-04  0:43 zach
  2005-08-04  0:47 ` H. Peter Anvin
  0 siblings, 1 reply; 5+ messages in thread
From: zach @ 2005-08-04  0:43 UTC (permalink / raw)
  To: akpm, chrisl, davej, hpa, linux-kernel, pratap, Riley, zach

Some more assembler cleanups I noticed along the way.

Diffs against: 2.6.13-rc4-mm1

Signed-off-by: Zachary Amsden <zach@vmware.com)
Index: linux-2.6.13/arch/i386/kernel/crash.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/crash.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/crash.c	2005-08-03 15:19:39.000000000 -0700
@@ -153,7 +153,7 @@
 	disable_local_APIC();
 	atomic_dec(&waiting_for_crash_ipi);
 	/* Assume hlt works */
-	__asm__("hlt");
+	halt();
 	for(;;);
 
 	return 1;
Index: linux-2.6.13/arch/i386/kernel/machine_kexec.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/machine_kexec.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/machine_kexec.c	2005-08-03 15:19:39.000000000 -0700
@@ -93,10 +93,7 @@
 	curidt.size    = limit;
 	curidt.address = (unsigned long)newidt;
 
-	__asm__ __volatile__ (
-		"lidtl %0\n"
-		: : "m" (curidt)
-		);
+	load_idt(&curidt);
 };
 
 
@@ -108,10 +105,7 @@
 	curgdt.size    = limit;
 	curgdt.address = (unsigned long)newgdt;
 
-	__asm__ __volatile__ (
-		"lgdtl %0\n"
-		: : "m" (curgdt)
-		);
+	load_gdt(&curgdt);
 };
 
 static void load_segments(void)
Index: linux-2.6.13/arch/i386/kernel/process.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/process.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/process.c	2005-08-03 17:27:48.000000000 -0700
@@ -165,7 +165,7 @@
 	 */
 	local_irq_disable();
 	while (1)
-		__asm__ __volatile__("hlt":::"memory");
+		halt();
 }
 #else
 static inline void play_dead(void)
Index: linux-2.6.13/arch/i386/kernel/msr.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/msr.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/msr.c	2005-08-03 15:19:39.000000000 -0700
@@ -46,23 +46,13 @@
 
 static struct class *msr_class;
 
-/* Note: "err" is handled in a funny way below.  Otherwise one version
-   of gcc or another breaks. */
-
 static inline int wrmsr_eio(u32 reg, u32 eax, u32 edx)
 {
 	int err;
 
-	asm volatile ("1:	wrmsr\n"
-		      "2:\n"
-		      ".section .fixup,\"ax\"\n"
-		      "3:	movl %4,%0\n"
-		      "	jmp 2b\n"
-		      ".previous\n"
-		      ".section __ex_table,\"a\"\n"
-		      "	.align 4\n" "	.long 1b,3b\n" ".previous":"=&bDS" (err)
-		      :"a"(eax), "d"(edx), "c"(reg), "i"(-EIO), "0"(0));
-
+	err = wrmsr_safe(reg, eax, edx);
+	if (err)
+		err = -EIO;
 	return err;
 }
 
@@ -70,18 +60,9 @@
 {
 	int err;
 
-	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 4\n"
-		      "	.long 1b,3b\n"
-		      ".previous":"=&bDS" (err), "=a"(*eax), "=d"(*edx)
-		      :"c"(reg), "i"(-EIO), "0"(0));
-
+	err = rdmsr_safe(reg, eax, edx);
+	if (err)
+		err = -EIO;
 	return err;
 }
 
Index: linux-2.6.13/arch/i386/kernel/cpu/intel.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/cpu/intel.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/cpu/intel.c	2005-08-03 15:19:39.000000000 -0700
@@ -82,16 +82,12 @@
  */
 static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
 {
-	unsigned int eax;
+	unsigned int eax, ebx, ecx, edx;
 
 	if (c->cpuid_level < 4)
 		return 1;
 
-	__asm__("cpuid"
-		: "=a" (eax)
-		: "0" (4), "c" (0)
-		: "bx", "dx");
-
+	cpuid(4, &eax, &ebx, &ecx, &edx);
 	if (eax & 0x1f)
 		return ((eax >> 26) + 1);
 	else
Index: linux-2.6.13/arch/i386/mach-voyager/voyager_basic.c
===================================================================
--- linux-2.6.13.orig/arch/i386/mach-voyager/voyager_basic.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/mach-voyager/voyager_basic.c	2005-08-03 15:19:39.000000000 -0700
@@ -234,10 +234,9 @@
 #endif
 	}
 	/* and wait for it to happen */
-	for(;;) {
-		__asm("cli");
-		__asm("hlt");
-	}
+	local_irq_disable();
+	for(;;) 
+		halt();
 }
 
 /* copied from process.c */
@@ -272,10 +271,9 @@
 		outb(basebd | 0x08, VOYAGER_MC_SETUP);
 		outb(0x02, catbase + 0x21);
 	}
-	for(;;) {
-		asm("cli");
-		asm("hlt");
-	}
+	local_irq_disable();
+	for(;;) 
+		halt();
 }
 
 void
Index: linux-2.6.13/arch/i386/mach-voyager/voyager_smp.c
===================================================================
--- linux-2.6.13.orig/arch/i386/mach-voyager/voyager_smp.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/mach-voyager/voyager_smp.c	2005-08-03 15:19:39.000000000 -0700
@@ -1015,7 +1015,7 @@
 	cpu_clear(smp_processor_id(), cpu_online_map);
 	local_irq_disable();
 	for(;;)
-	       __asm__("hlt");
+		halt();
 }
 
 static DEFINE_SPINLOCK(call_lock);
Index: linux-2.6.13/include/asm-i386/msr.h
===================================================================
--- linux-2.6.13.orig/include/asm-i386/msr.h	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/include/asm-i386/msr.h	2005-08-03 17:16:07.000000000 -0700
@@ -47,6 +47,21 @@
 		     : "c" (msr), "0" (a), "d" (b), "i" (-EFAULT));\
 	ret__; })
 
+/* rdmsr with exception handling */
+#define rdmsr_safe(msr,a,b) ({ int ret__;						\
+	asm volatile("2: rdmsr ; 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 4\n\t"							\
+		     "   .long 	2b,3b\n\t"						\
+		     ".previous"							\
+		     : "=r" (ret__), "=a" (*(a)), "=d" (*(b))				\
+		     : "c" (msr), "i" (-EFAULT));\
+	ret__; })
+
 #define rdtsc(low,high) \
      __asm__ __volatile__("rdtsc" : "=a" (low), "=d" (high))
 

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

* Re: [PATCH] 1/5 more-asm-cleanup
  2005-08-04  0:43 [PATCH] 1/5 more-asm-cleanup zach
@ 2005-08-04  0:47 ` H. Peter Anvin
  2005-08-04  3:06   ` Zachary Amsden
  2005-08-04  3:33   ` Zachary Amsden
  0 siblings, 2 replies; 5+ messages in thread
From: H. Peter Anvin @ 2005-08-04  0:47 UTC (permalink / raw)
  To: zach; +Cc: akpm, chrisl, davej, linux-kernel, pratap, Riley

zach@vmware.com wrote:
> Some more assembler cleanups I noticed along the way.

> Index: linux-2.6.13/arch/i386/kernel/cpu/intel.c
> ===================================================================
> --- linux-2.6.13.orig/arch/i386/kernel/cpu/intel.c	2005-08-03 15:18:18.000000000 -0700
> +++ linux-2.6.13/arch/i386/kernel/cpu/intel.c	2005-08-03 15:19:39.000000000 -0700
> @@ -82,16 +82,12 @@
>   */
>  static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
>  {
> -	unsigned int eax;
> +	unsigned int eax, ebx, ecx, edx;
>  
>  	if (c->cpuid_level < 4)
>  		return 1;
>  
> -	__asm__("cpuid"
> -		: "=a" (eax)
> -		: "0" (4), "c" (0)
> -		: "bx", "dx");
> -
> +	cpuid(4, &eax, &ebx, &ecx, &edx);
>  	if (eax & 0x1f)
>  		return ((eax >> 26) + 1);

Reject!  This is a bogus patch; Intel's CPUID level 4 has a nonstandard 
dependency on ECX (idiots...) and therefore this needs special handling.

	-hpa

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

* Re: [PATCH] 1/5 more-asm-cleanup
  2005-08-04  0:47 ` H. Peter Anvin
@ 2005-08-04  3:06   ` Zachary Amsden
  2005-08-04  6:01     ` H. Peter Anvin
  2005-08-04  3:33   ` Zachary Amsden
  1 sibling, 1 reply; 5+ messages in thread
From: Zachary Amsden @ 2005-08-04  3:06 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: akpm, chrisl, davej, linux-kernel, pratap, Riley

Please explain why this is a reject after looking at the cpuid macro.  
It changed recently.  Note 0 -> %ecx.

Would you prefer that I call cpuid_count and pass an explicit zero 
parameter for ecx?


/*
 * Generic CPUID function
 * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
 * resulting in stale register contents being returned.
 */
static inline void cpuid(unsigned int op, unsigned int *eax, unsigned 
int *ebx, unsigned int *ecx, unsigned int *edx)
{
        __asm__("cpuid"
                : "=a" (*eax),
                  "=b" (*ebx),
                  "=c" (*ecx),
                  "=d" (*edx)
                : "0" (op), "c"(0));
}


H. Peter Anvin wrote:

> zach@vmware.com wrote:
>
>> Some more assembler cleanups I noticed along the way.
>
>
>> Index: linux-2.6.13/arch/i386/kernel/cpu/intel.c
>> ===================================================================
>> --- linux-2.6.13.orig/arch/i386/kernel/cpu/intel.c    2005-08-03 
>> 15:18:18.000000000 -0700
>> +++ linux-2.6.13/arch/i386/kernel/cpu/intel.c    2005-08-03 
>> 15:19:39.000000000 -0700
>> @@ -82,16 +82,12 @@
>>   */
>>  static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
>>  {
>> -    unsigned int eax;
>> +    unsigned int eax, ebx, ecx, edx;
>>  
>>      if (c->cpuid_level < 4)
>>          return 1;
>>  
>> -    __asm__("cpuid"
>> -        : "=a" (eax)
>> -        : "0" (4), "c" (0)
>> -        : "bx", "dx");
>> -
>> +    cpuid(4, &eax, &ebx, &ecx, &edx);
>>      if (eax & 0x1f)
>>          return ((eax >> 26) + 1);
>
>
> Reject!  This is a bogus patch; Intel's CPUID level 4 has a 
> nonstandard dependency on ECX (idiots...) and therefore this needs 
> special handling.
>
>     -hpa
>


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

* Re: [PATCH] 1/5 more-asm-cleanup
  2005-08-04  0:47 ` H. Peter Anvin
  2005-08-04  3:06   ` Zachary Amsden
@ 2005-08-04  3:33   ` Zachary Amsden
  1 sibling, 0 replies; 5+ messages in thread
From: Zachary Amsden @ 2005-08-04  3:33 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: akpm, chrisl, davej, linux-kernel, pratap, Riley

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

H. Peter Anvin wrote:

>
> Reject!  This is a bogus patch; Intel's CPUID level 4 has a 
> nonstandard dependency on ECX (idiots...) and therefore this needs 
> special handling.
>
>     -hpa


Here's a better idea.  Let's comment that unusual dependency and make it 
explicit in the macro.


[-- Attachment #2: more-asm-cleanup --]
[-- Type: text/plain, Size: 5970 bytes --]

Some more assembler cleanups I noticed along the way.

Diffs against: 2.6.13-rc4-mm1

Signed-off-by: Zachary Amsden <zach@vmware.com)
Index: linux-2.6.13/arch/i386/kernel/crash.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/crash.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/crash.c	2005-08-03 15:19:39.000000000 -0700
@@ -153,7 +153,7 @@
 	disable_local_APIC();
 	atomic_dec(&waiting_for_crash_ipi);
 	/* Assume hlt works */
-	__asm__("hlt");
+	halt();
 	for(;;);
 
 	return 1;
Index: linux-2.6.13/arch/i386/kernel/machine_kexec.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/machine_kexec.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/machine_kexec.c	2005-08-03 15:19:39.000000000 -0700
@@ -93,10 +93,7 @@
 	curidt.size    = limit;
 	curidt.address = (unsigned long)newidt;
 
-	__asm__ __volatile__ (
-		"lidtl %0\n"
-		: : "m" (curidt)
-		);
+	load_idt(&curidt);
 };
 
 
@@ -108,10 +105,7 @@
 	curgdt.size    = limit;
 	curgdt.address = (unsigned long)newgdt;
 
-	__asm__ __volatile__ (
-		"lgdtl %0\n"
-		: : "m" (curgdt)
-		);
+	load_gdt(&curgdt);
 };
 
 static void load_segments(void)
Index: linux-2.6.13/arch/i386/kernel/process.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/process.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/process.c	2005-08-03 17:27:48.000000000 -0700
@@ -165,7 +165,7 @@
 	 */
 	local_irq_disable();
 	while (1)
-		__asm__ __volatile__("hlt":::"memory");
+		halt();
 }
 #else
 static inline void play_dead(void)
Index: linux-2.6.13/arch/i386/kernel/msr.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/msr.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/msr.c	2005-08-03 15:19:39.000000000 -0700
@@ -46,23 +46,13 @@
 
 static struct class *msr_class;
 
-/* Note: "err" is handled in a funny way below.  Otherwise one version
-   of gcc or another breaks. */
-
 static inline int wrmsr_eio(u32 reg, u32 eax, u32 edx)
 {
 	int err;
 
-	asm volatile ("1:	wrmsr\n"
-		      "2:\n"
-		      ".section .fixup,\"ax\"\n"
-		      "3:	movl %4,%0\n"
-		      "	jmp 2b\n"
-		      ".previous\n"
-		      ".section __ex_table,\"a\"\n"
-		      "	.align 4\n" "	.long 1b,3b\n" ".previous":"=&bDS" (err)
-		      :"a"(eax), "d"(edx), "c"(reg), "i"(-EIO), "0"(0));
-
+	err = wrmsr_safe(reg, eax, edx);
+	if (err)
+		err = -EIO;
 	return err;
 }
 
@@ -70,18 +60,9 @@
 {
 	int err;
 
-	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 4\n"
-		      "	.long 1b,3b\n"
-		      ".previous":"=&bDS" (err), "=a"(*eax), "=d"(*edx)
-		      :"c"(reg), "i"(-EIO), "0"(0));
-
+	err = rdmsr_safe(reg, eax, edx);
+	if (err)
+		err = -EIO;
 	return err;
 }
 
Index: linux-2.6.13/arch/i386/kernel/cpu/intel.c
===================================================================
--- linux-2.6.13.orig/arch/i386/kernel/cpu/intel.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/kernel/cpu/intel.c	2005-08-03 20:22:55.000000000 -0700
@@ -82,16 +82,13 @@
  */
 static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
 {
-	unsigned int eax;
+	unsigned int eax, ebx, ecx, edx;
 
 	if (c->cpuid_level < 4)
 		return 1;
 
-	__asm__("cpuid"
-		: "=a" (eax)
-		: "0" (4), "c" (0)
-		: "bx", "dx");
-
+	/* Intel has a non-standard dependency on %ecx for this CPUID level. */
+	cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
 	if (eax & 0x1f)
 		return ((eax >> 26) + 1);
 	else
Index: linux-2.6.13/arch/i386/mach-voyager/voyager_basic.c
===================================================================
--- linux-2.6.13.orig/arch/i386/mach-voyager/voyager_basic.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/mach-voyager/voyager_basic.c	2005-08-03 15:19:39.000000000 -0700
@@ -234,10 +234,9 @@
 #endif
 	}
 	/* and wait for it to happen */
-	for(;;) {
-		__asm("cli");
-		__asm("hlt");
-	}
+	local_irq_disable();
+	for(;;) 
+		halt();
 }
 
 /* copied from process.c */
@@ -272,10 +271,9 @@
 		outb(basebd | 0x08, VOYAGER_MC_SETUP);
 		outb(0x02, catbase + 0x21);
 	}
-	for(;;) {
-		asm("cli");
-		asm("hlt");
-	}
+	local_irq_disable();
+	for(;;) 
+		halt();
 }
 
 void
Index: linux-2.6.13/arch/i386/mach-voyager/voyager_smp.c
===================================================================
--- linux-2.6.13.orig/arch/i386/mach-voyager/voyager_smp.c	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/arch/i386/mach-voyager/voyager_smp.c	2005-08-03 15:19:39.000000000 -0700
@@ -1015,7 +1015,7 @@
 	cpu_clear(smp_processor_id(), cpu_online_map);
 	local_irq_disable();
 	for(;;)
-	       __asm__("hlt");
+		halt();
 }
 
 static DEFINE_SPINLOCK(call_lock);
Index: linux-2.6.13/include/asm-i386/msr.h
===================================================================
--- linux-2.6.13.orig/include/asm-i386/msr.h	2005-08-03 15:18:18.000000000 -0700
+++ linux-2.6.13/include/asm-i386/msr.h	2005-08-03 17:16:07.000000000 -0700
@@ -47,6 +47,21 @@
 		     : "c" (msr), "0" (a), "d" (b), "i" (-EFAULT));\
 	ret__; })
 
+/* rdmsr with exception handling */
+#define rdmsr_safe(msr,a,b) ({ int ret__;						\
+	asm volatile("2: rdmsr ; 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 4\n\t"							\
+		     "   .long 	2b,3b\n\t"						\
+		     ".previous"							\
+		     : "=r" (ret__), "=a" (*(a)), "=d" (*(b))				\
+		     : "c" (msr), "i" (-EFAULT));\
+	ret__; })
+
 #define rdtsc(low,high) \
      __asm__ __volatile__("rdtsc" : "=a" (low), "=d" (high))
 

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

* Re: [PATCH] 1/5 more-asm-cleanup
  2005-08-04  3:06   ` Zachary Amsden
@ 2005-08-04  6:01     ` H. Peter Anvin
  0 siblings, 0 replies; 5+ messages in thread
From: H. Peter Anvin @ 2005-08-04  6:01 UTC (permalink / raw)
  To: Zachary Amsden; +Cc: akpm, chrisl, davej, linux-kernel, pratap, Riley

Zachary Amsden wrote:
> Please explain why this is a reject after looking at the cpuid macro.  
> It changed recently.  Note 0 -> %ecx.

Then just use cpuid_eax(4)?  Or do those macros not behave that way?

> Would you prefer that I call cpuid_count and pass an explicit zero 
> parameter for ecx?

I guess I really don't like the implicit zero when it matters, so yes.

	-hpa

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

end of thread, other threads:[~2005-08-04  6:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-04  0:43 [PATCH] 1/5 more-asm-cleanup zach
2005-08-04  0:47 ` H. Peter Anvin
2005-08-04  3:06   ` Zachary Amsden
2005-08-04  6:01     ` H. Peter Anvin
2005-08-04  3:33   ` Zachary Amsden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox