linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: jason.wessel@windriver.com, gregkh@linuxfoundation.org,
	corbet@lwn.net, frowand.list@gmail.com,
	bjorn.andersson@linaro.org, linux-serial@vger.kernel.org,
	mingo@redhat.com, hpa@zytor.com, jslaby@suse.com,
	kgdb-bugreport@lists.sourceforge.net, sumit.garg@linaro.org,
	will@kernel.org, tglx@linutronix.de, agross@kernel.org,
	catalin.marinas@arm.com, bp@alien8.de,
	Andrew Morton <akpm@linux-foundation.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH v4 04/12] kgdb: Delay "kgdbwait" to dbg_late_init() by default
Date: Fri, 15 May 2020 17:18:12 +0100	[thread overview]
Message-ID: <20200515161812.vs3ry35qcatgf7ut@holly.lan> (raw)
In-Reply-To: <20200507130644.v4.4.I3113aea1b08d8ce36dc3720209392ae8b815201b@changeid>

On Thu, May 07, 2020 at 01:08:42PM -0700, Douglas Anderson wrote:
> Using kgdb requires at least some level of architecture-level
> initialization.  If nothing else, it relies on the architecture to
> pass breakpoints / crashes onto kgdb.
> 
> On some architectures this all works super early, specifically it
> starts working at some point in time before Linux parses
> early_params's.  On other architectures it doesn't.  A survey of a few
> platforms:
> 
> a) x86: Presumably it all works early since "ekgdboc" is documented to
>    work here.
> b) arm64: Catching crashes works; with a simple patch breakpoints can
>    also be made to work.
> c) arm: Nothing in kgdb works until
>    paging_init() -> devicemaps_init() -> early_trap_init()
> 
> Let's be conservative and, by default, process "kgdbwait" (which tells
> the kernel to drop into the debugger ASAP at boot) a bit later at
> dbg_late_init() time.  If an architecture has tested it and wants to
> re-enable super early debugging, they can select the
> ARCH_HAS_EARLY_DEBUG KConfig option.  We'll do this for x86 to start.
> It should be noted that dbg_late_init() is still called quite early in
> the system.
> 
> Note that this patch doesn't affect when kgdb runs its init.  If kgdb
> is set to initialize early it will still initialize when parsing
> early_param's.  This patch _only_ inhibits the initial breakpoint from
> "kgdbwait".  This means:
> 
> * Without any extra patches arm64 platforms will at least catch
>   crashes after kgdb inits.
> * arm platforms will catch crashes (and could handle a hardcoded
>   kgdb_breakpoint()) any time after early_trap_init() runs, even
>   before dbg_late_init().
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I hope to pull this set into the kgdb tree shortly. Any objections to
the arch/x86 changes this would bring?


Daniel.


> ---
> 
> Changes in v4:
> - Add "if KGDB" to "select ARCH_HAS_EARLY_DEBUG" in Kconfig.
> 
> Changes in v3:
> - Change boolean weak function to KConfig.
> 
> Changes in v2: None
> 
>  arch/x86/Kconfig          |  1 +
>  kernel/debug/debug_core.c | 25 +++++++++++++++----------
>  lib/Kconfig.kgdb          | 18 ++++++++++++++++++
>  3 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 1197b5596d5a..5f44955ee21c 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -60,6 +60,7 @@ config X86
>  	select ARCH_HAS_ACPI_TABLE_UPGRADE	if ACPI
>  	select ARCH_HAS_DEBUG_VIRTUAL
>  	select ARCH_HAS_DEVMEM_IS_ALLOWED
> +	select ARCH_HAS_EARLY_DEBUG		if KGDB
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_FAST_MULTIPLIER
>  	select ARCH_HAS_FILTER_PGPROT
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 950dc667c823..503c1630ca76 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -950,6 +950,14 @@ void kgdb_panic(const char *msg)
>  	kgdb_breakpoint();
>  }
>  
> +static void kgdb_initial_breakpoint(void)
> +{
> +	kgdb_break_asap = 0;
> +
> +	pr_crit("Waiting for connection from remote gdb...\n");
> +	kgdb_breakpoint();
> +}
> +
>  void __weak kgdb_arch_late(void)
>  {
>  }
> @@ -960,6 +968,9 @@ void __init dbg_late_init(void)
>  	if (kgdb_io_module_registered)
>  		kgdb_arch_late();
>  	kdb_init(KDB_INIT_FULL);
> +
> +	if (kgdb_io_module_registered && kgdb_break_asap)
> +		kgdb_initial_breakpoint();
>  }
>  
>  static int
> @@ -1055,14 +1066,6 @@ void kgdb_schedule_breakpoint(void)
>  }
>  EXPORT_SYMBOL_GPL(kgdb_schedule_breakpoint);
>  
> -static void kgdb_initial_breakpoint(void)
> -{
> -	kgdb_break_asap = 0;
> -
> -	pr_crit("Waiting for connection from remote gdb...\n");
> -	kgdb_breakpoint();
> -}
> -
>  /**
>   *	kgdb_register_io_module - register KGDB IO module
>   *	@new_dbg_io_ops: the io ops vector
> @@ -1099,7 +1102,8 @@ int kgdb_register_io_module(struct kgdb_io *new_dbg_io_ops)
>  	/* Arm KGDB now. */
>  	kgdb_register_callbacks();
>  
> -	if (kgdb_break_asap)
> +	if (kgdb_break_asap &&
> +	    (!dbg_is_early || IS_ENABLED(CONFIG_ARCH_HAS_EARLY_DEBUG)))
>  		kgdb_initial_breakpoint();
>  
>  	return 0;
> @@ -1169,7 +1173,8 @@ static int __init opt_kgdb_wait(char *str)
>  	kgdb_break_asap = 1;
>  
>  	kdb_init(KDB_INIT_EARLY);
> -	if (kgdb_io_module_registered)
> +	if (kgdb_io_module_registered &&
> +	    IS_ENABLED(CONFIG_ARCH_HAS_EARLY_DEBUG))
>  		kgdb_initial_breakpoint();
>  
>  	return 0;
> diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> index 933680b59e2d..ffa7a76de086 100644
> --- a/lib/Kconfig.kgdb
> +++ b/lib/Kconfig.kgdb
> @@ -124,4 +124,22 @@ config KDB_CONTINUE_CATASTROPHIC
>  	  CONFIG_KDB_CONTINUE_CATASTROPHIC == 2. KDB forces a reboot.
>  	  If you are not sure, say 0.
>  
> +config ARCH_HAS_EARLY_DEBUG
> +	bool
> +	default n
> +	help
> +	  If an architecture can definitely handle entering the debugger
> +	  when early_param's are parsed then it select this config.
> +	  Otherwise, if "kgdbwait" is passed on the kernel command line it
> +	  won't actually be processed until dbg_late_init() just after the
> +	  call to kgdb_arch_late() is made.
> +
> +	  NOTE: Even if this isn't selected by an architecture we will
> +	  still try to register kgdb to handle breakpoints and crashes
> +	  when early_param's are parsed, we just won't act on the
> +	  "kgdbwait" parameter until dbg_late_init().  If you get a
> +	  crash and try to drop into kgdb somewhere between these two
> +	  places you might or might not end up being able to use kgdb
> +	  depending on exactly how far along the architecture has initted.
> +
>  endif # KGDB
> -- 
> 2.26.2.645.ge9eca65c58-goog
> 

  reply	other threads:[~2020-05-15 16:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-07 20:08 [PATCH v4 00/12] kgdb: Support late serial drivers; enable early debug w/ boot consoles Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 01/12] kgdb: Disable WARN_CONSOLE_UNLOCKED for all kgdb Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 02/12] Revert "kgdboc: disable the console lock when in kgdb" Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 03/12] kgdboc: Use a platform device to handle tty drivers showing up late Douglas Anderson
2020-05-18 16:46   ` Daniel Thompson
2020-05-07 20:08 ` [PATCH v4 04/12] kgdb: Delay "kgdbwait" to dbg_late_init() by default Douglas Anderson
2020-05-15 16:18   ` Daniel Thompson [this message]
2020-05-07 20:08 ` [PATCH v4 05/12] arm64: Add call_break_hook() to early_brk64() for early kgdb Douglas Anderson
2020-05-13 23:12   ` Doug Anderson
2020-05-07 20:08 ` [PATCH v4 06/12] kgdb: Prevent infinite recursive entries to the debugger Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 07/12] kgdboc: Remove useless #ifdef CONFIG_KGDB_SERIAL_CONSOLE in kgdboc Douglas Anderson
2020-05-11 16:02   ` Daniel Thompson
2020-05-07 20:08 ` [PATCH v4 08/12] kgdboc: Add kgdboc_earlycon to support early kgdb using boot consoles Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 09/12] Documentation: kgdboc: Document new kgdboc_earlycon parameter Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 10/12] serial: qcom_geni_serial: Support kgdboc_earlycon Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 11/12] serial: 8250_early: " Douglas Anderson
2020-05-07 20:08 ` [PATCH v4 12/12] serial: amba-pl011: " Douglas Anderson
2020-05-14 16:21 ` [PATCH v4 00/12] kgdb: Support late serial drivers; enable early debug w/ boot consoles Daniel Thompson
2020-05-14 16:34   ` Doug Anderson
2020-05-14 16:36     ` Greg Kroah-Hartman
2020-05-19 10:37       ` Daniel Thompson
2020-05-19 10:36 ` Daniel Thompson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200515161812.vs3ry35qcatgf7ut@holly.lan \
    --to=daniel.thompson@linaro.org \
    --cc=agross@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=dianders@chromium.org \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jason.wessel@windriver.com \
    --cc=jslaby@suse.com \
    --cc=kgdb-bugreport@lists.sourceforge.net \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sumit.garg@linaro.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).