All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Mukesh Kumar Chaurasiya <mkchauras@gmail.com>,
	catalin.marinas@arm.com, will@kernel.org, maddy@linux.ibm.com,
	mpe@ellerman.id.au, npiggin@gmail.com, chleroy@kernel.org,
	ritesh.list@gmail.com, sshegde@linux.ibm.com, pjw@kernel.org,
	palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr,
	hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com,
	borntraeger@linux.ibm.com, svens@linux.ibm.com, tglx@kernel.org,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	x86@kernel.org, hpa@zytor.com, ojeda@kernel.org,
	boqun@kernel.org, gary@garyguo.net, bjorn3_gh@protonmail.com,
	lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com,
	tmgross@umich.edu, dakr@kernel.org, daniel.almeida@collabora.com,
	tamird@kernel.org, acourbot@nvidia.com, work@onurozkan.dev,
	nathan@kernel.org, ndesaulniers@google.com, morbo@google.com,
	justinstitt@google.com, jszhang@kernel.org, japo@linux.ibm.com,
	jpoimboe@kernel.org, seanjc@google.com, pmladek@suse.com,
	thuth@redhat.com, ynorov@nvidia.com, joelagnelf@nvidia.com,
	david@davidgow.net, fujita.tomonori@gmail.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	rust-for-linux@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [RFC] rust: kernel: Add KUnit tests for ARCH_WARN_ASM bug table emission
Date: Thu, 24 Sep 2026 14:52:53 +0200	[thread overview]
Message-ID: <20260924125253.GY776954@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <arUbw_TIA0Uj7oXT@J2N7QTR9R3>

On Thu, Sep 24, 2026 at 01:46:59PM +0100, Mark Rutland wrote:

> > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > index 914282016069..3d8a969eab5f 100644
> > --- a/arch/arm64/kernel/traps.c
> > +++ b/arch/arm64/kernel/traps.c
> > @@ -987,14 +987,13 @@ void do_serror(struct pt_regs *regs, unsigned long esr)
> >  #ifdef CONFIG_GENERIC_BUG
> >  int is_valid_bugaddr(unsigned long addr)
> >  {
> > -	/*
> > -	 * bug_brk_handler() only called for BRK #BUG_BRK_IMM.
> > -	 * So the answer is trivial -- any spurious instances with no
> > -	 * bug table entry will be rejected by report_bug() and passed
> > -	 * back to the debug-monitors code and handled as a fatal
> > -	 * unexpected debug exception.
> > -	 */
> > -	return 1;
> > +	u32 insn;
> > +
> > +	if (aarch64_insn_read((u32 *)addr, &insn))
> > +		return 0;
> > +
> > +	/* Match ASM_BUG_FLAGS() / BUG() instructions. */
> > +	return insn == (0xd4200000 | (BUG_BRK_IMM << 5));
> >  }
> >  #endif
> 
> What problem is this trying to solve?
> 
> Mukesh's original patch seems to be trying to check whether we compiled
> things correctly (such that the bug_table entry points at the expected
> insturction), and I don't think that warrants a runtime check, but (as
> per my reply to him) I don't understand why that's a thing to check in
> the first place.
> 
> On arm64 we only call is_valid_bugaddr() under do_el1_brk64() ->
> call_el1_break_hook() -> bug_brk_handler(). We only call
> bug_brk_handler() when HW has taken a BRK exception with the BUG_BRK_IMM
> immediate, and that can only be triggered by executing the right
> instruction (BRK #BUG_BRK_IMM), which we ONLY use for BUG(). and WARN().
> 
> So I don't see why any change is necessary here.
> 
> If the HW has somehow taken a BRK exception from any other instruction
> (which would presumably be a HW bug), there won't be a bug_table entry,
> and so __report_bug() will fail to find an entry, and will return
> BUG_TRAP_TYPE_NONE, same as for the !is_valid_bugaddr() case.

Right, so I wasn't at all sure what he was doing, Rust is still mostly
unreadable garbage to me.

But I figured I'd much rather see him use is_valid_bugaddr() to do
runtime checks than whatever dodgy games he was playing with that label.

So *if* doing runtime checks is what is wanted, then is_valid_bugaddr()
needs to actually work, because then it grows a user outside of the BRK
handler.

Lots of IFs etc, but since I am having a head-ache and am mostly
useless, I figured I'd distract myself doing whatever :-)

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Mukesh Kumar Chaurasiya <mkchauras@gmail.com>,
	catalin.marinas@arm.com, will@kernel.org, maddy@linux.ibm.com,
	mpe@ellerman.id.au, npiggin@gmail.com, chleroy@kernel.org,
	ritesh.list@gmail.com, sshegde@linux.ibm.com, pjw@kernel.org,
	palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr,
	hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com,
	borntraeger@linux.ibm.com, svens@linux.ibm.com, tglx@kernel.org,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	x86@kernel.org, hpa@zytor.com, ojeda@kernel.org,
	boqun@kernel.org, gary@garyguo.net, bjorn3_gh@protonmail.com,
	lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com,
	tmgross@umich.edu, dakr@kernel.org, daniel.almeida@collabora.com,
	tamird@kernel.org, acourbot@nvidia.com, work@onurozkan.dev,
	nathan@kernel.org, ndesaulniers@google.com, morbo@google.com,
	justinstitt@google.com, jszhang@kernel.org, japo@linux.ibm.com,
	jpoimboe@kernel.org, seanjc@google.com, pmladek@suse.com,
	thuth@redhat.com, ynorov@nvidia.com, joelagnelf@nvidia.com,
	david@davidgow.net, fujita.tomonori@gmail.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	rust-for-linux@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [RFC] rust: kernel: Add KUnit tests for ARCH_WARN_ASM bug table emission
Date: Thu, 24 Sep 2026 14:52:53 +0200	[thread overview]
Message-ID: <20260924125253.GY776954@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <arUbw_TIA0Uj7oXT@J2N7QTR9R3>

On Thu, Sep 24, 2026 at 01:46:59PM +0100, Mark Rutland wrote:

> > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > index 914282016069..3d8a969eab5f 100644
> > --- a/arch/arm64/kernel/traps.c
> > +++ b/arch/arm64/kernel/traps.c
> > @@ -987,14 +987,13 @@ void do_serror(struct pt_regs *regs, unsigned long esr)
> >  #ifdef CONFIG_GENERIC_BUG
> >  int is_valid_bugaddr(unsigned long addr)
> >  {
> > -	/*
> > -	 * bug_brk_handler() only called for BRK #BUG_BRK_IMM.
> > -	 * So the answer is trivial -- any spurious instances with no
> > -	 * bug table entry will be rejected by report_bug() and passed
> > -	 * back to the debug-monitors code and handled as a fatal
> > -	 * unexpected debug exception.
> > -	 */
> > -	return 1;
> > +	u32 insn;
> > +
> > +	if (aarch64_insn_read((u32 *)addr, &insn))
> > +		return 0;
> > +
> > +	/* Match ASM_BUG_FLAGS() / BUG() instructions. */
> > +	return insn == (0xd4200000 | (BUG_BRK_IMM << 5));
> >  }
> >  #endif
> 
> What problem is this trying to solve?
> 
> Mukesh's original patch seems to be trying to check whether we compiled
> things correctly (such that the bug_table entry points at the expected
> insturction), and I don't think that warrants a runtime check, but (as
> per my reply to him) I don't understand why that's a thing to check in
> the first place.
> 
> On arm64 we only call is_valid_bugaddr() under do_el1_brk64() ->
> call_el1_break_hook() -> bug_brk_handler(). We only call
> bug_brk_handler() when HW has taken a BRK exception with the BUG_BRK_IMM
> immediate, and that can only be triggered by executing the right
> instruction (BRK #BUG_BRK_IMM), which we ONLY use for BUG(). and WARN().
> 
> So I don't see why any change is necessary here.
> 
> If the HW has somehow taken a BRK exception from any other instruction
> (which would presumably be a HW bug), there won't be a bug_table entry,
> and so __report_bug() will fail to find an entry, and will return
> BUG_TRAP_TYPE_NONE, same as for the !is_valid_bugaddr() case.

Right, so I wasn't at all sure what he was doing, Rust is still mostly
unreadable garbage to me.

But I figured I'd much rather see him use is_valid_bugaddr() to do
runtime checks than whatever dodgy games he was playing with that label.

So *if* doing runtime checks is what is wanted, then is_valid_bugaddr()
needs to actually work, because then it grows a user outside of the BRK
handler.

Lots of IFs etc, but since I am having a head-ache and am mostly
useless, I figured I'd distract myself doing whatever :-)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2026-09-24 12:53 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  5:50 [RFC] rust: kernel: Add KUnit tests for ARCH_WARN_ASM bug table emission Mukesh Kumar Chaurasiya (IBM)
2026-09-22  5:50 ` Mukesh Kumar Chaurasiya (IBM)
2026-09-22  6:47 ` Peter Zijlstra
2026-09-22  6:47   ` Peter Zijlstra
2026-09-23  6:28   ` Mukesh Kumar Chaurasiya
2026-09-23  6:28     ` Mukesh Kumar Chaurasiya
2026-09-24 11:15     ` Peter Zijlstra
2026-09-24 11:15       ` Peter Zijlstra
2026-09-24 12:46       ` Mark Rutland
2026-09-24 12:46         ` Mark Rutland
2026-09-24 12:52         ` Peter Zijlstra [this message]
2026-09-24 12:52           ` Peter Zijlstra
2026-09-24 13:05           ` Mark Rutland
2026-09-24 13:05             ` Mark Rutland
2026-09-25  6:32             ` Mukesh Kumar Chaurasiya
2026-09-25  6:32               ` Mukesh Kumar Chaurasiya
2026-09-24 15:13       ` Heiko Carstens
2026-09-24 15:13         ` Heiko Carstens
2026-09-25  6:36         ` Mukesh Kumar Chaurasiya
2026-09-25  6:36           ` Mukesh Kumar Chaurasiya
2026-09-25  8:44           ` Heiko Carstens
2026-09-25  8:44             ` Heiko Carstens
2026-09-25  6:38       ` Mukesh Kumar Chaurasiya
2026-09-25  6:38         ` Mukesh Kumar Chaurasiya
2026-09-24 12:31 ` Mark Rutland
2026-09-24 12:31   ` Mark Rutland
2026-09-25  6:21   ` Mukesh Kumar Chaurasiya
2026-09-25  6:21     ` Mukesh Kumar Chaurasiya
2026-09-25  8:06     ` Mark Rutland
2026-09-25  8:06       ` Mark Rutland
2026-09-24 13:00 ` Christophe Leroy (CS GROUP)
2026-09-24 13:00   ` Christophe Leroy (CS GROUP)
2026-09-25  6:34   ` Mukesh Kumar Chaurasiya
2026-09-25  6:34     ` Mukesh Kumar Chaurasiya

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=20260924125253.GY776954@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=agordeev@linux.ibm.com \
    --cc=alex@ghiti.fr \
    --cc=aliceryhl@google.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=chleroy@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@davidgow.net \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=japo@linux.ibm.com \
    --cc=joelagnelf@nvidia.com \
    --cc=jpoimboe@kernel.org \
    --cc=jszhang@kernel.org \
    --cc=justinstitt@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=llvm@lists.linux.dev \
    --cc=lossin@kernel.org \
    --cc=maddy@linux.ibm.com \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mkchauras@gmail.com \
    --cc=morbo@google.com \
    --cc=mpe@ellerman.id.au \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=npiggin@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=pmladek@suse.com \
    --cc=ritesh.list@gmail.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=sshegde@linux.ibm.com \
    --cc=svens@linux.ibm.com \
    --cc=tamird@kernel.org \
    --cc=tglx@kernel.org \
    --cc=thuth@redhat.com \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    --cc=work@onurozkan.dev \
    --cc=x86@kernel.org \
    --cc=ynorov@nvidia.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.