On Mon, 1 Sep 2025 18:23:44 +0800 Jinchao Wang <wangjinchao...@gmail.com> wrote:
> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote: > > Hi Jinchao, > > > Hi Masami, > > > On Mon, 18 Aug 2025 20:26:07 +0800 > > Jinchao Wang <wangjinchao...@gmail.com> wrote: > > > >> Add arch_reinstall_hw_breakpoint() to enable atomic context modification > >> of hardware breakpoint parameters without deallocating and reallocating > >> the breakpoint slot. > >> > >> The existing arch_install_hw_breakpoint() allocates a new debug register > >> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some > >> use cases require modifying breakpoint parameters (address, length, type) > >> atomically without losing the allocated slot, particularly when operating > >> in atomic contexts where allocation might fail or be unavailable. > >> > >> This is particularly useful for debugging tools like kstackwatch that > >> need to dynamically update breakpoint targets in atomic contexts while > >> maintaining consistent hardware state. > >> > > > > I'm also trying to find this interface for my wprobe. So the idea is good. > > But this looks hacky and only for x86. I think the interface should be > > more generic and do not use this arch internal function directly. > > > > I agree with your point about the architectural dependency. I have been > considering this problem not only for the hardware breakpoint > reinstallation, > but also for other related parts of the series, such as canary finding and > stack address resolving. These parts also rely on arch-specific code. Yes, even though, the hw-breakpoint is an independent feature. Directly using arch_*() functions (which are expected to be used internally) introduces a hidden dependency between these two components and looses maintainability. > > It seems that the slot is allocated by "type", thus, if this reinstall > > hwbp without deallocate/allocate slot, it must NOT change the type. > > See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking > > whether the architecture support that interface. > > > Regarding the slot allocation, I would like to clarify my point. I > believe the > event->attr.type should not be changed when reinstalling a hardware > breakpoint, as this defines the fundamental nature of the event. The type > must always be PERF_TYPE_BREAKPOINT. > > The event->attr.bp_type, however, can be changed. For example, from a > HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and > reallocate the slot. This is useful for future applications, even though the > current use case for KStackWatch only requires HW_BREAKPOINT_W. I understand your point, so it also needs another wrapper which checks the type is compatible on the architecture. > > By the way, I have sent an updated series. > https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao...@gmail.com/ Yeah, OK, let me review the series. Thanks for update! > > Thank you again for your valuable review. > -- > Best regards, > Jinchao -- Masami Hiramatsu (Google) <mhira...@kernel.org>
