From: Anton Vorontsov <hidden> Date: 2012-07-30 11:59:31
Hi all,
I do realize that we're in the middle of the merge window. But maybe
some of you will be bored enough to look into this; and no problem if
you don't feel like it -- I promise to send a brand new shiny v4 after
the merge window, so you won't miss a bit of this new cool stuff. :-)
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; and for
convenience it is now available in the following repo:
git://git.infradead.org/users/cbou/linux-nmi-kdb.git master
Rationale for this patch set:
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 | 169 +------------------------
arch/arm/kernel/entry-header.S | 176 ++++++++++++++++++++++++++-
arch/arm/kernel/kgdb_fiq.c | 159 ++++++++++++++++++++++++
arch/arm/kernel/kgdb_fiq_entry.S | 76 ++++++++++++
arch/arm/mach-versatile/Makefile | 1 +
arch/arm/mach-versatile/include/mach/irqs.h | 1 +
arch/arm/mach-versatile/kgdb_fiq.c | 31 +++++
drivers/tty/serial/amba-pl011.c | 13 ++
drivers/tty/serial/kgdboc.c | 9 ++
drivers/tty/serial/serial_core.c | 15 +++
include/linux/kgdb.h | 14 +++
include/linux/serial_core.h | 1 +
include/linux/tty_driver.h | 1 +
kernel/debug/debug_core.c | 13 +-
kernel/debug/kdb/kdb_debugger.c | 4 +
kernel/debug/kdb/kdb_main.c | 20 +++
21 files changed, 591 insertions(+), 170 deletions(-)
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
--
Anton Vorontsov
Email: cbouatmailru at gmail.com
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:01:42
Currently kernel never set KGDB_REASON_NMI. We do now, when we enter
KGDB/KDB from an NMI.
This is not to be confused with kgdb_nmicallback(), NMI callback is
an entry for the slave CPUs during CPUs roundup, but REASON_NMI is the
entry for the master CPU.
Signed-off-by: Anton Vorontsov <redacted>
---
kernel/debug/kdb/kdb_debugger.c | 4 ++++
1 file changed, 4 insertions(+)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:01:47
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 <redacted>
---
include/linux/kgdb.h | 13 +++++++++++++
kernel/debug/debug_core.c | 13 ++++++++++++-
2 files changed, 25 insertions(+), 1 deletion(-)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:01:51
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 <redacted>
---
drivers/tty/serial/kgdboc.c | 9 +++++++++
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, 27 insertions(+)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:01:55
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 <redacted>
---
drivers/tty/serial/amba-pl011.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:01:57
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 <redacted>
---
arch/arm/kernel/entry-armv.S | 167 +--------------------------------------
arch/arm/kernel/entry-header.S | 170 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 171 insertions(+), 166 deletions(-)
@@ -136,57 +136,6 @@ common_invalid:bbad_modeENDPROC(__und_invalid)-/*-*SVCmodehandlers-*/--#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)-#define SPFIX(code...) code-#else-#define SPFIX(code...)-#endif--.macrosvc_entry,stack_hole=0-UNWIND(.fnstart)-UNWIND(.save{r0-pc})-subsp,sp,#(S_FRAME_SIZE + \stack_hole - 4)-#ifdef CONFIG_THUMB2_KERNEL-SPFIX(strr0,[sp])@temporarilysaved-SPFIX(movr0,sp)-SPFIX(tstr0,#4 ) @ test original stack alignment-SPFIX(ldrr0,[sp])@restored-#else-SPFIX(tstsp,#4 )-#endif-SPFIX(subeqsp,sp,#4 )-stmiasp,{r1-r12}--ldmiar0,{r3-r5}-addr7,sp,#S_SP - 4 @ here for interlock avoidance-movr6,#-1 @ "" "" "" ""-addr2,sp,#(S_FRAME_SIZE + \stack_hole - 4)-SPFIX(addeqr2,r2,#4 )-strr3,[sp,#-4]! @ save the "real" r0 copied-@fromtheexceptionstack--movr3,lr--@-@Wearenowreadytofillintheremainingblanksonthestack:-@-@r2-sp_svc-@r3-lr_svc-@r4-lr_<exception>,alreadyfixedupforcorrectreturn/restart-@r5-spsr_<exception>-@r6-orig_r0 (seept_regsdefinitioninptrace.h)-@-stmiar7,{r2-r6}--#ifdef CONFIG_TRACE_IRQFLAGS-bltrace_hardirqs_off-#endif-.endm-.align5 __dabt_svc:svc_entry
@@ -328,71 +277,8 @@ ENDPROC(__pabt_svc)/**Usermodehandlers-*-*EABInote:sp_svcisalways64-bitalignedhere,soshouldS_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--.macrousr_entry-UNWIND(.fnstart)-UNWIND(.cantunwind)@don't unwind the user space-subsp,sp,#S_FRAME_SIZE-ARM(stmibsp,{r1-r12})-THUMB(stmiasp,{r0-r12})--ldmiar0,{r3-r5}-addr0,sp,#S_PC @ here for interlock avoidance-movr6,#-1 @ "" "" "" ""--strr3,[sp]@savethe"real"r0copied-@fromtheexceptionstack--@-@Wearenowreadytofillintheremainingblanksonthestack:-@-@r4-lr_<exception>,alreadyfixedupforcorrectreturn/restart-@r5-spsr_<exception>-@r6-orig_r0 (seept_regsdefinitioninptrace.h)-@-@Also,separatelysavesp_usrandlr_usr-@-stmiar0,{r4-r6}-ARM(stmdbr0,{sp,lr}^)-THUMB(store_user_sp_lrr0,r1,S_SP-S_PC)--@-@Enablethealignmenttrapwhileinkernelmode-@-alignment_trapr0--@-@ClearFPtomarkthefirststackframe-@-zero_fp--#ifdef CONFIG_IRQSOFF_TRACER-bltrace_hardirqs_off-#endif-.endm--.macrokuser_cmpxchg_check-#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)-#ifndef CONFIG_MMU-#warning "NPTL on non MMU needs fixing"-#else-@Makesureouruserspaceatomichelperisrestarted-@ifitwasinterruptedinacriticalregion.Herewe-@performaquicktestinlinesinceitshouldbefalse-@99.9999%ofthetime.Therestisdoneoutofline.-cmpr4,#TASK_SIZE-blhskuser_cmpxchg64_fixup-#endif-#endif-.endm-.align5 __dabt_usr:usr_entry
@@ -73,6 +73,109 @@msrcpsr_c,\rtemp@switchbacktotheSVCmode.endm+/*+*Vectorstubs.+*+*Thiscodeiscopiedto0xffff0200sowecanusebranchesinthe+*vectors,ratherthanldr's. Note that this code must not+*exceed0x300bytes.+*+*Commonstubentrymacro:+*EnterinIRQmode,spsr=SVC/USRCPSR,lr=SVC/USRPC+*+*SPpointstoaminimalamountofprocessor-privatememory,theaddress+*ofwhichiscopiedintor0forthemodespecificaborthandler.+*/+.macrovector_stub,name,mode,correction=0+.align5++vector_\name:+.if\correction+sublr,lr,#\correction+.endif++@+@Saver0,lr_<exception>(parentPC)andspsr_<exception>+@(parentCPSR)+@+stmiasp,{r0,lr}@saver0,lr+mrslr,spsr+strlr,[sp,#8] @ save spsr++@+@PrepareforSVC32mode.IRQsremaindisabled.+@+mrsr0,cpsr+eorr0,r0,#(\mode ^ SVC_MODE | PSR_ISETSTATE)+msrspsr_cxsf,r0++@+@thebranchtablemustimmediatelyfollowthiscode+@+andlr,lr,#0x0f+THUMB(adrr0,1f)+THUMB(ldrlr,[r0,lr,lsl#2] )+movr0,sp+ARM(ldrlr,[pc,lr,lsl#2] )+movspc,lr@branchtohandlerinSVCmode+ENDPROC(vector_\name)++.align2+@handleraddressesfollowthislabel+1:+.endm++/*+*SVCmodehandlers+*/++#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)+#define SPFIX(code...) code+#else+#define SPFIX(code...)+#endif++.macrosvc_entry,stack_hole=0+UNWIND(.fnstart)+UNWIND(.save{r0-pc})+subsp,sp,#(S_FRAME_SIZE + \stack_hole - 4)+#ifdef CONFIG_THUMB2_KERNEL+SPFIX(strr0,[sp])@temporarilysaved+SPFIX(movr0,sp)+SPFIX(tstr0,#4 ) @ test original stack alignment+SPFIX(ldrr0,[sp])@restored+#else+SPFIX(tstsp,#4 )+#endif+SPFIX(subeqsp,sp,#4 )+stmiasp,{r1-r12}++ldmiar0,{r3-r5}+addr7,sp,#S_SP - 4 @ here for interlock avoidance+movr6,#-1 @ "" "" "" ""+addr2,sp,#(S_FRAME_SIZE + \stack_hole - 4)+SPFIX(addeqr2,r2,#4 )+strr3,[sp,#-4]! @ save the "real" r0 copied+@fromtheexceptionstack++movr3,lr++@+@Wearenowreadytofillintheremainingblanksonthestack:+@+@r2-sp_svc+@r3-lr_svc+@r4-lr_<exception>,alreadyfixedupforcorrectreturn/restart+@r5-spsr_<exception>+@r6-orig_r0 (seept_regsdefinitioninptrace.h)+@+stmiar7,{r2-r6}++#ifdef CONFIG_TRACE_IRQFLAGS+bltrace_hardirqs_off+#endif+.endm+#ifndef CONFIG_THUMB2_KERNEL.macrosvc_exit,rpsrmsrspsr_cxsf,\rpsr
@@ -164,6 +267,73 @@#endif /* !CONFIG_THUMB2_KERNEL *//*+*Usermodehandlers+*+*EABInote:sp_svcisalways64-bitalignedhere,soshouldS_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++.macrousr_entry+UNWIND(.fnstart)+UNWIND(.cantunwind)@don't unwind the user space+subsp,sp,#S_FRAME_SIZE+ARM(stmibsp,{r1-r12})+THUMB(stmiasp,{r0-r12})++ldmiar0,{r3-r5}+addr0,sp,#S_PC @ here for interlock avoidance+movr6,#-1 @ "" "" "" ""++strr3,[sp]@savethe"real"r0copied+@fromtheexceptionstack++@+@Wearenowreadytofillintheremainingblanksonthestack:+@+@r4-lr_<exception>,alreadyfixedupforcorrectreturn/restart+@r5-spsr_<exception>+@r6-orig_r0 (seept_regsdefinitioninptrace.h)+@+@Also,separatelysavesp_usrandlr_usr+@+stmiar0,{r4-r6}+ARM(stmdbr0,{sp,lr}^)+THUMB(store_user_sp_lrr0,r1,S_SP-S_PC)++@+@Enablethealignmenttrapwhileinkernelmode+@+alignment_trapr0++@+@ClearFPtomarkthefirststackframe+@+zero_fp++#ifdef CONFIG_IRQSOFF_TRACER+bltrace_hardirqs_off+#endif+.endm++.macrokuser_cmpxchg_check+#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)+#ifndef CONFIG_MMU+#warning "NPTL on non MMU needs fixing"+#else+@Makesureouruserspaceatomichelperisrestarted+@ifitwasinterruptedinacriticalregion.Herewe+@performaquicktestinlinesinceitshouldbefalse+@99.9999%ofthetime.Therestisdoneoutofline.+cmpr4,#TASK_SIZE+blhskuser_cmpxchg64_fixup+#endif+#endif+.endm++/**Thesearetheregistersusedinthesyscallhandler,andallowusto*haveintheoryupto7argumentstoafunction-r0tor6.*
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:02:05
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 <redacted>
---
arch/arm/common/vic.c | 28 ++++++++++++++++++++++++++++
arch/arm/include/asm/hardware/vic.h | 2 ++
2 files changed, 30 insertions(+)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:02:16
This makes the code more izolated.
The downside of this is that we now have an additional branch and the
code itself is 8 bytes longer. But on the bright side, this new layout
can be more cache friendly since cr_alignment address might be already
in the cache line (not that I measured anything, it's just fun to think
about it).
Signed-off-by: Anton Vorontsov <redacted>
---
arch/arm/kernel/entry-armv.S | 2 --
arch/arm/kernel/entry-header.S | 6 +++++-
arch/arm/kernel/kgdb_fiq_entry.S | 3 ---
3 files changed, 5 insertions(+), 6 deletions(-)
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:02:34
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 <redacted>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-versatile/Makefile | 1 +
arch/arm/mach-versatile/include/mach/irqs.h | 1 +
arch/arm/mach-versatile/kgdb_fiq.c | 31 +++++++++++++++++++++++++++
4 files changed, 34 insertions(+)
create mode 100644 arch/arm/mach-versatile/kgdb_fiq.c
@@ -0,0 +1,31 @@+/*+*KGDBFIQboardsupport+*+*Copyright2012LinaroLtd.+*AntonVorontsov<anton.vorontsov@linaro.org>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseversion2aspublished+*bytheFreeSoftwareFoundation.+*/++#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>++staticintkgdb_fiq;+module_param_named(uart_num,kgdb_fiq,int,0600);+MODULE_PARM_DESC(uart_num,"UART<number> port to use for KGDB FIQ");++staticint__initkgdb_fiq_init(void)+{+WARN_ON(kgdb_fiq>INT_UARTINT2-INT_UARTINT0);++returnkgdb_register_fiq(INT_UARTINT0+kgdb_fiq,+vic_fiq_select,+vic_is_fiq_rised);+}+console_initcall(kgdb_fiq_init);
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:02:49
As Colin Cross noticed, serial ports could be noisy, so occasional
characters once in a while are possible. So, considering the noise
possibility, entering the debugger on any received byte is unacceptable
for production devices.
This changes KGDB FIQ behaviour in a such way so that we have to type the
GDB-protocol "$3#33" command to actually enter the debugger, the kernel
will print the following prompt:
Type $3#33 to enter the debugger>
This is the exactly the same command we use to escape from KGDB to KDB,
so it should be all pretty familiar.
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>
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 <redacted>
Signed-off-by: Anton Vorontsov <redacted>
---
arch/arm/kernel/kgdb_fiq.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
@@ -26,6 +27,60 @@ 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");+staticintkgdb_fiq_knock=1;+module_param_named(knock,kgdb_fiq_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");++/*+*"Serial ports are often noisy, especially when muxed over another port (we+*oftenuseserialovertheheadsetconnector).Noiseontheasynccommand+*linejustcausescharactersthatareignored,onacommandlinethatblocked+*executionnoisewouldbecatastrophic." -- Colin Cross+*+*So,thissmallfunctionimplementsKGDB/KDBknockingontheserialline:we+*won'tenterthedebuggeruntilwereceiveaknownmagicphrase(whichis+*actually"$3#33",knownas"escape to KDB"command.Ifknockingisdisabled,+*justpressingthereturnkeyisenoughtoenterthedebugger.+*/+staticboolkgdb_fiq_poll_knock(void)+{+staticintn;+intc=-1;+get_char_func*getc;+charmagic[]="$3#33";+size_tm=strlen(magic);++if(kgdb_fiq_knock<0)+return1;++for(getc=&kdb_poll_funcs[0];*getc;++getc){+c=(*getc)();+if(c>=0)+break;+}++if(!kgdb_fiq_knock&&(c=='\r'||c=='\n')){+return1;+}elseif(c==magic[n]){+kdb_printf("%c",c);+n=(n+1)%m;+if(!n)+return1;+}else{+n=0;+kdb_printf("\r%s %s to enter the debugger> %*s",+kgdb_fiq_knock?"Type":"Hit",+kgdb_fiq_knock?magic:"<return>",m,"");+memset(magic,'\b',m);+kdb_printf("%s",magic);+}+return0;+}+staticunsignedintkgdb_fiq;staticvoid(*kgdb_enable_fiq)(unsignedintirq,boolon);staticbool(*is_kgdb_fiq)(unsignedintirq);
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:03:11
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 <redacted>
---
arch/arm/Kconfig | 18 +++++++
arch/arm/include/asm/kgdb.h | 8 +++
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/kgdb_fiq.c | 101 ++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/kgdb_fiq_entry.S | 79 +++++++++++++++++++++++++++++
5 files changed, 207 insertions(+)
create mode 100644 arch/arm/kernel/kgdb_fiq.c
create mode 100644 arch/arm/kernel/kgdb_fiq_entry.S
@@ -0,0 +1,101 @@+/*+*KGDBFIQentry+*+*Copyright2010Google,Inc.+*ArveHj?nnev?g<arve@android.com>+*ColinCross<ccross@android.com>+*Copyright2012LinaroLtd.+*AntonVorontsov<anton.vorontsov@linaro.org>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseversion2aspublished+*bytheFreeSoftwareFoundation.+*/++#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/kgdb.h>+#include<asm/fiq.h>+#include<asm/exception.h>++staticintkgdb_fiq_enabled;+module_param_named(enable,kgdb_fiq_enabled,int,0600);+MODULE_PARM_DESC(enable,"set to 1 to enable FIQ KGDB");++staticunsignedintkgdb_fiq;+staticvoid(*kgdb_enable_fiq)(unsignedintirq,boolon);+staticbool(*is_kgdb_fiq)(unsignedintirq);++asmlinkagevoid__exception_irq_entrykgdb_fiq_do_handle(structpt_regs*regs)+{+if(!is_kgdb_fiq(kgdb_fiq))+return;+dbg_io_ops->clear_irqs();++nmi_enter();+kgdb_handle_exception(1,0,0,regs);+nmi_exit();+}++staticstructfiq_handlerkgdb_fiq_desc={+.name="kgdb",+};++staticlongkgdb_fiq_setup_stack(void*info)+{+structpt_regsregs;++regs.ARM_sp=__get_free_pages(GFP_KERNEL,THREAD_SIZE_ORDER)++THREAD_START_SP;+WARN_ON(!regs.ARM_sp);++set_fiq_regs(®s);+return0;+}++intkgdb_arch_enable_nmi(boolon)+{+staticintcnt;++if(cnt>0&&on)+returncnt;+cnt+=on?1:-1;+kgdb_enable_fiq(kgdb_fiq,cnt>0);+returncnt;+}++int__initkgdb_register_fiq(unsignedintmach_kgdb_fiq,+void(*mach_kgdb_enable_fiq)(unsignedintirq,boolon),+bool(*mach_is_kgdb_fiq)(unsignedintirq))+{+interr;+intcpu;++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__);+returnerr;+}++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);++kgdb_arch_enable_nmi(1);+return0;+}
From: Anton Vorontsov <hidden> Date: 2012-07-30 12:03:47
This command disables NMI-entry. If NMI source was 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 <redacted>
---
kernel/debug/kdb/kdb_main.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
@@ -2873,6 +2891,8 @@ static void __init kdb_inittab(void)kdb_register_flags("dmesg",kdb_dmesg,"[lines]","Display syslog buffer",0,KDB_SAFE);#endif+kdb_register_flags("disable_nmi",kdb_disable_nmi,"",+"Disable NMI entry to KDB",0,KDB_SAFE);kdb_register_flags("defcmd",kdb_defcmd,"name \"usage\"\"help\"","Define a set of commands, down to endefcmd",0,KDB_SAFE);kdb_register_flags("kill",kdb_kill,"<-signal> <pid>",
I am not convinced that this does not cause loss of state from the parent
context. Let's review what happens when a FIQ is received from SVC mode
with the above code.
- The CPU will be in SVC mode.
- FIQ received.
- CPU saves CPSR into SPSR_fiq and PC into LR_fiq, and jumps to the FIQ
vector.
- We apply the 4 byte correction to LR_fiq, and store r0, LR_fiq and
SPSR_fiq to the FIQ 'stack'
- We switch to SVC mode and jump to __fiq_svc
- svc_entry:
- adjusts the SVC stack pointer down, and saves r1 - r12
- loads r0, LR_fiq and SPSR_fiq and saves them as ARM_r0, ARM_pc, ARM_cpsr
into the pt_regs
- the original value of the SVC stack pointer is saved as ARM_r13
- LR_svc is saved as ARM_r14
At this point, we have saved everything *except* for the SPSR_svc register.
Now, when we return from the above, we use svc_exit:
- write SPSR_svc with ARM_cpsr (from SPSR_fiq)
- load r0-pc from the pt_regs and load CPSR from SPSR_svc
Now the thing here is that even if we did preserve SPSR_svc, with the
above exit sequence, there is _no_ way to preserve the value of SPSR_svc.
Normally, this doesn't matter because we know that the regions we care
about this have IRQs disabled.
However, what this means, if we receive an FIQ and use this path from any
part of the kernel which expects SPSR_svc to be preserved (eg, the exit
path from any exception) the kernel will blow up.
I guess you could do something like this instead:
- disable FIQs
- load SPSR_svc with a saved value of it from entry.
- load r1-r14 from ARM_r1..ARM_lr
- switch to FIQ mode
- load SPSR_fiq from saved ARM_cpsr
- load r0 from ARM_r0
- load pc from ARM_pc
So, maybe something like this for the svc return path:
cpsid f
ldr r1, [saved_spsr_svc]
mov r0, sp
mrs spsr_cxsf, r1
ldmib r0, {r1 - r14}
msr cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
add r7, r0, #S_PC
ldr r8, [r0, #S_CPSR]
mrs spsr_cxsf, r8
ldr r0, [r0, #S_R0]
ldmia r7, {pc}^
From: Russell King - ARM Linux <hidden> Date: 2012-07-30 14:16:05
On Mon, Jul 30, 2012 at 04:58:20AM -0700, Anton Vorontsov wrote:
This makes the code more izolated.
The downside of this is that we now have an additional branch and the
code itself is 8 bytes longer. But on the bright side, this new layout
can be more cache friendly since cr_alignment address might be already
in the cache line (not that I measured anything, it's just fun to think
about it).
The caches are harvard, so mixing data and code together does not increase
performance. Having data which is used by the same code in the same cache
line results in better performance.
The additional branch will also cause a pipeline stall on older CPUs.
So no, I don't see any way that this is a performance improvement. Please
leave this as is.
From: Colin Cross <hidden> Date: 2012-07-30 17:33:39
On Mon, Jul 30, 2012 at 4:58 AM, Anton Vorontsov
[off-list ref] wrote:
This command disables NMI-entry. If NMI source was 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
This is very different behavior from the FIQ debugger "console"
command you are trying to replace. In the FIQ debugger, everything
goes through the FIQ/NMI, even when in console mode. That means that
the user can always get back to FIQ debugger/KDB mode using a special
sequence (we use a break character). With your implementation, if you
switch from KDB to console to see if the console is working, and find
that it is not working, you can never get back into KDB.
From: Jason Wessel <jason.wessel@windriver.com> Date: 2012-07-31 03:53:44
On 07/30/2012 06:58 AM, Anton Vorontsov wrote:
Currently kernel never set KGDB_REASON_NMI. We do now, when we enter
KGDB/KDB from an NMI.
This is not to be confused with kgdb_nmicallback(), NMI callback is
an entry for the slave CPUs during CPUs roundup, but REASON_NMI is the
entry for the master CPU.
No need for confusion here :-)
I'll take this one for the kernel merge window if it passes regression tests, no reason not to be setting the stop codes properly.
Thanks,
Jason.
From: Anton Vorontsov <hidden> Date: 2012-08-01 20:54:36
On Mon, Jul 30, 2012 at 03:07:24PM +0100, Russell King - ARM Linux wrote:
[....]
Now the thing here is that even if we did preserve SPSR_svc, with the
above exit sequence, there is _no_ way to preserve the value of SPSR_svc.
Normally, this doesn't matter because we know that the regions we care
about this have IRQs disabled.
Wow, that is quite subtle. Indeed, now I clearly see that we do mangle
SVC's SPSR.
Thanks!
[...]
So, maybe something like this for the svc return path:
cpsid f
ldr r1, [saved_spsr_svc]
mov r0, sp
mrs spsr_cxsf, r1
ldmib r0, {r1 - r14}
msr cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
...
ldmia r7, {pc}^
Yup, I got the idea: we have to restore the SPSR in SVC mode, but
then we switch to FIQ mode and exit the exception while being in
the FIQ mode, that way CPU won't change our restored SPSR_svc.
As for "saved_spsr_svc", I think the easiest way to get it, is to
switch to FIQ mode temporary, and read it from the stack.
(Plus, cpsid is not available for older CPUs, but the thing is that
we don't touch global enable/disable FIQs/IRQs flags, so I guess
I don't actually need it.)
So, that's what I've applied on top:
And the whole fixed up patch is down below:
- - - -
From: Anton Vorontsov <redacted>
Subject: 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 <redacted>
---
arch/arm/Kconfig | 18 +++++++
arch/arm/include/asm/kgdb.h | 8 +++
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/kgdb_fiq.c | 101 ++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/kgdb_fiq_entry.S | 92 ++++++++++++++++++++++++++++++++++
5 files changed, 220 insertions(+)
create mode 100644 arch/arm/kernel/kgdb_fiq.c
create mode 100644 arch/arm/kernel/kgdb_fiq_entry.S
@@ -0,0 +1,101 @@+/*+*KGDBFIQentry+*+*Copyright2010Google,Inc.+*ArveHj?nnev?g<arve@android.com>+*ColinCross<ccross@android.com>+*Copyright2012LinaroLtd.+*AntonVorontsov<anton.vorontsov@linaro.org>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseversion2aspublished+*bytheFreeSoftwareFoundation.+*/++#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/kgdb.h>+#include<asm/fiq.h>+#include<asm/exception.h>++staticintkgdb_fiq_enabled;+module_param_named(enable,kgdb_fiq_enabled,int,0600);+MODULE_PARM_DESC(enable,"set to 1 to enable FIQ KGDB");++staticunsignedintkgdb_fiq;+staticvoid(*kgdb_enable_fiq)(unsignedintirq,boolon);+staticbool(*is_kgdb_fiq)(unsignedintirq);++asmlinkagevoid__exception_irq_entrykgdb_fiq_do_handle(structpt_regs*regs)+{+if(!is_kgdb_fiq(kgdb_fiq))+return;+dbg_io_ops->clear_irqs();++nmi_enter();+kgdb_handle_exception(1,0,0,regs);+nmi_exit();+}++staticstructfiq_handlerkgdb_fiq_desc={+.name="kgdb",+};++staticlongkgdb_fiq_setup_stack(void*info)+{+structpt_regsregs;++regs.ARM_sp=__get_free_pages(GFP_KERNEL,THREAD_SIZE_ORDER)++THREAD_START_SP;+WARN_ON(!regs.ARM_sp);++set_fiq_regs(®s);+return0;+}++intkgdb_arch_enable_nmi(boolon)+{+staticintcnt;++if(cnt>0&&on)+returncnt;+cnt+=on?1:-1;+kgdb_enable_fiq(kgdb_fiq,cnt>0);+returncnt;+}++int__initkgdb_register_fiq(unsignedintmach_kgdb_fiq,+void(*mach_kgdb_enable_fiq)(unsignedintirq,boolon),+bool(*mach_is_kgdb_fiq)(unsignedintirq))+{+interr;+intcpu;++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__);+returnerr;+}++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);++kgdb_arch_enable_nmi(1);+return0;+}
From: Anton Vorontsov <hidden> Date: 2012-08-01 20:55:24
On Mon, Jul 30, 2012 at 03:15:44PM +0100, Russell King - ARM Linux wrote:
On Mon, Jul 30, 2012 at 04:58:20AM -0700, Anton Vorontsov wrote:
quoted
This makes the code more izolated.
The downside of this is that we now have an additional branch and the
code itself is 8 bytes longer. But on the bright side, this new layout
can be more cache friendly since cr_alignment address might be already
in the cache line (not that I measured anything, it's just fun to think
about it).
The caches are harvard, so mixing data and code together does not increase
performance. Having data which is used by the same code in the same cache
line results in better performance.
The additional branch will also cause a pipeline stall on older CPUs.
So no, I don't see any way that this is a performance improvement. Please
leave this as is.
Sure, will drop it.
Thanks!
--
Anton Vorontsov
Email: cbouatmailru at gmail.com
From: Anton Vorontsov <hidden> Date: 2012-08-01 21:04:48
On Mon, Jul 30, 2012 at 10:33:34AM -0700, Colin Cross wrote:
On Mon, Jul 30, 2012 at 4:58 AM, Anton Vorontsov
[off-list ref] wrote:
quoted
This command disables NMI-entry. If NMI source was 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
This is very different behavior from the FIQ debugger "console"
command you are trying to replace. In the FIQ debugger, everything
goes through the FIQ/NMI, even when in console mode. That means that
the user can always get back to FIQ debugger/KDB mode using a special
sequence (we use a break character). With your implementation, if you
switch from KDB to console to see if the console is working, and find
that it is not working, you can never get back into KDB.
Ah, I see. But with disable_nmi, in addition to kernel console,
applications can use /dev/ttyXX as normal, and with 'console' command
that is not possible (at least w/o modifying applications to escape
magic sequence). So, I think we should have both commands, each would
handle its own use case.
(Initially I just tried to avoid adding another console driver, but
it seems there is no other way, heh.)
Thanks!
--
Anton Vorontsov
Email: cbouatmailru at gmail.com