2.6.20-rc4-rt0: i386 assembler in kernel/sys.c

Hi all,

Looks like some debug code accidently leaked into this kernel/sys.c:

	if (option == 9998) {
		int i;
		for (i = 0; i < arg2; i++)
			asm ("movl $0x12341234, %%eax; movl %%eax, %%cr3":::"eax");
		return 1;
	}
	if (option == 9999) {
		int i;
		for (i = 0; i < arg2; i++)
			asm (".byte 0x0f, 0x01, 0xc1");
		return 0;
	}

In include/linux/irqflags.h safe_halt changed from a macro to a static inline function which results in tons of warnings for architectures other than x86[-64]:

include/linux/irqflags.h:99: warning: implicit declaration of function 'raw_safe_halt'

Apart from these two issues 2.6.20-rc4-rt0 seems to run on my arm system.

Sascha

 Dipl.-Ing. Sascha Hauer | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry
    Handelsregister: Amtsgericht Hildesheim, HRA 2686
      Hannoversche Str. 2, 31134 Hildesheim, Germany
    Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

ok, this is still not fixed in rt2 - which is using Rusty's paravirt-unexport patch.

Andrew, we need to fix this before v2.6.20 is released. Simplest upstream fix is to just disable the CONFIG_PARAVIRT Kbuild option, as per the patch below. Rusty's unexport patch is promising in many ways, but it's v2.6.21 stuff IMO.

i'm keeping it enabled in -rt, which makes actual use of this infrastructure via KVM-paravirt, so it wont be forgotten.

Ingo

-------------->
From: Ingo Molnar <[email protected]>
Subject: [patch] paravirt: disable Kbuild option for now

disable CONFIG_PARAVIRT for now - it's not like that it is useful for anything in-tree at the moment. I'll make sure it's fixed in v2.6.21.

Index: linux/arch/i386/Kconfig
===================================================================
@@ -199,6 +199,12 @@ endchoice
 
 config PARAVIRT
 	bool "Paravirtualization support (EXPERIMENTAL)"
+
+	#
+	# Work in progress, disable for now:
+	#
+	depends on 0
+
 	depends on EXPERIMENTAL
 	depends on !(X86_VISWS || X86_VOYAGER)
 	help

No. Just drop the whole "don't export paravirt_ops" idea, and we'll work on that for 2.6.21.

I don't see a problem with exporting paravirt_ops, I was just trying to keep you happy. I'm happy to remove the export in 2.6.21, but as we've seen it's non-trivial to ensure all configs still work.

Rusty.

I do see a problem with it: it's been poorly done /because/ nothing in-kernel uses it. We can fix it for 2.6.21, but lets disable it for 2.6.20 - or make the export _GPL as per the patch below. We can enable it no problem. Or if that doesnt happen i'm strongly NACK-ing the whole paravirt subsystem in its current form.

Ingo

---------------->
From: Ingo Molnar <[email protected]>
Subject: [patch] paravirt: mark the export GPL

the paravirt subsystem is still in flux so all exports from it are definitely internal use only. The APIs around this /will/ change.

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
@@ -566,4 +566,4 @@ struct paravirt_ops paravirt_ops = {
 	.irq_enable_sysexit = native_irq_enable_sysexit,
 	.iret = native_iret,
 };
-EXPORT_SYMBOL(paravirt_ops);
+EXPORT_SYMBOL_GPL(paravirt_ops);

Or, apply this. Compile & boot tested.

Name: Revert safe_halt to a macro: instead rename paravirt_ops op to avoid the namespace pollution

Making "safe_halt()" an inline function gives lots of warning on other archs, so it's easiest to just avoid that name.

diff -r 3bbc1e7384c9 arch/i386/kernel/paravirt.c
@@ -683,13 +773,13 @@ EXPORT_SYMBOL(wbinvd);
 
 void raw_safe_halt(void)
 {
-	paravirt_ops.safe_halt();
+	paravirt_ops.halt_safe();
 }
 EXPORT_SYMBOL_GPL(raw_safe_halt);
 
 void halt(void)
 {
-	paravirt_ops.safe_halt();
+	paravirt_ops.halt();
 }
 EXPORT_SYMBOL_GPL(halt);
 

@@ -750,7 +840,7 @@ struct paravirt_ops paravirt_ops = {
 	.restore_fl = native_restore_fl,
 	.irq_disable = native_irq_disable,
 	.irq_enable = native_irq_enable,
-	.safe_halt = native_safe_halt,
+	.halt_safe = native_safe_halt,
 	.halt = native_halt,
 	.wbinvd = native_wbinvd,
 	.read_msr = native_read_msr,

diff -r 3bbc1e7384c9 include/asm-i386/paravirt.h
@@ -94,7 +94,7 @@ struct paravirt_ops
 	void (fastcall *restore_fl)(unsigned long);
 	void (fastcall *irq_disable)(void);
 	void (fastcall *irq_enable)(void);
-	void (fastcall *safe_halt)(void);
+	void (fastcall *halt_safe)(void);
 	void (fastcall *halt)(void);
 	void (fastcall *wbinvd)(void);
 

diff -r 3bbc1e7384c9 include/linux/irqflags.h
@@ -74,11 +74,11 @@
 #endif /* CONFIG_TRACE_IRQFLAGS_SUPPORT */
 
 #ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
-static inline void safe_halt(void)
-{
-	trace_hardirqs_on();
-	raw_safe_halt();
-}
+#define safe_halt()						\
+	do {							\
+		trace_hardirqs_on();				\
+		raw_safe_halt();				\
+	} while (0)
 
 #define local_save_flags(flags)		raw_local_save_flags(flags)