2012-08-20 14:47:14

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH v4 0/12] KGDB/KDB FIQ (NMI) debugger

Hi all,

In v4:

- Implemented kgdb_nmi serial driver, it provides 'nmi_console' KDB
command. With the driver we can use our debugger port as a normal
console, except that we can always get back to the debugger using the
magic sequence. Note that I still somewhat reluctant to introduce
software-raised interrupts, as they're arch-specific and not always
possible. So today the driver uses a tasklet, it should be pretty
cheap: we're checking for the input on timer interrupts, but we don't
cause needless wakeups. The pro of this is that it works everywhere
(but arches still have an option to optimize things, of course);

- Two new patches added to propagate init_poll() callbacks from tty
to uart drivers. As a side-effect, a long-standing bug fixed in
amba-pl1011 driver;

- Dropped patch 'Get rid of .LCcralign local label';

- Some more fixes in SVC return path. Now it seems rock-solid;

- The patches were rebased on the latest Linus' tree, and as usual can
be found in the following repo:

git://git.infradead.org/users/cbou/linux-nmi-kdb.git master


Boilerplate:

These patches introduce KGDB FIQ debugger support. The idea (and some
code, of course) comes from Google's FIQ debugger[2]. There are some
differences (mostly implementation details, feature-wise they're almost
equivalent, or can be made equivalent, if desired).

The FIQ debugger is a facility that can be used to debug situations when
the kernel stuck in uninterruptable sections, e.g. the kernel infinitely
loops or deadlocked in an interrupt or with interrupts disabled. On some
development boards there is even a special NMI button, which is very
useful for debugging weird kernel hangs.

And FIQ is basically an NMI, it has a higher priority than IRQs, and
upon IRQ exception FIQs are not disabled. It is still possible to
disable FIQs (as well as some "NMIs" on other architectures), but via
special means.

So, here FIQs and NMIs are synonyms, but in the code I use NMI term for
arch-independent code, and FIQs for ARM code.

A few years ago KDB wasn't yet ready for production, or even not
well-known, so originally Google implemented its own FIQ debugger that
included its own shell, ring-buffer, commands, dumping, backtracing
logic and whatnot. This is very much like PowerPC's xmon
(arch/powerpc/xmon), except that xmon was there for a decade, so it even
predates KDB.

Anyway, nowadays KGDB/KDB is the cross-platform debugger, and the only
feature that was missing is NMI handling. This is now fixed for ARM.

There are a few differences comparing to the original (Google's) FIQ
debugger:

- Doing stuff in FIQ context is dangerous, as there we are not allowed
to cause aborts or faults. In the original FIQ debugger there was a
"signal" software-induced interrupt, upon exit from FIQ it would fire,
and we would continue to execute "dangerous" commands from there.

In KGDB/KDB we don't use signal interrupts. We can do easier: set up a
breakpoint, continue, and you'll trap into KGDB again in a safe
context.

It works for most cases, but I can imagine cases when you can't set up
a breakpoint. For these cases we'd better introduce a KDB command
"exit_nmi", that will rise the SW IRQ, after which we're allowed to do
anything.

- KGDB/KDB FIQ debugger shell is synchronous. In Google's version you
could have a dedicated shell always running in the FIQ context, so
when you type something on a serial line, you won't actually cause any
debugging actions, FIQ would save the characters in its own buffer and
continue execution normally. But when you hit return key after the
command, then the command is executed.

In KGDB/KDB FIQ debugger it is different. Once you enter KGDB, the
kernel will stop until you instruct it to continue.

This might look as a drastic change, but it is not. There is actually
no difference whether you have sync or async shell, or at least I
couldn't find any use-case where this would matter at all. Anyways, it
is still possible to do async shell in KDB, just don't see any need
for this.

- Original FIQ debugger used a custom FIQ vector handling code, w/ a lot
of logic in it. In this approach I'm using the fact that FIQs are
basically IRQs, except that we there are a bit more registers banked,
and we can actually trap from the IRQ context.

But this all does not prevent us from using a simple jump-table based
approach as used in the generic ARM entry code. So, here I just reuse
the generic approach.

Note that I test the code on a modelled ARM machine (QEMU Versatile), so
there might be some issues on a real HW, but it works in QEMU tho. :-)

Assuming you have QEMU >= 1.1.0, you can easily play with the code using
ARM/versatile defconfig and command like this:

qemu-system-arm -nographic -machine versatilepb -kernel
linux/arch/arm/boot/zImage -append "console=ttyAMA0 kgdboc=ttyAMA0
kgdb_fiq.enable=1"

Thanks,

--
arch/arm/Kconfig | 19 ++
arch/arm/common/vic.c | 28 +++
arch/arm/include/asm/hardware/vic.h | 2 +
arch/arm/include/asm/kgdb.h | 8 +
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/entry-armv.S | 167 +----------------
arch/arm/kernel/entry-header.S | 170 ++++++++++++++++++
arch/arm/kernel/kgdb_fiq.c | 99 ++++++++++
arch/arm/kernel/kgdb_fiq_entry.S | 87 +++++++++
arch/arm/mach-versatile/Makefile | 1 +
arch/arm/mach-versatile/kgdb_fiq.c | 31 ++++
drivers/tty/serial/Kconfig | 19 ++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/amba-pl011.c | 60 +++++--
drivers/tty/serial/kgdb_nmi.c | 348 ++++++++++++++++++++++++++++++++++++
drivers/tty/serial/kgdboc.c | 16 ++
drivers/tty/serial/serial_core.c | 30 ++++
include/linux/kdb.h | 29 +--
include/linux/kgdb.h | 34 ++++
include/linux/serial_core.h | 2 +
include/linux/tty_driver.h | 1 +
kernel/debug/debug_core.c | 36 +++-
kernel/debug/kdb/kdb_main.c | 29 +++
23 files changed, 1024 insertions(+), 194 deletions(-)

In v3:

- Per Colin Cross suggestion, added a way to release a debug console for
normal use. This is done via 'disable_nmi' command (in the original
FIQ debugger it was 'console' command). For this I added a new
callback in the tty ops, and serial drivers have to provide a way to
clear its interrupts. The patch 'tty/serial/kgdboc: Add and wire up
clear_irqs callback' explains the concept in details.
- Made the debug entry prompt more shell-like;
- A new knocking mode '-1'. It disables the feature altogether, and thus
makes it possible to hook KDB entry to a dedicated button.
- The code was rebased on 'v3.5 + kdb kiosk'[1] patches;

In v2:

- Per Colin Cross' suggestion, we should not enter the debugger on any
received byte (this might be a problem when there's a noise on the
serial line). So there is now an additional patch that implements
"knocking" to the KDB (either via $3#33 command or return key, this is
configurable);
- Reworked {enable,select}_fiq/is_fiq callbacks, now multi-mach kernels
should not be a problem;
- For versatile machines there are run-time checks for proper UART port
(kernel will scream aloud if out of range port is specified);
- Added some __init annotations;
- Since not every architecture defines FIQ_START, we can't just blindly
select CONFIG_FIQ symbol. So ARCH_MIGHT_HAVE_FIQ introduced;
- Add !THUMB2_KERNEL dependency for KGDB_FIQ, we don't support Thumb2
kernels;
- New patch that is used to get rid of LCcralign label in alignment_trap
macro.

[1] https://lkml.org/lkml/2012/7/26/260
[2]
Original Google's FIQ debugger, fiq_* files:
http://android.git.linaro.org/gitweb?p=kernel/common.git;a=tree;f=arch/arm/common;hb=refs/heads/android-3.4
And board support as an example of using it:
http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=commitdiff;h=461cb80c16e4e266ab6207a00767b59212148086


2012-08-20 14:50:08

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 01/12] kernel/debug: Mask KGDB NMI upon entry

The new arch callback should manage NMIs that usually cause KGDB to
enter. That is, not all NMIs should be enabled/disabled, but only
those that issue kgdb_handle_exception().

We must mask it as serial-line interrupt can be used as an NMI, so
if the original KGDB-entry cause was say a breakpoint, then every
input to KDB console will cause KGDB to reenter, which we don't want.

Signed-off-by: Anton Vorontsov <[email protected]>
---
include/linux/kgdb.h | 23 +++++++++++++++++++++++
kernel/debug/debug_core.c | 36 +++++++++++++++++++++++++++++++++---
2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c4d2fc1..3b111a6 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -221,6 +221,29 @@ extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
*/
extern void kgdb_arch_late(void);

+/**
+ * kgdb_arch_enable_nmi - Enable or disable KGDB-entry NMI
+ * @on: Flag to either enable or disable an NMI
+ *
+ * This is an architecture-specific "back-end" for kgdb_enable_nmi(). The
+ * call does not count disable/enable requests, do not use it directly.
+ */
+extern void kgdb_arch_enable_nmi(bool on);
+
+/**
+ * kgdb_enable_nmi - Enable or disable KGDB-entry NMI
+ * @on: Flag to either enable or disable an NMI
+ *
+ * This function manages NMIs that usually cause KGDB to enter. That is,
+ * not all NMIs should be enabled or disabled, but only those that issue
+ * kgdb_handle_exception().
+ *
+ * The call counts disable requests, and thus allows to nest disables.
+ * But trying to enable already enabled NMI is an error. The call returns
+ * 1 if NMI has been actually enabled after the call, and a value <= 0 if
+ * it is still disabled.
+ */
+extern int kgdb_enable_nmi(bool on);

/**
* struct kgdb_arch - Describe architecture specific values.
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..b621d1e 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -214,6 +214,30 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs)
return 0;
}

+void __weak kgdb_arch_enable_nmi(bool on)
+{
+}
+
+int kgdb_enable_nmi(bool on)
+{
+ static atomic_t cnt;
+ int ret;
+
+ ret = atomic_add_return(on ? 1 : -1, &cnt);
+ if (ret > 1 && on) {
+ /*
+ * There should be only one instance that calls this function
+ * in "enable, disable" order. All other users must call
+ * disable first, then enable. If not, something is wrong.
+ */
+ WARN_ON(1);
+ return 1;
+ }
+
+ kgdb_arch_enable_nmi(ret > 0);
+ return ret;
+}
+
/*
* Some architectures need cache flushes when we set/clear a
* breakpoint:
@@ -672,6 +696,9 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
{
struct kgdb_state kgdb_var;
struct kgdb_state *ks = &kgdb_var;
+ int ret = 0;
+
+ kgdb_enable_nmi(0);

ks->cpu = raw_smp_processor_id();
ks->ex_vector = evector;
@@ -681,11 +708,14 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
ks->linux_regs = regs;

if (kgdb_reenter_check(ks))
- return 0; /* Ouch, double exception ! */
+ goto out; /* Ouch, double exception ! */
if (kgdb_info[ks->cpu].enter_kgdb != 0)
- return 0;
+ goto out;

- return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+ ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+out:
+ kgdb_enable_nmi(1);
+ return ret;
}

int kgdb_nmicallback(int cpu, void *regs)
--
1.7.11.5

2012-08-20 14:50:13

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 03/12] kdb: Turn KGDB_KDB=n stubs into static inlines

This makes the stubs actually usable, since e.g. 'foo = kdb_register();'
leads to build errors in !KGDB_KDB case. Plus, with static inlines we
do type checking.

Signed-off-by: Anton Vorontsov <[email protected]>
---
include/linux/kdb.h | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 42d9e86..7f6fe6e 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -13,6 +13,14 @@
* Copyright (C) 2009 Jason Wessel <[email protected]>
*/

+typedef enum {
+ KDB_REPEAT_NONE = 0, /* Do not repeat this command */
+ KDB_REPEAT_NO_ARGS, /* Repeat the command without arguments */
+ KDB_REPEAT_WITH_ARGS, /* Repeat the command including its arguments */
+} kdb_repeat_t;
+
+typedef int (*kdb_func_t)(int, const char **);
+
#ifdef CONFIG_KGDB_KDB
#include <linux/init.h>
#include <linux/sched.h>
@@ -32,14 +40,6 @@ extern atomic_t kdb_event;

#define KDB_MAXARGS 16 /* Maximum number of arguments to a function */

-typedef enum {
- KDB_REPEAT_NONE = 0, /* Do not repeat this command */
- KDB_REPEAT_NO_ARGS, /* Repeat the command without arguments */
- KDB_REPEAT_WITH_ARGS, /* Repeat the command including its arguments */
-} kdb_repeat_t;
-
-typedef int (*kdb_func_t)(int, const char **);
-
/* KDB return codes from a command or internal kdb function */
#define KDB_NOTFOUND (-1)
#define KDB_ARGCOUNT (-2)
@@ -149,11 +149,14 @@ extern int kdb_register_repeat(char *, kdb_func_t, char *, char *,
short, kdb_repeat_t);
extern int kdb_unregister(char *);
#else /* ! CONFIG_KGDB_KDB */
-#define kdb_printf(...)
-#define kdb_init(x)
-#define kdb_register(...)
-#define kdb_register_repeat(...)
-#define kdb_uregister(x)
+static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; }
+static inline void kdb_init(int level) {}
+static inline int kdb_register(char *cmd, kdb_func_t func, char *usage,
+ char *help, short minlen) { return 0; }
+static inline int kdb_register_repeat(char *cmd, kdb_func_t func, char *usage,
+ char *help, short minlen,
+ kdb_repeat_t repeat) { return 0; }
+static inline int kdb_unregister(char *cmd) { return 0; }
#endif /* CONFIG_KGDB_KDB */
enum {
KDB_NOT_INITIALIZED,
--
1.7.11.5

2012-08-20 14:50:20

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 04/12] tty/serial/core: Introduce poll_init callback

It was noticed that polling drivers (like KGDB) are not able to use
serial ports if the ports were not previously initialized via console.
I.e. when booting with console=ttyAMA0 kgdboc=ttyAMA0, everything works
fine, but with console=ttyFOO kgdboc=ttyAMA0, the kgdboc doesn't work.

This is because we don't initialize the hardware. Calling ->startup() is
not an option, because drivers request interrupts there, and drivers
fail to handle situations when tty isn't opened with interrupts enabled.

So, we have to implement a new callback (actually, tty_ops already have
a similar callback), which does everything needed to initialize just the
hardware.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/tty/serial/serial_core.c | 15 +++++++++++++++
include/linux/serial_core.h | 1 +
2 files changed, 16 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a21dc8e..cba8443 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2108,11 +2108,13 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
{
struct uart_driver *drv = driver->driver_state;
struct uart_state *state = drv->state + line;
+ struct tty_port *tport;
struct uart_port *port;
int baud = 9600;
int bits = 8;
int parity = 'n';
int flow = 'n';
+ int ret;

if (!state || !state->uart_port)
return -1;
@@ -2121,6 +2123,19 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
if (!(port->ops->poll_get_char && port->ops->poll_put_char))
return -1;

+ tport = &state->port;
+ if (!(tport->flags & ASYNC_INITIALIZED) && port->ops->poll_init) {
+ mutex_lock(&tport->mutex);
+ ret = port->ops->poll_init(port);
+ /*
+ * We don't set ASYNCB_INITIALIZED as we only initialized the
+ * hw, e.g. state->xmit is still uninitialized.
+ */
+ mutex_unlock(&tport->mutex);
+ if (ret)
+ return ret;
+ }
+
if (options) {
uart_parse_options(options, &baud, &parity, &bits, &flow);
return uart_set_options(port, NULL, baud, parity, bits, flow);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 0253c20..3642710 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -274,6 +274,7 @@ struct uart_ops {
int (*verify_port)(struct uart_port *, struct serial_struct *);
int (*ioctl)(struct uart_port *, unsigned int, unsigned long);
#ifdef CONFIG_CONSOLE_POLL
+ int (*poll_init)(struct uart_port *);
void (*poll_put_char)(struct uart_port *, unsigned char);
int (*poll_get_char)(struct uart_port *);
#endif
--
1.7.11.5

2012-08-20 14:50:27

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 11/12] ARM: VIC: Add a couple of low-level FIQ management helpers

Just a couple of calls to manage VIC FIQ routing. We'll use them for
KGDB FIQ support on ARM Versatile machines.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/common/vic.c | 28 ++++++++++++++++++++++++++++
arch/arm/include/asm/hardware/vic.h | 2 ++
2 files changed, 30 insertions(+)

diff --git a/arch/arm/common/vic.c b/arch/arm/common/vic.c
index e0d5388..df2fc82 100644
--- a/arch/arm/common/vic.c
+++ b/arch/arm/common/vic.c
@@ -66,6 +66,34 @@ static struct vic_device vic_devices[CONFIG_ARM_VIC_NR];

static int vic_id;

+static void __iomem *vic_base(struct irq_data *d)
+{
+ return (void __iomem *)irq_data_get_irq_chip_data(d);
+}
+
+void vic_fiq_select(unsigned int irq, bool on)
+{
+ void __iomem *base = vic_base(&irq_to_desc(irq)->irq_data);
+ void __iomem *sel = base + VIC_INT_SELECT;
+ u32 msk = 1 << irq;
+ u32 val;
+
+ pr_debug("rerouting VIC vector %d to %s\n", irq, on ? "FIQ" : "IRQ");
+
+ val = readl(sel);
+ val &= ~msk;
+ if (on)
+ val |= msk;
+ writel(val, sel);
+}
+
+bool vic_is_fiq_rised(unsigned int irq)
+{
+ void __iomem *base = vic_base(&irq_to_desc(irq)->irq_data);
+
+ return readl(base + VIC_FIQ_STATUS) & (1 << irq);
+}
+
/**
* vic_init2 - common initialisation code
* @base: Base of the VIC.
diff --git a/arch/arm/include/asm/hardware/vic.h b/arch/arm/include/asm/hardware/vic.h
index e14af1a..2728975 100644
--- a/arch/arm/include/asm/hardware/vic.h
+++ b/arch/arm/include/asm/hardware/vic.h
@@ -52,6 +52,8 @@ void __vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources,
void vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources, u32 resume_sources);
int vic_of_init(struct device_node *node, struct device_node *parent);
void vic_handle_irq(struct pt_regs *regs);
+void vic_fiq_select(unsigned int irq, bool on);
+bool vic_is_fiq_rised(unsigned int irq);

#endif /* __ASSEMBLY__ */
#endif
--
1.7.11.5

2012-08-20 14:50:43

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 12/12] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger

If enabled, kernel will able to enter KGDB upon serial line activity on
UART ports.

Note that even with this patch and CONFIG_KGDB_FIQ is enabled, you still
need to pass kgdb_fiq.enable=1 kernel command line option, otherwise UART
will behave in a normal way.

By default UART0 is used, but this can be changed via kgdb_fiq.uart_num
kernel command line option.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-versatile/Makefile | 1 +
arch/arm/mach-versatile/kgdb_fiq.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 33 insertions(+)
create mode 100644 arch/arm/mach-versatile/kgdb_fiq.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c978c74..689b716 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -330,6 +330,7 @@ config ARCH_VERSATILE
select ICST
select GENERIC_CLOCKEVENTS
select ARCH_WANT_OPTIONAL_GPIOLIB
+ select ARCH_MIGHT_HAVE_KGDB_FIQ
select NEED_MACH_IO_H if PCI
select PLAT_VERSATILE
select PLAT_VERSATILE_CLOCK
diff --git a/arch/arm/mach-versatile/Makefile b/arch/arm/mach-versatile/Makefile
index 81fa3fe..bfd761f 100644
--- a/arch/arm/mach-versatile/Makefile
+++ b/arch/arm/mach-versatile/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_VERSATILE_PB) += versatile_pb.o
obj-$(CONFIG_MACH_VERSATILE_AB) += versatile_ab.o
obj-$(CONFIG_MACH_VERSATILE_DT) += versatile_dt.o
obj-$(CONFIG_PCI) += pci.o
+obj-$(CONFIG_KGDB_FIQ) += kgdb_fiq.o
diff --git a/arch/arm/mach-versatile/kgdb_fiq.c b/arch/arm/mach-versatile/kgdb_fiq.c
new file mode 100644
index 0000000..3cdf71d
--- /dev/null
+++ b/arch/arm/mach-versatile/kgdb_fiq.c
@@ -0,0 +1,31 @@
+/*
+ * KGDB FIQ board support
+ *
+ * Copyright 2012 Linaro Ltd.
+ * Anton Vorontsov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kgdb.h>
+#include <mach/hardware.h>
+#include <mach/platform.h>
+#include <asm/hardware/vic.h>
+
+static int kgdb_fiq;
+module_param_named(uart_num, kgdb_fiq, int, 0600);
+MODULE_PARM_DESC(uart_num, "UART<number> port to use for KGDB FIQ");
+
+static int __init kgdb_fiq_init(void)
+{
+ WARN_ON(kgdb_fiq > INT_UARTINT2 - INT_UARTINT0);
+
+ return kgdb_register_fiq(INT_UARTINT0 + kgdb_fiq,
+ vic_fiq_select,
+ vic_is_fiq_rised);
+}
+console_initcall(kgdb_fiq_init);
--
1.7.11.5

2012-08-20 14:50:16

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 02/12] kdb: Implement disable_nmi command

This command disables NMI-entry. If NMI source has been previously shared
with a serial console ("debug port"), this effectively releases the port
from KDB exclusive use, and makes the console available for normal use.

Of course, NMI can be reenabled, enable_nmi modparam is used for that:

echo 1 > /sys/module/kdb/parameters/enable_nmi

Signed-off-by: Anton Vorontsov <[email protected]>
---
kernel/debug/kdb/kdb_main.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 31df170..9fadff1 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -21,6 +21,7 @@
#include <linux/smp.h>
#include <linux/utsname.h>
#include <linux/vmalloc.h>
+#include <linux/atomic.h>
#include <linux/module.h>
#include <linux/mm.h>
#include <linux/init.h>
@@ -2107,6 +2108,32 @@ static int kdb_dmesg(int argc, const char **argv)
return 0;
}
#endif /* CONFIG_PRINTK */
+
+/* Make sure we balance enable/disable calls, must disable first. */
+static atomic_t kdb_nmi_disabled;
+
+static int kdb_disable_nmi(int argc, const char *argv[])
+{
+ if (atomic_read(&kdb_nmi_disabled))
+ return 0;
+ atomic_set(&kdb_nmi_disabled, 1);
+ kgdb_enable_nmi(0);
+ return 0;
+}
+
+static int kdb_param_enable_nmi(const char *val, const struct kernel_param *kp)
+{
+ if (!atomic_add_unless(&kdb_nmi_disabled, -1, 0))
+ return -EINVAL;
+ kgdb_enable_nmi(1);
+ return 0;
+}
+
+static const struct kernel_param_ops kdb_param_ops_enable_nmi = {
+ .set = kdb_param_enable_nmi,
+};
+module_param_cb(enable_nmi, &kdb_param_ops_enable_nmi, NULL, 0600);
+
/*
* kdb_cpu - This function implements the 'cpu' command.
* cpu [<cpunum>]
@@ -2851,6 +2878,8 @@ static void __init kdb_inittab(void)
kdb_register_repeat("dmesg", kdb_dmesg, "[lines]",
"Display syslog buffer", 0, KDB_REPEAT_NONE);
#endif
+ kdb_register_repeat("disable_nmi", kdb_disable_nmi, "",
+ "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE);
kdb_register_repeat("defcmd", kdb_defcmd, "name \"usage\" \"help\"",
"Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE);
kdb_register_repeat("kill", kdb_kill, "<-signal> <pid>",
--
1.7.11.5

2012-08-20 14:51:38

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 09/12] ARM: Move some macros from entry-armv to entry-header

Just move the macros into header file as we would want to use them for
KGDB FIQ entry code.

The following macros were moved:

- svc_entry
- usr_entry
- kuser_cmpxchg_check
- vector_stub

To make kuser_cmpxchg_check actually work across different files, we
also have to make kuser_cmpxchg64_fixup global.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/kernel/entry-armv.S | 167 +---------------------------------------
arch/arm/kernel/entry-header.S | 170 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 171 insertions(+), 166 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 0f82098..0f15368 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -136,57 +136,6 @@ common_invalid:
b bad_mode
ENDPROC(__und_invalid)

-/*
- * SVC mode handlers
- */
-
-#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)
-#define SPFIX(code...) code
-#else
-#define SPFIX(code...)
-#endif
-
- .macro svc_entry, stack_hole=0
- UNWIND(.fnstart )
- UNWIND(.save {r0 - pc} )
- sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
-#ifdef CONFIG_THUMB2_KERNEL
- SPFIX( str r0, [sp] ) @ temporarily saved
- SPFIX( mov r0, sp )
- SPFIX( tst r0, #4 ) @ test original stack alignment
- SPFIX( ldr r0, [sp] ) @ restored
-#else
- SPFIX( tst sp, #4 )
-#endif
- SPFIX( subeq sp, sp, #4 )
- stmia sp, {r1 - r12}
-
- ldmia r0, {r3 - r5}
- add r7, sp, #S_SP - 4 @ here for interlock avoidance
- mov r6, #-1 @ "" "" "" ""
- add r2, sp, #(S_FRAME_SIZE + \stack_hole - 4)
- SPFIX( addeq r2, r2, #4 )
- str r3, [sp, #-4]! @ save the "real" r0 copied
- @ from the exception stack
-
- mov r3, lr
-
- @
- @ We are now ready to fill in the remaining blanks on the stack:
- @
- @ r2 - sp_svc
- @ r3 - lr_svc
- @ r4 - lr_<exception>, already fixed up for correct return/restart
- @ r5 - spsr_<exception>
- @ r6 - orig_r0 (see pt_regs definition in ptrace.h)
- @
- stmia r7, {r2 - r6}
-
-#ifdef CONFIG_TRACE_IRQFLAGS
- bl trace_hardirqs_off
-#endif
- .endm
-
.align 5
__dabt_svc:
svc_entry
@@ -348,71 +297,8 @@ ENDPROC(__pabt_svc)

/*
* User mode handlers
- *
- * EABI note: sp_svc is always 64-bit aligned here, so should S_FRAME_SIZE
*/

-#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5) && (S_FRAME_SIZE & 7)
-#error "sizeof(struct pt_regs) must be a multiple of 8"
-#endif
-
- .macro usr_entry
- UNWIND(.fnstart )
- UNWIND(.cantunwind ) @ don't unwind the user space
- sub sp, sp, #S_FRAME_SIZE
- ARM( stmib sp, {r1 - r12} )
- THUMB( stmia sp, {r0 - r12} )
-
- ldmia r0, {r3 - r5}
- add r0, sp, #S_PC @ here for interlock avoidance
- mov r6, #-1 @ "" "" "" ""
-
- str r3, [sp] @ save the "real" r0 copied
- @ from the exception stack
-
- @
- @ We are now ready to fill in the remaining blanks on the stack:
- @
- @ r4 - lr_<exception>, already fixed up for correct return/restart
- @ r5 - spsr_<exception>
- @ r6 - orig_r0 (see pt_regs definition in ptrace.h)
- @
- @ Also, separately save sp_usr and lr_usr
- @
- stmia r0, {r4 - r6}
- ARM( stmdb r0, {sp, lr}^ )
- THUMB( store_user_sp_lr r0, r1, S_SP - S_PC )
-
- @
- @ Enable the alignment trap while in kernel mode
- @
- alignment_trap r0
-
- @
- @ Clear FP to mark the first stack frame
- @
- zero_fp
-
-#ifdef CONFIG_IRQSOFF_TRACER
- bl trace_hardirqs_off
-#endif
- .endm
-
- .macro kuser_cmpxchg_check
-#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)
-#ifndef CONFIG_MMU
-#warning "NPTL on non MMU needs fixing"
-#else
- @ Make sure our user space atomic helper is restarted
- @ if it was interrupted in a critical region. Here we
- @ perform a quick test inline since it should be false
- @ 99.9999% of the time. The rest is done out of line.
- cmp r4, #TASK_SIZE
- blhs kuser_cmpxchg64_fixup
-#endif
-#endif
- .endm
-
.align 5
__dabt_usr:
usr_entry
@@ -846,6 +732,7 @@ __kuser_cmpxchg64: @ 0xffff0f60
ldmfd sp!, {r4, r5, r6, pc}

.text
+ .global kuser_cmpxchg64_fixup
kuser_cmpxchg64_fixup:
@ Called from kuser_cmpxchg_fixup.
@ r4 = address of interrupted insn (must be preserved).
@@ -976,58 +863,6 @@ __kuser_helper_end:

THUMB( .thumb )

-/*
- * Vector stubs.
- *
- * This code is copied to 0xffff0200 so we can use branches in the
- * vectors, rather than ldr's. Note that this code must not
- * exceed 0x300 bytes.
- *
- * Common stub entry macro:
- * Enter in IRQ mode, spsr = SVC/USR CPSR, lr = SVC/USR PC
- *
- * SP points to a minimal amount of processor-private memory, the address
- * of which is copied into r0 for the mode specific abort handler.
- */
- .macro vector_stub, name, mode, correction=0
- .align 5
-
-vector_\name:
- .if \correction
- sub lr, lr, #\correction
- .endif
-
- @
- @ Save r0, lr_<exception> (parent PC) and spsr_<exception>
- @ (parent CPSR)
- @
- stmia sp, {r0, lr} @ save r0, lr
- mrs lr, spsr
- str lr, [sp, #8] @ save spsr
-
- @
- @ Prepare for SVC32 mode. IRQs remain disabled.
- @
- mrs r0, cpsr
- eor r0, r0, #(\mode ^ SVC_MODE | PSR_ISETSTATE)
- msr spsr_cxsf, r0
-
- @
- @ the branch table must immediately follow this code
- @
- and lr, lr, #0x0f
- THUMB( adr r0, 1f )
- THUMB( ldr lr, [r0, lr, lsl #2] )
- mov r0, sp
- ARM( ldr lr, [pc, lr, lsl #2] )
- movs pc, lr @ branch to handler in SVC mode
-ENDPROC(vector_\name)
-
- .align 2
- @ handler addresses follow this label
-1:
- .endm
-
.globl __stubs_start
__stubs_start:
/*
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..c3c09ac 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -73,6 +73,109 @@
msr cpsr_c, \rtemp @ switch back to the SVC mode
.endm

+/*
+ * Vector stubs.
+ *
+ * This code is copied to 0xffff0200 so we can use branches in the
+ * vectors, rather than ldr's. Note that this code must not
+ * exceed 0x300 bytes.
+ *
+ * Common stub entry macro:
+ * Enter in IRQ mode, spsr = SVC/USR CPSR, lr = SVC/USR PC
+ *
+ * SP points to a minimal amount of processor-private memory, the address
+ * of which is copied into r0 for the mode specific abort handler.
+ */
+ .macro vector_stub, name, mode, correction=0
+ .align 5
+
+vector_\name:
+ .if \correction
+ sub lr, lr, #\correction
+ .endif
+
+ @
+ @ Save r0, lr_<exception> (parent PC) and spsr_<exception>
+ @ (parent CPSR)
+ @
+ stmia sp, {r0, lr} @ save r0, lr
+ mrs lr, spsr
+ str lr, [sp, #8] @ save spsr
+
+ @
+ @ Prepare for SVC32 mode. IRQs remain disabled.
+ @
+ mrs r0, cpsr
+ eor r0, r0, #(\mode ^ SVC_MODE | PSR_ISETSTATE)
+ msr spsr_cxsf, r0
+
+ @
+ @ the branch table must immediately follow this code
+ @
+ and lr, lr, #0x0f
+ THUMB( adr r0, 1f )
+ THUMB( ldr lr, [r0, lr, lsl #2] )
+ mov r0, sp
+ ARM( ldr lr, [pc, lr, lsl #2] )
+ movs pc, lr @ branch to handler in SVC mode
+ENDPROC(vector_\name)
+
+ .align 2
+ @ handler addresses follow this label
+1:
+ .endm
+
+/*
+ * SVC mode handlers
+ */
+
+#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)
+#define SPFIX(code...) code
+#else
+#define SPFIX(code...)
+#endif
+
+ .macro svc_entry, stack_hole=0
+ UNWIND(.fnstart )
+ UNWIND(.save {r0 - pc} )
+ sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+#ifdef CONFIG_THUMB2_KERNEL
+ SPFIX( str r0, [sp] ) @ temporarily saved
+ SPFIX( mov r0, sp )
+ SPFIX( tst r0, #4 ) @ test original stack alignment
+ SPFIX( ldr r0, [sp] ) @ restored
+#else
+ SPFIX( tst sp, #4 )
+#endif
+ SPFIX( subeq sp, sp, #4 )
+ stmia sp, {r1 - r12}
+
+ ldmia r0, {r3 - r5}
+ add r7, sp, #S_SP - 4 @ here for interlock avoidance
+ mov r6, #-1 @ "" "" "" ""
+ add r2, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+ SPFIX( addeq r2, r2, #4 )
+ str r3, [sp, #-4]! @ save the "real" r0 copied
+ @ from the exception stack
+
+ mov r3, lr
+
+ @
+ @ We are now ready to fill in the remaining blanks on the stack:
+ @
+ @ r2 - sp_svc
+ @ r3 - lr_svc
+ @ r4 - lr_<exception>, already fixed up for correct return/restart
+ @ r5 - spsr_<exception>
+ @ r6 - orig_r0 (see pt_regs definition in ptrace.h)
+ @
+ stmia r7, {r2 - r6}
+
+#ifdef CONFIG_TRACE_IRQFLAGS
+ bl trace_hardirqs_off
+#endif
+ .endm
+
#ifndef CONFIG_THUMB2_KERNEL
.macro svc_exit, rpsr
msr spsr_cxsf, \rpsr
@@ -164,6 +267,73 @@
#endif /* !CONFIG_THUMB2_KERNEL */

/*
+ * User mode handlers
+ *
+ * EABI note: sp_svc is always 64-bit aligned here, so should S_FRAME_SIZE
+ */
+
+#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5) && (S_FRAME_SIZE & 7)
+#error "sizeof(struct pt_regs) must be a multiple of 8"
+#endif
+
+ .macro usr_entry
+ UNWIND(.fnstart )
+ UNWIND(.cantunwind ) @ don't unwind the user space
+ sub sp, sp, #S_FRAME_SIZE
+ ARM( stmib sp, {r1 - r12} )
+ THUMB( stmia sp, {r0 - r12} )
+
+ ldmia r0, {r3 - r5}
+ add r0, sp, #S_PC @ here for interlock avoidance
+ mov r6, #-1 @ "" "" "" ""
+
+ str r3, [sp] @ save the "real" r0 copied
+ @ from the exception stack
+
+ @
+ @ We are now ready to fill in the remaining blanks on the stack:
+ @
+ @ r4 - lr_<exception>, already fixed up for correct return/restart
+ @ r5 - spsr_<exception>
+ @ r6 - orig_r0 (see pt_regs definition in ptrace.h)
+ @
+ @ Also, separately save sp_usr and lr_usr
+ @
+ stmia r0, {r4 - r6}
+ ARM( stmdb r0, {sp, lr}^ )
+ THUMB( store_user_sp_lr r0, r1, S_SP - S_PC )
+
+ @
+ @ Enable the alignment trap while in kernel mode
+ @
+ alignment_trap r0
+
+ @
+ @ Clear FP to mark the first stack frame
+ @
+ zero_fp
+
+#ifdef CONFIG_IRQSOFF_TRACER
+ bl trace_hardirqs_off
+#endif
+ .endm
+
+ .macro kuser_cmpxchg_check
+#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)
+#ifndef CONFIG_MMU
+#warning "NPTL on non MMU needs fixing"
+#else
+ @ Make sure our user space atomic helper is restarted
+ @ if it was interrupted in a critical region. Here we
+ @ perform a quick test inline since it should be false
+ @ 99.9999% of the time. The rest is done out of line.
+ cmp r4, #TASK_SIZE
+ blhs kuser_cmpxchg64_fixup
+#endif
+#endif
+ .endm
+
+/*
* These are the registers used in the syscall handler, and allow us to
* have in theory up to 7 arguments to a function - r0 to r6.
*
--
1.7.11.5

2012-08-20 14:51:34

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 10/12] ARM: Add KGDB/KDB FIQ debugger generic code

The FIQ debugger may be used to debug situations when the kernel stuck
in uninterruptable sections, e.g. the kernel infinitely loops or
deadlocked in an interrupt or with interrupts disabled.

By default KGDB FIQ is disabled in runtime, but can be enabled with
kgdb_fiq.enable=1 kernel command line option.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/Kconfig | 18 ++++++++
arch/arm/include/asm/kgdb.h | 8 ++++
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/kgdb_fiq.c | 99 ++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/kgdb_fiq_entry.S | 87 +++++++++++++++++++++++++++++++++++
5 files changed, 213 insertions(+)
create mode 100644 arch/arm/kernel/kgdb_fiq.c
create mode 100644 arch/arm/kernel/kgdb_fiq_entry.S

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6d6e18f..c978c74 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -171,6 +171,24 @@ config GENERIC_ISA_DMA
config FIQ
bool

+config ARCH_MIGHT_HAVE_KGDB_FIQ
+ bool
+
+config KGDB_FIQ
+ bool "KGDB/KDB FIQ debugger"
+ depends on KGDB_KDB && ARCH_MIGHT_HAVE_KGDB_FIQ && !THUMB2_KERNEL
+ select FIQ
+ help
+ The FIQ debugger may be used to debug situations when the
+ kernel stuck in uninterruptable sections, e.g. the kernel
+ infinitely loops or deadlocked in an interrupt or with
+ interrupts disabled.
+
+ By default KGDB FIQ is disabled in runtime, but can be
+ enabled with kgdb_fiq.enable=1 kernel command line option.
+
+ If unsure, say N.
+
config NEED_RET_TO_USER
bool

diff --git a/arch/arm/include/asm/kgdb.h b/arch/arm/include/asm/kgdb.h
index 48066ce..807e547 100644
--- a/arch/arm/include/asm/kgdb.h
+++ b/arch/arm/include/asm/kgdb.h
@@ -11,6 +11,8 @@
#define __ARM_KGDB_H__

#include <linux/ptrace.h>
+#include <linux/linkage.h>
+#include <asm/exception.h>

/*
* GDB assumes that we're a user process being debugged, so
@@ -47,6 +49,12 @@ static inline void arch_kgdb_breakpoint(void)
extern void kgdb_handle_bus_error(void);
extern int kgdb_fault_expected;

+extern char kgdb_fiq_handler;
+extern char kgdb_fiq_handler_end;
+asmlinkage void __exception_irq_entry kgdb_fiq_do_handle(struct pt_regs *regs);
+extern int __init kgdb_register_fiq(unsigned int mach_kgdb_fiq,
+ void (*mach_kgdb_enable_fiq)(unsigned int irq, bool on),
+ bool (*mach_is_kgdb_fiq)(unsigned int irq));
#endif /* !__ASSEMBLY__ */

/*
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 7ad2d5c..5aa079b 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_ATAGS_PROC) += atags.o
obj-$(CONFIG_OABI_COMPAT) += sys_oabi-compat.o
obj-$(CONFIG_ARM_THUMBEE) += thumbee.o
obj-$(CONFIG_KGDB) += kgdb.o
+obj-$(CONFIG_KGDB_FIQ) += kgdb_fiq_entry.o kgdb_fiq.o
obj-$(CONFIG_ARM_UNWIND) += unwind.o
obj-$(CONFIG_HAVE_TCM) += tcm.o
obj-$(CONFIG_OF) += devtree.o
diff --git a/arch/arm/kernel/kgdb_fiq.c b/arch/arm/kernel/kgdb_fiq.c
new file mode 100644
index 0000000..8443af1
--- /dev/null
+++ b/arch/arm/kernel/kgdb_fiq.c
@@ -0,0 +1,99 @@
+/*
+ * KGDB FIQ
+ *
+ * Copyright 2010 Google, Inc.
+ * Arve Hjønnevåg <[email protected]>
+ * Colin Cross <[email protected]>
+ * Copyright 2012 Linaro Ltd.
+ * Anton Vorontsov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/hardirq.h>
+#include <linux/kdb.h>
+#include <linux/kgdb.h>
+#include <asm/fiq.h>
+#include <asm/exception.h>
+
+static int kgdb_fiq_enabled;
+module_param_named(enable, kgdb_fiq_enabled, int, 0600);
+MODULE_PARM_DESC(enable, "set to 1 to enable FIQ KGDB");
+
+static unsigned int kgdb_fiq;
+static bool (*is_kgdb_fiq)(unsigned int irq);
+
+asmlinkage void __exception_irq_entry kgdb_fiq_do_handle(struct pt_regs *regs)
+{
+ if (!is_kgdb_fiq(kgdb_fiq))
+ return;
+ if (!kgdb_nmi_poll_knock())
+ return;
+
+ nmi_enter();
+ kgdb_handle_exception(1, 0, 0, regs);
+ nmi_exit();
+}
+
+static struct fiq_handler kgdb_fiq_desc = {
+ .name = "kgdb",
+};
+
+static long kgdb_fiq_setup_stack(void *info)
+{
+ struct pt_regs regs;
+
+ regs.ARM_sp = __get_free_pages(GFP_KERNEL, THREAD_SIZE_ORDER) +
+ THREAD_START_SP;
+ WARN_ON(!regs.ARM_sp);
+
+ set_fiq_regs(&regs);
+ return 0;
+}
+
+static void (*kgdb_enable_fiq)(unsigned int irq, bool on);
+
+void kgdb_arch_enable_nmi(bool on)
+{
+ if (!kgdb_enable_fiq)
+ return;
+ kgdb_enable_fiq(kgdb_fiq, on);
+}
+
+int __init kgdb_register_fiq(unsigned int mach_kgdb_fiq,
+ void (*mach_kgdb_enable_fiq)(unsigned int irq, bool on),
+ bool (*mach_is_kgdb_fiq)(unsigned int irq))
+{
+ int err;
+ int cpu;
+
+ if (!kgdb_fiq_enabled)
+ return -ENODEV;
+ if (kgdb_fiq)
+ return -EBUSY;
+
+ kgdb_fiq = mach_kgdb_fiq;
+ kgdb_enable_fiq = mach_kgdb_enable_fiq;
+ is_kgdb_fiq = mach_is_kgdb_fiq;
+
+ err = claim_fiq(&kgdb_fiq_desc);
+ if (err) {
+ pr_warn("%s: unable to claim fiq", __func__);
+ return err;
+ }
+
+ for_each_possible_cpu(cpu)
+ work_on_cpu(cpu, kgdb_fiq_setup_stack, NULL);
+
+ set_fiq_handler(&kgdb_fiq_handler,
+ &kgdb_fiq_handler_end - &kgdb_fiq_handler);
+
+ return 0;
+}
diff --git a/arch/arm/kernel/kgdb_fiq_entry.S b/arch/arm/kernel/kgdb_fiq_entry.S
new file mode 100644
index 0000000..d6becca
--- /dev/null
+++ b/arch/arm/kernel/kgdb_fiq_entry.S
@@ -0,0 +1,87 @@
+/*
+ * KGDB FIQ entry
+ *
+ * Copyright 1996,1997,1998 Russell King.
+ * Copyright 2012 Linaro Ltd.
+ * Anton Vorontsov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/memory.h>
+#include <asm/unwind.h>
+#include "entry-header.S"
+
+ .text
+
+@ This is needed for usr_entry/alignment_trap
+.LCcralign:
+ .long cr_alignment
+.LCdohandle:
+ .long kgdb_fiq_do_handle
+
+ .macro fiq_handler
+ ldr r1, =.LCdohandle
+ mov r0, sp
+ adr lr, BSYM(9997f)
+ ldr pc, [r1]
+9997:
+ .endm
+
+ .align 5
+__fiq_svc:
+ svc_entry
+ fiq_handler
+ mov r0, sp
+ ldmib r0, {r1 - r14}
+ msr cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
+ add r8, r0, #S_PC
+ ldr r9, [r0, #S_PSR]
+ msr spsr_cxsf, r9
+ ldr r0, [r0, #S_R0]
+ ldmia r8, {pc}^
+
+ UNWIND(.fnend )
+ENDPROC(__fiq_svc)
+ .ltorg
+
+ .align 5
+__fiq_usr:
+ usr_entry
+ kuser_cmpxchg_check
+ fiq_handler
+ get_thread_info tsk
+ mov why, #0
+ b ret_to_user_from_irq
+ UNWIND(.fnend )
+ENDPROC(__fiq_usr)
+ .ltorg
+
+ .global kgdb_fiq_handler
+kgdb_fiq_handler:
+
+ vector_stub fiq, FIQ_MODE, 4
+
+ .long __fiq_usr @ 0 (USR_26 / USR_32)
+ .long __fiq_svc @ 1 (FIQ_26 / FIQ_32)
+ .long __fiq_svc @ 2 (IRQ_26 / IRQ_32)
+ .long __fiq_svc @ 3 (SVC_26 / SVC_32)
+ .long __fiq_svc @ 4
+ .long __fiq_svc @ 5
+ .long __fiq_svc @ 6
+ .long __fiq_svc @ 7
+ .long __fiq_svc @ 8
+ .long __fiq_svc @ 9
+ .long __fiq_svc @ a
+ .long __fiq_svc @ b
+ .long __fiq_svc @ c
+ .long __fiq_svc @ d
+ .long __fiq_svc @ e
+ .long __fiq_svc @ f
+
+ .global kgdb_fiq_handler_end
+kgdb_fiq_handler_end:
--
1.7.11.5

2012-08-20 14:53:04

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 08/12] tty/serial: Add kgdb_nmi driver

This special driver makes it possible to temporary use NMI debugger port
as a normal console by issuing 'nmi_console' command (assuming that the
port is attached to KGDB).

Unlike KDB's disable_nmi command, with this driver you are always able
to go back to the debugger using KGDB escape sequence ($3#33). This is
because this console driver processes the input in NMI context, and thus
is able to intercept the magic sequence.

Note that since the console interprets input and uses polling
communication methods, for things like PPP it is still better to fully
detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
console.

Usually, to enter the debugger one have to type the magic sequence, so
initially the kernel will print the following prompt on the NMI debugger
console:

Type $3#33 to enter the debugger>

For convenience, there is a kgdb_fiq.knock kernel command line option,
when set to 0, this turns the special command to just a return key
press, so the kernel will be printing this:

Hit <return> to enter the debugger>

This is more convenient for long debugging sessions, although it makes
nmi_console feature somewhat useless.

And for the cases when NMI connected to a dedicated button, the knocking
can be disabled altogether by setting kgdb_fiq.knock to -1.

Suggested-by: Colin Cross <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/tty/serial/Kconfig | 19 +++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/kgdb_nmi.c | 348 ++++++++++++++++++++++++++++++++++++++++++
drivers/tty/serial/kgdboc.c | 6 +
include/linux/kgdb.h | 10 ++
5 files changed, 384 insertions(+)
create mode 100644 drivers/tty/serial/kgdb_nmi.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4720b4b..32d6d3f 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT

Say Y if you have an external 8250/16C550 UART. If unsure, say N.

+config SERIAL_KGDB_NMI
+ bool "Serial console over KGDB NMI debugger port"
+ depends on KGDB_SERIAL_CONSOLE
+ help
+ This special driver allows you to temporary use NMI debugger port
+ as a normal console (assuming that the port is attached to KGDB).
+
+ Unlike KDB's disable_nmi command, with this driver you are always
+ able to go back to the debugger using KGDB escape sequence ($3#33).
+ This is because this console driver processes the input in NMI
+ context, and thus is able to intercept the magic sequence.
+
+ Note that since the console interprets input and uses polling
+ communication methods, for things like PPP you still must fully
+ detach debugger port from the KGDB NMI (i.e. disable_nmi), and
+ use raw console.
+
+ If unsure, say N.
+
config SERIAL_KS8695
bool "Micrel KS8695 (Centaur) serial port support"
depends on ARCH_KS8695
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 7257c5d..7a61d52 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
+obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
new file mode 100644
index 0000000..e1cd1be
--- /dev/null
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -0,0 +1,348 @@
+/*
+ * KGDB NMI serial console
+ *
+ * Copyright 2010 Google, Inc.
+ * Arve Hjønnevåg <[email protected]>
+ * Colin Cross <[email protected]>
+ * Copyright 2012 Linaro Ltd.
+ * Anton Vorontsov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/atomic.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include <linux/tty_flip.h>
+#include <linux/interrupt.h>
+#include <linux/hrtimer.h>
+#include <linux/tick.h>
+#include <linux/kfifo.h>
+#include <linux/kgdb.h>
+#include <linux/kdb.h>
+
+static int kgdb_nmi_knock = 1;
+module_param_named(knock, kgdb_nmi_knock, int, 0600);
+MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command "
+ "must be used to enter the debugger; when set to 0, "
+ "hitting return key is enough to enter the debugger; "
+ "when set to -1, the debugger is entered immediately "
+ "upon NMI");
+
+static bool kgdb_nmi_tty_enabled;
+
+static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
+{
+ int i;
+
+ if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
+ return;
+
+ for (i = 0; i < c; i++)
+ dbg_io_ops->write_char(s[i]);
+}
+
+static struct tty_driver *kgdb_nmi_tty_driver;
+
+static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
+{
+ *idx = co->index;
+ return kgdb_nmi_tty_driver;
+}
+
+static struct console kgdb_nmi_console = {
+ .name = "ttyNMI",
+ .write = kgdb_nmi_console_write,
+ .device = kgdb_nmi_console_device,
+ .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
+ .index = -1,
+};
+
+/*
+ * This is usually the maximum rate on debug ports. We make fifo large enough
+ * to make copy-pasting to the terminal usable.
+ */
+#define KGDB_NMI_BAUD 115200
+#define KGDB_NMI_FIFO_SIZE roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
+
+struct kgdb_nmi_tty_priv {
+ int opened;
+ struct tty_struct *tty;
+ struct tasklet_struct tlet;
+ STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
+};
+
+/*
+ * Our debugging console is polled in a tasklet, so we'll check for input
+ * every tick. In HZ-less mode, we should program the next tick. We have
+ * to use the lowlevel stuff as no locks should be grabbed.
+ */
+#ifdef CONFIG_HIGH_RES_TIMERS
+static void kgdb_tty_poke(void)
+{
+ tick_program_event(ktime_get(), 0);
+}
+#else
+static inline void kgdb_tty_poke(void) {}
+#endif
+
+static struct tty_struct *kgdb_nmi_tty;
+
+static void kgdb_tty_recv(int ch)
+{
+ struct kgdb_nmi_tty_priv *priv;
+ char c = ch;
+
+ if (!kgdb_nmi_tty || ch < 0)
+ return;
+
+ priv = kgdb_nmi_tty->driver_data;
+ kfifo_in(&priv->fifo, &c, 1);
+ kgdb_tty_poke();
+}
+
+static int kgdb_nmi_poll_one_knock(void)
+{
+ static int n;
+ int c = -1;
+ char magic[] = "$3#33";
+ size_t m = strlen(magic);
+ bool printch = 0;
+
+ c = dbg_io_ops->read_char();
+ if (c == NO_POLL_CHAR)
+ return c;
+
+ if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
+ return 1;
+ } else if (c == magic[n]) {
+ n = (n + 1) % m;
+ if (!n)
+ return 1;
+ printch = 1;
+ } else {
+ n = 0;
+ }
+
+ if (kgdb_nmi_tty_enabled) {
+ kgdb_tty_recv(c);
+ return 0;
+ }
+
+ if (printch) {
+ kdb_printf("%c", c);
+ return 0;
+ }
+
+ kdb_printf("\r%s %s to enter the debugger> %*s",
+ kgdb_nmi_knock ? "Type" : "Hit",
+ kgdb_nmi_knock ? magic : "<return>", m, "");
+ memset(magic, '\b', m);
+ kdb_printf("%s", magic);
+ return 0;
+}
+
+/**
+ * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
+ *
+ * "Serial ports are often noisy, especially when muxed over another port (we
+ * often use serial over the headset connector). Noise on the async command
+ * line just causes characters that are ignored, on a command line that blocked
+ * execution noise would be catastrophic." -- Colin Cross
+ *
+ * So, this function implements KGDB/KDB knocking on the serial line: we won't
+ * enter the debugger until we receive a known magic phrase (which is actually
+ * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
+ * of knocking, i.e. just pressing the return key is enough to enter the
+ * debugger. And if knocking is disabled, the function always returns 1.
+ */
+bool kgdb_nmi_poll_knock(void)
+{
+ if (kgdb_nmi_knock < 0)
+ return 1;
+
+ dbg_io_ops->clear_irqs();
+
+ while (1) {
+ int ret;
+
+ ret = kgdb_nmi_poll_one_knock();
+ if (ret == NO_POLL_CHAR)
+ return 0;
+ else if (ret == 1)
+ break;
+ }
+ return 1;
+}
+
+static void kgdb_nmi_tty_receiver(unsigned long data)
+{
+ struct kgdb_nmi_tty_priv *priv = (void *)data;
+ char ch;
+
+ tasklet_schedule(&priv->tlet);
+
+ if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
+ return;
+
+ while (kfifo_out(&priv->fifo, &ch, 1))
+ tty_insert_flip_char(priv->tty, ch, TTY_NORMAL);
+ tty_flip_buffer_push(priv->tty);
+}
+
+static DEFINE_MUTEX(kgdb_nmi_tty_mutex);
+
+static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
+{
+ struct kgdb_nmi_tty_priv *priv;
+ int ret = 0;
+
+ mutex_lock(&kgdb_nmi_tty_mutex);
+
+ priv = tty->driver_data;
+ if (priv)
+ goto out;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ kgdb_nmi_tty = tty;
+ priv->tty = tty;
+ INIT_KFIFO(priv->fifo);
+ tty->driver_data = priv;
+
+ /*
+ * The tasklet is cheap, it does not cause wakeups when reschedules
+ * itself, instead it waits for the next tick.
+ */
+ tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
+ tasklet_schedule(&priv->tlet);
+out:
+ priv->opened++;
+err:
+ mutex_unlock(&kgdb_nmi_tty_mutex);
+ return ret;
+}
+
+static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
+{
+ struct kgdb_nmi_tty_priv *priv;
+
+ mutex_lock(&kgdb_nmi_tty_mutex);
+
+ priv = tty->driver_data;
+ if (--priv->opened)
+ goto out;
+
+ tasklet_kill(&priv->tlet);
+ kgdb_nmi_tty = NULL;
+ kfree(priv);
+out:
+ mutex_unlock(&kgdb_nmi_tty_mutex);
+}
+
+static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
+{
+ /* Actually, we can handle any amount as we use polled writes. */
+ return 2048;
+}
+
+static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
+{
+ int i;
+
+ for (i = 0; i < c; i++)
+ dbg_io_ops->write_char(buf[i]);
+ return c;
+}
+
+static const struct tty_operations kgdb_nmi_tty_ops = {
+ .open = kgdb_nmi_tty_open,
+ .close = kgdb_nmi_tty_close,
+ .write_room = kgdb_nmi_tty_write_room,
+ .write = kgdb_nmi_tty_write,
+};
+
+static int kgdb_nmi_enable_console(int argc, const char *argv[])
+{
+ kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
+ return 0;
+}
+
+int kgdb_register_nmi_console(void)
+{
+ int ret;
+
+ kgdb_nmi_tty_driver = alloc_tty_driver(1);
+ if (!kgdb_nmi_tty_driver) {
+ pr_err("%s: cannot allocate tty\n", __func__);
+ return -ENOMEM;
+ }
+ kgdb_nmi_tty_driver->driver_name = "ttyNMI";
+ kgdb_nmi_tty_driver->name = "ttyNMI";
+ kgdb_nmi_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
+ kgdb_nmi_tty_driver->subtype = SERIAL_TYPE_NORMAL;
+ kgdb_nmi_tty_driver->flags = TTY_DRIVER_REAL_RAW |
+ TTY_DRIVER_DYNAMIC_DEV;
+ kgdb_nmi_tty_driver->init_termios = tty_std_termios;
+ tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
+ KGDB_NMI_BAUD, KGDB_NMI_BAUD);
+ tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
+
+ ret = tty_register_driver(kgdb_nmi_tty_driver);
+ if (ret) {
+ pr_err("%s: unable to register tty: %d\n", __func__, ret);
+ goto err_tty_reg;
+ }
+
+ ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
+ "switch to Linux NMI console", 0);
+ if (ret) {
+ pr_err("%s: unable to register kdb command: %d\n",
+ __func__, ret);
+ goto err_kdb_reg;
+ }
+
+ register_console(&kgdb_nmi_console);
+ kgdb_enable_nmi(1);
+
+ return 0;
+err_kdb_reg:
+ tty_unregister_driver(kgdb_nmi_tty_driver);
+err_tty_reg:
+ put_tty_driver(kgdb_nmi_tty_driver);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
+
+int kgdb_unregister_nmi_console(void)
+{
+ int ret;
+
+ kgdb_enable_nmi(0);
+
+ ret = tty_unregister_driver(kgdb_nmi_tty_driver);
+ if (ret)
+ return ret;
+ put_tty_driver(kgdb_nmi_tty_driver);
+
+ ret = unregister_console(&kgdb_nmi_console);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 0aa08c8..bb69803 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);

static void cleanup_kgdboc(void)
{
+ if (kgdb_unregister_nmi_console())
+ return;
kgdboc_unregister_kbd();
if (configured == 1)
kgdb_unregister_io_module(&kgdboc_io_ops);
@@ -198,6 +200,10 @@ do_register:
if (err)
goto noconfig;

+ err = kgdb_register_nmi_console();
+ if (err)
+ goto noconfig;
+
configured = 1;

return 0;
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 1fd1cf0..fb7c55c 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -245,6 +245,16 @@ extern void kgdb_arch_enable_nmi(bool on);
*/
extern int kgdb_enable_nmi(bool on);

+#ifdef CONFIG_SERIAL_KGDB_NMI
+extern int kgdb_register_nmi_console(void);
+extern int kgdb_unregister_nmi_console(void);
+extern bool kgdb_nmi_poll_knock(void);
+#else
+static inline int kgdb_register_nmi_console(void) { return 0; }
+static inline int kgdb_unregister_nmi_console(void) { return 0; }
+static inline bool kgdb_nmi_poll_knock(void) { return 1; }
+#endif
+
/**
* struct kgdb_arch - Describe architecture specific values.
* @gdb_bpt_instr: The instruction to trigger a breakpoint.
--
1.7.11.5

2012-08-20 14:53:09

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 07/12] tty/serial/amba-pl011: Implement clear_irqs callback

It's all pretty straightforward, except for TXIM interrupt. The interrupt
has meaning "ready to transmit", so it's almost always raised, and the
only way to silence it is to mask it. But that's OK, ops->start_tx will
unmask it.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/tty/serial/amba-pl011.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 90f2302..cf9278c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1308,6 +1308,18 @@ static void pl010_put_poll_char(struct uart_port *port,
writew(ch, uap->port.membase + UART01x_DR);
}

+static void pl010_clear_irqs(struct uart_port *port)
+{
+ struct uart_amba_port *uap = (struct uart_amba_port *)port;
+ unsigned char __iomem *regs = uap->port.membase;
+
+ writew(readw(regs + UART011_MIS), regs + UART011_ICR);
+ /*
+ * There is no way to clear TXIM, this is "ready to transmit IRQ", so
+ * we simply mask it. ops->start_tx will unmask it.
+ */
+ writew(readw(regs + UART011_IMSC) & ~UART011_TXIM, regs + UART011_IMSC);
+}
#endif /* CONFIG_CONSOLE_POLL */

static int pl011_hwinit(struct uart_port *port)
@@ -1721,6 +1733,7 @@ static struct uart_ops amba_pl011_pops = {
.poll_init = pl011_hwinit,
.poll_get_char = pl010_get_poll_char,
.poll_put_char = pl010_put_poll_char,
+ .clear_irqs = pl010_clear_irqs,
#endif
};

--
1.7.11.5

2012-08-20 14:53:42

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 06/12] tty/serial/kgdboc: Add and wire up clear_irqs callback

This patch implements a new callback: clear_irqs. It is used for the
cases when KDB-entry (e.g. NMI) and KDB IO (e.g. serial port) shares
the same interrupt. To get the idea, let's take some real example (ARM
machine): we have a serial port which interrupt is routed to an NMI,
and the interrupt is used to enter KDB. Once there is some activity on
the serial port, the CPU receives NMI exception, and we fall into KDB
shell. So, it is our "debug console", and it is able to interrupt (and
thus debug) even IRQ handlers themselves.

When used that way, the interrupt never reaches serial driver's IRQ
handler routine, which means that serial driver will not silence the
interrupt. NMIs behaviour are quite arch-specific, and we can't assume
that we can use them as ordinary IRQs, e.g. on some arches (like ARM)
we can't handle data aborts, the behaviour is undefined then. So we
can't just handle execution to serial driver's IRQ handler from the
NMI context once we're done with KDB (plus this would defeat the
debugger's purpose: we want the NMI handler be as simple as possible,
so it will have less chances to hang).

So, given that have to deal with it somehow, we have two options:

1. Implement something that clears the interrupt;
2. Implement a whole new concept of grabbing tty for exclusive KDB use,
plus implement mask/unmask callbacks, i.e.:
- Since consoles might use ttys w/o opending them, we would have
to make kdb respect CON_ENABLED flag (maybe a good idea to do it
anyway);
- Add 'bool exclusive' argument to tty_find_polling_driver(), if set
to 1, the function will refuse to return an already tty; and will
use the flag in tty_reopen() to not allow multiple users (there are
already checks for pty masters, which are "open once" ttys);
- Once we got the tty exclusively, we would need to call some new
uart->mask_all_but_rx_interrupts call before we want to use the
port for NMI/KDB, and unmask_all_but_rx_interrupts after we're
done with it.

The second option is obviously more complex, needlessly so, and less
generic. So I went with the first one: we just consume all the interrupts.
The tty becomes silently unusable for the rest of the world when we use
it with KDB; but once we reroute the serial IRQ source back from NMI to
an ordinary IRQ (in KDB this can be done with 'disable_nmi' command), it
will behave as normal.

p.s. Since the callback is so far used only by polling user, we place
it under the appropriate #ifdef.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/tty/serial/kgdboc.c | 10 ++++++++++
drivers/tty/serial/serial_core.c | 15 +++++++++++++++
include/linux/kgdb.h | 1 +
include/linux/serial_core.h | 1 +
include/linux/tty_driver.h | 1 +
5 files changed, 28 insertions(+)

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 2b42a01..0aa08c8 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -227,6 +227,15 @@ static int kgdboc_get_char(void)
kgdb_tty_line);
}

+static void kgdboc_clear_irqs(void)
+{
+ if (!kgdb_tty_driver)
+ return;
+ if (kgdb_tty_driver->ops->clear_irqs)
+ kgdb_tty_driver->ops->clear_irqs(kgdb_tty_driver,
+ kgdb_tty_line);
+}
+
static void kgdboc_put_char(u8 chr)
{
if (!kgdb_tty_driver)
@@ -298,6 +307,7 @@ static struct kgdb_io kgdboc_io_ops = {
.name = "kgdboc",
.read_char = kgdboc_get_char,
.write_char = kgdboc_put_char,
+ .clear_irqs = kgdboc_clear_irqs,
.pre_exception = kgdboc_pre_exp_handler,
.post_exception = kgdboc_post_exp_handler,
};
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index cba8443..908d108 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2169,6 +2169,20 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
port = state->uart_port;
port->ops->poll_put_char(port, ch);
}
+
+static void uart_clear_irqs(struct tty_driver *driver, int line)
+{
+ struct uart_driver *drv = driver->driver_state;
+ struct uart_state *state = drv->state + line;
+ struct uart_port *port;
+
+ if (!state || !state->uart_port)
+ return;
+
+ port = state->uart_port;
+ if (port->ops->clear_irqs)
+ port->ops->clear_irqs(port);
+}
#endif

static const struct tty_operations uart_ops = {
@@ -2201,6 +2215,7 @@ static const struct tty_operations uart_ops = {
.poll_init = uart_poll_init,
.poll_get_char = uart_poll_get_char,
.poll_put_char = uart_poll_put_char,
+ .clear_irqs = uart_clear_irqs,
#endif
};

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 3b111a6..1fd1cf0 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -295,6 +295,7 @@ struct kgdb_io {
const char *name;
int (*read_char) (void);
void (*write_char) (u8);
+ void (*clear_irqs) (void);
void (*flush) (void);
int (*init) (void);
void (*pre_exception) (void);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 3642710..114b3f3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -277,6 +277,7 @@ struct uart_ops {
int (*poll_init)(struct uart_port *);
void (*poll_put_char)(struct uart_port *, unsigned char);
int (*poll_get_char)(struct uart_port *);
+ void (*clear_irqs)(struct uart_port *);
#endif
};

diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 6e6dbb7..94b14cd 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -287,6 +287,7 @@ struct tty_operations {
int (*poll_init)(struct tty_driver *driver, int line, char *options);
int (*poll_get_char)(struct tty_driver *driver, int line);
void (*poll_put_char)(struct tty_driver *driver, int line, char ch);
+ void (*clear_irqs)(struct tty_driver *driver, int line);
#endif
const struct file_operations *proc_fops;
};
--
1.7.11.5

2012-08-20 14:53:58

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 05/12] tty/serial/amba-pl011: Implement poll_init callback

The callback is used to initialize the hardware, nothing else should be
done, i.e. we should not request interrupts (but we can and do unmask
some of them, as they might be useful for NMI entry).

As a side-effect, the patch also fixes a division by zero[1] when booting
with kgdboc options specified (e.g. kgdboc=ttyAMA0,115200n8). The issue
happens because serial core calls set_termios callback, but the driver
doesn't know clock frequency, and thus cannot calculate proper baud rate
values.

[1]
WARNING: at drivers/tty/serial/serial_core.c:400 uart_get_baud_rate+0xe8/0x14c()
Modules linked in:
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c0020ae8>] (warn_slowpath_common+0x4c/0x64)
[<c0020ae8>] (warn_slowpath_common+0x4c/0x64) from [<c0020b1c>] (warn_slowpath_null+0x1c/0x24)
[<c0020b1c>] (warn_slowpath_null+0x1c/0x24) from [<c0185ed8>] (uart_get_baud_rate+0xe8/0x14c)
[<c0185ed8>] (uart_get_baud_rate+0xe8/0x14c) from [<c0187078>] (pl011_set_termios+0x48/0x278)
[<c0187078>] (pl011_set_termios+0x48/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)
---[ end trace 7d41c9186f342c40 ]---
Division by zero in kernel.
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c014546c>] (Ldiv0+0x8/0x10)
[<c014546c>] (Ldiv0+0x8/0x10) from [<c0187098>] (pl011_set_termios+0x68/0x278)
[<c0187098>] (pl011_set_termios+0x68/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)
Division by zero in kernel.
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c014546c>] (Ldiv0+0x8/0x10)
[<c014546c>] (Ldiv0+0x8/0x10) from [<c0183a98>] (uart_update_timeout+0x4c/0x5c)
[<c0183a98>] (uart_update_timeout+0x4c/0x5c) from [<c01870f8>] (pl011_set_termios+0xc8/0x278)
[<c01870f8>] (pl011_set_termios+0xc8/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/tty/serial/amba-pl011.c | 47 ++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d3553b5..90f2302 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1310,10 +1310,9 @@ static void pl010_put_poll_char(struct uart_port *port,

#endif /* CONFIG_CONSOLE_POLL */

-static int pl011_startup(struct uart_port *port)
+static int pl011_hwinit(struct uart_port *port)
{
struct uart_amba_port *uap = (struct uart_amba_port *)port;
- unsigned int cr;
int retval;

/* Optionaly enable pins to be muxed in and configured */
@@ -1342,6 +1341,39 @@ static int pl011_startup(struct uart_port *port)
UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);

/*
+ * Save interrupts enable mask, and enable RX interrupts in case if
+ * the interrupt is used for NMI entry.
+ */
+ uap->im = readw(uap->port.membase + UART011_IMSC);
+ writew(UART011_RTIM | UART011_RXIM, uap->port.membase + UART011_IMSC);
+
+ if (uap->port.dev->platform_data) {
+ struct amba_pl011_data *plat;
+
+ plat = uap->port.dev->platform_data;
+ if (plat->init)
+ plat->init();
+ }
+ return 0;
+clk_unprep:
+ clk_unprepare(uap->clk);
+out:
+ return retval;
+}
+
+static int pl011_startup(struct uart_port *port)
+{
+ struct uart_amba_port *uap = (struct uart_amba_port *)port;
+ unsigned int cr;
+ int retval;
+
+ retval = pl011_hwinit(port);
+ if (retval)
+ goto clk_dis;
+
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+
+ /*
* Allocate the IRQ
*/
retval = request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
@@ -1400,21 +1432,11 @@ static int pl011_startup(struct uart_port *port)
writew(uap->im, uap->port.membase + UART011_IMSC);
spin_unlock_irq(&uap->port.lock);

- if (uap->port.dev->platform_data) {
- struct amba_pl011_data *plat;
-
- plat = uap->port.dev->platform_data;
- if (plat->init)
- plat->init();
- }
-
return 0;

clk_dis:
clk_disable(uap->clk);
- clk_unprep:
clk_unprepare(uap->clk);
- out:
return retval;
}

@@ -1696,6 +1718,7 @@ static struct uart_ops amba_pl011_pops = {
.config_port = pl010_config_port,
.verify_port = pl010_verify_port,
#ifdef CONFIG_CONSOLE_POLL
+ .poll_init = pl011_hwinit,
.poll_get_char = pl010_get_poll_char,
.poll_put_char = pl010_put_poll_char,
#endif
--
1.7.11.5

2012-08-20 20:51:38

by Brian Swetland

[permalink] [raw]
Subject: Re: [PATCH v4 0/12] KGDB/KDB FIQ (NMI) debugger

On Mon, Aug 20, 2012 at 7:44 AM, Anton Vorontsov
<[email protected]> wrote:
>
> - KGDB/KDB FIQ debugger shell is synchronous. In Google's version you
> could have a dedicated shell always running in the FIQ context, so
> when you type something on a serial line, you won't actually cause any
> debugging actions, FIQ would save the characters in its own buffer and
> continue execution normally. But when you hit return key after the
> command, then the command is executed.
>
> In KGDB/KDB FIQ debugger it is different. Once you enter KGDB, the
> kernel will stop until you instruct it to continue.
>
> This might look as a drastic change, but it is not. There is actually
> no difference whether you have sync or async shell, or at least I
> couldn't find any use-case where this would matter at all. Anyways, it
> is still possible to do async shell in KDB, just don't see any need
> for this.

The main reason we did this asynchronously was that it's entirely possible
to get the occasional random character on the debug serial port (which is
often multiplexed with the audio path on the headphone jack), and having
the device freeze mysteriously when this happens is problematic.

Since the FIQ debugger is incredibly useful for diagnosing "my device is
stuck" type problems, we tend to leave it enabled on large numbers of
devices during internal testing, so that if somebody runs into a problem
an engineer can plug in a serial debug cable and take a look. It's
important that the presence of the debug feature doesn't lead to instability,
and thus we don't want a single random character to stop the normal
operation ofthe device.

Brian

2012-08-20 21:27:31

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH v4 0/12] KGDB/KDB FIQ (NMI) debugger

Hi Brian,

On Mon, Aug 20, 2012 at 01:51:33PM -0700, Brian Swetland wrote:
> > - KGDB/KDB FIQ debugger shell is synchronous. In Google's version you
> > could have a dedicated shell always running in the FIQ context,
[...]
> The main reason we did this asynchronously was that it's entirely possible
> to get the occasional random character on the debug serial port (which is
> often multiplexed with the audio path on the headphone jack), and having
> the device freeze mysteriously when this happens is problematic.
>
> Since the FIQ debugger is incredibly useful for diagnosing "my device is
> stuck" type problems, we tend to leave it enabled on large numbers of
> devices during internal testing, so that if somebody runs into a problem
> an engineer can plug in a serial debug cable and take a look. It's
> important that the presence of the debug feature doesn't lead to instability,
> and thus we don't want a single random character to stop the normal
> operation ofthe device.

Yup, and that's why in my approach I implemented a tiny async "shell" on
to of KDB, the shell accepts just one command "$3#33" -- GDB-protocol
escape sequence:

/**
* kgdb_nmi_poll_knock - Check if it is time to enter the debugger
*
* "Serial ports are often noisy, especially when muxed over another port (we
* often use serial over the headset connector). Noise on the async command
* line just causes characters that are ignored, on a command line that blocked
* execution noise would be catastrophic." -- Colin Cross
*
* So, this function implements KGDB/KDB knocking on the serial line: we won't
* enter the debugger until we receive a known magic phrase (which is actually
* "$3#33", known as "escape to KDB" command.
...

I.e. the kernel will print this prompt on the NMI debugger console:

Type $3#33 to enter the debugger>

And this command will be processed asynchronously.

Thanks!

Anton.