x86: Add support for ipipe_get_irq_regs

From: Jan Kiszka <[email protected]>

Implement the x86 arch bits for ipipe_get_irq_regs support. This allows to drop __ipipe_tick_regs and use the new service instead.

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

---
 arch/x86/include/asm/ipipe.h    |   43 +++++++++++++++++++++++++++++++++++++-
 arch/x86/include/asm/ipipe_32.h |    2 +-
 arch/x86/include/asm/ipipe_64.h |    2 +-
 arch/x86/kernel/ipipe.c         |   26 +++--------------------
 4 files changed, 47 insertions(+), 26 deletions(-)

arch/x86/include/asm/ipipe.h +41 -2

index 4d711dd..971b3f3 100644
@@ -31,8 +31,6 @@
 #define IPIPE_PATCH_NUMBER	4
 #endif
 
-DECLARE_PER_CPU(struct pt_regs, __ipipe_tick_regs);
-
 static inline unsigned __ipipe_get_irq_vector(int irq)
 {
 #ifdef CONFIG_X86_IO_APIC

@@ -153,4 +151,45 @@ int __ipipe_check_tickdev(const char *devname);
 #define __ipipe_move_root_irq(irq)	do { } while (0)
 #endif /* !(CONFIG_SMP && CONFIG_IPIPE) */
 
+static inline void
+__ipipe_setup_irq_regs(struct pt_regs **orig_regs, struct pt_regs *saved_regs)
+{
+	*orig_regs = ipipe_get_irq_regs();
+
+	if (*orig_regs)
+		saved_regs->orig_ax = (*orig_regs)->orig_ax;
+	else {
+		saved_regs->flags = X86_EFLAGS_IF;
+		saved_regs->cs = __KERNEL_CS;
+#ifdef CONFIG_X86_32
+		saved_regs->ss = __KERNEL_DS;
+		__asm__ __volatile__ ("here: movl $here, %0\n\t"
+				      "movl %%ebp, %1\n\t"
+				      "movl %%esp, %2\n\t"
+				      : "=m" (saved_regs->ip),
+					"=m" (saved_regs->bp),
+					"=m" (saved_regs->sp));
+#else /* CONFIG_X86_64 */
+		saved_regs->ss = 0;
+		__asm__ __volatile__ ("here: movq $here, %0\n\t"
+				      "movq %%rbp, %1\n\t"
+				      "movq %%rsp, %2\n\t"
+				      : "=m" (saved_regs->ip),
+					"=m" (saved_regs->bp),
+					"=m" (saved_regs->sp));
+#endif /* CONFIG_X86_64 */
+		__ipipe_get_cpu_var(ipipe_irq_regs) = saved_regs;
+	}
+}
+
+static inline void
+__ipipe_cleanup_irq_regs(struct pt_regs *orig_regs,
+			 struct pt_regs *saved_regs)
+{
+	if (orig_regs)
+		orig_regs->orig_ax = saved_regs->orig_ax;
+	else
+		__ipipe_get_cpu_var(ipipe_irq_regs) = NULL;
+}
+
 #endif	/* !__X86_IPIPE_H */

arch/x86/include/asm/ipipe_32.h +1 -1

index 8d1f4b5..ce3d417 100644
@@ -65,7 +65,7 @@ void __ipipe_end_edge_irq(unsigned irq, struct irq_desc *desc);
 static inline void __ipipe_call_root_xirq_handler(unsigned irq,
 						  ipipe_irq_handler_t handler)
 {
-	struct pt_regs *regs = &__raw_get_cpu_var(__ipipe_tick_regs);
+	struct pt_regs *regs = ipipe_get_irq_regs();
 
 	regs->orig_ax = ~__ipipe_get_irq_vector(irq);
 

arch/x86/include/asm/ipipe_64.h +1 -1

index bc427b8..4452662 100644
@@ -63,7 +63,7 @@ void __ipipe_end_edge_irq(unsigned irq, struct irq_desc *desc);
 static inline void __ipipe_call_root_xirq_handler(unsigned irq,
 						  void (*handler)(unsigned, void *))
 {
-	struct pt_regs *regs = &__raw_get_cpu_var(__ipipe_tick_regs);
+	struct pt_regs *regs = ipipe_get_irq_regs();
 
 	regs->orig_ax = ~__ipipe_get_irq_vector(irq);
 

arch/x86/kernel/ipipe.c +4 -22

index 521ec53..23b6908 100644
@@ -900,11 +900,14 @@ int __ipipe_syscall_root(struct pt_regs *regs)
  */
 int __ipipe_handle_irq(struct pt_regs *regs)
 {
+	struct pt_regs *old_regs = __ipipe_get_cpu_var(ipipe_irq_regs);
 	struct ipipe_domain *this_domain, *next_domain;
 	unsigned int vector = regs->orig_ax, irq;
 	struct list_head *head, *pos;
 	int m_ack;
 
+	__ipipe_get_cpu_var(ipipe_irq_regs) = regs;
+
 	if ((long)regs->orig_ax < 0) {
 		vector = ~vector;
 #ifdef CONFIG_X86_LOCAL_APIC

@@ -976,28 +979,7 @@ int __ipipe_handle_irq(struct pt_regs *regs)
 	__ipipe_walk_pipeline(head);
 
 finalize_nosync:
-
-	/*
-	 * Given our deferred dispatching model for regular IRQs, we
-	 * only record CPU regs for the last timer interrupt, so that
-	 * the timer handler charges CPU times properly. It is assumed
-	 * that other interrupt handlers don't actually care for such
-	 * information.
-	 */
-
-	if (irq == __ipipe_tick_irq) {
-		struct pt_regs *tick_regs = &__raw_get_cpu_var(__ipipe_tick_regs);
-		tick_regs->flags = regs->flags;
-		tick_regs->cs = regs->cs;
-		tick_regs->ip = regs->ip;
-		tick_regs->bp = regs->bp;
-#ifdef CONFIG_X86_64
-		tick_regs->ss = regs->ss;
-		tick_regs->sp = regs->sp;
-#endif
-		if (!ipipe_root_domain_p)
-			tick_regs->flags &= ~X86_EFLAGS_IF;
-	}
+	__ipipe_get_cpu_var(ipipe_irq_regs) = old_regs;
 
 	if (!ipipe_root_domain_p ||
 	    test_bit(IPIPE_STALL_FLAG, &ipipe_root_cpudom_var(status)))

Quoted 4 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Implement the x86 arch bits for ipipe_get_irq_regs support. This allows to drop __ipipe_tick_regs and use the new service instead.

I'm unsure whether this patch would actually replace __ipipe_tick_regs properly, particularly regarding how the profiling code works.

But I'm now convinced that we are on the wrong track, with trying to track the IRQ frame, for the purpose we discussed on the Xenomai mailing list. In fact, Gilles already told us about the best approach, we did not pay attention enough, though:
https://mail.gna.org/public/xenomai-help/2010-06/msg00033.html

Basically, we want to get a hold of the register frame of a task context before it returns to user-space, so that we can do some fixups. Therefore, the best option is clearly to add a special event, that the pipeline would dispatch upon request, when:

- __ipipe_grab/handle_irq is about to return to userland
- __ipipe_syscall_root is about to do the same

In those contexts, we do have the _real_ register frame for the context, which removes the restriction on faked IRQ frames induced by the deferred interrupt model.

We could then use that feature, not only for
having a more graceful watchdog, but for allowing Xenomai's real-time signals to preempt syscall-less code as well.

I worked on this lately, and eventually coupled that event to a generalized support for forcing userland to run a kernel exit. I'll post more info to the Xenomai list.

Quoted 7 lines · Philippe Gerum

Quoted 4 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Implement the x86 arch bits for ipipe_get_irq_regs support. This allows to drop __ipipe_tick_regs and use the new service instead.

I'm unsure whether this patch would actually replace __ipipe_tick_regs properly, particularly regarding how the profiling code works.

tick_regs are a "workaround", this approach appears to me way closer to how native works.

Quoted 8 lines · Jan Kiszka

Quoted 7 lines · Philippe Gerum

Quoted 4 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Implement the x86 arch bits for ipipe_get_irq_regs support. This allows to drop __ipipe_tick_regs and use the new service instead.

I'm unsure whether this patch would actually replace __ipipe_tick_regs properly, particularly regarding how the profiling code works.

Quoted 2 lines

tick_regs are a "workaround", this approach appears to me way closer to how native works.

We do not want ipipe_tick_regs to work the way the native kernel works. We want to fool the kernel by passing it the value of the registers at the moment of the real timer tick so that its accounting works more or less reliably. When the kernel used the real registers it got the accounting wrong.

Quoted 9 lines · Gilles Chanteperdrix

Quoted 8 lines · Jan Kiszka

Quoted 7 lines · Philippe Gerum

Quoted 4 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Implement the x86 arch bits for ipipe_get_irq_regs support. This allows to drop __ipipe_tick_regs and use the new service instead.

I'm unsure whether this patch would actually replace __ipipe_tick_regs properly, particularly regarding how the profiling code works.

Quoted 8 lines

Quoted 2 lines

tick_regs are a "workaround", this approach appears to me way closer to how native works.

We do not want ipipe_tick_regs to work the way the native kernel works. We want to fool the kernel by passing it the value of the registers at the moment of the real timer tick so that its accounting works more or less reliably. When the kernel used the real registers it got the accounting wrong.

Right, though current tick_regs approach is not accurate either: the preempted task gets the full time slice accounted, neglecting the actual schedule inside the preempting domain.

Jan

_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main