Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 27 Oct 2020 12:01:34 +0000
From:   Daniel Thompson <daniel.thompson@linaro.org>
To:     Sasha Levin <sashal@kernel.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Douglas Anderson <dianders@chromium.org>,
        kgdb-bugreport@lists.sourceforge.net
Subject: Re: [PATCH AUTOSEL 5.4 31/80] kgdb: Make "kgdbcon" work properly
 with "kgdb_earlycon"
Message-ID: <20201027120134.iq44uw6bftumkivh@holly.lan>
References: <20201026235516.1025100-1-sashal@kernel.org>
 <20201026235516.1025100-31-sashal@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201026235516.1025100-31-sashal@kernel.org>
Precedence: bulk

On Mon, Oct 26, 2020 at 07:54:27PM -0400, Sasha Levin wrote:
> From: Douglas Anderson <dianders@chromium.org>
> 
> [ Upstream commit b18b099e04f450cdc77bec72acefcde7042bd1f3 ]
> 
> On my system the kernel processes the "kgdb_earlycon" parameter before
> the "kgdbcon" parameter.  When we setup "kgdb_earlycon" we'll end up
> in kgdb_register_callbacks() and "kgdb_use_con" won't have been set
> yet so we'll never get around to starting "kgdbcon".  Let's remedy
> this by detecting that the IO module was already registered when
> setting "kgdb_use_con" and registering the console then.
> 
> As part of this, to avoid pre-declaring things, move the handling of
> the "kgdbcon" further down in the file.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Link: https://lore.kernel.org/r/20200630151422.1.I4aa062751ff5e281f5116655c976dff545c09a46@changeid
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

kgdb[oc]_earlycon was a new feature introduced in v5.8 so, based on the
summary above, this fix does not obviously apply to older kernels.

However after looking closely...

I think the issue described above would also occur if kgdbdbgp (an
incomprehensible sequence consonants that translates to "present
debugger via USB EHCI debug") were used in conjunction with kgdbcon
meaning backporting does make sense.


Daniel.


> ---
>  kernel/debug/debug_core.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 2222f3225e53d..097ab02989f92 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -96,14 +96,6 @@ int dbg_switch_cpu;
>  /* Use kdb or gdbserver mode */
>  int dbg_kdb_mode = 1;
>  
> -static int __init opt_kgdb_con(char *str)
> -{
> -	kgdb_use_con = 1;
> -	return 0;
> -}
> -
> -early_param("kgdbcon", opt_kgdb_con);
> -
>  module_param(kgdb_use_con, int, 0644);
>  module_param(kgdbreboot, int, 0644);
>  
> @@ -876,6 +868,20 @@ static struct console kgdbcons = {
>  	.index		= -1,
>  };
>  
> +static int __init opt_kgdb_con(char *str)
> +{
> +	kgdb_use_con = 1;
> +
> +	if (kgdb_io_module_registered && !kgdb_con_registered) {
> +		register_console(&kgdbcons);
> +		kgdb_con_registered = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +early_param("kgdbcon", opt_kgdb_con);
> +
>  #ifdef CONFIG_MAGIC_SYSRQ
>  static void sysrq_handle_dbg(int key)
>  {
> -- 
> 2.25.1
>