Re: [PATCH] fix: x86: remove cpu_vendor_dev

Previous thread: none

Next thread: sleeping function called from invalid context at kernel/mutex.c by Sitsofe Wheeler on Tuesday, September 30, 2008 - 10:00 am. (1 message)
From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 9:41 am

Hi Ingo,

Here are some more unification patches for traps_xx.c. They
are against the current x86/traps branch in the tip tree and
work fine for my miniconfigs.

The branch does not at the moment compile a defconfig kernel,
due to a missing PCI_DEVICE_ID_AMD_10H_NB_MISC define, however.

Moreover, a defconfig won't run :-/ (on qemu-system-x86_64).
Bisection pointed to commit 10a434fcb "x86: remove cpu_vendor_dev".
The kernel crashes early with a general protection fault in a call
to strnlen. I have no idea what goes wrong, yet.

Greetings,
	Alexander
--

From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 9:41 am

Split out math_error from do_coprocessor_error and simd_math_error
from do_simd_coprocessor_error, like on i386. While at it, add the
"error_code" parameter to do_coprocessor_error, do_simd_coprocessor_error
and do_spurious_interrupt_bug. This does not change the generated
code, but brings the declarations in line with all the other trap
handlers.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_64.c |   36 +++++++++++++++++++++---------------
 include/asm-x86/traps.h    |    6 +++---
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c
index 8ab8f81..201f98d 100644
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -457,18 +457,12 @@ static int kernel_math_error(struct pt_regs *regs, const char *str, int trapnr)
  * the correct behaviour even in the presence of the asynchronous
  * IRQ13 behaviour
  */
-asmlinkage void do_coprocessor_error(struct pt_regs *regs)
+void math_error(void __user *ip)
 {
-	void __user *ip = (void __user *)(regs->ip);
 	struct task_struct *task;
 	siginfo_t info;
 	unsigned short cwd, swd;
 
-	conditional_sti(regs);
-	if (!user_mode(regs) &&
-	    kernel_math_error(regs, "kernel x87 math error", 16))
-		return;
-
 	/*
 	 * Save the info for the exception handler and clear the error.
 	 */
@@ -521,23 +515,26 @@ asmlinkage void do_coprocessor_error(struct pt_regs *regs)
 	force_sig_info(SIGFPE, &info, task);
 }
 
+asmlinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
+{
+	conditional_sti(regs);
+	if (!user_mode(regs) &&
+	    kernel_math_error(regs, "kernel x87 math error", 16))
+		return;
+	math_error((void __user *)regs->ip);
+}
+
 asmlinkage void bad_intr(void)
 {
 	printk("bad interrupt");
 }
 
-asmlinkage void do_simd_coprocessor_error(struct pt_regs *regs)
+static void simd_math_error(void __user *ip)
 {
-	void __user *ip = (void __user *)(regs->ip);
 	struct task_struct ...
From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 9:41 am

x86_64 does not do the lazy io-bitmap dance. Putting it in
its own function makes i386's do_general_protection look
much more like x86_64's.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_32.c |   76 +++++++++++++++++++++++++-------------------
 1 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index dd183a2..78113d3 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -96,6 +96,47 @@ die_if_kernel(const char *str, struct pt_regs *regs, long err)
 		die(str, regs, err);
 }
 
+/*
+ * Perform the lazy TSS's I/O bitmap copy. If the TSS has an
+ * invalid offset set (the LAZY one) and the faulting thread has
+ * a valid I/O bitmap pointer, we copy the I/O bitmap in the TSS,
+ * we set the offset field correctly and return 1.
+ */
+static int lazy_iobitmap_copy(void)
+{
+	struct thread_struct *thread;
+	struct tss_struct *tss;
+	int cpu;
+
+	cpu = get_cpu();
+	tss = &per_cpu(init_tss, cpu);
+	thread = &current->thread;
+
+	if (tss->x86_tss.io_bitmap_base == INVALID_IO_BITMAP_OFFSET_LAZY &&
+	    thread->io_bitmap_ptr) {
+		memcpy(tss->io_bitmap, thread->io_bitmap_ptr,
+		       thread->io_bitmap_max);
+		/*
+		 * If the previously set map was extending to higher ports
+		 * than the current one, pad extra space with 0xff (no access).
+		 */
+		if (thread->io_bitmap_max < tss->io_bitmap_max) {
+			memset((char *) tss->io_bitmap +
+				thread->io_bitmap_max, 0xff,
+				tss->io_bitmap_max - thread->io_bitmap_max);
+		}
+		tss->io_bitmap_max = thread->io_bitmap_max;
+		tss->x86_tss.io_bitmap_base = IO_BITMAP_OFFSET;
+		tss->io_bitmap_owner = thread;
+		put_cpu();
+
+		return 1;
+	}
+	put_cpu();
+
+	return 0;
+}
+
 static void __kprobes
 do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
 	long error_code, siginfo_t *info)
@@ -188,44 +229,13 @@ void __kprobes
 do_general_protection(struct pt_regs *regs, long ...
From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 9:41 am

Mark the exception handlers with "dotraplinkage" to hide the
calling convention differences between i386 and x86_64.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/entry_64.S |    2 +-
 arch/x86/kernel/traps_32.c |   28 +++++++++-------
 arch/x86/kernel/traps_64.c |   31 +++++++++++-------
 include/asm-x86/traps.h    |   73 +++++++++++++++++++++----------------------
 4 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 1924659..291dd21 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1266,7 +1266,7 @@ ENTRY(simd_coprocessor_error)
 END(simd_coprocessor_error)
 
 ENTRY(device_not_available)
-	zeroentry math_state_restore
+	zeroentry do_device_not_available
 END(device_not_available)
 
 	/* runs on exception stack */
diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index 78113d3..6ecc1b7 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -191,7 +191,7 @@ vm86_trap:
 }
 
 #define DO_ERROR(trapnr, signr, str, name)				\
-void do_##name(struct pt_regs *regs, long error_code)			\
+dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 {									\
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr)	\
 							== NOTIFY_STOP)	\
@@ -201,7 +201,7 @@ void do_##name(struct pt_regs *regs, long error_code)			\
 }
 
 #define DO_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr)		\
-void do_##name(struct pt_regs *regs, long error_code)			\
+dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 {									\
 	siginfo_t info;							\
 	info.si_signo = signr;						\
@@ -225,7 +225,7 @@ DO_ERROR(11, SIGBUS, "segment not present", segment_not_present)
 DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
 DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
 
-void __kprobes
+dotraplinkage void __kprobes
 ...
From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 9:41 am

Make the x86_64-version and the i386-version of do_debug
more similar.

 - introduce preempt_conditional_sti/cli to i386. The preempt-count
	is now elevated during the trap handler, like on x86_64. It
	does not run on a separate stack, however.
 - replace an open-coded "send_sigtrap"
 - copy some comments

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/kernel/traps_32.c |   30 +++++++++++++++++++++---------
 arch/x86/kernel/traps_64.c |   17 +++++++++--------
 include/asm-x86/ptrace.h   |    4 ----
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
index 6ecc1b7..da97cd2 100644
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -89,6 +89,20 @@ static inline void conditional_sti(struct pt_regs *regs)
 		local_irq_enable();
 }
 
+static inline void preempt_conditional_sti(struct pt_regs *regs)
+{
+	inc_preempt_count();
+	if (regs->flags & X86_EFLAGS_IF)
+		local_irq_enable();
+}
+
+static inline void preempt_conditional_cli(struct pt_regs *regs)
+{
+	if (regs->flags & X86_EFLAGS_IF)
+		local_irq_disable();
+	dec_preempt_count();
+}
+
 static inline void
 die_if_kernel(const char *str, struct pt_regs *regs, long err)
 {
@@ -499,7 +513,7 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
 dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 {
 	struct task_struct *tsk = current;
-	unsigned int condition;
+	unsigned long condition;
 	int si_code;
 
 	get_debugreg(condition, 6);
@@ -517,9 +531,9 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
 	if (notify_die(DIE_DEBUG, "debug", regs, condition, error_code,
 						SIGTRAP) == NOTIFY_STOP)
 		return;
+
 	/* It's safe to allow irq's after DR6 has been saved */
-	if (regs->flags & X86_EFLAGS_IF)
-		local_irq_enable();
+	preempt_conditional_sti(regs);
 
 	/* Mask out spurious debug traps due to lazy DR7 setting ...
From: Alexander van Heukelum
Date: Tuesday, September 30, 2008 - 12:37 pm

x86_64-kernels after commit 10a434fcb "x86: remove cpu_vendor_dev"
crashed on qemu-system-x86_64 due to a typo in vmlinux_64.lds.S.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>

---


It took quite some time, but I found the problem... I'll leave
the other one to you ;).

Greetings,
	Alexander

 arch/x86/kernel/vmlinux_64.lds.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/vmlinux_64.lds.S b/arch/x86/kernel/vmlinux_64.lds.S
index 201e81a..46e0544 100644
--- a/arch/x86/kernel/vmlinux_64.lds.S
+++ b/arch/x86/kernel/vmlinux_64.lds.S
@@ -172,8 +172,8 @@ SECTIONS
   .x86_cpu_dev.init : AT(ADDR(.x86_cpu_dev.init) - LOAD_OFFSET) {
 	*(.x86_cpu_dev.init)
   }
-  SECURITY_INIT
   __x86_cpu_dev_end = .;
+  SECURITY_INIT
 
   . = ALIGN(8);
   .parainstructions : AT(ADDR(.parainstructions) - LOAD_OFFSET) {
--

From: Yinghai Lu
Date: Tuesday, September 30, 2008 - 1:37 pm

On Tue, Sep 30, 2008 at 12:37 PM, Alexander van Heukelum

that is merging problem, Ingo should fix that already...

YH
--

From: Ingo Molnar
Date: Tuesday, September 30, 2008 - 11:49 pm

indeed. I've fixed it in tip/x86/traps by applying Alexander's patch.

	Ingo
--

From: Yinghai Lu
Date: Wednesday, October 1, 2008 - 12:22 am

still have merging problem... in tip/master we have

  __x86_cpu_dev_start = .;
  .x86_cpu_dev.init : AT(ADDR(.x86_cpu_dev.init) - LOAD_OFFSET) {
        *(.x86_cpu_dev.init)
  }
  __x86_cpu_dev_end = .;
  SECURITY_INIT

  DYN_ARRAY_INIT(8)

  SECURITY_INIT

there is two copy of SECURITY_INIT

YH
--

From: Ingo Molnar
Date: Wednesday, October 1, 2008 - 12:36 am

indeed. I fixed this up now.

interestingly, this seems to be one of the rare cases where Git 
auto-merge does the wrong thing - tt should have detected a conflict.

	Ingo
--

From: Yinghai Lu
Date: Wednesday, October 1, 2008 - 7:53 am

should let git guys know that.

YH
--

From: Ingo Molnar
Date: Wednesday, October 1, 2008 - 12:06 am

hm, commit 021f8b7 was missing, cf16970 got detached from that commit. I 
fixed it all up in tip/x86/traps.

	Ingo
--

Previous thread: none

Next thread: sleeping function called from invalid context at kernel/mutex.c by Sitsofe Wheeler on Tuesday, September 30, 2008 - 10:00 am. (1 message)