[PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[hongzha1]

inband sirq request through synthetic_irq_domain and free and post
srq.

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
index be9dc587f..1da9d13b2 100644
--- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
+++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
@@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
 	 * Allocate an IRQ from the synthetic interrupt domain then
 	 * trap it to @handler, to be fired from the in-band stage.
 	 */
-	TODO();
+	int sirq, ret;
 
-	return 0;
+	sirq = irq_create_direct_mapping(synthetic_irq_domain);
+	if (sirq == 0)
+		return -EAGAIN;
+
+	ret = __request_percpu_irq(sirq,
+			handler,
+			IRQF_NO_THREAD,
+			"Inband sirq",
+			&cobalt_machine_cpudata);
+
+	if (ret) {
+		irq_dispose_mapping(sirq);
+		return ret;
+	}
+
+	return sirq;
 }
 
 static inline
@@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
 	 * Free the synthetic IRQ then deallocate it to its
 	 * originating domain.
 	 */
-	TODO();
+	free_percpu_irq(sirq,
+		&cobalt_machine_cpudata);
+
+	irq_dispose_mapping(sirq);
 }
 
 static inline void pipeline_post_sirq(int sirq)
 {
 	/* Trigger the synthetic IRQ */
-	TODO();
+	irq_post_inband(sirq);
 }
 
-#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
+#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */
-- 
2.17.1

[PATCH 02/10] dovetail/clock: implement pipeline_read_cycle_counter

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/dovetail/pipeline/clock.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index 19e3d8986..5176b1506 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -13,9 +13,7 @@ struct timespec64;
 static inline u64 pipeline_read_cycle_counter(void)
 {
 	/* Read the raw cycle counter of the core clock. */
-	TODO();
-
-	return 0;
+	return  ktime_get_raw_fast_ns();
 }
 
 static inline void pipeline_set_timer_shot(unsigned long cycles)
-- 
2.17.1

[PATCH 03/10] dovetail/clock: implement pipeline_get_host_time

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/dovetail/pipeline/clock.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index 5176b1506..28d89b44b 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -7,6 +7,7 @@
 
 #include <cobalt/uapi/kernel/types.h>
 #include <cobalt/kernel/assert.h>
+#include <linux/ktime.h>
 
 struct timespec64;
 
@@ -50,7 +51,7 @@ static inline const char *pipeline_clock_name(void)
 static inline int pipeline_get_host_time(struct timespec64 *tp)
 {
 	/* Convert ktime_get_real_fast_ns() to timespec. */
-	TODO();
+	*tp = ktime_to_timespec64(ktime_get_real_fast_ns());
 
 	return 0;
 }
-- 
2.17.1

[PATCH 04/10] dovetail/sched: implement pipeline_init_shadow_tcb and pipeline_init_root_tcb

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 kernel/cobalt/dovetail/sched.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/dovetail/sched.c b/kernel/cobalt/dovetail/sched.c
index 71f763d39..82e29136c 100644
--- a/kernel/cobalt/dovetail/sched.c
+++ b/kernel/cobalt/dovetail/sched.c
@@ -36,7 +36,7 @@ void pipeline_init_shadow_tcb(struct xnthread *thread)
 	/*
 	 * Initialize the alternate scheduling control block.
 	 */
-	TODO();
+	dovetail_init_altsched(&xnthread_archtcb(thread)->altsched);
 
 	trace_cobalt_shadow_map(thread);
 }
@@ -46,7 +46,7 @@ void pipeline_init_root_tcb(struct xnthread *thread)
 	/*
 	 * Initialize the alternate scheduling control block.
 	 */
-	TODO();
+	dovetail_init_altsched(&xnthread_archtcb(thread)->altsched);
 }
 
 int pipeline_leave_inband(void)
-- 
2.17.1

[PATCH 05/10] dovetail/init: implement Xenomai stage enabling and disabling

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 kernel/cobalt/dovetail/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/dovetail/init.c b/kernel/cobalt/dovetail/init.c
index 983186abe..bc891b4c1 100644
--- a/kernel/cobalt/dovetail/init.c
+++ b/kernel/cobalt/dovetail/init.c
@@ -20,7 +20,7 @@ int __init pipeline_init(void)
 	}
 
 	/* Enable the Xenomai out-of-band stage */
-	TODO();
+	enable_oob_stage("Xenomai");
 
 	ret = xnclock_init();
 	if (ret)
@@ -46,7 +46,7 @@ int __init pipeline_late_init(void)
 __init void pipeline_cleanup(void)
 {
 	/* Disable the Xenomai stage */
-	TODO();
+	disable_oob_stage();
 
 	xnclock_cleanup();
 }
-- 
2.17.1

[PATCH 06/10] dovetail/pipeline: implement oob irq request and free and post for both TIMER_OOB_IPI and RESCHEDULE_OOB_IPI

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 .../kernel/dovetail/pipeline/pipeline.h       | 44 ++++++++++++++-----
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/pipeline.h b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
index 23c4a2c18..a345e1399 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;
 
+extern 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,29 @@ 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 +64,31 @@ static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
 	 * Trigger the out-of-band rescheduling interrupt on remote
 	 * CPU(s).
 	 */
-	TODO();
+	irq_pipeline_send_remote(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 +96,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_pipeline_send_remote(TIMER_OOB_IPI, dest);
 }
 
 #endif
-- 
2.17.1

[PATCH 07/10] dovetail/tick: implement proxy tick device installing and uninstalling

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 kernel/cobalt/dovetail/tick.c | 85 ++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

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 08/10] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 .../cobalt/kernel/dovetail/pipeline/clock.h   |  4 +++-
 kernel/cobalt/dovetail/tick.c                 | 24 +++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index 28d89b44b..ce9a5c867 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -11,6 +11,8 @@
 
 struct timespec64;
 
+extern inline void xnproxy_timer_set(unsigned long delta);
+
 static inline u64 pipeline_read_cycle_counter(void)
 {
 	/* Read the raw cycle counter of the core clock. */
@@ -26,7 +28,7 @@ static inline void pipeline_set_timer_shot(unsigned long cycles)
 	 * pipeline_set_timer_shot() should not be part of the
 	 * pipeline interface.
 	 */
-	TODO();
+	xnproxy_timer_set(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..58662e94e 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -16,6 +16,30 @@ extern struct xnintr nktimer;
 
 static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
 
+inline void xnproxy_timer_set(unsigned long delta)
+{
+	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
+	struct clock_event_device *real_dev = dev->real_device;
+	int ret;
+	u64 cycles;
+
+	if (delta <= 0)
+		delta = real_dev->min_delta_ns;
+	else {
+		delta = min_t(int64_t, delta,
+				(int64_t)real_dev->max_delta_ns);
+		delta = max_t(int64_t, delta,
+					(int64_t)real_dev->min_delta_ns);
+	}
+	cycles = ((u64)delta * real_dev->mult) >> real_dev->shift;
+
+	ret = real_dev->set_next_event(cycles, 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 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/dovetail/pipeline/clock.h | 5 ++---
 kernel/cobalt/dovetail/tick.c                   | 8 ++++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
index ce9a5c867..771104ab4 100644
--- a/include/cobalt/kernel/dovetail/pipeline/clock.h
+++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
@@ -12,6 +12,7 @@
 struct timespec64;
 
 extern inline void xnproxy_timer_set(unsigned long delta);
+inline const char *xn_get_timer_name(void);
 
 static inline u64 pipeline_read_cycle_counter(void)
 {
@@ -37,9 +38,7 @@ 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 "?";
+	return xn_get_timer_name();
 }
 
 static inline const char *pipeline_clock_name(void)
diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
index 58662e94e..e40ec06cb 100644
--- a/kernel/cobalt/dovetail/tick.c
+++ b/kernel/cobalt/dovetail/tick.c
@@ -16,6 +16,14 @@ extern struct xnintr nktimer;
 
 static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
 
+inline const char *xn_get_timer_name(void)
+{
+	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
+	struct clock_event_device *real_dev = dev->real_device;
+
+	return real_dev->name;
+}
+
 inline void xnproxy_timer_set(unsigned long delta)
 {
 	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
-- 
2.17.1

[PATCH 10/10] dovetail/kevents: dovetail: implement handle_ptrace_cont

[hongzha1]

Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
---
 kernel/cobalt/dovetail/kevents.c | 78 ++++++++++++++++++++++++++++++--
 1 file changed, 75 insertions(+), 3 deletions(-)

diff --git a/kernel/cobalt/dovetail/kevents.c b/kernel/cobalt/dovetail/kevents.c
index b5dc50ea7..13c31297c 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,79 @@ 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)
+		goto out;
+
+	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));
+out:
+	return KEVENT_PROPAGATE;
 }
 
 void handle_inband_event(enum inband_event_type event, void *data)
@@ -505,7 +577,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

Re: [PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[Jan Kiszka]

On 11.01.21 07:43, hongzha1 via Xenomai wrote:
> inband sirq request through synthetic_irq_domain and free and post
> srq.
> 
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
> index be9dc587f..1da9d13b2 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
> @@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
>  	 * Allocate an IRQ from the synthetic interrupt domain then
>  	 * trap it to @handler, to be fired from the in-band stage.
>  	 */
> -	TODO();
> +	int sirq, ret;
>  
> -	return 0;
> +	sirq = irq_create_direct_mapping(synthetic_irq_domain);
> +	if (sirq == 0)
> +		return -EAGAIN;
> +
> +	ret = __request_percpu_irq(sirq,
> +			handler,
> +			IRQF_NO_THREAD,
> +			"Inband sirq",
> +			&cobalt_machine_cpudata);
> +
> +	if (ret) {
> +		irq_dispose_mapping(sirq);
> +		return ret;
> +	}
> +
> +	return sirq;
>  }
>  
>  static inline
> @@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
>  	 * Free the synthetic IRQ then deallocate it to its
>  	 * originating domain.
>  	 */
> -	TODO();
> +	free_percpu_irq(sirq,
> +		&cobalt_machine_cpudata);
> +
> +	irq_dispose_mapping(sirq);
>  }
>  
>  static inline void pipeline_post_sirq(int sirq)
>  {
>  	/* Trigger the synthetic IRQ */
> -	TODO();
> +	irq_post_inband(sirq);
>  }
>  
> -#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
> +#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */
> 

Does this series merely target review / discussion?

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[Philippe Gerum]

Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:

> On 11.01.21 07:43, hongzha1 via Xenomai wrote:
>> inband sirq request through synthetic_irq_domain and free and post
>> srq.
>> 
>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>> ---
>>  .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>> 
>> diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>> index be9dc587f..1da9d13b2 100644
>> --- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
>> +++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>> @@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
>>  	 * Allocate an IRQ from the synthetic interrupt domain then
>>  	 * trap it to @handler, to be fired from the in-band stage.
>>  	 */
>> -	TODO();
>> +	int sirq, ret;
>>  
>> -	return 0;
>> +	sirq = irq_create_direct_mapping(synthetic_irq_domain);
>> +	if (sirq == 0)
>> +		return -EAGAIN;
>> +
>> +	ret = __request_percpu_irq(sirq,
>> +			handler,
>> +			IRQF_NO_THREAD,
>> +			"Inband sirq",
>> +			&cobalt_machine_cpudata);
>> +
>> +	if (ret) {
>> +		irq_dispose_mapping(sirq);
>> +		return ret;
>> +	}
>> +
>> +	return sirq;
>>  }
>>  
>>  static inline
>> @@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
>>  	 * Free the synthetic IRQ then deallocate it to its
>>  	 * originating domain.
>>  	 */
>> -	TODO();
>> +	free_percpu_irq(sirq,
>> +		&cobalt_machine_cpudata);
>> +
>> +	irq_dispose_mapping(sirq);
>>  }
>>  
>>  static inline void pipeline_post_sirq(int sirq)
>>  {
>>  	/* Trigger the synthetic IRQ */
>> -	TODO();
>> +	irq_post_inband(sirq);
>>  }
>>  
>> -#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
>> +#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */
>> 
>
> Does this series merely target review / discussion?
>

It is always open for discussion particularly if something needs to be
clarified wrt the way Dovetail works. If not, then this should be
considered as a merge merge request.

-- 
Philippe.

Re: [PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[Jan Kiszka]

On 11.01.21 14:05, Philippe Gerum wrote:
> 
> Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:
> 
>> On 11.01.21 07:43, hongzha1 via Xenomai wrote:
>>> inband sirq request through synthetic_irq_domain and free and post
>>> srq.
>>>
>>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>>> ---
>>>  .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
>>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>> index be9dc587f..1da9d13b2 100644
>>> --- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>> +++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>> @@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
>>>  	 * Allocate an IRQ from the synthetic interrupt domain then
>>>  	 * trap it to @handler, to be fired from the in-band stage.
>>>  	 */
>>> -	TODO();
>>> +	int sirq, ret;
>>>  
>>> -	return 0;
>>> +	sirq = irq_create_direct_mapping(synthetic_irq_domain);
>>> +	if (sirq == 0)
>>> +		return -EAGAIN;
>>> +
>>> +	ret = __request_percpu_irq(sirq,
>>> +			handler,
>>> +			IRQF_NO_THREAD,
>>> +			"Inband sirq",
>>> +			&cobalt_machine_cpudata);
>>> +
>>> +	if (ret) {
>>> +		irq_dispose_mapping(sirq);
>>> +		return ret;
>>> +	}
>>> +
>>> +	return sirq;
>>>  }
>>>  
>>>  static inline
>>> @@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
>>>  	 * Free the synthetic IRQ then deallocate it to its
>>>  	 * originating domain.
>>>  	 */
>>> -	TODO();
>>> +	free_percpu_irq(sirq,
>>> +		&cobalt_machine_cpudata);
>>> +
>>> +	irq_dispose_mapping(sirq);
>>>  }
>>>  
>>>  static inline void pipeline_post_sirq(int sirq)
>>>  {
>>>  	/* Trigger the synthetic IRQ */
>>> -	TODO();
>>> +	irq_post_inband(sirq);
>>>  }
>>>  
>>> -#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
>>> +#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */
>>>
>>
>> Does this series merely target review / discussion?
>>
> 
> It is always open for discussion particularly if something needs to be
> clarified wrt the way Dovetail works. If not, then this should be
> considered as a merge merge request.
> 

I read "TODO", and I thought of the markers we had in wip/dovetail. So
this series is targeting next already?

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[Philippe Gerum]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 11.01.21 14:05, Philippe Gerum wrote:
>> 
>> Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:
>> 
>>> On 11.01.21 07:43, hongzha1 via Xenomai wrote:
>>>> inband sirq request through synthetic_irq_domain and free and post
>>>> srq.
>>>>
>>>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>>>> ---
>>>>  .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
>>>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>>> index be9dc587f..1da9d13b2 100644
>>>> --- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>>> +++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
>>>> @@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
>>>>  	 * Allocate an IRQ from the synthetic interrupt domain then
>>>>  	 * trap it to @handler, to be fired from the in-band stage.
>>>>  	 */
>>>> -	TODO();
>>>> +	int sirq, ret;
>>>>  
>>>> -	return 0;
>>>> +	sirq = irq_create_direct_mapping(synthetic_irq_domain);
>>>> +	if (sirq == 0)
>>>> +		return -EAGAIN;
>>>> +
>>>> +	ret = __request_percpu_irq(sirq,
>>>> +			handler,
>>>> +			IRQF_NO_THREAD,
>>>> +			"Inband sirq",
>>>> +			&cobalt_machine_cpudata);
>>>> +
>>>> +	if (ret) {
>>>> +		irq_dispose_mapping(sirq);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	return sirq;
>>>>  }
>>>>  
>>>>  static inline
>>>> @@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
>>>>  	 * Free the synthetic IRQ then deallocate it to its
>>>>  	 * originating domain.
>>>>  	 */
>>>> -	TODO();
>>>> +	free_percpu_irq(sirq,
>>>> +		&cobalt_machine_cpudata);
>>>> +
>>>> +	irq_dispose_mapping(sirq);
>>>>  }
>>>>  
>>>>  static inline void pipeline_post_sirq(int sirq)
>>>>  {
>>>>  	/* Trigger the synthetic IRQ */
>>>> -	TODO();
>>>> +	irq_post_inband(sirq);
>>>>  }
>>>>  
>>>> -#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
>>>> +#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */
>>>>
>>>
>>> Does this series merely target review / discussion?
>>>
>> 
>> It is always open for discussion particularly if something needs to be
>> clarified wrt the way Dovetail works. If not, then this should be
>> considered as a merge merge request.
>> 
>
> I read "TODO", and I thought of the markers we had in wip/dovetail. So
> this series is targeting next already?
>

Yes. TODOs are being gradually fixed up by Hongzhan, whose patch series
will be following mine once I have reviewed it. Those TODOs do not
affect the behavior of I-pipe side, only the Dovetail-related section
which is WIP. This way, it is possible for me to prep the code base for
introducing Dovetail support, leaving the actual implementation to
Hongzhan, so that we can increase the bus factor by at least one
regarding this stuff.

-- 
Philippe.

Re: [PATCH 01/10] dovetail/sirq: implement sirq request and free and post

[Philippe Gerum]

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

> inband sirq request through synthetic_irq_domain and free and post
> srq.
>
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  .../cobalt/kernel/dovetail/pipeline/sirq.h    | 28 +++++++++++++++----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/sirq.h b/include/cobalt/kernel/dovetail/pipeline/sirq.h
> index be9dc587f..1da9d13b2 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/sirq.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/sirq.h
> @@ -23,9 +23,24 @@ int pipeline_create_inband_sirq(irqreturn_t (*handler)(int irq, void *dev_id))
>  	 * Allocate an IRQ from the synthetic interrupt domain then
>  	 * trap it to @handler, to be fired from the in-band stage.
>  	 */
> -	TODO();
> +	int sirq, ret;
>  
> -	return 0;
> +	sirq = irq_create_direct_mapping(synthetic_irq_domain);
> +	if (sirq == 0)
> +		return -EAGAIN;
> +
> +	ret = __request_percpu_irq(sirq,
> +			handler,
> +			IRQF_NO_THREAD,
> +			"Inband sirq",
> +			&cobalt_machine_cpudata);
> +
> +	if (ret) {
> +		irq_dispose_mapping(sirq);
> +		return ret;
> +	}
> +
> +	return sirq;
>  }
>  
>  static inline
> @@ -35,13 +50,16 @@ void pipeline_delete_inband_sirq(int sirq)
>  	 * Free the synthetic IRQ then deallocate it to its
>  	 * originating domain.
>  	 */
> -	TODO();
> +	free_percpu_irq(sirq,
> +		&cobalt_machine_cpudata);
> +

Nitpicking: lines should be joined. I'll fix this while merging.

> +	irq_dispose_mapping(sirq);
>  }
>  
>  static inline void pipeline_post_sirq(int sirq)
>  {
>  	/* Trigger the synthetic IRQ */
> -	TODO();
> +	irq_post_inband(sirq);
>  }
>  
> -#endif /* !_COBALT_KERNEL_IPIPE_SIRQ_H */
> +#endif /* !_COBALT_KERNEL_DOVETAIL_SIRQ_H */

Looks good, I'm picking up this patch.

Thanks,

-- 
Philippe.

Re: [PATCH 02/10] dovetail/clock: implement pipeline_read_cycle_counter

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  include/cobalt/kernel/dovetail/pipeline/clock.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index 19e3d8986..5176b1506 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -13,9 +13,7 @@ struct timespec64;
>  static inline u64 pipeline_read_cycle_counter(void)
>  {
>  	/* Read the raw cycle counter of the core clock. */
> -	TODO();
> -
> -	return 0;
> +	return  ktime_get_raw_fast_ns();
>  }
>  
>  static inline void pipeline_set_timer_shot(unsigned long cycles)

Ack, picking up this patch. Thanks.

NOTE: this can only work because Dovetail requires us to align on the
in-band kernel's idea of time, i.e. nanosecs. In that case, one Cobalt
"TSC" tick equals one nanosecond.

-- 
Philippe.

Re: [PATCH 03/10] dovetail/clock: implement pipeline_get_host_time

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  include/cobalt/kernel/dovetail/pipeline/clock.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index 5176b1506..28d89b44b 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -7,6 +7,7 @@
>  
>  #include <cobalt/uapi/kernel/types.h>
>  #include <cobalt/kernel/assert.h>
> +#include <linux/ktime.h>
>  
>  struct timespec64;
>  
> @@ -50,7 +51,7 @@ static inline const char *pipeline_clock_name(void)
>  static inline int pipeline_get_host_time(struct timespec64 *tp)
>  {
>  	/* Convert ktime_get_real_fast_ns() to timespec. */
> -	TODO();
> +	*tp = ktime_to_timespec64(ktime_get_real_fast_ns());
>  
>  	return 0;
>  }

Yummie. Ack, picking up this patch. Thanks.

-- 
Philippe.

Re: [PATCH 04/10] dovetail/sched: implement pipeline_init_shadow_tcb and pipeline_init_root_tcb

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  kernel/cobalt/dovetail/sched.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cobalt/dovetail/sched.c b/kernel/cobalt/dovetail/sched.c
> index 71f763d39..82e29136c 100644
> --- a/kernel/cobalt/dovetail/sched.c
> +++ b/kernel/cobalt/dovetail/sched.c
> @@ -36,7 +36,7 @@ void pipeline_init_shadow_tcb(struct xnthread *thread)
>  	/*
>  	 * Initialize the alternate scheduling control block.
>  	 */
> -	TODO();
> +	dovetail_init_altsched(&xnthread_archtcb(thread)->altsched);
>  
>  	trace_cobalt_shadow_map(thread);
>  }
> @@ -46,7 +46,7 @@ void pipeline_init_root_tcb(struct xnthread *thread)
>  	/*
>  	 * Initialize the alternate scheduling control block.
>  	 */
> -	TODO();
> +	dovetail_init_altsched(&xnthread_archtcb(thread)->altsched);
>  }
>  
>  int pipeline_leave_inband(void)

Looks good, picking up this one too.

Thanks.

-- 
Philippe.

Re: [PATCH 05/10] dovetail/init: implement Xenomai stage enabling and disabling

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  kernel/cobalt/dovetail/init.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cobalt/dovetail/init.c b/kernel/cobalt/dovetail/init.c
> index 983186abe..bc891b4c1 100644
> --- a/kernel/cobalt/dovetail/init.c
> +++ b/kernel/cobalt/dovetail/init.c
> @@ -20,7 +20,7 @@ int __init pipeline_init(void)
>  	}
>  
>  	/* Enable the Xenomai out-of-band stage */
> -	TODO();
> +	enable_oob_stage("Xenomai");
>  
>  	ret = xnclock_init();
>  	if (ret)
> @@ -46,7 +46,7 @@ int __init pipeline_late_init(void)
>  __init void pipeline_cleanup(void)
>  {
>  	/* Disable the Xenomai stage */
> -	TODO();
> +	disable_oob_stage();
>  
>  	xnclock_cleanup();
>  }

Works for me, queuing for upstream merge via my tree.

Thanks,

-- 
Philippe.

Re: [PATCH 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device

[chensong]

On 2021年01月11日 14:43, hongzha1 via Xenomai wrote:
> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>   include/cobalt/kernel/dovetail/pipeline/clock.h | 5 ++---
>   kernel/cobalt/dovetail/tick.c                   | 8 ++++++++
>   2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index ce9a5c867..771104ab4 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -12,6 +12,7 @@
>   struct timespec64;
>
>   extern inline void xnproxy_timer_set(unsigned long delta);
> +inline const char *xn_get_timer_name(void);
>
>   static inline u64 pipeline_read_cycle_counter(void)
>   {
> @@ -37,9 +38,7 @@ 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 "?";
> +	return xn_get_timer_name();
>   }
>

PATCH 02/10, PATCH 08/10 and this one, all of them have the changes for 
pipeline_read_cycle_counter in 
include/cobalt/kernel/dovetail/pipeline/clock.h, and they look 
different, how so? or they are incremental implementation,but PATCH 
02/10 returns ktime_get_raw_fast_ns() and PATCH 09/10 returns 
xn_get_timer_name();.

>   static inline const char *pipeline_clock_name(void)
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index 58662e94e..e40ec06cb 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -16,6 +16,14 @@ extern struct xnintr nktimer;
>
>   static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>
> +inline const char *xn_get_timer_name(void)
> +{
> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
> +	struct clock_event_device *real_dev = dev->real_device;
> +
> +	return real_dev->name;
> +}
> +
>   inline void xnproxy_timer_set(unsigned long delta)
>   {
>   	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>

RE: [PATCH 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device

[Chen, Hongzhan]

>
>
>-----Original Message-----
>From: chensong <chensong@tj.kylinos.cn>
>Sent: Tuesday, January 12, 2021 9:21 AM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; Xenomai@xenomai.org
>Subject: Re: [PATCH 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device
>
>
>
>On 2021?01?11? 14:43, hongzha1 via Xenomai wrote:
>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>> ---
>>   include/cobalt/kernel/dovetail/pipeline/clock.h | 5 ++---
>>   kernel/cobalt/dovetail/tick.c                   | 8 ++++++++
>>   2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> index ce9a5c867..771104ab4 100644
>> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
>> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> @@ -12,6 +12,7 @@
>>   struct timespec64;
>>
>>   extern inline void xnproxy_timer_set(unsigned long delta);
>> +inline const char *xn_get_timer_name(void);
>>
>>   static inline u64 pipeline_read_cycle_counter(void)
>>   {
>> @@ -37,9 +38,7 @@ 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 "?";
>> +       return xn_get_timer_name();
>>   }
>>
>
>PATCH 02/10, PATCH 08/10 and this one, all of them have the changes for
>pipeline_read_cycle_counter in
>include/cobalt/kernel/dovetail/pipeline/clock.h, and they look
>different, how so? or they are incremental implementation,but PATCH
>02/10 returns ktime_get_raw_fast_ns() and PATCH 09/10 returns
>xn_get_timer_name();.
>

Patch 02/10 really modify pipeline_read_cycle_counte but 08/10 actually modify
pipeline_set_timer_shot and this one modify pipeline_timer_name. The patch format
created by git send-email is really not friendly to read, I also
made such mistake before.

Regards

Hongzhan Chen
>>   static inline const char *pipeline_clock_name(void)
>> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
>> index 58662e94e..e40ec06cb 100644
>> --- a/kernel/cobalt/dovetail/tick.c
>> +++ b/kernel/cobalt/dovetail/tick.c
>> @@ -16,6 +16,14 @@ extern struct xnintr nktimer;
>>
>>   static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>>
>> +inline const char *xn_get_timer_name(void)
>> +{
>> +       struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>> +       struct clock_event_device *real_dev = dev->real_device;
>> +
>> +       return real_dev->name;
>> +}
>> +
>>   inline void xnproxy_timer_set(unsigned long delta)
>>   {
>>          struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>>

Re: [PATCH 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device

[chensong]

On 2021年01月12日 09:35, Chen, Hongzhan wrote:
>  >
>
>  >
>
>  >-----Original Message-----
>
>  >From: chensong <chensong@tj.kylinos.cn>
>
>  >Sent: Tuesday, January 12, 2021 9:21 AM
>
>  >To: Chen, Hongzhan <hongzhan.chen@intel.com>; Xenomai@xenomai.org
>
>  >Subject: Re: [PATCH 09/10] dovetail/clock: implement
> pipeline_timer_name to get name of real device
>
>  >
>
>  >
>
>  >
>
>  >On 2021?01?11? 14:43, hongzha1 via Xenomai wrote:
>
>  >> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>
>  >> ---
>
>  >>   include/cobalt/kernel/dovetail/pipeline/clock.h | 5 ++---
>
>  >>   kernel/cobalt/dovetail/tick.c                   | 8 ++++++++
>
>  >>   2 files changed, 10 insertions(+), 3 deletions(-)
>
>  >>
>
>  >> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h
> b/include/cobalt/kernel/dovetail/pipeline/clock.h
>
>  >> index ce9a5c867..771104ab4 100644
>
>  >> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
>
>  >> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
>
>  >> @@ -12,6 +12,7 @@
>
>  >>   struct timespec64;
>
>  >>
>
>  >>   extern inline void xnproxy_timer_set(unsigned long delta);
>
>  >> +inline const char *xn_get_timer_name(void);
>
>  >>
>
>  >>   static inline u64 pipeline_read_cycle_counter(void)
>
>  >>   {
>
>  >> @@ -37,9 +38,7 @@ 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 "?";
>
>  >> +       return xn_get_timer_name();
>
>  >>   }
>
>  >>
>
>  >
>
>  >PATCH 02/10, PATCH 08/10 and this one, all of them have the changes for
>
>  >pipeline_read_cycle_counter in
>
>  >include/cobalt/kernel/dovetail/pipeline/clock.h, and they look
>
>  >different, how so? or they are incremental implementation,but PATCH
>
>  >02/10 returns ktime_get_raw_fast_ns() and PATCH 09/10 returns
>
>  >xn_get_timer_name();.
>
>  >
>
> Patch 02/10 really modify pipeline_read_cycle_counte but 08/10 actually
> modify
>
> pipeline_set_timer_shot and this one modify pipeline_timer_name. The
> patch format
>
> created by git send-email is really not friendly to read, I also
>
> made such mistake before.

it's really confusing,anyway,clear to me,many thanks.

song
>
> Regards
>
> Hongzhan Chen
>
>  >>   static inline const char *pipeline_clock_name(void)
>
>  >> diff --git a/kernel/cobalt/dovetail/tick.c
> b/kernel/cobalt/dovetail/tick.c
>
>  >> index 58662e94e..e40ec06cb 100644
>
>  >> --- a/kernel/cobalt/dovetail/tick.c
>
>  >> +++ b/kernel/cobalt/dovetail/tick.c
>
>  >> @@ -16,6 +16,14 @@ extern struct xnintr nktimer;
>
>  >>
>
>  >>   static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>
>  >>
>
>  >> +inline const char *xn_get_timer_name(void)
>
>  >> +{
>
>  >> +       struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>
>  >> +       struct clock_event_device *real_dev = dev->real_device;
>
>  >> +
>
>  >> +       return real_dev->name;
>
>  >> +}
>
>  >> +
>
>  >>   inline void xnproxy_timer_set(unsigned long delta)
>
>  >>   {
>
>  >>          struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>
>  >>
>

Re: [PATCH 06/10] dovetail/pipeline: implement oob irq request and free and post for both TIMER_OOB_IPI and RESCHEDULE_OOB_IPI

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  .../kernel/dovetail/pipeline/pipeline.h       | 44 ++++++++++++++-----
>  1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/pipeline.h b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
> index 23c4a2c18..a345e1399 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;
>  
> +extern void xnintr_core_clock_handler(void);
> +

No need for mentioning the storage class in the declaration. We know
this is extern by default, otherwise we would have mentioned
"static". Will fix while merging.

>  /*
>   * 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,29 @@ 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 +64,31 @@ static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
>  	 * Trigger the out-of-band rescheduling interrupt on remote
>  	 * CPU(s).
>  	 */
> -	TODO();
> +	irq_pipeline_send_remote(RESCHEDULE_OOB_IPI, dest);

This call was recently renamed irq_send_oob_ipi() in an effort to
clarify the interface. I'll fix this up while merging.

>  }
>  
> +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);

Single line. Will fix.

>  }
>  
>  static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
> @@ -72,7 +96,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_pipeline_send_remote(TIMER_OOB_IPI, dest);
>  }

Ditto.

Ack.

-- 
Philippe.

Re: [PATCH 06/10] dovetail/pipeline: implement oob irq request and free and post for both TIMER_OOB_IPI and RESCHEDULE_OOB_IPI

[Philippe Gerum]

Also, the short log should be more concise. The changes are all about
out-of-band IRQ management and handling, details about which IRQs this
applies to can appear in the main part of the commit log.

There are too many changes for me to fix up that patch on the fly while
merging eventually. You may want to resubmit it.

Thanks,

Philippe Gerum via Xenomai <xenomai@xenomai.org> writes:

> hongzha1 via Xenomai <xenomai@xenomai.org> writes:
>
>> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
>> ---
>>  .../kernel/dovetail/pipeline/pipeline.h       | 44 ++++++++++++++-----
>>  1 file changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/cobalt/kernel/dovetail/pipeline/pipeline.h b/include/cobalt/kernel/dovetail/pipeline/pipeline.h
>> index 23c4a2c18..a345e1399 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;
>>  
>> +extern void xnintr_core_clock_handler(void);
>> +
>
> No need for mentioning the storage class in the declaration. We know
> this is extern by default, otherwise we would have mentioned
> "static". Will fix while merging.
>
>>  /*
>>   * 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,29 @@ 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 +64,31 @@ static inline void pipeline_send_resched_ipi(const struct cpumask *dest)
>>  	 * Trigger the out-of-band rescheduling interrupt on remote
>>  	 * CPU(s).
>>  	 */
>> -	TODO();
>> +	irq_pipeline_send_remote(RESCHEDULE_OOB_IPI, dest);
>
> This call was recently renamed irq_send_oob_ipi() in an effort to
> clarify the interface. I'll fix this up while merging.
>
>>  }
>>  
>> +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);
>
> Single line. Will fix.
>
>>  }
>>  
>>  static inline void pipeline_send_timer_ipi(const struct cpumask *dest)
>> @@ -72,7 +96,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_pipeline_send_remote(TIMER_OOB_IPI, dest);
>>  }
>
> Ditto.
>
> Ack.


-- 
Philippe.

Re: [PATCH 09/10] dovetail/clock: implement pipeline_timer_name to get name of real device

[Philippe Gerum]

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

> Signed-off-by: hongzha1 <hongzhan.chen@intel.com>
> ---
>  include/cobalt/kernel/dovetail/pipeline/clock.h | 5 ++---
>  kernel/cobalt/dovetail/tick.c                   | 8 ++++++++
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
> index ce9a5c867..771104ab4 100644
> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
> @@ -12,6 +12,7 @@
>  struct timespec64;
>  
>  extern inline void xnproxy_timer_set(unsigned long delta);
> +inline const char *xn_get_timer_name(void);
>
>  static inline u64 pipeline_read_cycle_counter(void)
>  {
> @@ -37,9 +38,7 @@ 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 "?";
> +	return xn_get_timer_name();
>  }
>  
>  static inline const char *pipeline_clock_name(void)
> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
> index 58662e94e..e40ec06cb 100644
> --- a/kernel/cobalt/dovetail/tick.c
> +++ b/kernel/cobalt/dovetail/tick.c
> @@ -16,6 +16,14 @@ extern struct xnintr nktimer;
>  
>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>  
> +inline const char *xn_get_timer_name(void)
> +{
> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
> +	struct clock_event_device *real_dev = dev->real_device;
> +
> +	return real_dev->name;
> +}
> +

Please rename xn_get_timer_name() directly to pipeline_timer_name(),
which should be out of line, so that we don't have to expose the
proxy_device variable. This stuff is rarely called, and definitely not
on the hot path.

-- 
Philippe.