All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v7 06/10] x86/hw_breakpoint: Unify breakpoint install/uninstall
Date: Wed, 15 Jul 2026 01:59:36 +0000	[thread overview]
Message-ID: <20260715015936.784251F000E9@smtp.kernel.org> (raw)
In-Reply-To: <178407990855.95826.6451473316040346375.stgit@devnote2>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing compiler memory barrier during hardware breakpoint disable allows reordering that breaks NMI safety.
--

--- Patch [6]: [PATCH v7 06/10] x86/hw_breakpoint: Unify breakpoint install/uninstall ---
Note: The format of this review has been altered to a free-form summary due to restrictions on quoting the original patch text.

commit 16356ea39e4e8d100873d56e9298c633a58cb692
Author: Jinchao Wang <wangjinchao600@gmail.com>
x86/hw_breakpoint: Unify breakpoint install/uninstall

This patch consolidates breakpoint management to reduce code duplication and
binary size. It introduces manage_bp_slot() and setup_hwbp() to handle the
logic previously duplicated in install and uninstall functions.

File: arch/x86/kernel/hw_breakpoint.c
Function: setup_hwbp()

[Severity: High]
Could the repositioning of the barrier() in the disable path introduce a
race condition with NMI handlers?

In setup_hwbp(), when enable is false, the code writes to the per-CPU cpu_dr7
cache after updating the hardware DR7 register via set_debugreg(). However,
the barrier() call was placed before set_debugreg().

Since neither the native set_debugreg() nor this_cpu_write() provides a
compiler memory clobber by default, is the compiler permitted to reorder
this_cpu_write(cpu_dr7, dr7) to occur before the hardware set_debugreg()
update?

If an NMI interrupts exactly between the reordered memory write to cpu_dr7
and the hardware DR7 register update, the NMI entry code (local_db_save)
would observe cpu_dr7 as zero and skip clearing the hardware DR7 register.
If the NMI handler then touches the watched address, would this trigger an
infinite #DB exception recursion and crash the system?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178407983818.95826.12714571928538799781.stgit@devnote2?part=6

  reply	other threads:[~2026-07-15  1:59 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15  1:43 [PATCH v7 00/10] tracing: wprobe: x86: Add wprobe for watchpoint Masami Hiramatsu (Google)
2026-07-15  1:44 ` [PATCH v7 01/10] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint Masami Hiramatsu (Google)
2026-07-15  2:01   ` sashiko-bot
2026-07-15  1:44 ` [PATCH v7 02/10] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires Masami Hiramatsu (Google)
2026-07-15  1:57   ` sashiko-bot
2026-07-15  1:44 ` [PATCH v7 03/10] selftests: tracing: Add a basic testcase for wprobe Masami Hiramatsu (Google)
2026-07-15  1:52   ` sashiko-bot
2026-07-15  1:44 ` [PATCH v7 04/10] selftests: tracing: Add syntax " Masami Hiramatsu (Google)
2026-07-15  1:44 ` [PATCH v7 05/10] tracing: wprobe: Use a new seq_print_ip_sym_offset() wrapper Masami Hiramatsu (Google)
2026-07-15  2:02   ` sashiko-bot
2026-07-15  1:45 ` [PATCH v7 06/10] x86/hw_breakpoint: Unify breakpoint install/uninstall Masami Hiramatsu (Google)
2026-07-15  1:59   ` sashiko-bot [this message]
2026-07-15  1:45 ` [PATCH v7 07/10] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint Masami Hiramatsu (Google)
2026-07-15  2:13   ` sashiko-bot
2026-07-15  1:45 ` [PATCH v7 08/10] HWBP: Add modify_wide_hw_breakpoint_local() API Masami Hiramatsu (Google)
2026-07-15  2:00   ` sashiko-bot
2026-07-15  1:45 ` [PATCH v7 09/10] tracing: wprobe: Add wprobe event trigger Masami Hiramatsu (Google)
2026-07-15  2:05   ` sashiko-bot
2026-07-15  1:45 ` [PATCH v7 10/10] selftests: ftrace: Add wprobe trigger testcase Masami Hiramatsu (Google)
2026-07-15  2:16   ` sashiko-bot

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=20260715015936.784251F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.