[PATCH V2 1/8] dovetail/pipeline: implement out-of-band irq management and handling

[PATCH V2 1/8] dovetail/pipeline: implement out-of-band irq management and handling

[hongzha1]

implement oob irq request and free and post for both
TIMER_OOB_IPI and RESCHEDULE_OOB_IPI

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/dovetail/pipeline/pipeline.h b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
index 23c4a2c18..13fbd7942 100644
--- a/include/cobalt/kernel/dovetail/pipeline/pipeline.h
+++ b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
@@ -8,9 +8,12 @@
 #include <linux/irq_pipeline.h>
 #include <cobalt/kernel/assert.h>
 #include <asm/xenomai/features.h>
+#include <pipeline/machine.h>
 
 typedef unsigned long spl_t;
 
+void xnintr_core_clock_handler(void);
+
 /*
  * We only keep the LSB when testing in SMP mode in order to strip off
  * the recursion marker (0x2) the nklock may store there.
@@ -30,18 +33,28 @@ typedef unsigned long spl_t;
 
 #ifdef CONFIG_SMP
 
+static irqreturn_t reschedule_interrupt_handler(int irq, void *dev_id)
+{
+
+	/* Will reschedule from irq_exit_pipeline. */
+
+	return IRQ_HANDLED;
+}
+
 static inline int pipeline_request_resched_ipi(void (*handler)(void))
 {
 	/* Trap the out-of-band rescheduling interrupt. */
-	TODO();
-
-	return 0;
+	return __request_percpu_irq(RESCHEDULE_OOB_IPI,
+			reschedule_interrupt_handler,
+			IRQF_OOB,
+			"Xenomai reschedule",
+			&cobalt_machine_cpudata);
 }
 
 static inline void pipeline_free_resched_ipi(void)
 {
 	/* Release the out-of-band rescheduling interrupt. */
-	TODO();
+	free_percpu_irq(RESCHEDULE_OOB_IPI, &cobalt_machine_cpudata);
 }
 
 static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
@@ -50,21 +63,29 @@ static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
 	 * Trigger the out-of-band rescheduling interrupt on remote
 	 * CPU(s).
 	 */
-	TODO();
+	irq_send_oob_ipi(RESCHEDULE_OOB_IPI, dest);
+}
+
+static irqreturn_t timer_ipi_interrupt_handler(int irq, void *dev_id)
+{
+	xnintr_core_clock_handler();
+
+	return IRQ_HANDLED;
 }
 
 static inline int pipeline_request_timer_ipi(void (*handler)(void))
 {
 	/* Trap the out-of-band timer interrupt. */
-	TODO();
-
-	return 0;
+	return __request_percpu_irq(TIMER_OOB_IPI,
+			timer_ipi_interrupt_handler,
+			IRQF_OOB, "Xenomai timer IPI",
+			&cobalt_machine_cpudata);
 }
 
 static inline void pipeline_free_timer_ipi(void)
 {
 	/* Release the out-of-band timer interrupt. */
-	TODO();
+	free_percpu_irq(TIMER_OOB_IPI, &cobalt_machine_cpudata);
 }
 
 static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
@@ -72,7 +93,7 @@ static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
 	/*
 	 * Trigger the out-of-band timer interrupt on remote CPU(s).
 	 */
-	TODO();
+	irq_send_oob_ipi(TIMER_OOB_IPI, dest);
 }
 
 #endif
-- 
2.17.1

[PATCH 2/8] dovetail/tick: implement proxy tick device installing and uninstalling

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
index 02cd86690..6a196496c 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -6,11 +6,92 @@
  */
 
 #include <linux/tick.h>
+#include <linux/clockchips.h>
 #include <cobalt/kernel/intr.h>
 #include <pipeline/tick.h>
+#include <cobalt/kernel/sched.h>
+#include <cobalt/kernel/timer.h>
 
 extern struct xnintr nktimer;
 
+static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
+
+static int proxy_set_next_ktime(ktime_t expires,
+				struct clock_event_device *proxy_dev)
+{
+	struct xnsched *sched;
+	ktime_t delta;
+	unsigned long flags;
+	int ret;
+
+	/*
+	 * When Negative delta have been observed, we set delta zero.
+	 * Or else exntimer_start() will return -ETIMEDOUT and do not
+	 * trigger shot
+	 */
+	delta = ktime_sub(expires, ktime_get_mono_fast_ns());
+	if (delta < 0)
+		delta = 0;
+
+	flags = hard_local_irq_save(); /* Prevent CPU migration. */
+	sched = xnsched_current();
+	ret = xntimer_start(&sched->htimer, delta, XN_INFINITE, XN_RELATIVE);
+	hard_local_irq_restore(flags);
+
+	return ret ? -ETIME : 0;
+}
+
+
+void xn_core_tick(struct clock_event_device *dummy) /* hard irqs off */
+{
+	xnintr_core_clock_handler();
+}
+
+static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
+{
+	struct clock_event_device *real_dev;
+	struct clock_proxy_device *dev;
+	struct xnsched *sched;
+	spl_t s;
+
+	dev = container_of(proxy_dev, struct clock_proxy_device, proxy_device);
+
+	/*
+	 * In-band wants to disable the clock hardware on entering a
+	 * tickless state, so we have to stop our in-band tick
+	 * emulation. Propagate the request for shutting down the
+	 * hardware to the real device only if we have no outstanding
+	 * OOB timers. CAUTION: the in-band timer is counted when
+	 * assessing the RQ_IDLE condition, so we need to stop it
+	 * prior to testing the latter.
+	 */
+	xnlock_get_irqsave(&nklock, s);
+	sched = xnsched_current();
+	xntimer_stop(&sched->htimer);
+	//sched->lflags |= XNTSTOP;
+
+	if (sched->lflags & XNIDLE) {
+		real_dev = dev->real_device;
+		real_dev->set_state_oneshot_stopped(real_dev);
+	}
+
+	xnlock_put_irqrestore(&nklock, s);
+
+	return 0;
+}
+
+static void setup_proxy(struct clock_proxy_device *dev)
+{
+	struct clock_event_device *proxy_dev = &dev->proxy_device;
+
+	dev->handle_oob_event = xn_core_tick;
+	proxy_dev->features |= CLOCK_EVT_FEAT_KTIME;
+	proxy_dev->set_next_ktime = proxy_set_next_ktime;
+	if (proxy_dev->set_state_oneshot_stopped)
+		proxy_dev->set_state_oneshot_stopped = proxy_set_oneshot_stopped;
+	__this_cpu_write(proxy_device, dev);
+}
+
 int pipeline_install_tick_proxy(void)
 {
 	int ret;
@@ -20,7 +101,7 @@ int pipeline_install_tick_proxy(void)
 		return ret;
 
 	/* Install the proxy tick device */
-	TODO();	ret = 0;
+	ret = tick_install_proxy(setup_proxy, &xnsched_realtime_cpus);
 	if (ret)
 		goto fail_proxy;
 
@@ -35,7 +116,7 @@ fail_proxy:
 void pipeline_uninstall_tick_proxy(void)
 {
 	/* Uninstall the proxy tick device. */
-	TODO();
+	tick_uninstall_proxy(&xnsched_realtime_cpus);
 
 	pipeline_free_timer_ipi();
 
-- 
2.17.1

[PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index 28d89b44b..db982d969 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -17,17 +17,7 @@ static inline u64 pipeline_read_cycle_counter(void)
 	return  ktime_get_raw_fast_ns();
 }
 
-static inline void pipeline_set_timer_shot(unsigned long cycles)
-{
-	/*
-	 * N/A. Revisit: xnclock_core_local_shot() should go to the
-	 * I-pipe section, we do things differently on Dovetail via
-	 * the proxy tick device. As a consequence,
-	 * pipeline_set_timer_shot() should not be part of the
-	 * pipeline interface.
-	 */
-	TODO();
-}
+inline void pipeline_set_timer_shot(unsigned long cycles);
 
 static inline const char *pipeline_timer_name(void)
 {
diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
index 6a196496c..55039f96e 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -16,6 +16,43 @@ extern struct xnintr nktimer;
 
 static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
 
+inline void pipeline_set_timer_shot(unsigned long cycles)
+{
+	/*
+	 * N/A. Revisit: xnclock_core_local_shot() should go to the
+	 * I-pipe section, we do things differently on Dovetail via
+	 * the proxy tick device. As a consequence,
+	 * pipeline_set_timer_shot() should not be part of the
+	 * pipeline interface.
+	 */
+	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
+	struct clock_event_device *real_dev = dev->real_device;
+	int ret;
+	u64 sumcyc;
+	ktime_t t;
+
+	if (real_dev->features & CLOCK_EVT_FEAT_KTIME) {
+		t = ktime_add(cycles, xnclock_core_read_raw());
+		real_dev->set_next_ktime(t, real_dev);
+	} else {
+		if (cycles <= 0)
+			cycles = real_dev->min_delta_ns;
+		else {
+			cycles = min_t(int64_t, cycles,
+					real_dev->max_delta_ns);
+			cycles = max_t(int64_t, cycles,
+					real_dev->min_delta_ns);
+		}
+		sumcyc = ((u64)cycles * real_dev->mult) >> real_dev->shift;
+
+		ret = real_dev->set_next_event(sumcyc, real_dev);
+		if (ret) {
+			ret = real_dev->set_next_event(real_dev->min_delta_ticks,
+							real_dev);
+		}
+	}
+}
+
 static int proxy_set_next_ktime(ktime_t expires,
 				struct clock_event_device *proxy_dev)
 {
-- 
2.17.1

[PATCH V2 4/8] dovetail/clock: implement pipeline_timer_name

[hongzha1]

to get the name of the current clock event chip, which is the real
device controlled by the proxy tick device.

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index db982d969..d2de1cd34 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -19,16 +19,7 @@ static inline u64 pipeline_read_cycle_counter(void)
 
 inline void pipeline_set_timer_shot(unsigned long cycles);
 
-static inline const char *pipeline_timer_name(void)
-{
-	/*
-	 * Return the name of the current clock event chip, which is
-	 * the real device controlled by the proxy tick device.
-	 */
-	TODO();
-
-	return "?";
-}
+inline const char *pipeline_timer_name(void);
 
 static inline const char *pipeline_clock_name(void)
 {
diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
index 55039f96e..bf267b9d9 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -16,6 +16,18 @@ extern struct xnintr nktimer;
 
 static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
 
+inline const char *pipeline_timer_name(void)
+{
+	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
+	struct clock_event_device *real_dev = dev->real_device;
+
+	/*
+	 * Return the name of the current clock event chip, which is
+	 * the real device controlled by the proxy tick device.
+	 */
+	return real_dev->name;
+}
+
 inline void pipeline_set_timer_shot(unsigned long cycles)
 {
 	/*
-- 
2.17.1

[PATCH 5/8] dovetail/kevents: dovetail: implement handle_ptrace_cont

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
index b5dc50ea7..378017bea 100644
--- a/kernel/cobalt/dovetail/kevents.c
+++ b/kernel/cobalt/dovetail/kevents.c
@@ -473,7 +473,7 @@ int handle_ptrace_resume(struct task_struct *tracee)
 	return KEVENT_PROPAGATE;
 }
 
-static void handle_ptrace_cont(void)
+static void handle_ptrace_cont(struct task_struct *next_task)
 {
 	/*
 	 * This is the place where the ptrace-related work which used
@@ -482,7 +482,77 @@ static void handle_ptrace_cont(void)
 	 * stopped state, which is what look for in
 	 * handle_schedule_event().
 	 */
-	TODO();
+	struct task_struct *prev_task;
+	struct xnthread *next;
+	sigset_t pending;
+	spl_t s;
+
+	cobalt_signal_yield();
+
+	prev_task = current;
+	next = xnthread_from_task(next_task);
+	if (next == NULL)
+		return;
+
+	xnlock_get_irqsave(&nklock, s);
+
+	/*
+	 * Track tasks leaving the ptraced state.  Check both SIGSTOP
+	 * (NPTL) and SIGINT (LinuxThreads) to detect ptrace
+	 * continuation.
+	 */
+	if (xnthread_test_state(next, XNSSTEP)) {
+		if (signal_pending(next_task)) {
+			/*
+			 * Do not grab the sighand lock here: it's
+			 * useless, and we already own the runqueue
+			 * lock, so this would expose us to deadlock
+			 * situations on SMP.
+			 */
+			sigorsets(&pending,
+				  &next_task->pending.signal,
+				  &next_task->signal->shared_pending.signal);
+			if (sigismember(&pending, SIGSTOP) ||
+			    sigismember(&pending, SIGINT))
+				goto no_ptrace;
+		}
+
+		/*
+		 * Do not unregister before the thread migrated.
+		 * unregister_debugged_thread will then be called by our
+		 * resume_oob_task.
+		 */
+		if (!xnthread_test_info(next, XNCONTHI))
+			unregister_debugged_thread(next);
+
+		xnthread_set_localinfo(next, XNHICCUP);
+	}
+
+no_ptrace:
+	xnlock_put_irqrestore(&nklock, s);
+
+	/*
+	 * Do basic sanity checks on the incoming thread state.
+	 * NOTE: we allow ptraced threads to run shortly in order to
+	 * properly recover from a stopped state.
+	 */
+	if (!XENO_WARN(COBALT, !xnthread_test_state(next, XNRELAX),
+		       "hardened thread %s[%d] running in Linux domain?! "
+		       "(status=0x%x, sig=%d, prev=%s[%d])",
+		       next->name, task_pid_nr(next_task),
+		       xnthread_get_state(next),
+		       signal_pending(next_task),
+		       prev_task->comm, task_pid_nr(prev_task)))
+		XENO_WARN(COBALT,
+			  !(next_task->ptrace & PT_PTRACED) &&
+			   !xnthread_test_state(next, XNDORMANT)
+			  && xnthread_test_state(next, XNPEND),
+			  "blocked thread %s[%d] rescheduled?! "
+			  "(status=0x%x, sig=%d, prev=%s[%d])",
+			  next->name, task_pid_nr(next_task),
+			  xnthread_get_state(next),
+			  signal_pending(next_task), prev_task->comm,
+			  task_pid_nr(prev_task));
 }
 
 void handle_inband_event(enum inband_event_type event, void *data)
@@ -505,7 +575,7 @@ void handle_inband_event(enum inband_event_type event, void *data)
 		handle_ptrace_resume(data);
 		break;
 	case INBAND_TASK_PTCONT:
-		handle_ptrace_cont();
+		handle_ptrace_cont(data);
 		break;
 	case INBAND_TASK_PTSTOP:
 		break;
-- 
2.17.1

[PATCH 6/8] cobalt/timer: pipeline: abstract signal test of XNTSTOP

[hongzha1]

The I-pipe kind of internally "emulates" a shutdown+restart sequence
automatically upon resuming a timer after the ONESHOT_STOPPED state, so
we actually do not need XNTSTOP in this case. We should
specifically abstract the single test of XNTSTOP when arming a
timer, so that the Dovetail version does the right thing.
We can leave XNTSTOP in the set/clear mask bits operations in either
the I-pipe and Dovetail cases, this has zero overhead since other bits
are being set/cleared in these operations anyway.

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/ipipe/pipeline/tick.h b/include/cobalt/kernel/ipipe/pipeline/tick.h
index 409581a3c..41347f7b1 100644
--- a/include/cobalt/kernel/ipipe/pipeline/tick.h
+++ b/include/cobalt/kernel/ipipe/pipeline/tick.h
@@ -9,4 +9,10 @@ int pipeline_install_tick_proxy(void);
 
 void pipeline_uninstall_tick_proxy(void);
 
+struct xnsched;
+static inline bool pipeline_must_force_program_tick(struct xnsched *sched)
+{
+	return false;
+}
+
 #endif /* !_COBALT_KERNEL_IPIPE_TICK_H */
diff --git a/include/cobalt/kernel/sched.h b/include/cobalt/kernel/sched.h
index c13f46ff7..aa24d5442 100644
--- a/include/cobalt/kernel/sched.h
+++ b/include/cobalt/kernel/sched.h
@@ -48,6 +48,11 @@
 #define XNINIRQ		0x00004000	/* In IRQ handling context */
 #define XNHDEFER	0x00002000	/* Host tick deferred */
 
+/*
+ * Hardware timer is stopped.
+ */
+#define XNTSTOP		0x00000800
+
 struct xnsched_rt {
 	xnsched_queue_t runnable;	/*!< Runnable thread queue. */
 };
diff --git a/kernel/cobalt/clock.c b/kernel/cobalt/clock.c
index bf24e1693..2115b15ef 100644
--- a/kernel/cobalt/clock.c
+++ b/kernel/cobalt/clock.c
@@ -147,7 +147,7 @@ void xnclock_core_local_shot(struct xnsched *sched)
 	 * or a timer with an earlier timeout date is scheduled,
 	 * whichever comes first.
 	 */
-	sched->lflags &= ~(XNHDEFER|XNIDLE);
+	sched->lflags &= ~(XNHDEFER|XNIDLE|XNTSTOP);
 	timer = container_of(h, struct xntimer, aplink);
 	if (unlikely(timer == &sched->htimer)) {
 		if (xnsched_resched_p(sched) ||
diff --git a/kernel/cobalt/timer.c b/kernel/cobalt/timer.c
index f9aa457ce..f667a3878 100644
--- a/kernel/cobalt/timer.c
+++ b/kernel/cobalt/timer.c
@@ -18,6 +18,7 @@
  * 02111-1307, USA.
  */
 #include <linux/sched.h>
+#include <pipeline/tick.h>
 #include <cobalt/kernel/sched.h>
 #include <cobalt/kernel/thread.h>
 #include <cobalt/kernel/timer.h>
@@ -63,8 +64,10 @@ int xntimer_heading_p(struct xntimer *timer)
 
 void xntimer_enqueue_and_program(struct xntimer *timer, xntimerq_t *q)
 {
+	struct xnsched *sched = xntimer_sched(timer);
+
 	xntimer_enqueue(timer, q);
-	if (xntimer_heading_p(timer)) {
+	if (pipeline_must_force_program_tick(sched) || xntimer_heading_p(timer)) {
 		struct xnsched *sched = xntimer_sched(timer);
 		struct xnclock *clock = xntimer_clock(timer);
 		if (sched != xnsched_current())
-- 
2.17.1

[PATCH 6/8] cobalt/timer: pipeline: abstract signal test of XNTSTOP

[hongzha1]

The I-pipe kind of internally "emulates" a shutdown+restart sequence
automatically upon resuming a timer after the ONESHOT_STOPPED state, so
we actually do not need XNTSTOP in this case. We should
specifically abstract the single test of XNTSTOP when arming a
timer, so that the Dovetail version does the right thing.
We can leave XNTSTOP in the set/clear mask bits operations in either
the I-pipe and Dovetail cases, this has zero overhead since other bits
are being set/cleared in these operations anyway.

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/ipipe/pipeline/tick.h b/include/cobalt/kernel/ipipe/pipeline/tick.h
index 409581a3c..41347f7b1 100644
--- a/include/cobalt/kernel/ipipe/pipeline/tick.h
+++ b/include/cobalt/kernel/ipipe/pipeline/tick.h
@@ -9,4 +9,10 @@ int pipeline_install_tick_proxy(void);
 
 void pipeline_uninstall_tick_proxy(void);
 
+struct xnsched;
+static inline bool pipeline_must_force_program_tick(struct xnsched *sched)
+{
+	return false;
+}
+
 #endif /* !_COBALT_KERNEL_IPIPE_TICK_H */
diff --git a/include/cobalt/kernel/sched.h b/include/cobalt/kernel/sched.h
index c13f46ff7..aa24d5442 100644
--- a/include/cobalt/kernel/sched.h
+++ b/include/cobalt/kernel/sched.h
@@ -48,6 +48,11 @@
 #define XNINIRQ		0x00004000	/* In IRQ handling context */
 #define XNHDEFER	0x00002000	/* Host tick deferred */
 
+/*
+ * Hardware timer is stopped.
+ */
+#define XNTSTOP		0x00000800
+
 struct xnsched_rt {
 	xnsched_queue_t runnable;	/*!< Runnable thread queue. */
 };
diff --git a/kernel/cobalt/clock.c b/kernel/cobalt/clock.c
index bf24e1693..2115b15ef 100644
--- a/kernel/cobalt/clock.c
+++ b/kernel/cobalt/clock.c
@@ -147,7 +147,7 @@ void xnclock_core_local_shot(struct xnsched *sched)
 	 * or a timer with an earlier timeout date is scheduled,
 	 * whichever comes first.
 	 */
-	sched->lflags &= ~(XNHDEFER|XNIDLE);
+	sched->lflags &= ~(XNHDEFER|XNIDLE|XNTSTOP);
 	timer = container_of(h, struct xntimer, aplink);
 	if (unlikely(timer == &sched->htimer)) {
 		if (xnsched_resched_p(sched) ||
diff --git a/kernel/cobalt/timer.c b/kernel/cobalt/timer.c
index f9aa457ce..f667a3878 100644
--- a/kernel/cobalt/timer.c
+++ b/kernel/cobalt/timer.c
@@ -18,6 +18,7 @@
  * 02111-1307, USA.
  */
 #include <linux/sched.h>
+#include <pipeline/tick.h>
 #include <cobalt/kernel/sched.h>
 #include <cobalt/kernel/thread.h>
 #include <cobalt/kernel/timer.h>
@@ -63,8 +64,10 @@ int xntimer_heading_p(struct xntimer *timer)
 
 void xntimer_enqueue_and_program(struct xntimer *timer, xntimerq_t *q)
 {
+	struct xnsched *sched = xntimer_sched(timer);
+
 	xntimer_enqueue(timer, q);
-	if (xntimer_heading_p(timer)) {
+	if (pipeline_must_force_program_tick(sched) || xntimer_heading_p(timer)) {
 		struct xnsched *sched = xntimer_sched(timer);
 		struct xnclock *clock = xntimer_clock(timer);
 		if (sched != xnsched_current())
-- 
2.17.1

[PATCH 7/8] dovetail/tick: pipeline: impmement pipeline_must_force_program_tick

[hongzha1]

1. set XNTSTOP to force to program tick after the ONESHOT_STOPPED
   state
2. implement pipeline_must_force_program_tick for dovetail

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/dovetail/pipeline/tick.h b/include/cobalt/kernel/dovetail/pipeline/tick.h
index 409581a3c..8ac4760ec 100644
--- a/include/cobalt/kernel/dovetail/pipeline/tick.h
+++ b/include/cobalt/kernel/dovetail/pipeline/tick.h
@@ -9,4 +9,8 @@ int pipeline_install_tick_proxy(void);
 
 void pipeline_uninstall_tick_proxy(void);
 
+struct xnsched;
+
+inline bool pipeline_must_force_program_tick(struct xnsched *sched);
+
 #endif /* !_COBALT_KERNEL_IPIPE_TICK_H */
diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
index bf267b9d9..8d6dc1812 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -96,6 +96,11 @@ void xn_core_tick(struct clock_event_device *dummy) /* hard irqs off */
 	xnintr_core_clock_handler();
 }
 
+inline bool pipeline_must_force_program_tick(struct xnsched *sched)
+{
+	return sched->lflags & XNTSTOP;
+}
+
 static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
 {
 	struct clock_event_device *real_dev;
@@ -117,7 +122,7 @@ static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
 	xnlock_get_irqsave(&nklock, s);
 	sched = xnsched_current();
 	xntimer_stop(&sched->htimer);
-	//sched->lflags |= XNTSTOP;
+	sched->lflags |= XNTSTOP;
 
 	if (sched->lflags & XNIDLE) {
 		real_dev = dev->real_device;
-- 
2.17.1

[PATCH 8/8] dovetail/kevents: enable back tracing

[hongzha1]

enable back tracing for handle_oob_trap_entry

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>

diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
index 378017bea..51946e21c 100644
--- a/kernel/cobalt/dovetail/kevents.c
+++ b/kernel/cobalt/dovetail/kevents.c
@@ -97,10 +97,8 @@ void handle_oob_trap_entry(unsigned int trapnr, struct pt_regs *regs)
 
 	/*
 	 * Enable back tracing.
-	 *
-	 * trace_cobalt_thread_fault(xnarch_fault_pc(regs), trapnr);
 	 */
-	TODO();
+	trace_cobalt_thread_fault(xnarch_fault_pc(regs), trapnr);
 
 	if (xnthread_test_state(thread, XNROOT))
 		return;
-- 
2.17.1

Re: [PATCH V2 1/8] dovetail/pipeline: implement out-of-band irq management and handling

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> implement oob irq request and free and post for both
> TIMER_OOB_IPI and RESCHEDULE_OOB_IPI
>
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/pipeline.h b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
> index 23c4a2c18..13fbd7942 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/pipeline.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
> @@ -8,9 +8,12 @@
>  #include <linux/irq_pipeline.h>
>  #include <cobalt/kernel/assert.h>
>  #include <asm/xenomai/features.h>
> +#include <pipeline/machine.h>
>  
>  typedef unsigned long spl_t;
>  
> +void xnintr_core_clock_handler(void);
> +
>  /*
>   * We only keep the LSB when testing in SMP mode in order to strip off
>   * the recursion marker (0x2) the nklock may store there.
> @@ -30,18 +33,28 @@ typedef unsigned long spl_t;
>  
>  #ifdef CONFIG_SMP
>  
> +static irqreturn_t reschedule_interrupt_handler(int irq, void *dev_id)
> +{
> +
> +	/* Will reschedule from irq_exit_pipeline. */
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static inline int pipeline_request_resched_ipi(void (*handler)(void))
>  {
>  	/* Trap the out-of-band rescheduling interrupt. */
> -	TODO();
> -
> -	return 0;
> +	return __request_percpu_irq(RESCHEDULE_OOB_IPI,
> +			reschedule_interrupt_handler,
> +			IRQF_OOB,
> +			"Xenomai reschedule",
> +			&cobalt_machine_cpudata);
>  }
>  
>  static inline void pipeline_free_resched_ipi(void)
>  {
>  	/* Release the out-of-band rescheduling interrupt. */
> -	TODO();
> +	free_percpu_irq(RESCHEDULE_OOB_IPI, &cobalt_machine_cpudata);
>  }
>  
>  static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
> @@ -50,21 +63,29 @@ static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
>  	 * Trigger the out-of-band rescheduling interrupt on remote
>  	 * CPU(s).
>  	 */
> -	TODO();
> +	irq_send_oob_ipi(RESCHEDULE_OOB_IPI, dest);
> +}
> +
> +static irqreturn_t timer_ipi_interrupt_handler(int irq, void *dev_id)
> +{
> +	xnintr_core_clock_handler();
> +
> +	return IRQ_HANDLED;
>  }
>  
>  static inline int pipeline_request_timer_ipi(void (*handler)(void))
>  {
>  	/* Trap the out-of-band timer interrupt. */
> -	TODO();
> -
> -	return 0;
> +	return __request_percpu_irq(TIMER_OOB_IPI,
> +			timer_ipi_interrupt_handler,
> +			IRQF_OOB, "Xenomai timer IPI",
> +			&cobalt_machine_cpudata);
>  }
>  
>  static inline void pipeline_free_timer_ipi(void)
>  {
>  	/* Release the out-of-band timer interrupt. */
> -	TODO();
> +	free_percpu_irq(TIMER_OOB_IPI, &cobalt_machine_cpudata);
>  }
>  
>  static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
> @@ -72,7 +93,7 @@ static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
>  	/*
>  	 * Trigger the out-of-band timer interrupt on remote CPU(s).
>  	 */
> -	TODO();
> +	irq_send_oob_ipi(TIMER_OOB_IPI, dest);
>  }
>  
>  #endif

Merged, fixing up the short log on the fly, in order to align on the
matching I-pipe patch as follows:

"dovetail/irq: dovetail: implement out-of-band irq management and
handling"

Thanks,

-- 
Philippe.

Re: [PATCH 2/8] dovetail/tick: implement proxy tick device installing and uninstalling

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index 02cd86690..6a196496c 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -6,11 +6,92 @@
>   */
>  
>  #include <linux/tick.h>
> +#include <linux/clockchips.h>
>  #include <cobalt/kernel/intr.h>
>  #include <pipeline/tick.h>
> +#include <cobalt/kernel/sched.h>
> +#include <cobalt/kernel/timer.h>
>  
>  extern struct xnintr nktimer;
>  
> +static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
> +
> +static int proxy_set_next_ktime(ktime_t expires,
> +				struct clock_event_device *proxy_dev)
> +{
> +	struct xnsched *sched;
> +	ktime_t delta;
> +	unsigned long flags;
> +	int ret;
> +
> +	/*
> +	 * When Negative delta have been observed, we set delta zero.
> +	 * Or else exntimer_start() will return -ETIMEDOUT and do not
> +	 * trigger shot
> +	 */
> +	delta = ktime_sub(expires, ktime_get_mono_fast_ns());
> +	if (delta < 0)
> +		delta = 0;
> +
> +	flags = hard_local_irq_save(); /* Prevent CPU migration. */
> +	sched = xnsched_current();
> +	ret = xntimer_start(&sched->htimer, delta, XN_INFINITE, XN_RELATIVE);
> +	hard_local_irq_restore(flags);
> +
> +	return ret ? -ETIME : 0;
> +}
> +
> +
> +void xn_core_tick(struct clock_event_device *dummy) /* hard irqs off */
> +{
> +	xnintr_core_clock_handler();
> +}
> +
> +static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
> +{
> +	struct clock_event_device *real_dev;
> +	struct clock_proxy_device *dev;
> +	struct xnsched *sched;
> +	spl_t s;
> +
> +	dev = container_of(proxy_dev, struct clock_proxy_device, proxy_device);
> +
> +	/*
> +	 * In-band wants to disable the clock hardware on entering a
> +	 * tickless state, so we have to stop our in-band tick
> +	 * emulation. Propagate the request for shutting down the
> +	 * hardware to the real device only if we have no outstanding
> +	 * OOB timers. CAUTION: the in-band timer is counted when
> +	 * assessing the RQ_IDLE condition, so we need to stop it
> +	 * prior to testing the latter.
> +	 */
> +	xnlock_get_irqsave(&nklock, s);
> +	sched = xnsched_current();
> +	xntimer_stop(&sched->htimer);
> +	//sched->lflags |= XNTSTOP;
> +
> +	if (sched->lflags & XNIDLE) {
> +		real_dev = dev->real_device;
> +		real_dev->set_state_oneshot_stopped(real_dev);
> +	}
> +
> +	xnlock_put_irqrestore(&nklock, s);
> +
> +	return 0;
> +}
> +
> +static void setup_proxy(struct clock_proxy_device *dev)
> +{
> +	struct clock_event_device *proxy_dev = &dev->proxy_device;
> +
> +	dev->handle_oob_event = xn_core_tick;
> +	proxy_dev->features |= CLOCK_EVT_FEAT_KTIME;
> +	proxy_dev->set_next_ktime = proxy_set_next_ktime;
> +	if (proxy_dev->set_state_oneshot_stopped)
> +		proxy_dev->set_state_oneshot_stopped = proxy_set_oneshot_stopped;
> +	__this_cpu_write(proxy_device, dev);
> +}
> +
>  int pipeline_install_tick_proxy(void)
>  {
>  	int ret;
> @@ -20,7 +101,7 @@ int pipeline_install_tick_proxy(void)
>  		return ret;
>  
>  	/* Install the proxy tick device */
> -	TODO();	ret = 0;
> +	ret = tick_install_proxy(setup_proxy, &xnsched_realtime_cpus);
>  	if (ret)
>  		goto fail_proxy;
>  
> @@ -35,7 +116,7 @@ fail_proxy:
>  void pipeline_uninstall_tick_proxy(void)
>  {
>  	/* Uninstall the proxy tick device. */
> -	TODO();
> +	tick_uninstall_proxy(&xnsched_realtime_cpus);
>  
>  	pipeline_free_timer_ipi();


Merged, fixing up the short log for consistency with other patches, as:

cobalt/tick: dovetail: install/uninstall proxy tick device

-- 
Philippe.

Re: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[Philippe Gerum]

The shortlog should mention the subsystem and the pipeline variant to
allow for a quick comprehension of the scope; that would be
"cobalt/tick: dovetail: ..." in this case.

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index 28d89b44b..db982d969 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -17,17 +17,7 @@ static inline u64 pipeline_read_cycle_counter(void)
>  	return  ktime_get_raw_fast_ns();
>  }
>  
> -static inline void pipeline_set_timer_shot(unsigned long cycles)
> -{
> -	/*
> -	 * N/A. Revisit: xnclock_core_local_shot() should go to the
> -	 * I-pipe section, we do things differently on Dovetail via
> -	 * the proxy tick device. As a consequence,
> -	 * pipeline_set_timer_shot() should not be part of the
> -	 * pipeline interface.
> -	 */
> -	TODO();
> -}
> +inline void pipeline_set_timer_shot(unsigned long cycles);
>  
>  static inline const char *pipeline_timer_name(void)
>  {
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index 6a196496c..55039f96e 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -16,6 +16,43 @@ extern struct xnintr nktimer;
>  
>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>  
> +inline void pipeline_set_timer_shot(unsigned long cycles)
> +{
> +	/*
> +	 * N/A. Revisit: xnclock_core_local_shot() should go to the
> +	 * I-pipe section, we do things differently on Dovetail via
> +	 * the proxy tick device. As a consequence,
> +	 * pipeline_set_timer_shot() should not be part of the
> +	 * pipeline interface.
> +	 */
> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
> +	struct clock_event_device *real_dev = dev->real_device;
> +	int ret;
> +	u64 sumcyc;
> +	ktime_t t;
> +
> +	if (real_dev->features & CLOCK_EVT_FEAT_KTIME) {
> +		t = ktime_add(cycles, xnclock_core_read_raw());
> +		real_dev->set_next_ktime(t, real_dev);
> +	} else {
> +		if (cycles <= 0)
> +			cycles = real_dev->min_delta_ns;
> +		else {
> +			cycles = min_t(int64_t, cycles,
> +					real_dev->max_delta_ns);
> +			cycles = max_t(int64_t, cycles,
> +					real_dev->min_delta_ns);
> +		}
> +		sumcyc = ((u64)cycles * real_dev->mult) >> real_dev->shift;
> +
> +		ret = real_dev->set_next_event(sumcyc, real_dev);
> +		if (ret) {
> +			ret = real_dev->set_next_event(real_dev->min_delta_ticks,
> +							real_dev);
> +		}
> +	}
> +}
> +
>  static int proxy_set_next_ktime(ktime_t expires,
>  				struct clock_event_device *proxy_dev)
>  {

This patch does not apply cleanly, raising a conflict in
dovetail/pipeline/clock.h. Besides, as discussed recently, the comment
mentioning the need to revisit the code may not be accurate anymore. You
may want to drop it as part of the fix up.

-- 
Philippe.

Re: [PATCH V2 4/8] dovetail/clock: implement pipeline_timer_name

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> to get the name of the current clock event chip, which is the real
> device controlled by the proxy tick device.
>
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index db982d969..d2de1cd34 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -19,16 +19,7 @@ static inline u64 pipeline_read_cycle_counter(void)
>  
>  inline void pipeline_set_timer_shot(unsigned long cycles);
>
> -static inline const char *pipeline_timer_name(void)
> -{
> -	/*
> -	 * Return the name of the current clock event chip, which is
> -	 * the real device controlled by the proxy tick device.
> -	 */
> -	TODO();
> -
> -	return "?";
> -}
> +inline const char *pipeline_timer_name(void);
>  

Since the storage class of pipeline_timer_name() is actually extern, it
will be generated as stand-alone object code in the translation unit for
tick.o, in order for other units to be able to call it (e.g. clock.o),
so "inline" is moot in this particular case.

>  static inline const char *pipeline_clock_name(void)
>  {
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index 55039f96e..bf267b9d9 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -16,6 +16,18 @@ extern struct xnintr nktimer;
>  
>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>  
> +inline const char *pipeline_timer_name(void)
> +{
> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
> +	struct clock_event_device *real_dev = dev->real_device;
> +
> +	/*
> +	 * Return the name of the current clock event chip, which is
> +	 * the real device controlled by the proxy tick device.
> +	 */
> +	return real_dev->name;
> +}
> +
>  inline void pipeline_set_timer_shot(unsigned long cycles)
>  {
>  	/*


-- 
Philippe.

Re: [PATCH 5/8] dovetail/kevents: dovetail: implement handle_ptrace_cont

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
> index b5dc50ea7..378017bea 100644
> --- a/kernel/cobalt/dovetail/kevents.c
> +++ b/kernel/cobalt/dovetail/kevents.c
> @@ -473,7 +473,7 @@ int handle_ptrace_resume(struct task_struct *tracee)
>  	return KEVENT_PROPAGATE;
>  }
>  
> -static void handle_ptrace_cont(void)
> +static void handle_ptrace_cont(struct task_struct *next_task)
>  {

Dovetail does not pass any argument to the client core along with
INBAND_TASK_PTCONT, because "current" may always be assumed. So
handle_ptrace_cont() receives no argument, and next_task is actually
"current" in this case.

The reason was that the I-pipe gave us no way to observe ptraced tasks
waking up from a stopped state, so we resorted to observing context
switches, figuring out when/if the incoming task was about to resume
from a ptrace-induced stopped state. Since Dovetail can send us such
event directly from the context of the resuming task, next_task ==
current in this context.

-- 
Philippe.

Re: [PATCH 6/8] cobalt/timer: pipeline: abstract signal test of XNTSTOP

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> The I-pipe kind of internally "emulates" a shutdown+restart sequence
> automatically upon resuming a timer after the ONESHOT_STOPPED state, so
> we actually do not need XNTSTOP in this case. We should
> specifically abstract the single test of XNTSTOP when arming a
> timer, so that the Dovetail version does the right thing.
> We can leave XNTSTOP in the set/clear mask bits operations in either
> the I-pipe and Dovetail cases, this has zero overhead since other bits
> are being set/cleared in these operations anyway.
>

This change log is confusing: citing one of my past remarks it seems, it
explains how we should be abstracting XNTSTOP, but the patch actually
introduces it. Therefore we should explain _why_ we are introducing
XNTSTOP in the first place, then maybe retain the last paragraph only to
justify some details of the implementation.

The patch actually adds a way to force the timer management code to
reprogram the hardware on option, to make the real device controlled by
the proxy tick again as it leaves the ONESHOT_STOPPED mode. We should go
on saying that the I-pipe does not require any further action in this
case, leading to a nop (for this patch). Patch 7/10 would then implement
the Dovetail side, which does require the next tick to be force
programmed in the hardware as a result of leaving the ONESHOT_STOPPED
mode.

-- 
Philippe.

Re: [PATCH 7/8] dovetail/tick: pipeline: impmement pipeline_must_force_program_tick

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

The short log would rather start with "cobalt/tick: dovetail: ..."
here.

> 1. set XNTSTOP to force to program tick after the ONESHOT_STOPPED
>    state
> 2. implement pipeline_must_force_program_tick for dovetail
>
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/tick.h b/include/cobalt/kernel/dovetail/pipeline/tick.h
> index 409581a3c..8ac4760ec 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/tick.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/tick.h
> @@ -9,4 +9,8 @@ int pipeline_install_tick_proxy(void);
>  
>  void pipeline_uninstall_tick_proxy(void);
>  
> +struct xnsched;
> +
> +inline bool pipeline_must_force_program_tick(struct xnsched *sched);
> +
>  #endif /* !_COBALT_KERNEL_IPIPE_TICK_H */
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index bf267b9d9..8d6dc1812 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -96,6 +96,11 @@ void xn_core_tick(struct clock_event_device *dummy) /* hard irqs off */
>  	xnintr_core_clock_handler();
>  }
>  
> +inline bool pipeline_must_force_program_tick(struct xnsched *sched)
> +{
> +	return sched->lflags & XNTSTOP;
> +}
> +
>  static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
>  {
>  	struct clock_event_device *real_dev;
> @@ -117,7 +122,7 @@ static int proxy_set_oneshot_stopped(struct clock_event_device *proxy_dev)
>  	xnlock_get_irqsave(&nklock, s);
>  	sched = xnsched_current();
>  	xntimer_stop(&sched->htimer);
> -	//sched->lflags |= XNTSTOP;
> +	sched->lflags |= XNTSTOP;
>  
>  	if (sched->lflags & XNIDLE) {
>  		real_dev = dev->real_device;


-- 
Philippe.

Re: [PATCH 8/8] dovetail/kevents: enable back tracing

[Philippe Gerum]

hongzha1 via Xenomai <xenomai@xenomai.org> writes:

> enable back tracing for handle_oob_trap_entry
>
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
> diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
> index 378017bea..51946e21c 100644
> --- a/kernel/cobalt/dovetail/kevents.c
> +++ b/kernel/cobalt/dovetail/kevents.c
> @@ -97,10 +97,8 @@ void handle_oob_trap_entry(unsigned int trapnr, struct pt_regs *regs)
>  
>  	/*
>  	 * Enable back tracing.
> -	 *
> -	 * trace_cobalt_thread_fault(xnarch_fault_pc(regs), trapnr);
>  	 */
> -	TODO();
> +	trace_cobalt_thread_fault(xnarch_fault_pc(regs), trapnr);
>  
>  	if (xnthread_test_state(thread, XNROOT))
>  		return;

Merged, thanks.

-- 
Philippe.

RE: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[Chen, Hongzhan]

>-----Original Message-----
>From: Philippe Gerum <rpm@xenomai.org> 
>Sent: Saturday, January 16, 2021 7:56 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>
>Cc: xenomai@xenomai.org
>Subject: Re: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot
>
>
>The shortlog should mention the subsystem and the pipeline variant to
>allow for a quick comprehension of the scope; that would be
>"cobalt/tick: dovetail: ..." in this case.
>
>hongzha1 via Xenomai <xenomai@xenomai.org> writes:
>
>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>>
>> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> index 28d89b44b..db982d969 100644
>> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
>> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> @@ -17,17 +17,7 @@ static inline u64 pipeline_read_cycle_counter(void)
>>  	return  ktime_get_raw_fast_ns();
>>  }
>>  
>> -static inline void pipeline_set_timer_shot(unsigned long cycles)
>> -{
>> -	/*
>> -	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>> -	 * I-pipe section, we do things differently on Dovetail via
>> -	 * the proxy tick device. As a consequence,
>> -	 * pipeline_set_timer_shot() should not be part of the
>> -	 * pipeline interface.
>> -	 */
>> -	TODO();
>> -}
>> +inline void pipeline_set_timer_shot(unsigned long cycles);
>>  
>>  static inline const char *pipeline_timer_name(void)
>>  {
>> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
>> index 6a196496c..55039f96e 100644
>> --- a/kernel/cobalt/dovetail/tick.c
>> +++ b/kernel/cobalt/dovetail/tick.c
>> @@ -16,6 +16,43 @@ extern struct xnintr nktimer;
>>  
>>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>>  
>> +inline void pipeline_set_timer_shot(unsigned long cycles)
>> +{
>> +	/*
>> +	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>> +	 * I-pipe section, we do things differently on Dovetail via
>> +	 * the proxy tick device. As a consequence,
>> +	 * pipeline_set_timer_shot() should not be part of the
>> +	 * pipeline interface.
>> +	 */
>> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>> +	struct clock_event_device *real_dev = dev->real_device;
>> +	int ret;
>> +	u64 sumcyc;
>> +	ktime_t t;
>> +
>> +	if (real_dev->features & CLOCK_EVT_FEAT_KTIME) {
>> +		t = ktime_add(cycles, xnclock_core_read_raw());
>> +		real_dev->set_next_ktime(t, real_dev);
>> +	} else {
>> +		if (cycles <= 0)
>> +			cycles = real_dev->min_delta_ns;
>> +		else {
>> +			cycles = min_t(int64_t, cycles,
>> +					real_dev->max_delta_ns);
>> +			cycles = max_t(int64_t, cycles,
>> +					real_dev->min_delta_ns);
>> +		}
>> +		sumcyc = ((u64)cycles * real_dev->mult) >> real_dev->shift;
>> +
>> +		ret = real_dev->set_next_event(sumcyc, real_dev);
>> +		if (ret) {
>> +			ret = real_dev->set_next_event(real_dev->min_delta_ticks,
>> +							real_dev);
>> +		}
>> +	}
>> +}
>> +
>>  static int proxy_set_next_ktime(ktime_t expires,
>>  				struct clock_event_device *proxy_dev)
>>  {
>
>This patch does not apply cleanly, raising a conflict in
>dovetail/pipeline/clock.h. Besides, as discussed recently, the comment   

The patch actually depends on previous 5 patches you already approved 
to merge. One of them modified dovetail/pipeline/clock.h.  

https://xenomai.org/pipermail/xenomai/2021-January/044123.html
https://xenomai.org/pipermail/xenomai/2021-January/044124.html
https://xenomai.org/pipermail/xenomai/2021-January/044125.html
https://xenomai.org/pipermail/xenomai/2021-January/044126.html
https://xenomai.org/pipermail/xenomai/2021-January/044127.html

>mentioning the need to revisit the code may not be accurate anymore. You

Thanks for your suggestions, I will refine it though.

>may want to drop it as part of the fix up.
>
>-- 
>Philippe.

Re: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[Philippe Gerum]

Chen, Hongzhan <hongzhan.chen@intel.com> writes:

>>-----Original Message-----
>>From: Philippe Gerum <rpm@xenomai.org> 
>>Sent: Saturday, January 16, 2021 7:56 PM
>>To: Chen, Hongzhan <hongzhan.chen@intel.com>
>>Cc: xenomai@xenomai.org
>>Subject: Re: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot
>>
>>
>>The shortlog should mention the subsystem and the pipeline variant to
>>allow for a quick comprehension of the scope; that would be
>>"cobalt/tick: dovetail: ..." in this case.
>>
>>hongzha1 via Xenomai <xenomai@xenomai.org> writes:
>>
>>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>>>
>>> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
>>> index 28d89b44b..db982d969 100644
>>> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
>>> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
>>> @@ -17,17 +17,7 @@ static inline u64 pipeline_read_cycle_counter(void)
>>>  	return  ktime_get_raw_fast_ns();
>>>  }
>>>  
>>> -static inline void pipeline_set_timer_shot(unsigned long cycles)
>>> -{
>>> -	/*
>>> -	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>>> -	 * I-pipe section, we do things differently on Dovetail via
>>> -	 * the proxy tick device. As a consequence,
>>> -	 * pipeline_set_timer_shot() should not be part of the
>>> -	 * pipeline interface.
>>> -	 */
>>> -	TODO();
>>> -}
>>> +inline void pipeline_set_timer_shot(unsigned long cycles);
>>>  
>>>  static inline const char *pipeline_timer_name(void)
>>>  {
>>> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
>>> index 6a196496c..55039f96e 100644
>>> --- a/kernel/cobalt/dovetail/tick.c
>>> +++ b/kernel/cobalt/dovetail/tick.c
>>> @@ -16,6 +16,43 @@ extern struct xnintr nktimer;
>>>  
>>>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>>>  
>>> +inline void pipeline_set_timer_shot(unsigned long cycles)
>>> +{
>>> +	/*
>>> +	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>>> +	 * I-pipe section, we do things differently on Dovetail via
>>> +	 * the proxy tick device. As a consequence,
>>> +	 * pipeline_set_timer_shot() should not be part of the
>>> +	 * pipeline interface.
>>> +	 */
>>> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>>> +	struct clock_event_device *real_dev = dev->real_device;
>>> +	int ret;
>>> +	u64 sumcyc;
>>> +	ktime_t t;
>>> +
>>> +	if (real_dev->features & CLOCK_EVT_FEAT_KTIME) {
>>> +		t = ktime_add(cycles, xnclock_core_read_raw());
>>> +		real_dev->set_next_ktime(t, real_dev);
>>> +	} else {
>>> +		if (cycles <= 0)
>>> +			cycles = real_dev->min_delta_ns;
>>> +		else {
>>> +			cycles = min_t(int64_t, cycles,
>>> +					real_dev->max_delta_ns);
>>> +			cycles = max_t(int64_t, cycles,
>>> +					real_dev->min_delta_ns);
>>> +		}
>>> +		sumcyc = ((u64)cycles * real_dev->mult) >> real_dev->shift;
>>> +
>>> +		ret = real_dev->set_next_event(sumcyc, real_dev);
>>> +		if (ret) {
>>> +			ret = real_dev->set_next_event(real_dev->min_delta_ticks,
>>> +							real_dev);
>>> +		}
>>> +	}
>>> +}
>>> +
>>>  static int proxy_set_next_ktime(ktime_t expires,
>>>  				struct clock_event_device *proxy_dev)
>>>  {
>>
>>This patch does not apply cleanly, raising a conflict in
>>dovetail/pipeline/clock.h. Besides, as discussed recently, the comment   
>
> The patch actually depends on previous 5 patches you already approved 
> to merge. One of them modified dovetail/pipeline/clock.h.  
>
> https://xenomai.org/pipermail/xenomai/2021-January/044123.html
> https://xenomai.org/pipermail/xenomai/2021-January/044124.html
> https://xenomai.org/pipermail/xenomai/2021-January/044125.html
> https://xenomai.org/pipermail/xenomai/2021-January/044126.html
> https://xenomai.org/pipermail/xenomai/2021-January/044127.html
>

Correct, my bad. I have four of your patches pending in my queue which I
still need to push to the Dovetail branch. I'll be doing that asap.

>>mentioning the need to revisit the code may not be accurate anymore. You
>
> Thanks for your suggestions, I will refine it though.
>

Thanks,
 
-- 
Philippe.