Jan Kiszka wrote:
> 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.
Attached the equivalent x86_64 version. Works fine here as well.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
---
arch/x86_64/kernel/ipipe.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
Index: linux-2.6.23.1-xeno_64/arch/x86_64/kernel/ipipe.c
===================================================================
--- linux-2.6.23.1-xeno_64.orig/arch/x86_64/kernel/ipipe.c
+++ linux-2.6.23.1-xeno_64/arch/x86_64/kernel/ipipe.c
@@ -612,17 +612,32 @@ asmlinkage int __ipipe_handle_exception(
}
#endif /* CONFIG_KGDB */
- if (!ipipe_trap_notify(vector, regs)) {
- __ipipe_exptr handler = __ipipe_std_extable[vector];
- handler(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(instruction_pointer(regs))) {
+ 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;
}
asmlinkage int __ipipe_divert_exception(struct pt_regs *regs, int vector)
_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main
