From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4B5EB6D2.9040206@domain.hid> Date: Tue, 26 Jan 2010 10:33:06 +0100 From: Jan Kiszka MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return List-Id: Xenomai life and development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Philippe Gerum , Gilles Chanteperdrix , Wolfgang Mauerer Cc: adeos-main , xenomai-core If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. arch/x86/kernel/ipipe.c | 81 +++++++++++++++++++++++++++++----------------- 1 files changed, 51 insertions(+), 30 deletions(-) diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c index 4442d96..b471355 100644 --- a/arch/x86/kernel/ipipe.c +++ b/arch/x86/kernel/ipipe.c @@ -702,19 +702,23 @@ static int __ipipe_xlate_signo[] = { int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) { - unsigned long flags; - - /* Pick up the root domain state of the interrupted context. */ - local_save_flags(flags); + bool root_entry = false; + unsigned long flags = 0; if (ipipe_root_domain_p) { - /* - * Replicate hw interrupt state into the virtual mask before - * calling the I-pipe event handler over the root domain. Also - * required later when calling the Linux exception handler. - */ - if (irqs_disabled_hw()) + root_entry = true; + + local_save_flags(flags); + + if (irqs_disabled_hw()) { + /* + * Replicate hw interrupt state into the virtual mask + * before calling the I-pipe event handler over the + * root domain. Also required later when calling the + * Linux exception handler. + */ local_irq_disable(); + } } #ifdef CONFIG_KGDB /* catch exception KGDB is interested in over non-root domains */ @@ -725,18 +729,22 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 1; } - /* - * 32-bit: In case we migrated to root domain inside the event - * handler, restore the original IF from exception entry as the - * low-level return code will evaluate it. - */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); - - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* + * 32-bit: In case we faulted in the iret path, regs.flags do + * not match the root domain state as the low-level return + * code will evaluate it. Fix this up, either by the root + * state sampled on entry or, if we migrated to root, with the + * current state. + */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); + } else { /* Detect unhandled faults over non-root domains. */ struct ipipe_domain *ipd = ipipe_current_domain; @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) * Relevant for 64-bit: Restore root domain state as the low-level * return code will not align it to regs.flags. */ - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 0; } int __ipipe_divert_exception(struct pt_regs *regs, int vector) { - unsigned long flags; - - /* Same root state handling as in __ipipe_handle_exception. */ - local_save_flags(flags); + bool root_entry = false; + unsigned long flags = 0; if (ipipe_root_domain_p) { - if (irqs_disabled_hw()) + root_entry = true; + + local_save_flags(flags); + + if (irqs_disabled_hw()) { + /* + * Same root state handling as in + * __ipipe_handle_exception. + */ local_irq_disable(); + } } #ifdef CONFIG_KGDB /* catch int1 and int3 over non-root domains */ @@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 1; } + if (likely(ipipe_root_domain_p)) { + /* see __ipipe_handle_exception */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); + } + /* - * 32-bit: Due to possible migration inside the event handler, we have - * to restore IF so that low-level return code sets the root domain - * state correctly. + * No need to restore root state in the 64-bit case, the Linux handler + * and the return code will take care of it. */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); return 0; } -- 1.6.0.2