linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes
@ 2015-06-26 18:52 Timur Tabi
  2015-06-26 18:52 ` [PATCH 2/3] hvc_dcc: don't ignore errors during initialization Timur Tabi
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Timur Tabi @ 2015-06-26 18:52 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, Shanker Donthineni, awallis,
	abhimany, will.deacon, sboyd, Vipul Gandhi

Some debuggers, such as Trace32 from Lauterbach GmbH, do not handle
reads/writes from/to DCC on secondary cores.  Each core has its
own DCC device registers, so when a core reads or writes from/to DCC,
it only accesses its own DCC device.  Since kernel code can run on
any core, every time the kernel wants to write to the console, it
might write to a different DCC.

In SMP mode, Trace32 only uses the DCC on core 0.  In AMP mode, it
creates multiple windows, and each window shows the DCC output
only from that core's DCC.  The result is that console output is
either lost or scattered across windows.

Selecting this option will enable code that serializes all console
input and output to core 0.  The DCC driver will create input and
output FIFOs that all cores will use.  Reads and writes from/to DCC
are handled by a workqueue that runs only core 0.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
Acked-by: Adam Wallis <awallis@codeaurora.org>
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/tty/hvc/Kconfig   |  21 +++++++
 drivers/tty/hvc/hvc_dcc.c | 157 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 177 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 8902f9b..2c6883c 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -95,6 +95,27 @@ config HVC_DCC
 	 driver. This console is used through a JTAG only on ARM. If you don't have
 	 a JTAG then you probably don't want this option.
 
+config HVC_DCC_SERIALIZE_SMP
+	bool "Use DCC only on core 0"
+	depends on SMP && HVC_DCC
+	help
+	  Some debuggers, such as Trace32 from Lauterbach GmbH, do not handle
+	  reads/writes from/to DCC on more than one core.  Each core has its
+	  own DCC device registers, so when a core reads or writes from/to DCC,
+	  it only accesses its own DCC device.  Since kernel code can run on
+	  any core, every time the kernel wants to write to the console, it
+	  might write to a different DCC.
+
+	  In SMP mode, Trace32 only uses the DCC on core 0.  In AMP mode, it
+	  creates multiple windows, and each window shows the DCC output
+	  only from that core's DCC.  The result is that console output is
+	  either lost or scattered across windows.
+
+	  Selecting this option will enable code that serializes all console
+	  input and output to core 0.  The DCC driver will create input and
+	  output FIFOs that all cores will use.  Reads and writes from/to DCC
+	  are handled by a workqueue that runs only core 0.
+
 config HVC_BFIN_JTAG
 	bool "Blackfin JTAG console"
 	depends on BLACKFIN
diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 809920d..33657dc 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -11,6 +11,10 @@
  */
 
 #include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/spinlock.h>
+#include <linux/moduleparam.h>
+#include <linux/console.h>
 
 #include <asm/dcc.h>
 #include <asm/processor.h>
@@ -48,26 +52,177 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
 	return i;
 }
 
+/*
+ * Check if the DCC is enabled.  If CONFIG_HVC_DCC_SERIALIZE_SMP is enabled,
+ * then we assume then this function will be called first on core 0.  That
+ * way, dcc_core0_available will be true only if it's available on core 0.
+ */
 static bool hvc_dcc_check(void)
 {
 	unsigned long time = jiffies + (HZ / 10);
 
+#ifdef CONFIG_HVC_DCC_SERIALIZE_SMP
+	static bool dcc_core0_available;
+
+	/*
+	 * If we're not on core 0, but we previously confirmed that DCC is
+	 * active, then just return true.
+	 */
+	if (smp_processor_id() && dcc_core0_available)
+		return true;
+#endif
+
 	/* Write a test character to check if it is handled */
 	__dcc_putchar('\n');
 
 	while (time_is_after_jiffies(time)) {
-		if (!(__dcc_getstatus() & DCC_STATUS_TX))
+		if (!(__dcc_getstatus() & DCC_STATUS_TX)) {
+#ifdef CONFIG_HVC_DCC_SERIALIZE_SMP
+			dcc_core0_available = true;
+#endif
 			return true;
+		}
 	}
 
 	return false;
 }
 
+#ifdef CONFIG_HVC_DCC_SERIALIZE_SMP
+
+static void dcc_put_work_fn(struct work_struct *work);
+static void dcc_get_work_fn(struct work_struct *work);
+static DECLARE_WORK(dcc_pwork, dcc_put_work_fn);
+static DECLARE_WORK(dcc_gwork, dcc_get_work_fn);
+static DEFINE_SPINLOCK(dcc_lock);
+static DEFINE_KFIFO(inbuf, unsigned char, 128);
+static DEFINE_KFIFO(outbuf, unsigned char, 1024);
+
+/*
+ * Workqueue function that writes the output FIFO to the DCC on core 0.
+ */
+static void dcc_put_work_fn(struct work_struct *work)
+{
+	unsigned char ch;
+
+	spin_lock(&dcc_lock);
+
+	/* While there's data in the output FIFO, write it to the DCC */
+	while (kfifo_get(&outbuf, &ch))
+		hvc_dcc_put_chars(0, &ch, 1);
+
+	/* While we're at it, check for any input characters */
+	while (!kfifo_is_full(&inbuf)) {
+		if (!hvc_dcc_get_chars(0, &ch, 1))
+			break;
+		kfifo_put(&inbuf, ch);
+	}
+
+	spin_unlock(&dcc_lock);
+}
+
+/*
+ * Workqueue function that reads characters from DCC and puts them into the
+ * input FIFO.
+ */
+static void dcc_get_work_fn(struct work_struct *work)
+{
+	unsigned char ch;
+
+	/*
+	 * Read characters from DCC and put them into the input FIFO, as
+	 * long as there is room and we have characters to read.
+	 */
+	spin_lock(&dcc_lock);
+
+	while (!kfifo_is_full(&inbuf)) {
+		if (!hvc_dcc_get_chars(0, &ch, 1))
+			break;
+		kfifo_put(&inbuf, ch);
+	}
+	spin_unlock(&dcc_lock);
+}
+
+/*
+ * Write characters directly to the DCC if we're on core 0 and the FIFO
+ * is empty, or write them to the FIFO if we're not.
+ */
+static int hvc_dcc0_put_chars(uint32_t vt, const char *buf,
+					     int count)
+{
+	int len;
+
+	spin_lock(&dcc_lock);
+	if (smp_processor_id() || (!kfifo_is_empty(&outbuf))) {
+		len = kfifo_in(&outbuf, buf, count);
+		spin_unlock(&dcc_lock);
+		/*
+		 * We just push data to the output FIFO, so schedule the
+		 * workqueue that will actually write that data to DCC.
+		 */
+		schedule_work_on(0, &dcc_pwork);
+		return len;
+	}
+
+	/*
+	 * If we're already on core 0, and the FIFO is empty, then just
+	 * write the data to DCC.
+	 */
+	len = hvc_dcc_put_chars(vt, buf, count);
+	spin_unlock(&dcc_lock);
+
+	return len;
+}
+
+/*
+ * Read characters directly from the DCC if we're on core 0 and the FIFO
+ * is empty, or read them from the FIFO if we're not.
+ */
+static int hvc_dcc0_get_chars(uint32_t vt, char *buf, int count)
+{
+	int len;
+
+	spin_lock(&dcc_lock);
+
+	if (smp_processor_id() || (!kfifo_is_empty(&inbuf))) {
+		len = kfifo_out(&inbuf, buf, count);
+		spin_unlock(&dcc_lock);
+
+		/*
+		 * If the FIFO was empty, there may be characters in the DCC
+		 * that we haven't read yet.  Schedule a workqueue to fill
+		 * the input FIFO, so that the next time this function is
+		 * called, we'll have data.
+		*/
+		if (!len)
+			schedule_work_on(0, &dcc_gwork);
+
+		return len;
+	}
+
+	/*
+	 * If we're already on core 0, and the FIFO is empty, then just
+	 * read the data from DCC.
+	 */
+	len = hvc_dcc_get_chars(vt, buf, count);
+	spin_unlock(&dcc_lock);
+
+	return len;
+}
+
+static const struct hv_ops hvc_dcc_get_put_ops = {
+	.get_chars = hvc_dcc0_get_chars,
+	.put_chars = hvc_dcc0_put_chars,
+};
+
+#else
+
 static const struct hv_ops hvc_dcc_get_put_ops = {
 	.get_chars = hvc_dcc_get_chars,
 	.put_chars = hvc_dcc_put_chars,
 };
 
+#endif
+
 static int __init hvc_dcc_console_init(void)
 {
 	if (!hvc_dcc_check())
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] hvc_dcc: don't ignore errors during initialization
  2015-06-26 18:52 [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
@ 2015-06-26 18:52 ` Timur Tabi
  2015-06-26 18:52 ` [PATCH 3/3] [v2] ARM64: TTY: hvc_dcc: Add support for ARM64 dcc Timur Tabi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Timur Tabi @ 2015-06-26 18:52 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, Shanker Donthineni, awallis,
	abhimany, will.deacon, sboyd, Vipul Gandhi

hvc_instantiate() and hvc_alloc() return errors if they fail, so don't
ignore them.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/tty/hvc/hvc_dcc.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 33657dc..f8b8cf2 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -225,20 +225,29 @@ static const struct hv_ops hvc_dcc_get_put_ops = {
 
 static int __init hvc_dcc_console_init(void)
 {
+	int ret;
+
+	/* This always runs on boot core */
 	if (!hvc_dcc_check())
 		return -ENODEV;
 
-	hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
-	return 0;
+	/* Returns -1 if error */
+	ret = hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
+
+	return ret < 0 ? -ENODEV : 0;
 }
 console_initcall(hvc_dcc_console_init);
 
 static int __init hvc_dcc_init(void)
 {
+	struct hvc_struct *p;
+
+	/* This can run on any core */
 	if (!hvc_dcc_check())
 		return -ENODEV;
 
-	hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
-	return 0;
+	p = hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
+
+	return IS_ERR(p) ? PTR_ERR(p) : 0;
 }
 device_initcall(hvc_dcc_init);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] [v2] ARM64: TTY: hvc_dcc: Add support for ARM64 dcc
  2015-06-26 18:52 [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
  2015-06-26 18:52 ` [PATCH 2/3] hvc_dcc: don't ignore errors during initialization Timur Tabi
@ 2015-06-26 18:52 ` Timur Tabi
  2015-06-26 19:28 ` [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
  2015-06-30 13:58 ` Will Deacon
  3 siblings, 0 replies; 6+ messages in thread
From: Timur Tabi @ 2015-06-26 18:52 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, Shanker Donthineni, awallis,
	abhimany, will.deacon, sboyd, Vipul Gandhi

From: Abhimanyu Kapur <abhimany@codeaurora.org>

Add support for debug communications channel based
hvc console for arm64 cpus.

Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 arch/arm64/include/asm/dcc.h | 52 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/tty/hvc/Kconfig      |  2 +-
 drivers/tty/hvc/hvc_dcc.c    | 11 ++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/dcc.h

diff --git a/arch/arm64/include/asm/dcc.h b/arch/arm64/include/asm/dcc.h
new file mode 100644
index 0000000..d550173
--- /dev/null
+++ b/arch/arm64/include/asm/dcc.h
@@ -0,0 +1,52 @@
+/* Copyright (c) 2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * A call to __dcc_getchar() or __dcc_putchar() is typically followed by
+ * a call to __dcc_getstatus().  We want to make sure that the CPU does
+ * not speculative read the DCC status before executing the read or write
+ * instruction.  That's what the ISBs are for.
+ *
+ * The 'volatile' ensures that the compiler does not cache the status bits,
+ * and instead reads the DCC register every time.
+ */
+#ifndef __ASM_DCC_H
+#define __ASM_DCC_H
+
+#include <asm/barrier.h>
+
+static inline u32 __dcc_getstatus(void)
+{
+	u32 ret;
+
+	asm volatile("mrs %0, mdccsr_el0" : "=r" (ret));
+
+	return ret;
+}
+
+static inline char __dcc_getchar(void)
+{
+	char c;
+
+	asm volatile("mrs %0, dbgdtrrx_el0" : "=r" (c));
+	isb();
+
+	return c;
+}
+
+static inline void __dcc_putchar(char c)
+{
+	asm volatile("msr dbgdtrtx_el0, %0"
+			: /* No output register */
+			: "r" (c));
+	isb();
+}
+
+#endif
diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 2c6883c..9a60d18 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -88,7 +88,7 @@ config HVC_UDBG
 
 config HVC_DCC
        bool "ARM JTAG DCC console"
-       depends on ARM
+       depends on ARM || ARM64
        select HVC_DRIVER
        help
          This console uses the JTAG DCC on ARM to create a console under the HVC
diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index f8b8cf2..d4c7073 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -226,11 +226,22 @@ static const struct hv_ops hvc_dcc_get_put_ops = {
 static int __init hvc_dcc_console_init(void)
 {
 	int ret;
+#ifdef CONFIG_ARM64
+	u32 val;
+#endif
 
 	/* This always runs on boot core */
 	if (!hvc_dcc_check())
 		return -ENODEV;
 
+#ifdef CONFIG_ARM64
+	/* Disable user-space access to DCC */
+	asm("msr mdscr_el1, %0    "
+		"orr %0, %0, #4096    " /* TDCC */
+		"msr %0, mdscr_el1    "
+		: "=r" (val));
+#endif
+
 	/* Returns -1 if error */
 	ret = hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes
  2015-06-26 18:52 [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
  2015-06-26 18:52 ` [PATCH 2/3] hvc_dcc: don't ignore errors during initialization Timur Tabi
  2015-06-26 18:52 ` [PATCH 3/3] [v2] ARM64: TTY: hvc_dcc: Add support for ARM64 dcc Timur Tabi
@ 2015-06-26 19:28 ` Timur Tabi
  2015-06-30 13:58 ` Will Deacon
  3 siblings, 0 replies; 6+ messages in thread
From: Timur Tabi @ 2015-06-26 19:28 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linux-arm-kernel, lkml, Shanker Donthineni, awallis,
	Abhimanyu Kapur, Will Deacon, Stephen Boyd, Vipul Gandhi

On Fri, Jun 26, 2015 at 1:52 PM, Timur Tabi <timur@codeaurora.org> wrote:
>
> Selecting this option will enable code that serializes all console
> input and output to core 0.  The DCC driver will create input and
> output FIFOs that all cores will use.  Reads and writes from/to DCC
> are handled by a workqueue that runs only core 0.
>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>

Somehow the original author got dropped when I made this patch.  This
patch should say:

From: Shanker Donthineni <shankerd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes
  2015-06-26 18:52 [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
                   ` (2 preceding siblings ...)
  2015-06-26 19:28 ` [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
@ 2015-06-30 13:58 ` Will Deacon
  2015-06-30 14:07   ` Timur Tabi
  3 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2015-06-30 13:58 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linux-arm-kernel, linux-kernel, Shanker Donthineni, awallis,
	abhimany, sboyd, Vipul Gandhi

On Fri, Jun 26, 2015 at 07:52:34PM +0100, Timur Tabi wrote:
> Some debuggers, such as Trace32 from Lauterbach GmbH, do not handle
> reads/writes from/to DCC on secondary cores.  Each core has its
> own DCC device registers, so when a core reads or writes from/to DCC,
> it only accesses its own DCC device.  Since kernel code can run on
> any core, every time the kernel wants to write to the console, it
> might write to a different DCC.
> 
> In SMP mode, Trace32 only uses the DCC on core 0.  In AMP mode, it
> creates multiple windows, and each window shows the DCC output
> only from that core's DCC.  The result is that console output is
> either lost or scattered across windows.
> 
> Selecting this option will enable code that serializes all console
> input and output to core 0.  The DCC driver will create input and
> output FIFOs that all cores will use.  Reads and writes from/to DCC
> are handled by a workqueue that runs only core 0.

What happens if CPU0 is hotplugged off?

Will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes
  2015-06-30 13:58 ` Will Deacon
@ 2015-06-30 14:07   ` Timur Tabi
  0 siblings, 0 replies; 6+ messages in thread
From: Timur Tabi @ 2015-06-30 14:07 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-kernel, Shanker Donthineni, awallis,
	abhimany, sboyd, Vipul Gandhi

Will Deacon wrote:
>> >Selecting this option will enable code that serializes all console
>> >input and output to core 0.  The DCC driver will create input and
>> >output FIFOs that all cores will use.  Reads and writes from/to DCC
>> >are handled by a workqueue that runs only core 0.

> What happens if CPU0 is hotplugged off?

I guess the whole thing just breaks.

I don't know what to say.  Trace32's DCC window is latched to CPU0.  If 
that CPU is hotplugged off, then I don't think there's a mechanism for 
Trace32 to migrate to another CPU.  It will broken no matter what.

That's why this feature is disabled by default.  If you're working on an 
SMP ARM system and using Trace32, you will need this patch.  It might be 
possible for Lauterbach to fix Trace32 to provide this feature 
internally in some situations (e.g. SMP mode where there's one window 
for all cores), but not in every situation.

I can add a "depends on !CPU_HOTPLUG" to the Kconfig, but I think that's 
overkill.  This is really a "use it if you need it" patch, and DCC is 
used mostly for debugging.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-06-30 14:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-26 18:52 [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
2015-06-26 18:52 ` [PATCH 2/3] hvc_dcc: don't ignore errors during initialization Timur Tabi
2015-06-26 18:52 ` [PATCH 3/3] [v2] ARM64: TTY: hvc_dcc: Add support for ARM64 dcc Timur Tabi
2015-06-26 19:28 ` [PATCH 1/3] hvc_dcc: bind driver to core0 for reads and writes Timur Tabi
2015-06-30 13:58 ` Will Deacon
2015-06-30 14:07   ` Timur Tabi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).