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:   Mon, 8 Mar 2021 13:31:49 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Sangmoon Kim <sangmoon.kim@samsung.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, jordan.lim@samsung.com,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Martin <Dave.Martin@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Dmitry Safonov <0x7f454c46@gmail.com>,
        Amit Daniel Kachhap <amit.kachhap@arm.com>,
        Peter Collingbourne <pcc@google.com>,
        Gavin Shan <gshan@redhat.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: traps: add tracepoints for unusal exception cases
Message-ID: <20210308133149.GA4656@sirena.org.uk>
References: <CGME20210305124537epcas1p1930302083680f1b1cf87e37630556460@epcas1p1.samsung.com>
 <20210305123635.27492-1-sangmoon.kim@samsung.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="Qxx1br4bt0+wmkIi"
Content-Disposition: inline
In-Reply-To: <20210305123635.27492-1-sangmoon.kim@samsung.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--Qxx1br4bt0+wmkIi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 05, 2021 at 09:36:30PM +0900, Sangmoon Kim wrote:

> When kernel panic occurs, a kernel module can use either the
> panic_notifier or die_notifier to obtain the debugging information.

> However, in case of these exceptions like do_undefinstr(), regs and
> esr data are not passed on. Although a module might be able to find
> those data in the console messages, parsing text messages is very
> expensive behavior for a module especially on mobile devices.

> These bare tracepoints allow a module to probe regs and esr information
> for debugging purpose. _tp suffix comes from bare tracepoints of
> sched/core.c

This use case sounds a lot like what the enterprise and Android people
do via pstore - it seems like it would be better for this to integrate
via the interfaces that other systems are using for similar purposes and
then ensure that whatever information is useful is getting passed
through in a format that makes sense.  That'd be more structured and
more readily usable by a wider range of systems than something that's
more of a building block, going via the trace infrastructure seems like
a bit of an indirection.

> @@ -832,6 +846,7 @@ void __noreturn arm64_serror_panic(struct pt_regs *re=
gs, u32 esr)
>  	if (regs)
>  		__show_regs(regs);
> =20
> +	trace_traps_serror_panic_tp(regs, esr);
>  	nmi_panic(regs, "Asynchronous SError Interrupt");

One of the concerns people have with adding tracepoints is that they can
end up defining ABI so if we *are* going to add any then we need to
think carefully about how they're defined.  As things currently stand
they'll pass in the full pt_regs struct which includes not only what's
defined by the hardware but also additional software defined information
we store along with it like the stackframe which would be even more of a
problem if it ends up getting used by someone in a way that ends up as
ABI.  These are defined as bare tracehooks which does mitigate against
things ending up getting used in ways that cause problems but people are
still going to worry about things ending up getting relied on one way or
another.

That said it's not clear to me that this will record anything beyond the
pointer directly in the trace buffer so the value might not be useful
for terribly long, that itself feels like it might not be as robust an
interface as it should be.

--Qxx1br4bt0+wmkIi
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmBGJ0QACgkQJNaLcl1U
h9DSHwf+KF0FUBNraZjSJH0LKhgVabREZDAIk49+0JDFJ3gxP3ZFmZvbYrzq3TtY
nf+q2IsxMY2GIBNfr5QcjbT6DRU4OCJHv5iel7NH0fyJlwvIKTDRziA5pydoMUn4
nP5x1FdGvHi4GO7cmi7FQ3uB43jencWcDGIi1Pwz1Hppe1wSnmcdffapBCZmkcEk
eS4kZmklNElayMTdVYSukcUwI+V4gJTkKQNfGkACSFc2dWq0yLqgb62kL7mmhdXp
5RhKBxz4snpjXwiNiBbkgU0wUwF77vSm5hs2eJQu10e9vul3d/1zFrIDPLWX09sr
0VWy+6st5LjE4F79ooXkzyyL97LFgw==
=wHoA
-----END PGP SIGNATURE-----

--Qxx1br4bt0+wmkIi--