Skip to contentSkip to conversation

Regparm for x86 machine check handlers

PATCH In review Brian Gerst 3 participants 4 messages Nov 15, 2004

lore
Share & tools
lore

Brian Gerst

author
#0 lore raw

PATCH 1/1 Regparm for x86 machine check handlers

The patch to change traps and interrupts to the fastcall convention missed the machine check handlers.

Signed-off-by Brian Gerst <[email protected]>
arch/i386/kernel/cpu/mcheck/k7.c +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/k7.c linux/arch/i386/kernel/cpu/mcheck/k7.c
@@ -18,7 +18,7 @@
 #include "mce.h"
 
 /* Machine Check Handler For AMD Athlon/Duron */
-static asmlinkage void k7_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void k7_machine_check(struct pt_regs * regs, long error_code)
 {
 	int recover=1;
 	u32 alow, ahigh, high, low;
arch/i386/kernel/cpu/mcheck/mce.c +2 -2
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/mce.c linux/arch/i386/kernel/cpu/mcheck/mce.c
@@ -22,13 +22,13 @@
 EXPORT_SYMBOL_GPL(nr_mce_banks);	/* non-fatal.o */
 
 /* Handle unconfigured int18 (should never happen) */
-static asmlinkage void unexpected_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void unexpected_machine_check(struct pt_regs * regs, long error_code)
 {	
 	printk(KERN_ERR "CPU#%d: Unexpected int18 (Machine Check).\n", smp_processor_id());
 }
 
 /* Call the installed machine check handler for this CPU setup. */
-void asmlinkage (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;
+void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;
 
 /* This has to be run for each processor */
 void __init mcheck_init(struct cpuinfo_x86 *c)
arch/i386/kernel/cpu/mcheck/mce.h +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/mce.h linux/arch/i386/kernel/cpu/mcheck/mce.h
@@ -7,7 +7,7 @@
 void winchip_mcheck_init(struct cpuinfo_x86 *c);
 
 /* Call the installed machine check handler for this CPU setup. */
-extern asmlinkage void (*machine_check_vector)(struct pt_regs *, long error_code);
+extern fastcall void (*machine_check_vector)(struct pt_regs *, long error_code);
 
 extern int mce_disabled __initdata;
 extern int nr_mce_banks;
arch/i386/kernel/cpu/mcheck/p4.c +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/p4.c linux/arch/i386/kernel/cpu/mcheck/p4.c
@@ -159,7 +159,7 @@
 	return mce_num_extended_msrs;
 }
 
-static asmlinkage void intel_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void intel_machine_check(struct pt_regs * regs, long error_code)
 {
 	int recover=1;
 	u32 alow, ahigh, high, low;
arch/i386/kernel/cpu/mcheck/p5.c +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/p5.c linux/arch/i386/kernel/cpu/mcheck/p5.c
@@ -17,7 +17,7 @@
 #include "mce.h"
 
 /* Machine check handler for Pentium class Intel */
-static asmlinkage void pentium_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void pentium_machine_check(struct pt_regs * regs, long error_code)
 {
 	u32 loaddr, hi, lotype;
 	rdmsr(MSR_IA32_P5_MC_ADDR, loaddr, hi);
arch/i386/kernel/cpu/mcheck/p6.c +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/p6.c linux/arch/i386/kernel/cpu/mcheck/p6.c
@@ -17,7 +17,7 @@
 #include "mce.h"
 
 /* Machine Check Handler For PII/PIII */
-static asmlinkage void intel_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void intel_machine_check(struct pt_regs * regs, long error_code)
 {
 	int recover=1;
 	u32 alow, ahigh, high, low;
arch/i386/kernel/cpu/mcheck/winchip.c +1 -1
diff -urN linux-2.6.10-rc1-bk/arch/i386/kernel/cpu/mcheck/winchip.c linux/arch/i386/kernel/cpu/mcheck/winchip.c
@@ -16,7 +16,7 @@
 #include "mce.h"
 
 /* Machine check handler for WinChip C6 */
-static asmlinkage void winchip_machine_check(struct pt_regs * regs, long error_code)
+static fastcall void winchip_machine_check(struct pt_regs * regs, long error_code)
 {
 	printk(KERN_EMERG "CPU0: Machine Check Exception.\n");
 	add_taint(TAINT_MACHINE_CHECK);
#1 lore raw
Quoted 3 lines · Brian Gerst

The patch to change traps and interrupts to the fastcall convention missed the machine check handlers.

Thanks, that was silly.

Anybody want to write a script that verifies that the only remaining "asmlinkage" entries are of the type "sys_xxxx()"?

"grep" shows that there's a number of incorrect ones left, but most of them seem to take no arguments, so ir doesn't matter. And there's the FP emulation stuff, which really -does- use the old interfaces.

Linus

#2 lore raw
Quoted 9 lines · Linus Torvalds
Quoted 1 line · Brian Gerst

The patch to change traps and interrupts to the fastcall convention missed the machine check handlers.

Thanks, that was silly.

Anybody want to write a script that verifies that the only remaining "asmlinkage" entries are of the type "sys_xxxx()"?

Is part of the problem definition missing here?
or I missed it?

E.g., printk() and vprint() are asmlinkage but not sys_xyz()... but I have a suspicion that they are OK.

Quoted 1 line

"grep" shows that there's a number of incorrect ones left, but most of them seem to take no arguments, so ir doesn't matter. And there's the FP emulation stuff, which really -does- use the old interfaces.

so ignore the FP emulation, ignore functions with no arguments, right?

and omit "asmlinkage.*sys_xyz". that leaves a handful of functions which are <asm>, like:

acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state); csum_partial(), csum_partial_copy_generic(), schedule_tail(), aes_enc_blk(), aes_dec_blk().

I don't see others than need to be fixed, but a script would be a safer way to check, so I'm trying to nail down the requirements ... and what tool to use, like is there already a PERL [or python or xyz] script that parses C, or would you *coff* recommend sparse?

Signature · 1 line
~Randy
#3 lore raw
Quoted 7 lines · Randy.Dunlap

and omit "asmlinkage.*sys_xyz". that leaves a handful of functions which are <asm>, like:

acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state); csum_partial(), csum_partial_copy_generic(), schedule_tail(), aes_enc_blk(), aes_dec_blk().

The schedule_tail() usage seems a bit suspect. The asm code on x86 seems to play it safe, and pass the argument _both_ on the stack and in registers. So it looks correct, but a bit schitzo.

The others should probably be changed to use register calling conventions, but it might not be worth doing so right now, and at least they seem to agree on how to pass arguments in callers and callee's. The csum functions are certainly performance-critical, but it's not like the argument passing convention is likely to make _that_ big of a difference, and they are "asmcall" not because they are called from asm, but because they are _implemented_ in asm (so unlike the other cases, these guys don't have the problem with the callee thinking it owns the argument frame).

Quoted 5 lines

I don't see others than need to be fixed, but a script would be a safer way to check, so I'm trying to nail down the requirements ... and what tool to use, like is there already a PERL [or python or xyz] script that parses C, or would you *coff* recommend sparse?

Sparse can't look into the asm code, but it could certainly be made to report on functions that use "regparm(0)". But it would be good to have something that matches up the functions with the asm code and verifies that they agree on passing convention.

There doesn't seem to be enough of them to bother with a tool, though.

Linus