Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Jan Kiszka wrote:
This patch addresses the recently discovered issue that I-pipe actually
need to deal with faults over non-root domain in which the current
domain shows no interest in. Such faults could be triggered inside
copy_*_user, thus can cleanly be handled by Linux - if we only allow for
this. Currently, if debugging is on, we warn about a potential bug, and
corrupt the pipeline states otherwise.
The new approach is to unconditionally drop to root domain in such
cases, but - for debugging purposes of non-fixable faults - keep track
of the original domain and report it on oops.
Similar patches are required for other archs. Maybe I can look into
x86_64 later.
Nak, this patch would not work as wanted. Again, what you need is to
always fixup, and conditionally send a bug report to the kernel log if
CONFIG_IPIPE_DEBUG is enabled, nothing more.
This patch assumes that die() is always going to be fired for any
in-kernel fault, so that all reports only need to go through this
routine, which is wrong. Kernel fixups through exception tables may fix
the fault early and silently, and this is particularly the case for
copy_to_user helpers, which do include kernel fixup code. By being
silent when fixing up things in __ipipe_handle_exception() like your
patch currently is, we would be left with no trace at all that some
unhandled fault just happened, except by looking at /proc/xenomai/faults.
As you are still remain vague on the actual problematic scenarios, I
will try to go through them, and maybe you can add/correct what I miss:
- faults in user land = can be silently handled by Linux after
dropping to root domain. This lowering is perfectly fine as the
higher domains showed no interest in the fault, thus are currently
running in domain-agnostic code paths anyway.
- faults on fixable kernel addresses = same as above. If the high
domains fail to evaluate the fix-up result, it's not I-pipe's fault.
- minor faults on kernel addresses (more precisely: in the I-pipe core
or some I-pipe user) = those would now went unnoticed and need
further thoughts, granted.
+
- major faults on kernel addresses = still generate major oopses and
will thus be visible.
Did I missed something? If not, I would now start addressing the
remaining problematic scenario directly instead of throwing all into the
same pot.
By sending the report immediately when fixing up in the latter routine,
you also avoid the ugly ipipe_orig_domain stuff.
It's not nice, but it is at least as ugly as reporting a kernel BUG when
there is only a gracefully fixable bug in user code. I definitely do not
agree with your approach as well, and I'm convinced we need to find a
third way here.
OK, I think this should be acceptable for everyone. It's basically your
approach, reworked to avoid false positives. Moreover, it
unconditionally sets the root domain - should be faster.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
---
arch/i386/kernel/ipipe.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
Index: linux-2.6.23-ipipe/arch/i386/kernel/ipipe.c
===
--- linux-2.6.23-ipipe.orig/arch/i386/kernel/ipipe.c
+++ linux-2.6.23-ipipe/arch/i386/kernel/ipipe.c
@@ -643,26 +643,32 @@ fastcall int __ipipe_handle_exception(st
}
#endif /* CONFIG_KGDB */
- if (!ipipe_trap_notify(vector, regs)) {
-#ifdef CONFIG_IPIPE_DEBUG
- if (!ipipe_root_domain_p) {
- /* Fix up domain so that Linux can handle this. */
- ipipe_current_domain = ipipe_root_domain;
- ipipe_trace_panic_freeze();
- printk(KERN_ERR BUG: Unhandled exception over domain
- %s - switching to ROOT\n,
- ipipe_current_domain-name);
- }
-#endif /* CONFIG_IPIPE_DEBUG */
- __ipipe_std_extable[vector](regs, error_code);
+ if (unlikely(ipipe_trap_notify(vector, regs))) {
local_irq_restore(flags);
- __fixup_if(regs);
- return 0;
+ return 1;
}
- local_irq_restore(flags);
+#ifdef CONFIG_IPIPE_DEBUG
+ /* Warn about unhandled faults over non-root domains in kernel space
+ * unless they occured at fixable locations. */
+ if (unlikely(!ipipe_root_domain_p) !(error_code 4)
+ !search_exception_tables(regs-eip)) {
+ struct ipipe_domain *ipd = ipipe_current_domain;
+ ipipe_current_domain = ipipe_root_domain;
+ ipipe_trace_panic_freeze();
+ printk(KERN_ERR BUG: Unhandled exception over domain
+ %s - switching to ROOT\n, ipd-name);
+ dump_stack();
+ }
+#endif /* CONFIG_IPIPE_DEBUG */
- return 1;
+ /* Always switch to root so that Linux can handle it cleanly. */
+ ipipe_current_domain = ipipe_root_domain;
+
+ __ipipe_std_extable[vector](regs, error_code);
+ local_irq_restore(flags);
+ __fixup_if(regs);
+ return 0;
}
fastcall int __ipipe_divert_exception(struct pt_regs *regs, int
