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 <jan.kis...@siemens.com>
---

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

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to