ipipe: Re-fix migration helper for legacy users

PATCH In review Jan Kiszka 4 participants 10 messages Jan 16, 2013 – Jan 17, 2013

Share & tools

Jan Kiszka

author

2013-01-16 19:17 UTC #0 lore raw

PATCH 1/1 ipipe: Re-fix migration helper for legacy users

In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.

Signed-off-by Jan Kiszka <[email protected]>

---

For the case my analysis was right. With this, Xenomai 2.6 works again here.

 kernel/ipipe/core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

kernel/ipipe/core.c +2 -0

index 6929156..4695ba8 100644
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
 	hard_local_irq_disable();
 #endif
 	x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
 	if (x)
+#endif
 		complete_domain_migration();
 
 #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH

Signature · 1 line

1.7.3.4

Gilles Chanteperdrix

2013-01-16 19:53 UTC #1 lore raw

Quoted 18 lines · Jan Kiszka

In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
 kernel/ipipe/core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644

kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132

Quoted 9 lines

kernel/ipipe/core.c +2 -0

@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
 	hard_local_irq_disable();
 #endif
 	x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
 	if (x)
+#endif
 		complete_domain_migration();
 
 #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH

Mmm, from the old patch it seems complete_domain_migration should not be called for legacy, so, what we would like instead is:

#ifdef CONFIG_IPIPE_LEGACY
	current->state &= ~TASK_HARDENING;
#else
	if (x)
		complete_domain_migration();

#endif

Signature · 1 line

                                                                Gilles.

Jan Kiszka

author

2013-01-16 20:09 UTC #2 lore raw

Quoted 34 lines · Gilles Chanteperdrix

Quoted 18 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
 kernel/ipipe/core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644
kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 9 lines
kernel/ipipe/core.c +2 -0
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
 	hard_local_irq_disable();
 #endif
 	x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
 	if (x)
+#endif
 		complete_domain_migration();
 
 #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
Mmm, from the old patch it seems complete_domain_migration should not be called for legacy, so, what we would like instead is:

Quoted 7 lines

#ifdef CONFIG_IPIPE_LEGACY
	current->state &= ~TASK_HARDENING;
#else
	if (x)
		complete_domain_migration();

#endif

That's equivalent after the new patch (there are two versions of complete_domain_migration now).

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Philippe Gerum

2013-01-17 08:26 UTC #3 lore raw

In reply to Jan Kiszka

Quoted 19 lines · Jan Kiszka

In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
  kernel/ipipe/core.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644

kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132

Quoted 9 lines

kernel/ipipe/core.c +2 -0

@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
  	hard_local_irq_disable();
  #endif
  	x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
  	if (x)
+#endif
  		complete_domain_migration();
 
  #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH

The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.

http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225

Signature · 1 line

Philippe.

Jan Kiszka

2013-01-17 08:34 UTC #4 lore raw

Quoted 27 lines · Philippe Gerum

Quoted 26 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
  kernel/ipipe/core.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +2 -0
index 6929156..4695ba8 100644
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
      hard_local_irq_disable();
  #endif
      x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
      if (x)
+#endif
          complete_domain_migration();
 

Quoted 7 lines

kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 2 lines
kernel/ipipe/core.c +0 -0
  #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.
http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225

Would you push your new version?

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20130117/752d95bd/attachment.pgp>

Philippe Gerum

2013-01-17 08:51 UTC #5 lore raw

Quoted 34 lines · Jan Kiszka

Quoted 33 lines · Philippe Gerum
Quoted 19 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
   kernel/ipipe/core.c |    2 ++
   1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644
kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 9 lines
kernel/ipipe/core.c +2 -0
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
       hard_local_irq_disable();
   #endif
       x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
       if (x)
+#endif
           complete_domain_migration();
 
   #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.

Quoted 5 lines

Quoted 1 line
http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225
Would you push your new version?
Jan

These patches introduced between core-3.5-2 and core-3.5-3 were not propagated to core-3.5.7. They are on their way now.

ba77ad1 powerpc/ipipe: prevent spurious hw IRQ enabling in activate_mm() 8b11dc5 ipipe: fix domain migration helper
1f0d763 x86_64/ipipe: introduce ipipe_clock_name()

Signature · 1 line

Philippe.

Philippe Gerum

2013-01-17 09:12 UTC #6 lore raw

Quoted 40 lines · Philippe Gerum

Quoted 39 lines · Jan Kiszka
Quoted 27 lines · Philippe Gerum
Quoted 26 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
   kernel/ipipe/core.c |    2 ++
   1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +2 -0
index 6929156..4695ba8 100644
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
       hard_local_irq_disable();
   #endif
       x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
       if (x)
+#endif
           complete_domain_migration();
 
Quoted 8 lines
kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 2 lines
kernel/ipipe/core.c +0 -0
   #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.
http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225
Would you push your new version?

Quoted 9 lines

Quoted 1 line
Jan
These patches introduced between core-3.5-2 and core-3.5-3 were not propagated to core-3.5.7. They are on their way now.
ba77ad1 powerpc/ipipe: prevent spurious hw IRQ enabling in activate_mm() 8b11dc5 ipipe: fix domain migration helper
1f0d763 x86_64/ipipe: introduce ipipe_clock_name()

8b11dc5 would reintroduce the issue you saw in legacy mode though. Fix on its way.

Signature · 1 line

Philippe.

Jan Kiszka

2013-01-17 09:14 UTC #7 lore raw

Quoted 48 lines · Philippe Gerum

Quoted 47 lines · Philippe Gerum
Quoted 35 lines · Jan Kiszka
Quoted 34 lines · Philippe Gerum
Quoted 20 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not
be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
   kernel/ipipe/core.c |    2 ++
   1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644
kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 9 lines
kernel/ipipe/core.c +2 -0
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
       hard_local_irq_disable();
   #endif
       x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
       if (x)
+#endif
           complete_domain_migration();
 
   #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.
Quoted 7 lines
Quoted 3 lines
http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225
Would you push your new version?
Jan
These patches introduced between core-3.5-2 and core-3.5-3 were not propagated to core-3.5.7. They are on their way now.

Quoted 7 lines

Quoted 4 lines
ba77ad1 powerpc/ipipe: prevent spurious hw IRQ enabling in activate_mm() 8b11dc5 ipipe: fix domain migration helper
1f0d763 x86_64/ipipe: introduce ipipe_clock_name()
8b11dc5 would reintroduce the issue you saw in legacy mode though. Fix on its way.

Not sure what you were looking at: 8b11dc5 is part of your public core-3.5 branch. I was rebasing over it, testing the result and found his bug.

Jan

Philippe Gerum

2013-01-17 09:19 UTC #8 lore raw

Quoted 54 lines · Jan Kiszka

Quoted 53 lines · Philippe Gerum
Quoted 51 lines · Philippe Gerum
Quoted 35 lines · Jan Kiszka
Quoted 34 lines · Philippe Gerum
Quoted 20 lines · Jan Kiszka
In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not
be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
    kernel/ipipe/core.c |    2 ++
    1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644
kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132
Quoted 9 lines
kernel/ipipe/core.c +2 -0
@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
        hard_local_irq_disable();
    #endif
        x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
        if (x)
+#endif
            complete_domain_migration();
 
    #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH
The analysis is right. I already fixed this bug in the 3.5 branch, we are missing a forward port to 3.5.7.
Quoted 7 lines
Quoted 3 lines
http://git.denx.de/?p=ipipe-2.6.git;a=commit;h=8b11dc5fee73842a24ec4367b57aaa2dcdd67225
Would you push your new version?
Jan
These patches introduced between core-3.5-2 and core-3.5-3 were not propagated to core-3.5.7. They are on their way now.
ba77ad1 powerpc/ipipe: prevent spurious hw IRQ enabling in activate_mm() 8b11dc5 ipipe: fix domain migration helper
1f0d763 x86_64/ipipe: introduce ipipe_clock_name()

Quoted 9 lines

Quoted 2 lines
8b11dc5 would reintroduce the issue you saw in legacy mode though. Fix on its way.
Not sure what you were looking at: 8b11dc5 is part of your public core-3.5 branch. I was rebasing over it, testing the result and found his bug.
Jan

We have two bugs in the migration handling in core-3.5.7: one preventing the hardening bit to be cleared in legacy mode when running over the head domain, the other allowing the migration hook to be fired over the head domain. The latter was fixed by 8b11dc5 in core-3.5, but we are missing that fix in core-3.5.7. However, that fix did not address the issue in legacy mode.

The new patch has to cope with both legacy and non-legacy in this area, and made available to 3.2, 3.4, 3.5 and 3.5.7.

Signature · 1 line

Philippe.

Philippe Gerum

2013-01-17 08:28 UTC #9 lore raw

In reply to Jan Kiszka

Quoted 19 lines · Jan Kiszka

In the legacy use case, __ipipe_switch_tail will be invoked over the non-root domain after hardening. So clearing of TASK_HARDENING must not be restricted to root.
Signed-off-by Jan Kiszka <[email protected]>
---
For the case my analysis was right. With this, Xenomai 2.6 works again here.
  kernel/ipipe/core.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
kernel/ipipe/core.c +0 -0
index 6929156..4695ba8 100644

kernel/ipipe/core.c int __ipipe_switch_tail(void) · line 1132

Quoted 9 lines

kernel/ipipe/core.c +2 -0

@@ -1124,7 +1124,9 @@ int __ipipe_switch_tail(void)
  	hard_local_irq_disable();
  #endif
  	x = __ipipe_root_p;
+#ifndef CONFIG_IPIPE_LEGACY
  	if (x)
+#endif
  		complete_domain_migration();
 
  #ifndef CONFIG_IPIPE_WANT_PREEMPTIBLE_SWITCH

We need this in the non-legacy case as well. Otherwise, a gatekeeper-less system like the one we use in 3.x would spuriously run the migration hook over the head domain.

Signature · 1 line

Philippe.