* [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 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
* 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
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