* [PATCH v2 0/6] rtla: Migrate to libsubcmd for command line option parsing
From: Tomas Glozar @ 2026-05-21 14:18 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar
Cc: John Kacur, Luis Goncalves, Crystal Wood, Costa Shulyupin,
Wander Lairson Costa, Ivan Pravdin, Namhyung Kim, Ian Rogers,
Arnaldo Carvalho de Melo, LKML, linux-trace-kernel,
linux-perf-users
[ CC to linux-perf-users for the libsubcmd code changes ]
rtla currently uses its own implementation that uses getopt_long() to
parse command-line arguments.
Migrate rtla to use libsubcmd for command line argument parsing,
similarly to what is already done by other tools like perf, bpftool,
and objtool. Among other benefits, this allows help messages to be
generated automatically rather than having to be typed out manually
for each tool.
libsubcmd is extended with a flag to parse optarg from separate
argument if a new flag is turned on. Without the flag, the old behavior
is preserved. That keeps the parsing working for tools that use
positional arguments, and allows RTLA to keep its flexible syntax for -C
and -t options and their long variants, --cgroup and --trace-output.
Another flag is added to disable automatic definition of --no-xy for
every option --xy and vice versa, which overlaps for RTLA's --irq and
--thread options.
The new implementation is moved into a separate file, cli.c, together
with a tiny header counterpart, cli.h. This helps separate the parsing
logic, which has little in common with the rest of RTLA, in a separate
module. Another new file, cli_p.h, is used as a private header to contain
macros and static function declarations that are also used by unit tests
next to cli.c, but should not be imported from elsewhere.
Macros to generate struct option array fields for libsubcmd's
parse_args() are used to preserve the consolidation of argument parsing
code across different RTLA tools. Kernel and user threads are, as
an exception, treated as common, although they are currently implemented
for timerlat only, in line with earlier consolidation changes.
The test suite is expanded to include two levels of unit tests, one testing
the already existing tool_parse_args() functions, one tests option callbacks,
which are a new level of the CLI parser added in this patchset. This helps
to verify that no regressions are caused by this refactoring.
I expect more improvements to the code being possible in the future,
like creating macros for option groups to further deduplicate the code,
reducing the amount of extra code in the _parse_args() functions, or
implementing support for unsetting options (which is currently only
supported for those that do not use a custom callback).
v2 changes:
- Two unit test suites are added to cover regressions, after several
options were broken by the v1. The test suites cover all parsing
issues reported in the v1 as well as those found during additional
testing.
- Return value of all paths that print help, including those that
are handled in RTLA, are set to libsubcmd's help exit code of 129.
Previously, only the tool help returned 129. While some other tools
(e.g. bpftool) do that, RTLA unifies those for consistency. The return
value is also added to the corresponding section in documentation.
- Incorrect parsing of --no-irq and --no-thread is fixed using a newly
added libsubcmd option flag.
- Incorrect parsing of -n and -u timerlat options (which erroneously
required an argument in v1) is fixed.
- A now stale declaration of removed function common_parse_options()
is removed from common.h.
- Segmentation fault on abbreviated --help (e.g. --he) is fixed.
- Incorrect formatting of OPT_END macro (spurious tab) is fixed.
- All opt_* callbacks now reject unimplemented unset (--no-) correctly.
- opt_trigger_cb() and opt_filter_cb() now take only the required events
field, not the whole params structure.
- Spurious opt_osnoise_threshold_cb() which is actually just a wrapper for
opt_llong_callback() is removed.
- Old off-by-one typo is fixed in --dma-latency and -E/--entries error
messages, to make it consistent with the newly added unit tests.
- Fix a bug in Makefile that defined LIBSUBCMD_INCLUDES as -I... and then
used it as a target name: define it as the path only, and then add -I$(...)
to CFLAGS, as this is the only include path that is generated during the build
itself.
v1: https://lore.kernel.org/linux-trace-kernel/20260320150651.51057-1-tglozar@redhat.com/T
Tomas Glozar (6):
rtla: Add libsubcmd dependency
tools subcmd: support optarg as separate argument
tools subcmd: allow parsing distinct --opt and --no-opt
rtla: Parse cmdline using libsubcmd
rtla/tests: Add unit tests for _parse_args() functions
rtla/tests: Add unit tests for CLI option callbacks
Documentation/tools/rtla/common_appendix.txt | 7 +-
tools/lib/subcmd/parse-options.c | 63 +-
tools/lib/subcmd/parse-options.h | 4 +
tools/tracing/rtla/.gitignore | 2 +
tools/tracing/rtla/Makefile | 66 +-
tools/tracing/rtla/src/Build | 2 +-
tools/tracing/rtla/src/cli.c | 537 +++++++++++++
tools/tracing/rtla/src/cli.h | 9 +
tools/tracing/rtla/src/cli_p.h | 670 +++++++++++++++++
tools/tracing/rtla/src/common.c | 109 ---
tools/tracing/rtla/src/common.h | 27 +-
tools/tracing/rtla/src/osnoise.c | 9 +-
tools/tracing/rtla/src/osnoise_hist.c | 221 +-----
tools/tracing/rtla/src/osnoise_top.c | 200 +----
tools/tracing/rtla/src/rtla.c | 92 ---
tools/tracing/rtla/src/timerlat.c | 9 +-
tools/tracing/rtla/src/timerlat.h | 6 +-
tools/tracing/rtla/src/timerlat_hist.c | 317 +-------
tools/tracing/rtla/src/timerlat_top.c | 286 +------
tools/tracing/rtla/src/utils.c | 28 +-
tools/tracing/rtla/src/utils.h | 9 +-
tools/tracing/rtla/tests/hwnoise.t | 2 +-
tools/tracing/rtla/tests/osnoise.t | 6 +-
tools/tracing/rtla/tests/timerlat.t | 6 +-
tools/tracing/rtla/tests/unit/Build | 5 +
tools/tracing/rtla/tests/unit/Makefile.unit | 4 +-
.../rtla/tests/unit/cli_opt_callback.c | 704 ++++++++++++++++++
.../rtla/tests/unit/cli_params_assert.h | 68 ++
.../rtla/tests/unit/osnoise_hist_cli.c | 557 ++++++++++++++
.../tracing/rtla/tests/unit/osnoise_top_cli.c | 503 +++++++++++++
.../rtla/tests/unit/timerlat_hist_cli.c | 702 +++++++++++++++++
.../rtla/tests/unit/timerlat_top_cli.c | 634 ++++++++++++++++
tools/tracing/rtla/tests/unit/unit_tests.c | 13 +
33 files changed, 4566 insertions(+), 1311 deletions(-)
create mode 100644 tools/tracing/rtla/src/cli.c
create mode 100644 tools/tracing/rtla/src/cli.h
create mode 100644 tools/tracing/rtla/src/cli_p.h
delete mode 100644 tools/tracing/rtla/src/rtla.c
create mode 100644 tools/tracing/rtla/tests/unit/cli_opt_callback.c
create mode 100644 tools/tracing/rtla/tests/unit/cli_params_assert.h
create mode 100644 tools/tracing/rtla/tests/unit/osnoise_hist_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/osnoise_top_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/timerlat_hist_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/timerlat_top_cli.c
--
2.54.0
^ permalink raw reply
* Re: [PATCH v20 02/10] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
From: Steven Rostedt @ 2026-05-21 14:17 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Catalin Marinas, Will Deacon,
Ian Rogers, sashiko-bot@kernel.org,
sashiko-reviews@lists.linux.dev
In-Reply-To: <20260520185017.219342323@kernel.org>
[
Replying to Sashiko from v19: https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/177751968499.2136606.17388366710182662849.stgit%40mhiramat.tok.corp.google.com
]
> > @@ -1892,9 +1895,27 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
> > * subbuf_size is considered invalid.
> > */
> > tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
> > - if (tail > meta->subbuf_size - BUF_PAGE_HDR_SIZE)
> > - return -1;
> > - return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
> > + if (tail <= meta->subbuf_size - BUF_PAGE_HDR_SIZE)
> > + ret = rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
> Since tail is read from untrusted persistent memory, does this call evaluating
The persistent memory is not untrusted.
> event headers verify enough bytes remain in the sub-buffer?
> If tail is set to the maximum allowed value, and preceding events are crafted
> so that the last event starts at tail - 1, the event pointer inside
> rb_read_data_buffer() will point to the very last byte.
> Calling rb_event_length(event) will evaluate event->type_len via a 32-bit
> read which could read past the page boundary and cause a regression if the
> adjacent page is unmapped.
> Should the parsing loop verify tail - e >= sizeof(struct ring_buffer_event)
> before evaluating the event?
> > + else
> > + ret = -1;
> > +
> > + /*
> > + * The timestamp must be greater than @prev_ts and smaller than @next_ts.
> > + * Since this function works in both forward (verify) and reverse (unwind)
> > + * loop, we don't know both @prev_ts and @next_ts at the same time.
> > + * So use the known boundary as the boundary.
> > + */
> > + if (ret < 0 || (prev_ts && prev_ts > ts) || (next_ts && ts > next_ts)) {
> Does this timestamp validation properly enforce strictly monotonic page ordering?
> Inside rb_validate_buffer(), ts is updated by rb_read_data_buffer() to
> the timestamp of the last event in the current page. The condition
> (prev_ts && prev_ts > ts) only checks that the previous page's start time
> is not greater than the current page's end time.
> Could a corrupted page with a start timestamp earlier than the previous
> page incorrectly pass validation if its final event timestamp is large enough?
> Would it prevent this regression to enforce that the current page starts after
> the previous page by checking prev_ts > dpage->time_stamp?
This may be an enhancement but shouldn't be critical for what is happening now.
> > + local_set(&bpage->entries, 0);
> [ ... ]
> > @@ -1915,25 +1936,29 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> > orig_head = head_page = cpu_buffer->head_page;
> > orig_reader = cpu_buffer->reader_page;
> >
> > - /* Do the reader page first */
> > - ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu, meta);
> > + /* Do the head page first */
> > + ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta, 0, 0);
> > + if (ret < 0) {
> > + pr_info("Ring buffer meta [%d] invalid head page detected\n",
> > + cpu_buffer->cpu);
> > + goto skip_rewind;
> > + }
> > + ts = head_page->page->time_stamp;
> > +
> > + /* Do the reader page - reader must be previous to head. */
> > + ret = rb_validate_buffer(orig_reader, cpu_buffer->cpu, meta, 0, ts);
> If rb_validate_buffer() for the head page fails, we take the goto skip_rewind
> path. Since skip_rewind jumps past this orig_reader validation, and the
> iteration loop explicitly skips orig_reader:
> /* Iterate until finding the commit page */
> for (i = 0; i < meta->nr_subbufs + 1; i++, rb_inc_page(&head_page)) {
> /* The original reader page has already been checked/counted. */
> if (head_page == orig_reader)
> continue;
> does this mean the reader page escapes validation entirely, introducing a
> regression?
This is a bug and needs to be fixed. If the head page is invalid it
shouldn't skip the rewind but instead jump to the "invalid:" label.
> If an attacker provides a corrupted persistent ring buffer with an invalid
> head page, would the kernel blindly trust the unvalidated commit length
> and event data of the reader page?
Not worried about attackers but I am worried about corrupted memory.
-- Steve
^ permalink raw reply
* [PATCH] tracing: Move trace_iterator_increment() into trace_find_next_entry_inc()
From: Steven Rostedt @ 2026-05-21 13:50 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers
From: Steven Rostedt <rostedt@goodmis.org>
trace_iterator_increment() is only called from trace_find_next_entry_inc().
It's a small enough function that really doesn't need to be separated.
Move the code from trace_iterator_increment() into
trace_find_next_entry_inc() and remove trace_iterator_increment().
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4573f65d68ce..0b43e88ac378 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2338,15 +2338,6 @@ void trace_last_func_repeats(struct trace_array *tr,
__buffer_unlock_commit(buffer, event);
}
-static void trace_iterator_increment(struct trace_iterator *iter)
-{
- struct ring_buffer_iter *buf_iter = trace_buffer_iter(iter, iter->cpu);
-
- iter->idx++;
- if (buf_iter)
- ring_buffer_iter_advance(buf_iter);
-}
-
static struct trace_entry *
peek_next_entry(struct trace_iterator *iter, int cpu, u64 *ts,
unsigned long *lost_events)
@@ -2676,11 +2667,17 @@ struct trace_entry *trace_find_next_entry(struct trace_iterator *iter,
/* Find the next real entry, and increment the iterator to the next entry */
void *trace_find_next_entry_inc(struct trace_iterator *iter)
{
+ struct ring_buffer_iter *buf_iter;
+
iter->ent = __find_next_entry(iter, &iter->cpu,
&iter->lost_events, &iter->ts);
- if (iter->ent)
- trace_iterator_increment(iter);
+ if (iter->ent) {
+ iter->idx++;
+ buf_iter = trace_buffer_iter(iter, iter->cpu);
+ if (buf_iter)
+ ring_buffer_iter_advance(buf_iter);
+ }
return iter->ent ? iter : NULL;
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v6 05/43] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Fuad Tabba @ 2026-05-21 13:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ag8JIlHjohAOC3-g@google.com>
On Thu, 21 May 2026 at 14:31, Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, May 21, 2026, Fuad Tabba wrote:
> > On Wed, 20 May 2026 at 22:44, Ackerley Tng <ackerleytng@google.com> wrote:
> > >
> > > Fuad Tabba <tabba@google.com> writes:
> > >
> > > >
> > > > [...snip...]
> > > >
> > > >> +unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> > > >> +{
> > > >> + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> > > >> + struct inode *inode;
> > > >> +
> > > >> + /*
> > > >> + * If this gfn has no associated memslot, there's no chance of the gfn
> > > >> + * being backed by private memory, since guest_memfd must be used for
> > > >> + * private memory, and guest_memfd must be associated with some memslot.
> > > >> + */
> > > >> + if (!slot)
> > > >> + return 0;
> > > >> +
> > > >> + CLASS(gmem_get_file, file)(slot);
> > > >> + if (!file)
> > > >> + return 0;
> > > >> +
> > > >> + inode = file_inode(file);
> > > >> +
> > > >> + /*
> > > >> + * Rely on the maple tree's internal RCU lock to ensure a
> > > >> + * stable result. This result can become stale as soon as the
> > > >> + * lock is dropped, so the caller _must_ still protect
> > > >> + * consumption of private vs. shared by checking
> > > >> + * mmu_invalidate_retry_gfn() under mmu_lock to serialize
> > > >> + * against ongoing attribute updates.
> > > >> + */
> > > >> + return kvm_gmem_get_attributes(inode, kvm_gmem_get_index(slot, gfn));
> > > >> +}
> > > >
> > > > Doesn't this imply that all consumers of kvm_mem_is_private() should
> > > > validate the result using mmu_lock and the invalidation sequence?
> > >
> > > Let me know how I can improve the comment.
> >
> > Given Sean's context, the comment is good I think. I would quibble
> > with the the "_must_ still protect" phrasing being a bit too strict.
> >
> > Maybe just soften it slightly to acknowledge the exception? Something like:
> >
> > * lock is dropped, so callers that require a strict result _must_ protect
> > * consumption of private vs. shared by checking mmu_invalidate_retry_gfn()
> > * under mmu_lock to serialize against ongoing attribute updates. Callers
> > * doing lockless reads must be able to tolerate a stale result.
> >
> > That aligns the comment with how KVM is actually using it today. That
> > said, this is nitpicking. Feel free to use or ignore.
>
> Hmm, I wonder if we can figure out a way to consolidate some documentation,
> because this is _exactly_ the same pattern that x86's host_pfn_mapping_level()
> deals with (see its big comment below).
>
> There's also the stale comment in kvm_invalidate_memslot(), which, stating the
> obvious, speaks to the memslot+SRCU side of things.
>
> Maybe it makes sense to to find a central location for one giant comment about
> how how MMU notifier events and memslot+SRCU protections work? And then refer
> to that in paths where some asset needs to be tied into MMU notifiers and/or
> memslots+SRCU?
>
> [*] https://lore.kernel.org/all/agcbWe8s9lmPuJwG@google.com
This would fix a few related issues at once. sgtm
/fuad
/fuad
>
> /*
> * Lookup the mapping level for @gfn in the current mm.
> *
> * WARNING! Use of host_pfn_mapping_level() requires the caller and the end
> * consumer to be tied into KVM's handlers for MMU notifier events!
> *
> * There are several ways to safely use this helper:
> *
> * - Check mmu_invalidate_retry_gfn() after grabbing the mapping level, before
> * consuming it. In this case, mmu_lock doesn't need to be held during the
> * lookup, but it does need to be held while checking the MMU notifier.
> *
> * - Hold mmu_lock AND ensure there is no in-progress MMU notifier invalidation
> * event for the hva. This can be done by explicit checking the MMU notifier
> * or by ensuring that KVM already has a valid mapping that covers the hva.
> *
> * - Do not use the result to install new mappings, e.g. use the host mapping
> * level only to decide whether or not to zap an entry. In this case, it's
> * not required to hold mmu_lock (though it's highly likely the caller will
> * want to hold mmu_lock anyways, e.g. to modify SPTEs).
> *
> * Note! The lookup can still race with modifications to host page tables, but
> * the above "rules" ensure KVM will not _consume_ the result of the walk if a
> * race with the primary MMU occurs.
> */
^ permalink raw reply
* Re: [PATCHv3 04/12] uprobes/x86: Move optimized uprobe from nop5 to nop10
From: Peter Zijlstra @ 2026-05-21 13:35 UTC (permalink / raw)
To: Jiri Olsa
Cc: Oleg Nesterov, Ingo Molnar, Masami Hiramatsu, Andrii Nakryiko,
bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-5-jolsa@kernel.org>
On Thu, May 21, 2026 at 02:44:03PM +0200, Jiri Olsa wrote:
> Andrii reported an issue with optimized uprobes [1] that can clobber
> redzone area with call instruction storing return address on stack
> where user code may keep temporary data without adjusting rsp.
>
> Fixing this by moving the optimized uprobes on top of 10-bytes nop
> instruction, so we can squeeze another instruction to escape the
> redzone area before doing the call, like:
>
> lea -0x80(%rsp), %rsp
> call tramp
>
> Note the lea instruction is used to adjust the rsp register without
> changing the flags.
>
> We use nop10 and following transofrmation to optimized instructions
> above and back as suggested by Peterz [2].
>
> Optimize path (int3_update_optimize):
>
> 1) Initial state after set_swbp() installed the uprobe:
> cc 2e 0f 1f 84 00 00 00 00 00
>
> From offset 0 this is INT3 followed by the tail of the original
> 10-byte NOP.
>
> 2) Trap the call slot before rewriting the NOP tail:
> cc 2e 0f 1f 84 [cc] 00 00 00 00
>
> From offset 0 this traps on the uprobe INT3. A thread reaching
> offset 5 traps on the temporary INT3 instead of seeing a partially
> patched call.
>
> 3) Rewrite the LEA tail and call displacement, keeping both INT3 bytes:
> cc [8d 64 24 80] cc [d0 d1 d2 d3]
>
> From offset 0 and offset 5 this still traps. The bytes between
> them are not executable entry points while both traps are in place.
>
> 4) Restore the call opcode at offset 5:
> cc 8d 64 24 80 [e8] d0 d1 d2 d3
>
> From offset 0 this still traps. From offset 5 the instruction is
> the final CALL to the uprobe trampoline.
>
> 5) Publish the first LEA byte:
> [48] 8d 64 24 80 e8 d0 d1 d2 d3
>
> From offset 0 this is:
> lea -0x80(%rsp), %rsp
> call <uprobe-trampoline>
>
> Unoptimize path (int3_update_unoptimize):
>
> 1) Initial optimized state:
> 48 8d 64 24 80 e8 d0 d1 d2 d3
> Same as 5) above.
>
> 2) Trap new entries before restoring the NOP bytes:
> [cc] 8d 64 24 80 e8 d0 d1 d2 d3
>
> From offset 0 this traps. A thread that had already executed the
> LEA can still reach the intact CALL at offset 5.
>
> 3) Restore bytes 1..4 of the original NOP while keeping byte 0 trapped
> and byte 5 as CALL.
> cc [2e 0f 1f 84] e8 d0 d1 d2 d3
>
> From offset 0 this still traps. Offset 5 is still the CALL for any
> thread that was already past the first LEA byte.
>
> 4) Publish the first byte of the original NOP:
> [66] 2e 0f 1f 84 e8 d0 d1 d2 d3
>
> From offset 0 this is the restored 10-byte NOP; the CALL opcode and
> displacement are now only NOP operands. Offset 5 still decodes as
> CALL for a thread that was already there.
>
> Note as explained in [2] we need to use following nop10:
> PF1 PF2 ESC NOPL MOD SIB DISP32
> NOP10: 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 -- cs nopw 0x00000000(%rax,%rax,1)
>
> which means we need to allow 0x2e prefix which maps to INAT_PFX_CS
> attribute in is_prefix_bad function.
>
> The optimized uprobe performance stays the same:
>
> uprobe-nop : 3.129 ± 0.013M/s
> uprobe-push : 3.045 ± 0.006M/s
> uprobe-ret : 1.095 ± 0.004M/s
> --> uprobe-nop10 : 7.170 ± 0.020M/s
> uretprobe-nop : 2.143 ± 0.021M/s
> uretprobe-push : 2.090 ± 0.000M/s
> uretprobe-ret : 0.942 ± 0.000M/s
> --> uretprobe-nop10: 3.381 ± 0.003M/s
> usdt-nop : 3.245 ± 0.004M/s
> --> usdt-nop10 : 7.256 ± 0.023M/s
>
> @@ -893,48 +918,134 @@ static int verify_insn(struct page *page, unsigned long vaddr, uprobe_opcode_t *
> }
>
> /*
> + * Modify the optimized instruction by using INT3 breakpoints on SMP.
> * We completely avoid using stop_machine() here, and achieve the
> * synchronization using INT3 breakpoints and SMP cross-calls.
> * (borrowed comment from smp_text_poke_batch_finish)
> *
> + * The way it is done for optimization (int3_update_optimize):
> + * 1) Start with the uprobe INT3 trap already installed
> + * 2) Add an INT3 trap to the call slot
> + * 3) Update everything but the first byte and the call opcode
> + * 4) Replace the call slot INT3 by the call opcode
> + * 5) Replace the first INT3 by the first byte of the LEA instruction
> + *
> + * The way it is done for unoptimization (int3_update_unoptimize):
> + * 1) Start with the optimized uprobe lea/call instructions
> + * 2) Add an INT3 trap to the address that will be patched
> + * 3) Restore the NOP bytes before the call opcode
> + * 4) Replace the first INT3 by the first byte of the NOP instruction
> + *
> + * Note that unoptimization deliberately keeps the call opcode and displacement
> + * in bytes 5..9. Those bytes become operands of the restored 10-byte NOP.
> */
One important thing to note is that (as earlier noted by Andrii) the
CALL address is never changed. A new optimization pass will not change
the CALL instruction again.
If you noted this anywhere, I failed to find it. This is crucially
important for the correctness of the scheme and should not be emitted.
That is, please add something like:
"Since there is only a single uprobe-trampoline, the CALL instruction
will not be changed across unoptimization/optimization cycles.
Therefore, any task that is preempted at the CALL instruction is
guaranteed to observe that CALL and not anything else."
^ permalink raw reply
* Re: [PATCH v6 05/43] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Sean Christopherson @ 2026-05-21 13:31 UTC (permalink / raw)
To: Fuad Tabba
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTzLCD-dU-euZKgzwyEr2ecPqFDNutcaHm2fCDGA+MHVXA@mail.gmail.com>
On Thu, May 21, 2026, Fuad Tabba wrote:
> On Wed, 20 May 2026 at 22:44, Ackerley Tng <ackerleytng@google.com> wrote:
> >
> > Fuad Tabba <tabba@google.com> writes:
> >
> > >
> > > [...snip...]
> > >
> > >> +unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> > >> +{
> > >> + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> > >> + struct inode *inode;
> > >> +
> > >> + /*
> > >> + * If this gfn has no associated memslot, there's no chance of the gfn
> > >> + * being backed by private memory, since guest_memfd must be used for
> > >> + * private memory, and guest_memfd must be associated with some memslot.
> > >> + */
> > >> + if (!slot)
> > >> + return 0;
> > >> +
> > >> + CLASS(gmem_get_file, file)(slot);
> > >> + if (!file)
> > >> + return 0;
> > >> +
> > >> + inode = file_inode(file);
> > >> +
> > >> + /*
> > >> + * Rely on the maple tree's internal RCU lock to ensure a
> > >> + * stable result. This result can become stale as soon as the
> > >> + * lock is dropped, so the caller _must_ still protect
> > >> + * consumption of private vs. shared by checking
> > >> + * mmu_invalidate_retry_gfn() under mmu_lock to serialize
> > >> + * against ongoing attribute updates.
> > >> + */
> > >> + return kvm_gmem_get_attributes(inode, kvm_gmem_get_index(slot, gfn));
> > >> +}
> > >
> > > Doesn't this imply that all consumers of kvm_mem_is_private() should
> > > validate the result using mmu_lock and the invalidation sequence?
> >
> > Let me know how I can improve the comment.
>
> Given Sean's context, the comment is good I think. I would quibble
> with the the "_must_ still protect" phrasing being a bit too strict.
>
> Maybe just soften it slightly to acknowledge the exception? Something like:
>
> * lock is dropped, so callers that require a strict result _must_ protect
> * consumption of private vs. shared by checking mmu_invalidate_retry_gfn()
> * under mmu_lock to serialize against ongoing attribute updates. Callers
> * doing lockless reads must be able to tolerate a stale result.
>
> That aligns the comment with how KVM is actually using it today. That
> said, this is nitpicking. Feel free to use or ignore.
Hmm, I wonder if we can figure out a way to consolidate some documentation,
because this is _exactly_ the same pattern that x86's host_pfn_mapping_level()
deals with (see its big comment below).
There's also the stale comment in kvm_invalidate_memslot(), which, stating the
obvious, speaks to the memslot+SRCU side of things.
Maybe it makes sense to to find a central location for one giant comment about
how how MMU notifier events and memslot+SRCU protections work? And then refer
to that in paths where some asset needs to be tied into MMU notifiers and/or
memslots+SRCU?
[*] https://lore.kernel.org/all/agcbWe8s9lmPuJwG@google.com
/*
* Lookup the mapping level for @gfn in the current mm.
*
* WARNING! Use of host_pfn_mapping_level() requires the caller and the end
* consumer to be tied into KVM's handlers for MMU notifier events!
*
* There are several ways to safely use this helper:
*
* - Check mmu_invalidate_retry_gfn() after grabbing the mapping level, before
* consuming it. In this case, mmu_lock doesn't need to be held during the
* lookup, but it does need to be held while checking the MMU notifier.
*
* - Hold mmu_lock AND ensure there is no in-progress MMU notifier invalidation
* event for the hva. This can be done by explicit checking the MMU notifier
* or by ensuring that KVM already has a valid mapping that covers the hva.
*
* - Do not use the result to install new mappings, e.g. use the host mapping
* level only to decide whether or not to zap an entry. In this case, it's
* not required to hold mmu_lock (though it's highly likely the caller will
* want to hold mmu_lock anyways, e.g. to modify SPTEs).
*
* Note! The lookup can still race with modifications to host page tables, but
* the above "rules" ensure KVM will not _consume_ the result of the walk if a
* race with the primary MMU occurs.
*/
^ permalink raw reply
* Re: [PATCH v6 16/43] KVM: guest_memfd: Use actual size for invalidation in kvm_gmem_release()
From: Fuad Tabba @ 2026-05-21 13:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ag8BmtzxTlcuA_zy@google.com>
On Thu, 21 May 2026 at 13:59, Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, May 21, 2026, Fuad Tabba wrote:
> > Hi Ackerley,
> >
> > On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
> > <devnull+ackerleytng.google.com@kernel.org> wrote:
> > >
> > > From: Ackerley Tng <ackerleytng@google.com>
> > >
> > > __kvm_gmem_invalidate_begin() and __kvm_gmem_invalidate_end() actually do
> > > not specially handle -1ul. -1ul is used as a huge number, which legal
> > > indices do not exceed, and hence the invalidation works as expected.
> > >
> > > Since a later patch is going to make use of the exact range, calculate the
> > > size of the guest_memfd inode and use it as the end range for invalidating
> > > SPTEs.
> > >
> > > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> >
> > Want to look at what Sashiko has to say? Seems to be a real issue:
> >
> > https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=16
> >
> > If I understand correctly, the fix should simple: use
> > check_add_overflow() to validate the offset and size parameters in
> > kvm_gmem_bind()
> >
> > int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> > unsigned int fd, loff_t offset)
> > {
> > loff_t size = slot->npages << PAGE_SHIFT;
> > + loff_t end;
> > unsigned long start, end_index;
> > struct gmem_file *f;
> > ...
> > - if (offset < 0 || !PAGE_ALIGNED(offset) ||
> > - offset + size > i_size_read(inode))
> > + if (offset < 0 || !PAGE_ALIGNED(offset) ||
> > + check_add_overflow(offset, size, &end) ||
>
> Eww, TIL I'm not a fan of check_add_overflow(). Burying an out-param in an
> if-statement is nasty.
>
> > + end > i_size_read(inode))
>
> This is all rather silly. @offset and and @slot->npages are fundamentally
> unsigned values. I don't see any reason to convert them to signed values, only
> to convert them *back* to unsigned values (when stored in start/end, because xarrays
> operate on "unsigned long" indices).
>
> i_size_read() obviously has to return a positive value, so can't we just do this?
lgtm,
/fuad
>
> diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
> index a35a55571a2d..9c6dbb54e800 100644
> --- virt/kvm/guest_memfd.c
> +++ virt/kvm/guest_memfd.c
> @@ -640,9 +640,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
> }
>
> int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> - unsigned int fd, loff_t offset)
> + unsigned int fd, u64 offset)
> {
> - loff_t size = slot->npages << PAGE_SHIFT;
> + u64 size = slot->npages << PAGE_SHIFT;
> unsigned long start, end;
> struct gmem_file *f;
> struct inode *inode;
> @@ -664,8 +664,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>
> inode = file_inode(file);
>
> - if (offset < 0 || !PAGE_ALIGNED(offset) ||
> - offset + size > i_size_read(inode))
> + if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
> goto err;
>
> filemap_invalidate_lock(inode->i_mapping);
> diff --git virt/kvm/kvm_mm.h virt/kvm/kvm_mm.h
> index 9fcc5d5b7f8d..3cb5ef86d0d9 100644
> --- virt/kvm/kvm_mm.h
> +++ virt/kvm/kvm_mm.h
> @@ -72,7 +72,7 @@ int kvm_gmem_init(struct module *module);
> void kvm_gmem_exit(void);
> int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
> int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> - unsigned int fd, loff_t offset);
> + unsigned int fd, u64 offset);
> void kvm_gmem_unbind(struct kvm_memory_slot *slot);
> #else
> static inline int kvm_gmem_init(struct module *module)
> @@ -80,9 +80,8 @@ static inline int kvm_gmem_init(struct module *module)
> return 0;
> }
> static inline void kvm_gmem_exit(void) {};
> -static inline int kvm_gmem_bind(struct kvm *kvm,
> - struct kvm_memory_slot *slot,
> - unsigned int fd, loff_t offset)
> +static inline int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> + unsigned int fd, u64 offset)
> {
> WARN_ON_ONCE(1);
> return -EIO;
>
^ permalink raw reply
* Re: [PATCHv3 02/12] uprobes/x86: Remove struct uprobe_trampoline object
From: bot+bpf-ci @ 2026-05-21 13:26 UTC (permalink / raw)
To: jolsa, oleg, peterz, mingo, mhiramat, andrii
Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260521124411.31133-3-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]
> commit 35d93b034b144060a547b507cfb4268692c328c2
> Author: Jiri Olsa <jolsa@kernel.org>
> Date: Thu May 21 14:44:01 2026 +0200
>
> uprobes/x86: Remove struct uprobe_trampoline object
>
> Removing struct uprobe_trampoline object and it's tracking code,
> because it's not needed. We can do same thing directly on top of
> struct vm_area_struct objects.
>
> This makes the code simpler and allows easy propagation of the
> trampoline vma object into child process in following change.
>
> Note the original code called destroy_uprobe_trampoline if the
> optimiation failed, but it only freed the struct uprobe_trampoline
> object, not the vma.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
This looks like a bug fix for a VMA resource leak. The commit message
notes that when uprobe optimization failed, the original code only freed
the struct uprobe_trampoline object but leaked the VMA created by
_install_special_mapping. Should this include:
Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26227520868
^ permalink raw reply
* Re: [PATCHv3 11/12] selftests/bpf: Add tests for uprobe nop10 red zone clobbering
From: bot+bpf-ci @ 2026-05-21 13:26 UTC (permalink / raw)
To: jolsa, oleg, peterz, mingo, mhiramat, andrii
Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260521124411.31133-12-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index 969f4deba9fd..efff0c515184 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
[ ... ]
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index fda3a298ccfc..8004c9568ffa 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/test_usdt.c b/tools/testing/selftests/bpf/progs/test_usdt.c
> index f00cb52874e0..0ee78fb050a1 100644
> --- a/tools/testing/selftests/bpf/progs/test_usdt.c
> +++ b/tools/testing/selftests/bpf/progs/test_usdt.c
[ ... ]
> diff --git a/tools/testing/selftests/bpf/usdt_2.c b/tools/testing/selftests/bpf/usdt_2.c
> index b359b389f6c0..5e38f8605b02 100644
> --- a/tools/testing/selftests/bpf/usdt_2.c
> +++ b/tools/testing/selftests/bpf/usdt_2.c
> @@ -13,4 +13,17 @@ void usdt_2(void)
> USDT(optimized_attach, usdt_2);
> }
>
> +static volatile unsigned long usdt_red_zone_arg1 = 0xDEADBEEF;
> +static volatile unsigned long usdt_red_zone_arg2 = 0xCAFEBABE;
> +static volatile unsigned long usdt_red_zone_arg3 = 0xFEEDFACE;
> +
> +void __attribute__((noinline)) usdt_red_zone_trigger(void)
> +{
> + unsigned long a1 = usdt_red_zone_arg1;
> + unsigned long a2 = usdt_red_zone_arg2;
> + unsigned long a3 = usdt_red_zone_arg3;
> +
> + USDT(optimized_attach, usdt_red_zone, a1, a2, a3);
> +}
Following up on a review comment from sashiko-bot: is this test reliable
across different compiler versions?
The x86-64 USDT macro uses the "nor" inline assembly constraint, which allows
the compiler to allocate these operands to CPU registers. Because there is no
register pressure in usdt_red_zone_trigger(), modern compilers might allocate
the local variables a1, a2, and a3 directly to registers rather than the
stack's red zone.
If the operands are placed in registers, the uprobe CALL optimization
clobbering [rsp-8] will not corrupt them. This could cause the test to pass
even on buggy kernels, creating a false positive.
The author acknowledged this concern but kept the test as-is, noting that
selftests compile with -O0 and there's also an assembly-based test.
Would it be safer to explicitly force these operands into the red zone using
inline assembly constraints, rather than depending on the compiler's register
allocator?
Reference:
https://lore.kernel.org/bpf/20260509021238.AE98FC2BCB0@smtp.kernel.org/
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26227520868
^ permalink raw reply
* Re: [PATCHv3 08/12] selftests/bpf: Change uprobe syscall tests to use nop10
From: bot+bpf-ci @ 2026-05-21 13:26 UTC (permalink / raw)
To: jolsa, oleg, peterz, mingo, mhiramat, andrii
Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260521124411.31133-9-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3335 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index c944136252c6..9653fb5608f2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
[ ... ]
> @@ -388,14 +388,15 @@ static int find_uprobes_trampoline(void *tramp_addr)
> return ret;
> }
>
> -static unsigned char nop5[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
> +static unsigned char nop10[10] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
> +static unsigned char lea_rsp[5] = { 0x48, 0x8d, 0x64, 0x24, 0x80 };
>
> -static void *find_nop5(void *fn)
> +static void *find_nop10(void *fn)
> {
> int i;
>
> - for (i = 0; i < 10; i++) {
> - if (!memcmp(nop5, fn + i, 5))
> + for (i = 0; i < 128; i++) {
> + if (!memcmp(nop10, fn + i, 10))
> return fn + i;
> }
> return NULL;
[ ... ]
> @@ -432,7 +434,7 @@ static void check_detach(void *addr, void *tramp)
> {
> /* [uprobes_trampoline] stays after detach */
> ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
> - ASSERT_OK(memcmp(addr, nop5, 5), "nop5");
> + ASSERT_OK(memcmp(addr, jmp2B, 2), "jmp2B");
> }
Is jmp2B declared somewhere? The identifier is used in memcmp() but does
not appear to be defined anywhere in the file or included headers. A
subsequent commit later in the series replaces this with a properly
declared nop10_prefix array, confirming this is likely a missing
definition.
The pre-patch code referenced the defined static array nop5, but the new
code references an undeclared symbol that will cause a compilation error.
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index 69759b27794d..fda3a298ccfc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
> @@ -252,7 +252,7 @@ extern void usdt_1(void);
> extern void usdt_2(void);
>
> static unsigned char nop1[1] = { 0x90 };
> -static unsigned char nop1_nop5_combo[6] = { 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 };
> +static unsigned char nop1_nop10_combo[11] = { 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
>
> static void *find_instr(void *fn, unsigned char *instr, size_t cnt)
> {
> int i;
>
> for (i = 0; i < 10; i++) {
> if (!memcmp(instr, fn + i, cnt))
> return fn + i;
> }
> return NULL;
> }
Is the loop in find_instr() adequate to find this instruction sequence?
In uprobe_syscall.c, find_nop10() was updated to search up to 128 bytes
to account for compiler-generated prologues:
tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c:find_nop10() {
for (i = 0; i < 128; i++) {
if (!memcmp(nop10, fn + i, 10))
return fn + i;
}
}
If a modern compiler generates a prologue longer than 9 bytes for
usdt_2(), find_instr() will prematurely terminate and cause the
subtest_optimized_attach() test to fail. This was acknowledged in v2
review with "yea find_instr needs same update, will fix" but remains
unchanged.
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26227520868
^ permalink raw reply
* Re: [PATCHv3 06/12] libbpf: Detect uprobe syscall with new error
From: bot+bpf-ci @ 2026-05-21 13:26 UTC (permalink / raw)
To: jolsa, oleg, peterz, mingo, mhiramat, andrii
Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260521124411.31133-7-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]
> commit adef46acd6e910a58509f054ca1d94c79b4c2bf2
> Author: Jiri Olsa <jolsa@kernel.org>
> Date: Thu May 21 14:44:05 2026 +0200
>
> libbpf: Detect uprobe syscall with new error
>
> In the previous optimized uprobe fix we changed the syscall
> error used for its detection from ENXIO to EPROTO.
>
> Changing related probe_uprobe_syscall detection check.
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> Fixes: 05738da0efa1 ("libbpf: Add uprobe syscall feature detection")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
The Fixes tag points to commit 05738da0efa1 ("libbpf: Add uprobe syscall
feature detection"), but wasn't the bug actually introduced by commit
40b54d7e3dab ("uprobes/x86: Move optimized uprobe from nop5 to nop10")?
Commit 05738da0efa1 originally added the ENXIO check correctly, but commit
40b54d7e3dab changed the kernel to return EPROTO instead of ENXIO, breaking
the libbpf detection.
The commit message even says "In the previous optimized uprobe fix we changed
the syscall error," which refers to 40b54d7e3dab.
Should this use:
Fixes: 40b54d7e3dab ("uprobes/x86: Move optimized uprobe from nop5 to nop10")
This concern was previously raised in review by bot+bpf-ci@kernel.org on
Mon, 18 May 2026 11:37:54 +0000:
https://lore.kernel.org/bpf/545ca16cc113f00a390baf1ffcafba33233af771fc7c879e3c850bb33949da29@mail.kernel.org/
> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index b7e388f99d0b..e5641fa60163 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
> @@ -577,10 +577,10 @@ static int probe_ldimm64_full_range_off(int token_fd)
> static int probe_uprobe_syscall(int token_fd)
> {
> /*
> - * If kernel supports uprobe() syscall, it will return -ENXIO when called
> + * If kernel supports uprobe() syscall, it will return -EPROTO when called
> * from the outside of a kernel-generated uprobe trampoline.
> */
> - return syscall(__NR_uprobe) < 0 && errno == ENXIO;
> + return syscall(__NR_uprobe) < 0 && errno == EPROTO;
> }
> #else
> static int probe_uprobe_syscall(int token_fd)
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26227520868
^ permalink raw reply
* Re: [PATCHv3 07/12] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch
From: bot+bpf-ci @ 2026-05-21 13:26 UTC (permalink / raw)
To: jolsa, oleg, peterz, mingo, mhiramat, andrii
Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260521124411.31133-8-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]
> diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
> index c71e21df38b3..75687f50f4e2 100644
> --- a/tools/testing/selftests/bpf/usdt.h
> +++ b/tools/testing/selftests/bpf/usdt.h
> @@ -313,7 +313,7 @@ struct usdt_sema { volatile unsigned short active; };
> #if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
> #define USDT_NOP nop 0
> #elif defined(__x86_64__)
> -#define USDT_NOP .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
> +#define USDT_NOP .byte 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 /* nop, nop10 */
> #else
> #define USDT_NOP nop
> #endif
Does changing this instruction sequence from 6 bytes to 11 bytes break
prog_tests/usdt.c? The test in subtest_optimized_attach() searches for the
old 6-byte nop1_nop5_combo sequence:
tools/testing/selftests/bpf/prog_tests/usdt.c:
/* usdt_2 USDT probe has nop,nop5 instructions combo */
addr_2 = find_instr(usdt_2, nop1_nop5_combo, 6);
if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop5_combo"))
goto cleanup;
Because the old sequence is no longer emitted by USDT_NOP, find_instr() will
return NULL, causing the ASSERT_OK_PTR assertion to fail. Should the test
update be squashed into this commit to avoid breaking git bisect?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26227520868
^ permalink raw reply
* Re: [PATCH v6 21/43] KVM: SEV: Make 'uaddr' parameter optional for KVM_SEV_SNP_LAUNCH_UPDATE
From: Sean Christopherson @ 2026-05-21 13:21 UTC (permalink / raw)
To: Fuad Tabba
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTwrygfMrZZSw4y7-ry8fidW2x0C7iuF2Q=dnPNHUmNtUg@mail.gmail.com>
On Thu, May 21, 2026, Fuad Tabba wrote:
> Hi,
>
> On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
> <devnull+ackerleytng.google.com@kernel.org> wrote:
> >
> > From: Michael Roth <michael.roth@amd.com>
> >
> > For vm_memory_attributes=1, in-place conversion/population is not
> > supported, so the initial contents necessarily must need to come
> > from a separate src address, which is enforced by the current
> > implementation. However, for vm_memory_attributes=0, it is possible for
> > guest memory to be initialized directly from userspace by mmap()'ing the
> > guest_memfd and writing to it while the corresponding GPA ranges are in
> > a 'shared' state before converting them to the 'private' state expected
> > by KVM_SEV_SNP_LAUNCH_UPDATE.
> >
> > Update the handling/documentation for KVM_SEV_SNP_LAUNCH_UPDATE to allow
> > for 'uaddr' to be set to NULL when vm_memory_attributes=0, which
> > SNP_LAUNCH_UPDATE will then use to determine when it should/shouldn't
> > copy in data from a separate memory location. Continue to enforce
> > non-NULL for the original vm_memory_attributes=1 case.
> >
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > [Added src_page check in error handling path when the firmware command fails]
> > [Dropped ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES]
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>
> I'm not very familiar with the SEV-SNP populate flows, but it looks
> like Sashiko is on to something:
> https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=21
>
> - a potential read-only page overwrite, because src_page is acquired
> via get_user_pages_fast() without the FOLL_WRITE flag, but is then
> overwritten via memcpy
Oof, yeah, that's bad. Adding FOLL_WRITE to kvm_gmem_populate() feels wrong, and
could break uABI, but doing gup() in SNP code would reintroduce the AB-BA issue
with filemap_invalidate_lock().
Aha! Not if we use get_user_page_fast_only(). Ugh, but then we'd have to plumb
the userspace address into the post-populated callback.
Hrm. Given that no one has yelled about overwriting their CPUID page, and given
that the CPUID page is likely dynamically created and thus is unlikely to be a
read-only mapping (e.g. versus the initial image), maybe this?
diff --git arch/x86/kvm/svm/sev.c arch/x86/kvm/svm/sev.c
index 37d4cfa5d980..c73c028d72c1 100644
--- arch/x86/kvm/svm/sev.c
+++ arch/x86/kvm/svm/sev.c
@@ -2456,6 +2456,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
sev_populate_args.type = params.type;
count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
+ params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID,
sev_gmem_post_populate, &sev_populate_args);
if (count < 0) {
argp->error = sev_populate_args.fw_error;
diff --git arch/x86/kvm/vmx/tdx.c arch/x86/kvm/vmx/tdx.c
index f97bcf580e6d..33f35be4455b 100644
--- arch/x86/kvm/vmx/tdx.c
+++ arch/x86/kvm/vmx/tdx.c
@@ -3188,7 +3188,7 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
};
gmem_ret = kvm_gmem_populate(kvm, gpa_to_gfn(region.gpa),
u64_to_user_ptr(region.source_addr),
- 1, tdx_gmem_post_populate, &arg);
+ 1, false, tdx_gmem_post_populate, &arg);
if (gmem_ret < 0) {
ret = gmem_ret;
break;
diff --git include/linux/kvm_host.h include/linux/kvm_host.h
index 61a3430957f2..b83cda2870ba 100644
--- include/linux/kvm_host.h
+++ include/linux/kvm_host.h
@@ -2596,7 +2596,8 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
struct page *page, void *opaque);
-long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
+long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
+ long npages, bool writable,
kvm_gmem_populate_cb post_populate, void *opaque);
#endif
diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
index a35a55571a2d..6553d4e032ce 100644
--- virt/kvm/guest_memfd.c
+++ virt/kvm/guest_memfd.c
@@ -858,7 +858,8 @@ static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
return ret;
}
-long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
+long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
+ long npages, bool writable,
kvm_gmem_populate_cb post_populate, void *opaque)
{
struct kvm_memory_slot *slot;
@@ -892,8 +893,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
if (src) {
unsigned long uaddr = (unsigned long)src + i * PAGE_SIZE;
+ unsigned int flags = writable ? FOLL_WRITE : 0;
- ret = get_user_pages_fast(uaddr, 1, 0, &src_page);
+ ret = get_user_pages_fast(uaddr, 1, flags, &src_page);
if (ret < 0)
break;
if (ret != 1) {
> - an ordering violation with the kunmap_local() calls
Yeesh, that's a new one for me. Thankfully this is 64-bit only, so it's not an
issue.
> These predate this patch series and are just being touched by the
> 'src_page' addition, but if Sashiko's right, these should probably be
> fixed sooner rather than later.
Yeah, ditto with the offset wrapping case.
^ permalink raw reply related
* [PATCH v6] stm: class: Add MIPI OST protocol support
From: Yingchao Deng @ 2026-05-21 13:14 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Alexander Shishkin, Alexandre Torgue
Cc: linux-kernel, linux-trace-kernel, linux-doc, linux-arm-kernel,
quic_yingdeng, tingwei.zhang, jinlong.mao, jie.gan,
yuanfang.zhang, Yingchao Deng
Add MIPI OST (Open System Trace) protocol support for stm to format the
traces. The OST Protocol abstracts the underlying layers from the sending
and receiving applications, thus removing dependencies on the connection
media and platform implementation.
OST over STP packet consists of Header/Payload/End. Header is designed to
include the information required by all OST packets. Information that is
not shared by all packets is left to the higher layer protocols. Thus, the
OST Protocol Header can be regarded as the first part of a complete OST
Packet Header, while a higher layer header can be regarded as an extension
designed for a specific purpose.
+--------+--------+--------+--------+
| start |version |entity |protocol|
+--------+--------+--------+--------+
| stm version | magic |
+-----------------------------------+
| cpu |
+-----------------------------------+
| timestamp |
| |
+-----------------------------------+
| tgid |
| |
+-----------------------------------+
| payload |
+-----------------------------------+
| ... | end |
+-----------------------------------+
In header, there will be STARTSIMPLE/VERSION/ENTITY/PROTOCOL.
STARTSIMPLE is used to signal the beginning of a simplified OST protocol.
The Version field is a one byte, unsigned number identifying the version
of the OST Protocol. The Entity ID field is a one byte unsigned number
that identifies the source.
Entity ID values (0~239) are defined and controlled by the TS owner, and
shall be unique for the whole TS. The configfs entity attribute allows the
user to configure which Entity ID is associated with each policy node.
The Protocol ID field is a one byte unsigned number identifying the higher
layer protocol of the OST Packet, i.e. identifying the format of the data
after the OST Protocol Header. OST Control Protocol ID value represents
the common control protocol, the remaining Protocol ID values may be used
by any higher layer protocols capable of being transported by the OST
Protocol.
Co-developed-by: Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>
Signed-off-by: Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>
Co-developed-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
Co-developed-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
Signed-off-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
Signed-off-by: Yingchao Deng <yingchao.deng@oss.qualcomm.com>
---
Changes in v6:
1. Rebase on top of linux-next-20260518.
2. Fix Kconfig: 'default CONFIG_STM' -> 'default STM'.
3. Fix documentation grammar issues.
4. Add p_ost entry to Documentation/trace/index.rst.
5. Add missing priv_sz field to stm_protocol_driver registration.
6. Use kzalloc_obj() instead of kzalloc() in ost_output_open().
7. Add mutex protection in entity configfs store handler.
8. Keep the configfs entity attribute: entity ID values (0~239) are
defined and controlled by the TS owner and are deployment-specific.
stm_source_type only carries a small number of in-kernel source
classifications and cannot represent the full range of OST entity
assignments needed in practice. The configfs attribute allows each
policy node to declare its entity.
OST_ENTITY_TYPE_NONE is an enum sentinel (not entity ID 0) that causes
ost_write() to return -EINVAL when no entity is configured, preventing
emission of packets with an unintended entity field.
OST_ENTITY_DIAG (0xEE) is a TS-owner-defined value used by Qualcomm's
diagnostic framework as the standard entity identifier for diagnostic
trace sources.
Link to v5: https://lore.kernel.org/all/20260129-p_ost-v5-1-2b14fff39428@oss.qualcomm.com/
Changes in v5:
1. Add Co-developed-by tag.
2. Use yearless copyright for new file.
- Link to v4: https://lore.kernel.org/all/20251024-p_ost-v4-1-3652a06fd055@oss.qualcomm.com/
Changes in v4:
1. Delete unused variable 'i'.
2. Fix build error: call to undeclared function 'task_tgid_nr'.
Link to v3 - https://lore.kernel.org/all/20251022071834.1658684-1-yingchao.deng@oss.qualcomm.com/
Changes in v3:
1. Add more details about OST.
2. Delete 'entity_available' node, and 'entity' node will show available
and currently selected (shown in square brackets) entity.
3. Removed the usage of config_item->ci_group->cg_subsys->su_mutex.
Link to v2 - https://lore.kernel.org/all/20230419141328.37472-1-quic_jinlmao@quicinc.com/
---
.../ABI/testing/configfs-stp-policy-p_ost | 9 +
Documentation/trace/index.rst | 1 +
Documentation/trace/p_ost.rst | 39 ++++
drivers/hwtracing/stm/Kconfig | 14 ++
drivers/hwtracing/stm/Makefile | 2 +
drivers/hwtracing/stm/p_ost.c | 241 +++++++++++++++++++++
6 files changed, 306 insertions(+)
diff --git a/Documentation/ABI/testing/configfs-stp-policy-p_ost b/Documentation/ABI/testing/configfs-stp-policy-p_ost
new file mode 100644
index 000000000000..8fb160b50c40
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-stp-policy-p_ost
@@ -0,0 +1,9 @@
+What: /config/stp-policy/<device>:p_ost.<policy>/<node>/entity
+Date: May 2026
+KernelVersion: 7.1
+Description:
+ Set the entity ID which identifies the trace source in the
+ OST packet header. Entity ID values (0~239) are defined by
+ the TS owner. Currently supported values are ftrace, console
+ and diag. RW.
+
diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
index 5d9bf4694d5d..9cd1e0b5af6d 100644
--- a/Documentation/trace/index.rst
+++ b/Documentation/trace/index.rst
@@ -72,6 +72,7 @@ interactions and system performance.
intel_th
stm
sys-t
+ p_ost
coresight/index
rv/index
hisi-ptt
diff --git a/Documentation/trace/p_ost.rst b/Documentation/trace/p_ost.rst
new file mode 100644
index 000000000000..2b92e2229653
--- /dev/null
+++ b/Documentation/trace/p_ost.rst
@@ -0,0 +1,39 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================
+MIPI OST over STP
+===================
+
+The OST (Open System Trace) driver is used with STM class devices to
+generate standardized trace stream. Trace sources can be identified
+by different entity IDs.
+
+CONFIG_STM_PROTO_OST is for p_ost driver enablement. Once this config
+is enabled, you can select the p_ost protocol by command below:
+
+# mkdir /sys/kernel/config/stp-policy/stm0:p_ost.policy
+
+The policy name format is extended like this:
+
+ <device_name>:<protocol_name>.<policy_name>
+
+With a coresight-stm device, it will look like "stm0:p_ost.policy".
+
+With the MIPI OST protocol driver, the attributes for each protocol node are:
+
+# mkdir /sys/kernel/config/stp-policy/stm0:p_ost.policy/default
+# ls /sys/kernel/config/stp-policy/stm0:p_ost.policy/default
+channels entity masters
+
+The entity here is the set of entities that p_ost supports. Currently
+p_ost supports ftrace, console and diag entities.
+
+Set entity:
+# echo 'ftrace' > /sys/kernel/config/stp-policy/stm0:p_ost.policy/default/entity
+
+Get available and currently selected (shown in square brackets) entity:
+# cat /sys/kernel/config/stp-policy/stm0:p_ost.policy/default/entity
+[ftrace] console diag
+
+See Documentation/ABI/testing/configfs-stp-policy-p_ost for more details.
+
diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index cd7f0b0f3fbe..4c83da5d95a0 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -40,6 +40,20 @@ config STM_PROTO_SYS_T
If you don't know what this is, say N.
+config STM_PROTO_OST
+ tristate "MIPI OST STM framing protocol driver"
+ default STM
+ help
+ This is an implementation of MIPI OST protocol to be used
+ over the STP transport. In addition to the data payload, it
+ also carries additional metadata for entity, better
+ means of trace source identification, etc.
+
+ The receiving side must be able to decode this protocol in
+ addition to the MIPI STP, in order to extract the data.
+
+ If you don't know what this is, say N.
+
config STM_DUMMY
tristate "Dummy STM driver"
help
diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
index 1692fcd29277..d9c8615849b9 100644
--- a/drivers/hwtracing/stm/Makefile
+++ b/drivers/hwtracing/stm/Makefile
@@ -5,9 +5,11 @@ stm_core-y := core.o policy.o
obj-$(CONFIG_STM_PROTO_BASIC) += stm_p_basic.o
obj-$(CONFIG_STM_PROTO_SYS_T) += stm_p_sys-t.o
+obj-$(CONFIG_STM_PROTO_OST) += stm_p_ost.o
stm_p_basic-y := p_basic.o
stm_p_sys-t-y := p_sys-t.o
+stm_p_ost-y := p_ost.o
obj-$(CONFIG_STM_DUMMY) += dummy_stm.o
diff --git a/drivers/hwtracing/stm/p_ost.c b/drivers/hwtracing/stm/p_ost.c
new file mode 100644
index 000000000000..d2174872b761
--- /dev/null
+++ b/drivers/hwtracing/stm/p_ost.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * MIPI OST framing protocol for STM devices.
+ */
+
+#include <linux/pid.h>
+#include <linux/sched/clock.h>
+#include <linux/slab.h>
+#include <linux/stm.h>
+#include "stm.h"
+
+/*
+ * OST Base Protocol Header
+ *
+ * Position Bits Field Name
+ * 0 8 STARTSIMPLE
+ * 1 8 Version
+ * 2 8 Entity ID
+ * 3 8 Protocol ID
+ */
+#define OST_FIELD_STARTSIMPLE 0
+#define OST_FIELD_VERSION 8
+#define OST_FIELD_ENTITY 16
+#define OST_FIELD_PROTOCOL 24
+
+#define OST_TOKEN_STARTSIMPLE 0x10
+#define OST_VERSION_MIPI1 0x10
+
+/* entity id to identify the source */
+#define OST_ENTITY_FTRACE 0x01
+#define OST_ENTITY_CONSOLE 0x02
+#define OST_ENTITY_DIAG 0xEE
+
+#define OST_CONTROL_PROTOCOL 0x0
+
+#define DATA_HEADER ((OST_TOKEN_STARTSIMPLE << OST_FIELD_STARTSIMPLE) | \
+ (OST_VERSION_MIPI1 << OST_FIELD_VERSION) | \
+ (OST_CONTROL_PROTOCOL << OST_FIELD_PROTOCOL))
+
+#define STM_MAKE_VERSION(ma, mi) (((ma) << 8) | (mi))
+#define STM_HEADER_MAGIC (0x5953)
+
+enum ost_entity_type {
+ OST_ENTITY_TYPE_NONE,
+ OST_ENTITY_TYPE_FTRACE,
+ OST_ENTITY_TYPE_CONSOLE,
+ OST_ENTITY_TYPE_DIAG,
+};
+
+static const char * const str_ost_entity_type[] = {
+ [OST_ENTITY_TYPE_NONE] = "none",
+ [OST_ENTITY_TYPE_FTRACE] = "ftrace",
+ [OST_ENTITY_TYPE_CONSOLE] = "console",
+ [OST_ENTITY_TYPE_DIAG] = "diag",
+};
+
+static const u32 ost_entity_value[] = {
+ [OST_ENTITY_TYPE_NONE] = 0,
+ [OST_ENTITY_TYPE_FTRACE] = OST_ENTITY_FTRACE,
+ [OST_ENTITY_TYPE_CONSOLE] = OST_ENTITY_CONSOLE,
+ [OST_ENTITY_TYPE_DIAG] = OST_ENTITY_DIAG,
+};
+
+struct ost_policy_node {
+ enum ost_entity_type entity_type;
+};
+
+struct ost_output {
+ struct ost_policy_node node;
+};
+
+/* Set default entity type as none */
+static void ost_policy_node_init(void *priv)
+{
+ struct ost_policy_node *pn = priv;
+
+ pn->entity_type = OST_ENTITY_TYPE_NONE;
+}
+
+static int ost_output_open(void *priv, struct stm_output *output)
+{
+ struct ost_policy_node *pn = priv;
+ struct ost_output *opriv;
+
+ opriv = kzalloc_obj(*opriv, GFP_ATOMIC);
+ if (!opriv)
+ return -ENOMEM;
+
+ memcpy(&opriv->node, pn, sizeof(opriv->node));
+ output->pdrv_private = opriv;
+ return 0;
+}
+
+static void ost_output_close(struct stm_output *output)
+{
+ kfree(output->pdrv_private);
+}
+
+static ssize_t ost_t_policy_entity_show(struct config_item *item,
+ char *page)
+{
+ struct ost_policy_node *pn = to_pdrv_policy_node(item);
+ ssize_t sz = 0;
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(str_ost_entity_type); i++) {
+ if (i == pn->entity_type)
+ sz += sysfs_emit_at(page, sz, "[%s] ", str_ost_entity_type[i]);
+ else
+ sz += sysfs_emit_at(page, sz, "%s ", str_ost_entity_type[i]);
+ }
+
+ sz += sysfs_emit_at(page, sz, "\n");
+ return sz;
+}
+
+static int entity_index(const char *str)
+{
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(str_ost_entity_type); i++) {
+ if (sysfs_streq(str, str_ost_entity_type[i]))
+ return i;
+ }
+
+ return 0;
+}
+
+static ssize_t
+ost_t_policy_entity_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct mutex *mutexp = &item->ci_group->cg_subsys->su_mutex;
+ struct ost_policy_node *pn = to_pdrv_policy_node(item);
+ int i;
+
+ i = entity_index(page);
+ if (i) {
+ mutex_lock(mutexp);
+ pn->entity_type = i;
+ mutex_unlock(mutexp);
+ } else {
+ return -EINVAL;
+ }
+
+ return count;
+}
+CONFIGFS_ATTR(ost_t_policy_, entity);
+
+static struct configfs_attribute *ost_t_policy_attrs[] = {
+ &ost_t_policy_attr_entity,
+ NULL,
+};
+
+static ssize_t
+notrace ost_write(struct stm_data *data, struct stm_output *output,
+ unsigned int chan, const char *buf, size_t count,
+ struct stm_source_data *source)
+{
+ struct ost_output *op = output->pdrv_private;
+ unsigned int c = output->channel + chan;
+ unsigned int m = output->master;
+ const unsigned char nil = 0;
+ u32 header = DATA_HEADER;
+ struct trc_hdr {
+ u16 version;
+ u16 magic;
+ u32 cpu;
+ u64 timestamp;
+ u64 tgid;
+ } hdr;
+ ssize_t sz;
+
+ /*
+ * Identify the source by entity type.
+ * If entity type is not set, return error value.
+ */
+ if (op->node.entity_type)
+ header |= (ost_entity_value[op->node.entity_type] << OST_FIELD_ENTITY);
+ else
+ return -EINVAL;
+
+ /*
+ * STP framing rules for OST frames:
+ * * the first packet of the OST frame is marked;
+ * * the last packet is a FLAG with timestamped tag.
+ */
+ /* Message layout: HEADER / DATA / TAIL */
+ /* HEADER */
+ sz = data->packet(data, m, c, STP_PACKET_DATA, STP_PACKET_MARKED,
+ 4, (u8 *)&header);
+ if (sz <= 0)
+ return sz;
+
+ /* DATA */
+ hdr.version = STM_MAKE_VERSION(0, 3);
+ hdr.magic = STM_HEADER_MAGIC;
+ hdr.cpu = raw_smp_processor_id();
+ hdr.timestamp = sched_clock();
+ hdr.tgid = task_tgid_nr(current);
+ sz = stm_data_write(data, m, c, false, &hdr, sizeof(hdr));
+ if (sz <= 0)
+ return sz;
+
+ sz = stm_data_write(data, m, c, false, buf, count);
+
+ /* TAIL */
+ if (sz > 0)
+ data->packet(data, m, c, STP_PACKET_FLAG,
+ STP_PACKET_TIMESTAMPED, 0, &nil);
+
+ return sz;
+}
+
+static const struct stm_protocol_driver ost_pdrv = {
+ .owner = THIS_MODULE,
+ .name = "p_ost",
+ .priv_sz = sizeof(struct ost_policy_node),
+ .write = ost_write,
+ .policy_attr = ost_t_policy_attrs,
+ .output_open = ost_output_open,
+ .output_close = ost_output_close,
+ .policy_node_init = ost_policy_node_init,
+};
+
+static int ost_stm_init(void)
+{
+ return stm_register_protocol(&ost_pdrv);
+}
+module_init(ost_stm_init);
+
+static void ost_stm_exit(void)
+{
+ stm_unregister_protocol(&ost_pdrv);
+}
+module_exit(ost_stm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MIPI Open System Trace STM framing protocol driver");
---
base-commit: 80dd246accce631c328ea43294e53b2b2dd2aa32
change-id: 20260521-stm_p_ost-3489f42a9e8c
Best regards,
--
Yingchao Deng <yingchao.deng@oss.qualcomm.com>
^ permalink raw reply related
* [PATCH v6] stm: class: Add MIPI OST protocol support
From: Yingchao Deng @ 2026-05-21 13:08 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Alexander Shishkin, Alexandre Torgue
Cc: linux-kernel, linux-trace-kernel, linux-doc, linux-arm-kernel,
Tingwei Zhang, Yuanfang Zhang, Jinlong Mao, Yingchao Deng
Add MIPI OST (Open System Trace) protocol support for stm to format the
traces. The OST Protocol abstracts the underlying layers from the sending
and receiving applications, thus removing dependencies on the connection
media and platform implementation.
OST over STP packet consists of Header/Payload/End. Header is designed to
include the information required by all OST packets. Information that is
not shared by all packets is left to the higher layer protocols. Thus, the
OST Protocol Header can be regarded as the first part of a complete OST
Packet Header, while a higher layer header can be regarded as an extension
designed for a specific purpose.
+--------+--------+--------+--------+
| start |version |entity |protocol|
+--------+--------+--------+--------+
| stm version | magic |
+-----------------------------------+
| cpu |
+-----------------------------------+
| timestamp |
| |
+-----------------------------------+
| tgid |
| |
+-----------------------------------+
| payload |
+-----------------------------------+
| ... | end |
+-----------------------------------+
In header, there will be STARTSIMPLE/VERSION/ENTITY/PROTOCOL.
STARTSIMPLE is used to signal the beginning of a simplified OST protocol.
The Version field is a one byte, unsigned number identifying the version
of the OST Protocol. The Entity ID field is a one byte unsigned number
that identifies the source.
Entity ID values (0~239) are defined and controlled by the TS owner, and
shall be unique for the whole TS. The configfs entity attribute allows the
user to configure which Entity ID is associated with each policy node.
The Protocol ID field is a one byte unsigned number identifying the higher
layer protocol of the OST Packet, i.e. identifying the format of the data
after the OST Protocol Header. OST Control Protocol ID value represents
the common control protocol, the remaining Protocol ID values may be used
by any higher layer protocols capable of being transported by the OST
Protocol.
Co-developed-by: Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>
Signed-off-by: Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>
Co-developed-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
Co-developed-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
Signed-off-by: Jinlong Mao <jinlong.mao@oss.qualcomm.com>
Signed-off-by: Yingchao Deng <yingchao.deng@oss.qualcomm.com>
---
Changes in v6:
1. Rebase on top of linux-next-20260518.
2. Fix Kconfig: 'default CONFIG_STM' -> 'default STM'.
3. Fix documentation grammar issues.
4. Add p_ost entry to Documentation/trace/index.rst.
5. Add missing priv_sz field to stm_protocol_driver registration.
6. Use kzalloc_obj() instead of kzalloc() in ost_output_open().
7. Add mutex protection in entity configfs store handler.
8. Keep the configfs entity attribute: entity ID values (0~239) are
defined and controlled by the TS owner and are deployment-specific.
stm_source_type only carries a small number of in-kernel source
classifications and cannot represent the full range of OST entity
assignments needed in practice. The configfs attribute allows each
policy node to declare its entity.
OST_ENTITY_TYPE_NONE is an enum sentinel (not entity ID 0) that causes
ost_write() to return -EINVAL when no entity is configured, preventing
emission of packets with an unintended entity field.
OST_ENTITY_DIAG (0xEE) is a TS-owner-defined value used by Qualcomm's
diagnostic framework as the standard entity identifier for diagnostic
trace sources.
Link to v5: https://lore.kernel.org/all/20260129-p_ost-v5-1-2b14fff39428@oss.qualcomm.com/
Changes in v5:
1. Add Co-developed-by tag.
2. Use yearless copyright for new file.
- Link to v4: https://lore.kernel.org/all/20251024-p_ost-v4-1-3652a06fd055@oss.qualcomm.com/
Changes in v4:
1. Delete unused variable 'i'.
2. Fix build error: call to undeclared function 'task_tgid_nr'.
Link to v3 - https://lore.kernel.org/all/20251022071834.1658684-1-yingchao.deng@oss.qualcomm.com/
Changes in v3:
1. Add more details about OST.
2. Delete 'entity_available' node, and 'entity' node will show available
and currently selected (shown in square brackets) entity.
3. Removed the usage of config_item->ci_group->cg_subsys->su_mutex.
Link to v2 - https://lore.kernel.org/all/20230419141328.37472-1-quic_jinlmao@quicinc.com/
---
.../ABI/testing/configfs-stp-policy-p_ost | 9 +
Documentation/trace/index.rst | 1 +
Documentation/trace/p_ost.rst | 39 ++++
drivers/hwtracing/stm/Kconfig | 14 ++
drivers/hwtracing/stm/Makefile | 2 +
drivers/hwtracing/stm/p_ost.c | 241 +++++++++++++++++++++
6 files changed, 306 insertions(+)
diff --git a/Documentation/ABI/testing/configfs-stp-policy-p_ost b/Documentation/ABI/testing/configfs-stp-policy-p_ost
new file mode 100644
index 000000000000..8fb160b50c40
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-stp-policy-p_ost
@@ -0,0 +1,9 @@
+What: /config/stp-policy/<device>:p_ost.<policy>/<node>/entity
+Date: May 2026
+KernelVersion: 7.1
+Description:
+ Set the entity ID which identifies the trace source in the
+ OST packet header. Entity ID values (0~239) are defined by
+ the TS owner. Currently supported values are ftrace, console
+ and diag. RW.
+
diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
index 5d9bf4694d5d..9cd1e0b5af6d 100644
--- a/Documentation/trace/index.rst
+++ b/Documentation/trace/index.rst
@@ -72,6 +72,7 @@ interactions and system performance.
intel_th
stm
sys-t
+ p_ost
coresight/index
rv/index
hisi-ptt
diff --git a/Documentation/trace/p_ost.rst b/Documentation/trace/p_ost.rst
new file mode 100644
index 000000000000..2b92e2229653
--- /dev/null
+++ b/Documentation/trace/p_ost.rst
@@ -0,0 +1,39 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================
+MIPI OST over STP
+===================
+
+The OST (Open System Trace) driver is used with STM class devices to
+generate standardized trace stream. Trace sources can be identified
+by different entity IDs.
+
+CONFIG_STM_PROTO_OST is for p_ost driver enablement. Once this config
+is enabled, you can select the p_ost protocol by command below:
+
+# mkdir /sys/kernel/config/stp-policy/stm0:p_ost.policy
+
+The policy name format is extended like this:
+
+ <device_name>:<protocol_name>.<policy_name>
+
+With a coresight-stm device, it will look like "stm0:p_ost.policy".
+
+With the MIPI OST protocol driver, the attributes for each protocol node are:
+
+# mkdir /sys/kernel/config/stp-policy/stm0:p_ost.policy/default
+# ls /sys/kernel/config/stp-policy/stm0:p_ost.policy/default
+channels entity masters
+
+The entity here is the set of entities that p_ost supports. Currently
+p_ost supports ftrace, console and diag entities.
+
+Set entity:
+# echo 'ftrace' > /sys/kernel/config/stp-policy/stm0:p_ost.policy/default/entity
+
+Get available and currently selected (shown in square brackets) entity:
+# cat /sys/kernel/config/stp-policy/stm0:p_ost.policy/default/entity
+[ftrace] console diag
+
+See Documentation/ABI/testing/configfs-stp-policy-p_ost for more details.
+
diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index cd7f0b0f3fbe..4c83da5d95a0 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -40,6 +40,20 @@ config STM_PROTO_SYS_T
If you don't know what this is, say N.
+config STM_PROTO_OST
+ tristate "MIPI OST STM framing protocol driver"
+ default STM
+ help
+ This is an implementation of MIPI OST protocol to be used
+ over the STP transport. In addition to the data payload, it
+ also carries additional metadata for entity, better
+ means of trace source identification, etc.
+
+ The receiving side must be able to decode this protocol in
+ addition to the MIPI STP, in order to extract the data.
+
+ If you don't know what this is, say N.
+
config STM_DUMMY
tristate "Dummy STM driver"
help
diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
index 1692fcd29277..d9c8615849b9 100644
--- a/drivers/hwtracing/stm/Makefile
+++ b/drivers/hwtracing/stm/Makefile
@@ -5,9 +5,11 @@ stm_core-y := core.o policy.o
obj-$(CONFIG_STM_PROTO_BASIC) += stm_p_basic.o
obj-$(CONFIG_STM_PROTO_SYS_T) += stm_p_sys-t.o
+obj-$(CONFIG_STM_PROTO_OST) += stm_p_ost.o
stm_p_basic-y := p_basic.o
stm_p_sys-t-y := p_sys-t.o
+stm_p_ost-y := p_ost.o
obj-$(CONFIG_STM_DUMMY) += dummy_stm.o
diff --git a/drivers/hwtracing/stm/p_ost.c b/drivers/hwtracing/stm/p_ost.c
new file mode 100644
index 000000000000..d2174872b761
--- /dev/null
+++ b/drivers/hwtracing/stm/p_ost.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * MIPI OST framing protocol for STM devices.
+ */
+
+#include <linux/pid.h>
+#include <linux/sched/clock.h>
+#include <linux/slab.h>
+#include <linux/stm.h>
+#include "stm.h"
+
+/*
+ * OST Base Protocol Header
+ *
+ * Position Bits Field Name
+ * 0 8 STARTSIMPLE
+ * 1 8 Version
+ * 2 8 Entity ID
+ * 3 8 Protocol ID
+ */
+#define OST_FIELD_STARTSIMPLE 0
+#define OST_FIELD_VERSION 8
+#define OST_FIELD_ENTITY 16
+#define OST_FIELD_PROTOCOL 24
+
+#define OST_TOKEN_STARTSIMPLE 0x10
+#define OST_VERSION_MIPI1 0x10
+
+/* entity id to identify the source */
+#define OST_ENTITY_FTRACE 0x01
+#define OST_ENTITY_CONSOLE 0x02
+#define OST_ENTITY_DIAG 0xEE
+
+#define OST_CONTROL_PROTOCOL 0x0
+
+#define DATA_HEADER ((OST_TOKEN_STARTSIMPLE << OST_FIELD_STARTSIMPLE) | \
+ (OST_VERSION_MIPI1 << OST_FIELD_VERSION) | \
+ (OST_CONTROL_PROTOCOL << OST_FIELD_PROTOCOL))
+
+#define STM_MAKE_VERSION(ma, mi) (((ma) << 8) | (mi))
+#define STM_HEADER_MAGIC (0x5953)
+
+enum ost_entity_type {
+ OST_ENTITY_TYPE_NONE,
+ OST_ENTITY_TYPE_FTRACE,
+ OST_ENTITY_TYPE_CONSOLE,
+ OST_ENTITY_TYPE_DIAG,
+};
+
+static const char * const str_ost_entity_type[] = {
+ [OST_ENTITY_TYPE_NONE] = "none",
+ [OST_ENTITY_TYPE_FTRACE] = "ftrace",
+ [OST_ENTITY_TYPE_CONSOLE] = "console",
+ [OST_ENTITY_TYPE_DIAG] = "diag",
+};
+
+static const u32 ost_entity_value[] = {
+ [OST_ENTITY_TYPE_NONE] = 0,
+ [OST_ENTITY_TYPE_FTRACE] = OST_ENTITY_FTRACE,
+ [OST_ENTITY_TYPE_CONSOLE] = OST_ENTITY_CONSOLE,
+ [OST_ENTITY_TYPE_DIAG] = OST_ENTITY_DIAG,
+};
+
+struct ost_policy_node {
+ enum ost_entity_type entity_type;
+};
+
+struct ost_output {
+ struct ost_policy_node node;
+};
+
+/* Set default entity type as none */
+static void ost_policy_node_init(void *priv)
+{
+ struct ost_policy_node *pn = priv;
+
+ pn->entity_type = OST_ENTITY_TYPE_NONE;
+}
+
+static int ost_output_open(void *priv, struct stm_output *output)
+{
+ struct ost_policy_node *pn = priv;
+ struct ost_output *opriv;
+
+ opriv = kzalloc_obj(*opriv, GFP_ATOMIC);
+ if (!opriv)
+ return -ENOMEM;
+
+ memcpy(&opriv->node, pn, sizeof(opriv->node));
+ output->pdrv_private = opriv;
+ return 0;
+}
+
+static void ost_output_close(struct stm_output *output)
+{
+ kfree(output->pdrv_private);
+}
+
+static ssize_t ost_t_policy_entity_show(struct config_item *item,
+ char *page)
+{
+ struct ost_policy_node *pn = to_pdrv_policy_node(item);
+ ssize_t sz = 0;
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(str_ost_entity_type); i++) {
+ if (i == pn->entity_type)
+ sz += sysfs_emit_at(page, sz, "[%s] ", str_ost_entity_type[i]);
+ else
+ sz += sysfs_emit_at(page, sz, "%s ", str_ost_entity_type[i]);
+ }
+
+ sz += sysfs_emit_at(page, sz, "\n");
+ return sz;
+}
+
+static int entity_index(const char *str)
+{
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(str_ost_entity_type); i++) {
+ if (sysfs_streq(str, str_ost_entity_type[i]))
+ return i;
+ }
+
+ return 0;
+}
+
+static ssize_t
+ost_t_policy_entity_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct mutex *mutexp = &item->ci_group->cg_subsys->su_mutex;
+ struct ost_policy_node *pn = to_pdrv_policy_node(item);
+ int i;
+
+ i = entity_index(page);
+ if (i) {
+ mutex_lock(mutexp);
+ pn->entity_type = i;
+ mutex_unlock(mutexp);
+ } else {
+ return -EINVAL;
+ }
+
+ return count;
+}
+CONFIGFS_ATTR(ost_t_policy_, entity);
+
+static struct configfs_attribute *ost_t_policy_attrs[] = {
+ &ost_t_policy_attr_entity,
+ NULL,
+};
+
+static ssize_t
+notrace ost_write(struct stm_data *data, struct stm_output *output,
+ unsigned int chan, const char *buf, size_t count,
+ struct stm_source_data *source)
+{
+ struct ost_output *op = output->pdrv_private;
+ unsigned int c = output->channel + chan;
+ unsigned int m = output->master;
+ const unsigned char nil = 0;
+ u32 header = DATA_HEADER;
+ struct trc_hdr {
+ u16 version;
+ u16 magic;
+ u32 cpu;
+ u64 timestamp;
+ u64 tgid;
+ } hdr;
+ ssize_t sz;
+
+ /*
+ * Identify the source by entity type.
+ * If entity type is not set, return error value.
+ */
+ if (op->node.entity_type)
+ header |= (ost_entity_value[op->node.entity_type] << OST_FIELD_ENTITY);
+ else
+ return -EINVAL;
+
+ /*
+ * STP framing rules for OST frames:
+ * * the first packet of the OST frame is marked;
+ * * the last packet is a FLAG with timestamped tag.
+ */
+ /* Message layout: HEADER / DATA / TAIL */
+ /* HEADER */
+ sz = data->packet(data, m, c, STP_PACKET_DATA, STP_PACKET_MARKED,
+ 4, (u8 *)&header);
+ if (sz <= 0)
+ return sz;
+
+ /* DATA */
+ hdr.version = STM_MAKE_VERSION(0, 3);
+ hdr.magic = STM_HEADER_MAGIC;
+ hdr.cpu = raw_smp_processor_id();
+ hdr.timestamp = sched_clock();
+ hdr.tgid = task_tgid_nr(current);
+ sz = stm_data_write(data, m, c, false, &hdr, sizeof(hdr));
+ if (sz <= 0)
+ return sz;
+
+ sz = stm_data_write(data, m, c, false, buf, count);
+
+ /* TAIL */
+ if (sz > 0)
+ data->packet(data, m, c, STP_PACKET_FLAG,
+ STP_PACKET_TIMESTAMPED, 0, &nil);
+
+ return sz;
+}
+
+static const struct stm_protocol_driver ost_pdrv = {
+ .owner = THIS_MODULE,
+ .name = "p_ost",
+ .priv_sz = sizeof(struct ost_policy_node),
+ .write = ost_write,
+ .policy_attr = ost_t_policy_attrs,
+ .output_open = ost_output_open,
+ .output_close = ost_output_close,
+ .policy_node_init = ost_policy_node_init,
+};
+
+static int ost_stm_init(void)
+{
+ return stm_register_protocol(&ost_pdrv);
+}
+module_init(ost_stm_init);
+
+static void ost_stm_exit(void)
+{
+ stm_unregister_protocol(&ost_pdrv);
+}
+module_exit(ost_stm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MIPI Open System Trace STM framing protocol driver");
---
base-commit: 80dd246accce631c328ea43294e53b2b2dd2aa32
change-id: 20260521-stm_p_ost-3489f42a9e8c
Best regards,
--
Yingchao Deng <yingchao.deng@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH v6 16/43] KVM: guest_memfd: Use actual size for invalidation in kvm_gmem_release()
From: Sean Christopherson @ 2026-05-21 12:59 UTC (permalink / raw)
To: Fuad Tabba
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTxcadguOfOo7RpJVtAzcY5JAFZTbrAT_wcN6akMi8gCUg@mail.gmail.com>
On Thu, May 21, 2026, Fuad Tabba wrote:
> Hi Ackerley,
>
> On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
> <devnull+ackerleytng.google.com@kernel.org> wrote:
> >
> > From: Ackerley Tng <ackerleytng@google.com>
> >
> > __kvm_gmem_invalidate_begin() and __kvm_gmem_invalidate_end() actually do
> > not specially handle -1ul. -1ul is used as a huge number, which legal
> > indices do not exceed, and hence the invalidation works as expected.
> >
> > Since a later patch is going to make use of the exact range, calculate the
> > size of the guest_memfd inode and use it as the end range for invalidating
> > SPTEs.
> >
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>
> Want to look at what Sashiko has to say? Seems to be a real issue:
>
> https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=16
>
> If I understand correctly, the fix should simple: use
> check_add_overflow() to validate the offset and size parameters in
> kvm_gmem_bind()
>
> int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> unsigned int fd, loff_t offset)
> {
> loff_t size = slot->npages << PAGE_SHIFT;
> + loff_t end;
> unsigned long start, end_index;
> struct gmem_file *f;
> ...
> - if (offset < 0 || !PAGE_ALIGNED(offset) ||
> - offset + size > i_size_read(inode))
> + if (offset < 0 || !PAGE_ALIGNED(offset) ||
> + check_add_overflow(offset, size, &end) ||
Eww, TIL I'm not a fan of check_add_overflow(). Burying an out-param in an
if-statement is nasty.
> + end > i_size_read(inode))
This is all rather silly. @offset and and @slot->npages are fundamentally
unsigned values. I don't see any reason to convert them to signed values, only
to convert them *back* to unsigned values (when stored in start/end, because xarrays
operate on "unsigned long" indices).
i_size_read() obviously has to return a positive value, so can't we just do this?
diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
index a35a55571a2d..9c6dbb54e800 100644
--- virt/kvm/guest_memfd.c
+++ virt/kvm/guest_memfd.c
@@ -640,9 +640,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
}
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset)
+ unsigned int fd, u64 offset)
{
- loff_t size = slot->npages << PAGE_SHIFT;
+ u64 size = slot->npages << PAGE_SHIFT;
unsigned long start, end;
struct gmem_file *f;
struct inode *inode;
@@ -664,8 +664,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
inode = file_inode(file);
- if (offset < 0 || !PAGE_ALIGNED(offset) ||
- offset + size > i_size_read(inode))
+ if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
goto err;
filemap_invalidate_lock(inode->i_mapping);
diff --git virt/kvm/kvm_mm.h virt/kvm/kvm_mm.h
index 9fcc5d5b7f8d..3cb5ef86d0d9 100644
--- virt/kvm/kvm_mm.h
+++ virt/kvm/kvm_mm.h
@@ -72,7 +72,7 @@ int kvm_gmem_init(struct module *module);
void kvm_gmem_exit(void);
int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset);
+ unsigned int fd, u64 offset);
void kvm_gmem_unbind(struct kvm_memory_slot *slot);
#else
static inline int kvm_gmem_init(struct module *module)
@@ -80,9 +80,8 @@ static inline int kvm_gmem_init(struct module *module)
return 0;
}
static inline void kvm_gmem_exit(void) {};
-static inline int kvm_gmem_bind(struct kvm *kvm,
- struct kvm_memory_slot *slot,
- unsigned int fd, loff_t offset)
+static inline int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
+ unsigned int fd, u64 offset)
{
WARN_ON_ONCE(1);
return -EIO;
^ permalink raw reply related
* Re: [PATCH] tracing: Fix unload_page for simple_ring_buffer init rollback
From: Vincent Donnefort @ 2026-05-21 12:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, kernel-team,
linux-kernel
In-Reply-To: <20260521083107.286c5f1b@gandalf.local.home>
On Thu, May 21, 2026 at 08:31:07AM -0400, Steven Rostedt wrote:
> On Tue, 12 May 2026 15:16:14 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
>
> > The unload_page callback expects the return value of load_page() as its
> > argument: ret = load_page(va); unload(ret). Fix the rollback code in
> > simple_ring_buffer_init_mm() where the descriptor's VA is used instead
> > of the loaded page address.
> >
> > Fixes: 635923081c79 ("tracing: load/unload page callbacks for simple_ring_buffer")
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> >
> > diff --git a/kernel/trace/simple_ring_buffer.c b/kernel/trace/simple_ring_buffer.c
> > index 02af2297ae5a..38cf9abe0be8 100644
> > --- a/kernel/trace/simple_ring_buffer.c
> > +++ b/kernel/trace/simple_ring_buffer.c
> > @@ -431,7 +431,7 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
> >
> > if (ret) {
> > for (i--; i >= 0; i--)
> > - unload_page((void *)desc->page_va[i]);
> > + unload_page(bpages[i].page);
> > unload_page(cpu_buffer->meta);
> >
> > return ret;
> >
> > base-commit: 5d6919055dec134de3c40167a490f33c74c12581
>
> Vincent,
>
> Did you make this patch because of the Sashiko report?
>
> https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260512135420.99194-1-devnexen%40gmail.com
>
> If so, I think we can add a: Reported-by: Sashiko <sashiko-bot@kernel.org>
I did not for this one, but I have used this tag few times since yesterday [1] though :)
https://lore.kernel.org/all/20260521102149.804874-2-vdonnefort@google.com/
https://lore.kernel.org/all/20260521124613.911067-4-vdonnefort@google.com/
>
> -- Steve
^ permalink raw reply
* [PATCHv3 12/12] selftests/bpf: Add tests for forked/cloned optimized uprobes
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
Adding tests for forked/cloned optimized uprobes and make
sure the child can properly execute optimized probe for
both fork (dups mm) and clone with CLONE_VM.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/uprobe_syscall.c | 88 +++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index efff0c515184..033d32b4cc27 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -4,6 +4,8 @@
#ifdef __x86_64__
+#define _GNU_SOURCE
+#include <sched.h>
#include <unistd.h>
#include <asm/ptrace.h>
#include <linux/compiler.h>
@@ -936,6 +938,88 @@ static void test_uprobe_error(void)
ASSERT_EQ(errno, EPROTO, "errno");
}
+__attribute__((aligned(16)))
+__nocf_check __weak __naked void uprobe_fork_test(void)
+{
+ asm volatile (
+ ".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
+ "ret\n"
+ );
+}
+
+static int child_func(void *arg)
+{
+ struct uprobe_syscall_executed *skel = arg;
+
+ /* Make sure the child's probe is still there and optimized.. */
+ if (memcmp(uprobe_fork_test, lea_rsp, sizeof(lea_rsp)))
+ _exit(1);
+
+ skel->bss->pid = getpid();
+
+ /* .. and it executes properly. */
+ uprobe_fork_test();
+
+ if (skel->bss->executed != 3)
+ _exit(2);
+
+ _exit(0);
+}
+
+static void test_uprobe_fork_optimized(bool clone_vm)
+{
+ struct uprobe_syscall_executed *skel = NULL;
+ struct bpf_link *link = NULL;
+ unsigned long offset;
+ int pid, status, err;
+ char stack[65535];
+
+ offset = get_uprobe_offset(&uprobe_fork_test);
+ if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
+ return;
+
+ skel = uprobe_syscall_executed__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ goto cleanup;
+
+ link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+ -1, "/proc/self/exe", offset, NULL);
+ if (!ASSERT_OK_PTR(link, "attach_uprobe"))
+ goto cleanup;
+
+ skel->bss->pid = getpid();
+
+ /* Trigger optimization of uprobe in uprobe_fork_test. */
+ uprobe_fork_test();
+ uprobe_fork_test();
+
+ /* Make sure it got optimied. */
+ if (!ASSERT_OK(memcmp(uprobe_fork_test, lea_rsp, sizeof(lea_rsp)), "optimized"))
+ goto cleanup;
+
+ if (clone_vm) {
+ pid = clone(child_func, stack + sizeof(stack), CLONE_VM|SIGCHLD, skel);
+ if (!ASSERT_GT(pid, 0, "clone"))
+ goto cleanup;
+ } else {
+ pid = fork();
+ if (!ASSERT_GE(pid, 0, "fork"))
+ goto cleanup;
+ if (pid == 0)
+ child_func(skel);
+ }
+
+ /* Wait for the child and verify it exited properly with 0. */
+ err = waitpid(pid, &status, 0);
+ if (ASSERT_EQ(err, pid, "waitpid")) {
+ ASSERT_EQ(WIFEXITED(status), 1, "child_exited");
+ ASSERT_EQ(WEXITSTATUS(status), 0, "child_exit_code");
+ }
+
+cleanup:
+ uprobe_syscall_executed__destroy(skel);
+}
+
static void __test_uprobe_syscall(void)
{
if (test__start_subtest("uretprobe_regs_equal"))
@@ -956,6 +1040,10 @@ static void __test_uprobe_syscall(void)
test_uprobe_race();
if (test__start_subtest("uprobe_red_zone"))
test_uprobe_red_zone();
+ if (test__start_subtest("uprobe_optimized_fork"))
+ test_uprobe_fork_optimized(false);
+ if (test__start_subtest("uprobe_optimized_clone_vm"))
+ test_uprobe_fork_optimized(true);
if (test__start_subtest("uprobe_error"))
test_uprobe_error();
if (test__start_subtest("uprobe_regs_equal"))
--
2.53.0
^ permalink raw reply related
* [PATCHv3 11/12] selftests/bpf: Add tests for uprobe nop10 red zone clobbering
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
From: Andrii Nakryiko <andrii@kernel.org>
The uprobe nop5 optimization used to replace a 5-byte NOP with a 5-byte
CALL to a trampoline. The CALL pushes a return address onto the stack at
[rsp-8], clobbering whatever was stored there.
On x86-64, the red zone is the 128 bytes below rsp that user code may use
for temporary storage without adjusting rsp. Compilers can place USDT
argument operands there, generating specs like "8@-8(%rbp)" when rbp ==
rsp. With the CALL-based optimization, the return address overwrites that
argument before the BPF-side USDT argument fetch runs.
Add two tests for this case. The uprobe_syscall subtest stores known values
at -8(%rsp), -16(%rsp), and -24(%rsp), executes an optimized nop10 uprobe,
and verifies the red-zone data is still intact. The USDT subtest triggers a
probe in a function where the compiler places three USDT operands in the
red zone and verifies that all 10 optimized invocations deliver the expected
argument values to BPF.
On an unfixed kernel, the first hit goes through the INT3 path and later
hits use the optimized CALL path, so the red-zone checks fail after
optimization.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
[ updates to use nop10 ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/uprobe_syscall.c | 75 +++++++++++++++++++
tools/testing/selftests/bpf/prog_tests/usdt.c | 49 ++++++++++++
tools/testing/selftests/bpf/progs/test_usdt.c | 25 +++++++
tools/testing/selftests/bpf/usdt_2.c | 13 ++++
4 files changed, 162 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 969f4deba9fd..efff0c515184 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -357,6 +357,48 @@ __nocf_check __weak void usdt_test(void)
USDT(optimized_uprobe, usdt);
}
+/*
+ * Assembly-level red zone clobbering test. Stores known values in the
+ * red zone (below RSP), executes a nop10 (uprobe site), and checks that
+ * the values survived. Returns 0 if intact, 1 if clobbered.
+ *
+ * The nop5 optimization used CALL (which pushes a return address to
+ * [rsp-8]), the value at -8(%rsp) was overwritten. The nop10 optimization
+ * should escape that by moving stackpointer below the redzone before
+ * doing the CALL.
+ */
+__attribute__((aligned(16)))
+__nocf_check __weak __naked unsigned long uprobe_red_zone_test(void)
+{
+ asm volatile (
+ "movabs $0x1111111111111111, %%rax\n"
+ "movq %%rax, -8(%%rsp)\n"
+ "movabs $0x2222222222222222, %%rax\n"
+ "movq %%rax, -16(%%rsp)\n"
+ "movabs $0x3333333333333333, %%rax\n"
+ "movq %%rax, -24(%%rsp)\n"
+
+ ".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10: uprobe site */
+
+ "movabs $0x1111111111111111, %%rax\n"
+ "cmpq %%rax, -8(%%rsp)\n"
+ "jne 1f\n"
+ "movabs $0x2222222222222222, %%rax\n"
+ "cmpq %%rax, -16(%%rsp)\n"
+ "jne 1f\n"
+ "movabs $0x3333333333333333, %%rax\n"
+ "cmpq %%rax, -24(%%rsp)\n"
+ "jne 1f\n"
+
+ "xorl %%eax, %%eax\n"
+ "retq\n"
+ "1:\n"
+ "movl $1, %%eax\n"
+ "retq\n"
+ ::: "rax", "memory"
+ );
+}
+
static int find_uprobes_trampoline(void *tramp_addr)
{
void *start, *end;
@@ -855,6 +897,37 @@ static void test_uprobe_race(void)
#define __NR_uprobe 336
#endif
+static void test_uprobe_red_zone(void)
+{
+ struct uprobe_syscall_executed *skel;
+ struct bpf_link *link;
+ void *nop10_addr;
+ size_t offset;
+ int i;
+
+ nop10_addr = find_nop10(uprobe_red_zone_test);
+ if (!ASSERT_NEQ(nop10_addr, NULL, "find_nop10"))
+ return;
+
+ skel = uprobe_syscall_executed__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ offset = get_uprobe_offset(nop10_addr);
+ link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+ 0, "/proc/self/exe", offset, NULL);
+ if (!ASSERT_OK_PTR(link, "attach_uprobe"))
+ goto cleanup;
+
+ for (i = 0; i < 10; i++)
+ ASSERT_EQ(uprobe_red_zone_test(), 0, "red_zone_intact");
+
+ bpf_link__destroy(link);
+
+cleanup:
+ uprobe_syscall_executed__destroy(skel);
+}
+
static void test_uprobe_error(void)
{
long err = syscall(__NR_uprobe);
@@ -881,6 +954,8 @@ static void __test_uprobe_syscall(void)
test_uprobe_usdt();
if (test__start_subtest("uprobe_race"))
test_uprobe_race();
+ if (test__start_subtest("uprobe_red_zone"))
+ test_uprobe_red_zone();
if (test__start_subtest("uprobe_error"))
test_uprobe_error();
if (test__start_subtest("uprobe_regs_equal"))
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index fda3a298ccfc..8004c9568ffa 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -250,6 +250,7 @@ static void subtest_basic_usdt(bool optimized)
#ifdef __x86_64__
extern void usdt_1(void);
extern void usdt_2(void);
+extern void usdt_red_zone_trigger(void);
static unsigned char nop1[1] = { 0x90 };
static unsigned char nop1_nop10_combo[11] = { 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
@@ -340,6 +341,52 @@ static void subtest_optimized_attach(void)
cleanup:
test_usdt__destroy(skel);
}
+
+/*
+ * Test that USDT arguments survive nop10 optimization in a function where
+ * the compiler places operands in the red zone.
+ *
+ * Signal handlers are prone to having the compiler place USDT argument
+ * operands in the red zone (below rsp).
+ *
+ * The nop5 optimization used CALL (which pushes a return address to
+ * [rsp-8]), the value at -8(%rsp) was overwritten. The nop10 optimization
+ * should escape that by moving stackpointer below the redzone before
+ * doing the CALL.
+ */
+static void subtest_optimized_red_zone(void)
+{
+ struct test_usdt *skel;
+ int i;
+
+ skel = test_usdt__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ skel->bss->expected_arg[0] = 0xDEADBEEF;
+ skel->bss->expected_arg[1] = 0xCAFEBABE;
+ skel->bss->expected_arg[2] = 0xFEEDFACE;
+ skel->bss->expected_pid = getpid();
+
+ skel->links.usdt_check_arg = bpf_program__attach_usdt(
+ skel->progs.usdt_check_arg, 0, "/proc/self/exe",
+ "optimized_attach", "usdt_red_zone", NULL);
+ if (!ASSERT_OK_PTR(skel->links.usdt_check_arg, "attach_usdt_red_zone"))
+ goto cleanup;
+
+ for (i = 0; i < 10; i++)
+ usdt_red_zone_trigger();
+
+ ASSERT_EQ(skel->bss->arg_total, 10, "arg_total");
+ ASSERT_EQ(skel->bss->arg_bad, 0, "arg_bad");
+ ASSERT_EQ(skel->bss->arg_last[0], 0xDEADBEEF, "arg_last_1");
+ ASSERT_EQ(skel->bss->arg_last[1], 0xCAFEBABE, "arg_last_2");
+ ASSERT_EQ(skel->bss->arg_last[2], 0xFEEDFACE, "arg_last_3");
+
+cleanup:
+ test_usdt__destroy(skel);
+}
+
#endif
unsigned short test_usdt_100_semaphore SEC(".probes");
@@ -613,6 +660,8 @@ void test_usdt(void)
subtest_basic_usdt(true);
if (test__start_subtest("optimized_attach"))
subtest_optimized_attach();
+ if (test__start_subtest("optimized_red_zone"))
+ subtest_optimized_red_zone();
#endif
if (test__start_subtest("multispec"))
subtest_multispec_usdt();
diff --git a/tools/testing/selftests/bpf/progs/test_usdt.c b/tools/testing/selftests/bpf/progs/test_usdt.c
index f00cb52874e0..0ee78fb050a1 100644
--- a/tools/testing/selftests/bpf/progs/test_usdt.c
+++ b/tools/testing/selftests/bpf/progs/test_usdt.c
@@ -149,5 +149,30 @@ int usdt_executed(struct pt_regs *ctx)
executed++;
return 0;
}
+
+int arg_total;
+int arg_bad;
+long arg_last[3];
+long expected_arg[3];
+int expected_pid;
+
+SEC("usdt")
+int BPF_USDT(usdt_check_arg, long arg1, long arg2, long arg3)
+{
+ if (expected_pid != (bpf_get_current_pid_tgid() >> 32))
+ return 0;
+
+ __sync_fetch_and_add(&arg_total, 1);
+ arg_last[0] = arg1;
+ arg_last[1] = arg2;
+ arg_last[2] = arg3;
+
+ if (arg1 != expected_arg[0] ||
+ arg2 != expected_arg[1] ||
+ arg3 != expected_arg[2])
+ __sync_fetch_and_add(&arg_bad, 1);
+
+ return 0;
+}
#endif
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/usdt_2.c b/tools/testing/selftests/bpf/usdt_2.c
index b359b389f6c0..5e38f8605b02 100644
--- a/tools/testing/selftests/bpf/usdt_2.c
+++ b/tools/testing/selftests/bpf/usdt_2.c
@@ -13,4 +13,17 @@ void usdt_2(void)
USDT(optimized_attach, usdt_2);
}
+static volatile unsigned long usdt_red_zone_arg1 = 0xDEADBEEF;
+static volatile unsigned long usdt_red_zone_arg2 = 0xCAFEBABE;
+static volatile unsigned long usdt_red_zone_arg3 = 0xFEEDFACE;
+
+void __attribute__((noinline)) usdt_red_zone_trigger(void)
+{
+ unsigned long a1 = usdt_red_zone_arg1;
+ unsigned long a2 = usdt_red_zone_arg2;
+ unsigned long a3 = usdt_red_zone_arg3;
+
+ USDT(optimized_attach, usdt_red_zone, a1, a2, a3);
+}
+
#endif
--
2.53.0
^ permalink raw reply related
* [PATCHv3 10/12] selftests/bpf: Add reattach tests for uprobe syscall
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
Adding reattach tests for uprobe syscall tests to make sure
we can re-attach and optimize same uprobe multiple times.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/uprobe_syscall.c | 116 ++++++++++++++++--
1 file changed, 106 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 9653fb5608f2..969f4deba9fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -430,21 +430,28 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
return tramp;
}
-static void check_detach(void *addr, void *tramp)
+static bool check_detach(void *addr, void *tramp)
{
+ static const char nop10_prefix[] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84 };
+ bool ok = true;
+
/* [uprobes_trampoline] stays after detach */
- ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
- ASSERT_OK(memcmp(addr, jmp2B, 2), "jmp2B");
+ if (!ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline"))
+ ok = false;
+ if (!ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix"))
+ ok = false;
+ return ok;
}
-static void check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
- trigger_t trigger, void *addr, int executed)
+static void *check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
+ trigger_t trigger, void *addr, int executed)
{
void *tramp;
tramp = check_attach(skel, trigger, addr, executed);
bpf_link__destroy(link);
check_detach(addr, tramp);
+ return tramp;
}
static void test_uprobe_legacy(void)
@@ -455,6 +462,7 @@ static void test_uprobe_legacy(void)
);
struct bpf_link *link;
unsigned long offset;
+ void *tramp;
offset = get_uprobe_offset(&uprobe_test);
if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -472,7 +480,28 @@ static void test_uprobe_legacy(void)
if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
goto cleanup;
- check(skel, link, uprobe_test, uprobe_test, 2);
+ tramp = check(skel, link, uprobe_test, uprobe_test, 2);
+
+ /* reattach and detach without triggering optimization */
+ link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+ 0, "/proc/self/exe", offset, NULL);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
+ goto cleanup;
+
+ bpf_link__destroy(link);
+ if (!check_detach(uprobe_test, tramp))
+ goto cleanup;
+
+ uprobe_test();
+ ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+ /* reattach with triggering optimization */
+ link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+ 0, "/proc/self/exe", offset, NULL);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
+ goto cleanup;
+
+ check(skel, link, uprobe_test, uprobe_test, 4);
/* uretprobe */
skel->bss->executed = 0;
@@ -494,6 +523,7 @@ static void test_uprobe_multi(void)
LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
struct bpf_link *link;
unsigned long offset;
+ void *tramp;
offset = get_uprobe_offset(&uprobe_test);
if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -514,7 +544,28 @@ static void test_uprobe_multi(void)
if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
goto cleanup;
- check(skel, link, uprobe_test, uprobe_test, 2);
+ tramp = check(skel, link, uprobe_test, uprobe_test, 2);
+
+ /* reattach and detach without triggering optimization */
+ link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_multi,
+ 0, "/proc/self/exe", NULL, &opts);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ bpf_link__destroy(link);
+ if (!check_detach(uprobe_test, tramp))
+ goto cleanup;
+
+ uprobe_test();
+ ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+ /* reattach with triggering optimization */
+ link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_multi,
+ 0, "/proc/self/exe", NULL, &opts);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ check(skel, link, uprobe_test, uprobe_test, 4);
/* uretprobe.multi */
skel->bss->executed = 0;
@@ -538,6 +589,7 @@ static void test_uprobe_session(void)
);
struct bpf_link *link;
unsigned long offset;
+ void *tramp;
offset = get_uprobe_offset(&uprobe_test);
if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -557,7 +609,28 @@ static void test_uprobe_session(void)
if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
goto cleanup;
- check(skel, link, uprobe_test, uprobe_test, 4);
+ tramp = check(skel, link, uprobe_test, uprobe_test, 4);
+
+ /* reattach and detach without triggering optimization */
+ link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_session,
+ 0, "/proc/self/exe", NULL, &opts);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ bpf_link__destroy(link);
+ if (!check_detach(uprobe_test, tramp))
+ goto cleanup;
+
+ uprobe_test();
+ ASSERT_EQ(skel->bss->executed, 4, "executed_no_probe");
+
+ /* reattach with triggering optimization */
+ link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_session,
+ 0, "/proc/self/exe", NULL, &opts);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ check(skel, link, uprobe_test, uprobe_test, 8);
cleanup:
uprobe_syscall_executed__destroy(skel);
@@ -567,7 +640,7 @@ static void test_uprobe_usdt(void)
{
struct uprobe_syscall_executed *skel;
struct bpf_link *link;
- void *addr;
+ void *addr, *tramp;
errno = 0;
addr = find_nop10(usdt_test);
@@ -586,7 +659,30 @@ static void test_uprobe_usdt(void)
if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
goto cleanup;
- check(skel, link, usdt_test, addr, 2);
+ tramp = check(skel, link, usdt_test, addr, 2);
+
+ /* reattach and detach without triggering optimization */
+ link = bpf_program__attach_usdt(skel->progs.test_usdt,
+ -1 /* all PIDs */, "/proc/self/exe",
+ "optimized_uprobe", "usdt", NULL);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
+ goto cleanup;
+
+ bpf_link__destroy(link);
+ if (!check_detach(addr, tramp))
+ goto cleanup;
+
+ usdt_test();
+ ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+ /* reattach with triggering optimization */
+ link = bpf_program__attach_usdt(skel->progs.test_usdt,
+ -1 /* all PIDs */, "/proc/self/exe",
+ "optimized_uprobe", "usdt", NULL);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
+ goto cleanup;
+
+ check(skel, link, usdt_test, addr, 4);
cleanup:
uprobe_syscall_executed__destroy(skel);
--
2.53.0
^ permalink raw reply related
* [PATCHv3 09/12] selftests/bpf: Change uprobe/usdt trigger bench code to use nop10
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
Changing uprobe/usdt trigger bench code to use nop10 instead
of nop5. Also changing run_bench_uprobes.sh to use nop10 triggers.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/bench.c | 20 +++++------
.../selftests/bpf/benchs/bench_trigger.c | 36 +++++++++----------
.../selftests/bpf/benchs/run_bench_uprobes.sh | 2 +-
3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
index 6155ce455c27..1252a1af2e84 100644
--- a/tools/testing/selftests/bpf/bench.c
+++ b/tools/testing/selftests/bpf/bench.c
@@ -539,12 +539,12 @@ extern const struct bench bench_trig_uretprobe_multi_push;
extern const struct bench bench_trig_uprobe_multi_ret;
extern const struct bench bench_trig_uretprobe_multi_ret;
#ifdef __x86_64__
-extern const struct bench bench_trig_uprobe_nop5;
-extern const struct bench bench_trig_uretprobe_nop5;
-extern const struct bench bench_trig_uprobe_multi_nop5;
-extern const struct bench bench_trig_uretprobe_multi_nop5;
+extern const struct bench bench_trig_uprobe_nop10;
+extern const struct bench bench_trig_uretprobe_nop10;
+extern const struct bench bench_trig_uprobe_multi_nop10;
+extern const struct bench bench_trig_uretprobe_multi_nop10;
extern const struct bench bench_trig_usdt_nop;
-extern const struct bench bench_trig_usdt_nop5;
+extern const struct bench bench_trig_usdt_nop10;
#endif
extern const struct bench bench_rb_libbpf;
@@ -619,12 +619,12 @@ static const struct bench *benchs[] = {
&bench_trig_uprobe_multi_ret,
&bench_trig_uretprobe_multi_ret,
#ifdef __x86_64__
- &bench_trig_uprobe_nop5,
- &bench_trig_uretprobe_nop5,
- &bench_trig_uprobe_multi_nop5,
- &bench_trig_uretprobe_multi_nop5,
+ &bench_trig_uprobe_nop10,
+ &bench_trig_uretprobe_nop10,
+ &bench_trig_uprobe_multi_nop10,
+ &bench_trig_uretprobe_multi_nop10,
&bench_trig_usdt_nop,
- &bench_trig_usdt_nop5,
+ &bench_trig_usdt_nop10,
#endif
/* ringbuf/perfbuf benchmarks */
&bench_rb_libbpf,
diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
index a60b8173cdc4..61513efc167a 100644
--- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
+++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
@@ -396,15 +396,15 @@ static void *uprobe_producer_ret(void *input)
}
#ifdef __x86_64__
-__nocf_check __weak void uprobe_target_nop5(void)
+__nocf_check __weak void uprobe_target_nop10(void)
{
asm volatile (".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00");
}
-static void *uprobe_producer_nop5(void *input)
+static void *uprobe_producer_nop10(void *input)
{
while (true)
- uprobe_target_nop5();
+ uprobe_target_nop10();
return NULL;
}
@@ -418,7 +418,7 @@ static void *uprobe_producer_usdt_nop(void *input)
return NULL;
}
-static void *uprobe_producer_usdt_nop5(void *input)
+static void *uprobe_producer_usdt_nop10(void *input)
{
while (true)
usdt_2();
@@ -542,24 +542,24 @@ static void uretprobe_multi_ret_setup(void)
}
#ifdef __x86_64__
-static void uprobe_nop5_setup(void)
+static void uprobe_nop10_setup(void)
{
- usetup(false, false /* !use_multi */, &uprobe_target_nop5);
+ usetup(false, false /* !use_multi */, &uprobe_target_nop10);
}
-static void uretprobe_nop5_setup(void)
+static void uretprobe_nop10_setup(void)
{
- usetup(true, false /* !use_multi */, &uprobe_target_nop5);
+ usetup(true, false /* !use_multi */, &uprobe_target_nop10);
}
-static void uprobe_multi_nop5_setup(void)
+static void uprobe_multi_nop10_setup(void)
{
- usetup(false, true /* use_multi */, &uprobe_target_nop5);
+ usetup(false, true /* use_multi */, &uprobe_target_nop10);
}
-static void uretprobe_multi_nop5_setup(void)
+static void uretprobe_multi_nop10_setup(void)
{
- usetup(true, true /* use_multi */, &uprobe_target_nop5);
+ usetup(true, true /* use_multi */, &uprobe_target_nop10);
}
static void usdt_setup(const char *name)
@@ -598,7 +598,7 @@ static void usdt_nop_setup(void)
usdt_setup("usdt_1");
}
-static void usdt_nop5_setup(void)
+static void usdt_nop10_setup(void)
{
usdt_setup("usdt_2");
}
@@ -665,10 +665,10 @@ BENCH_TRIG_USERMODE(uretprobe_multi_nop, nop, "uretprobe-multi-nop");
BENCH_TRIG_USERMODE(uretprobe_multi_push, push, "uretprobe-multi-push");
BENCH_TRIG_USERMODE(uretprobe_multi_ret, ret, "uretprobe-multi-ret");
#ifdef __x86_64__
-BENCH_TRIG_USERMODE(uprobe_nop5, nop5, "uprobe-nop5");
-BENCH_TRIG_USERMODE(uretprobe_nop5, nop5, "uretprobe-nop5");
-BENCH_TRIG_USERMODE(uprobe_multi_nop5, nop5, "uprobe-multi-nop5");
-BENCH_TRIG_USERMODE(uretprobe_multi_nop5, nop5, "uretprobe-multi-nop5");
+BENCH_TRIG_USERMODE(uprobe_nop10, nop10, "uprobe-nop10");
+BENCH_TRIG_USERMODE(uretprobe_nop10, nop10, "uretprobe-nop10");
+BENCH_TRIG_USERMODE(uprobe_multi_nop10, nop10, "uprobe-multi-nop10");
+BENCH_TRIG_USERMODE(uretprobe_multi_nop10, nop10, "uretprobe-multi-nop10");
BENCH_TRIG_USERMODE(usdt_nop, usdt_nop, "usdt-nop");
-BENCH_TRIG_USERMODE(usdt_nop5, usdt_nop5, "usdt-nop5");
+BENCH_TRIG_USERMODE(usdt_nop10, usdt_nop10, "usdt-nop10");
#endif
diff --git a/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh b/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
index 9ec59423b949..e490b337e960 100755
--- a/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
+++ b/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
@@ -2,7 +2,7 @@
set -eufo pipefail
-for i in usermode-count syscall-count {uprobe,uretprobe}-{nop,push,ret,nop5} usdt-nop usdt-nop5
+for i in usermode-count syscall-count {uprobe,uretprobe}-{nop,push,ret,nop10} usdt-nop usdt-nop10
do
summary=$(sudo ./bench -w2 -d5 -a trig-$i | tail -n1 | cut -d'(' -f1 | cut -d' ' -f3-)
printf "%-15s: %s\n" $i "$summary"
--
2.53.0
^ permalink raw reply related
* [PATCHv3 08/12] selftests/bpf: Change uprobe syscall tests to use nop10
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
Optimized uprobes are now on top of 10-bytes nop instructions,
reflect that in existing tests.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/benchs/bench_trigger.c | 2 +-
.../selftests/bpf/prog_tests/uprobe_syscall.c | 28 ++++++++++---------
tools/testing/selftests/bpf/prog_tests/usdt.c | 25 ++++++++++-------
tools/testing/selftests/bpf/usdt_2.c | 2 +-
4 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
index 2f22ec61667b..a60b8173cdc4 100644
--- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
+++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
@@ -398,7 +398,7 @@ static void *uprobe_producer_ret(void *input)
#ifdef __x86_64__
__nocf_check __weak void uprobe_target_nop5(void)
{
- asm volatile (".byte 0x0f, 0x1f, 0x44, 0x00, 0x00");
+ asm volatile (".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00");
}
static void *uprobe_producer_nop5(void *input)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index c944136252c6..9653fb5608f2 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -17,7 +17,7 @@
#include "uprobe_syscall_executed.skel.h"
#include "bpf/libbpf_internal.h"
-#define USDT_NOP .byte 0x0f, 0x1f, 0x44, 0x00, 0x00
+#define USDT_NOP .byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00
#include "usdt.h"
#pragma GCC diagnostic ignored "-Wattributes"
@@ -26,7 +26,7 @@ __attribute__((aligned(16)))
__nocf_check __weak __naked unsigned long uprobe_regs_trigger(void)
{
asm volatile (
- ".byte 0x0f, 0x1f, 0x44, 0x00, 0x00\n" /* nop5 */
+ ".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
"movq $0xdeadbeef, %rax\n"
"ret\n"
);
@@ -345,9 +345,9 @@ static void test_uretprobe_syscall_call(void)
__attribute__((aligned(16)))
__nocf_check __weak __naked void uprobe_test(void)
{
- asm volatile (" \n"
- ".byte 0x0f, 0x1f, 0x44, 0x00, 0x00 \n"
- "ret \n"
+ asm volatile (
+ ".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
+ "ret\n"
);
}
@@ -388,14 +388,15 @@ static int find_uprobes_trampoline(void *tramp_addr)
return ret;
}
-static unsigned char nop5[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+static unsigned char nop10[10] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
+static unsigned char lea_rsp[5] = { 0x48, 0x8d, 0x64, 0x24, 0x80 };
-static void *find_nop5(void *fn)
+static void *find_nop10(void *fn)
{
int i;
- for (i = 0; i < 10; i++) {
- if (!memcmp(nop5, fn + i, 5))
+ for (i = 0; i < 128; i++) {
+ if (!memcmp(nop10, fn + i, 10))
return fn + i;
}
return NULL;
@@ -420,7 +421,8 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
ASSERT_EQ(skel->bss->executed, executed, "executed");
/* .. and check the trampoline is as expected. */
- call = (struct __arch_relative_insn *) addr;
+ ASSERT_OK(memcmp(addr, lea_rsp, 5), "lea_rsp");
+ call = (struct __arch_relative_insn *)(addr + 5);
tramp = (void *) (call + 1) + call->raddr;
ASSERT_EQ(call->op, 0xe8, "call");
ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
@@ -432,7 +434,7 @@ static void check_detach(void *addr, void *tramp)
{
/* [uprobes_trampoline] stays after detach */
ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
- ASSERT_OK(memcmp(addr, nop5, 5), "nop5");
+ ASSERT_OK(memcmp(addr, jmp2B, 2), "jmp2B");
}
static void check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
@@ -568,8 +570,8 @@ static void test_uprobe_usdt(void)
void *addr;
errno = 0;
- addr = find_nop5(usdt_test);
- if (!ASSERT_OK_PTR(addr, "find_nop5"))
+ addr = find_nop10(usdt_test);
+ if (!ASSERT_OK_PTR(addr, "find_nop10"))
return;
skel = uprobe_syscall_executed__open_and_load();
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index 69759b27794d..fda3a298ccfc 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -252,7 +252,7 @@ extern void usdt_1(void);
extern void usdt_2(void);
static unsigned char nop1[1] = { 0x90 };
-static unsigned char nop1_nop5_combo[6] = { 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+static unsigned char nop1_nop10_combo[11] = { 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
static void *find_instr(void *fn, unsigned char *instr, size_t cnt)
{
@@ -271,17 +271,17 @@ static void subtest_optimized_attach(void)
__u8 *addr_1, *addr_2;
/* usdt_1 USDT probe has single nop instruction */
- addr_1 = find_instr(usdt_1, nop1_nop5_combo, 6);
- if (!ASSERT_NULL(addr_1, "usdt_1_find_nop1_nop5_combo"))
+ addr_1 = find_instr(usdt_1, nop1_nop10_combo, 11);
+ if (!ASSERT_NULL(addr_1, "usdt_1_find_nop1_nop10_combo"))
return;
addr_1 = find_instr(usdt_1, nop1, 1);
if (!ASSERT_OK_PTR(addr_1, "usdt_1_find_nop1"))
return;
- /* usdt_2 USDT probe has nop,nop5 instructions combo */
- addr_2 = find_instr(usdt_2, nop1_nop5_combo, 6);
- if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop5_combo"))
+ /* usdt_2 USDT probe has nop,nop10 instructions combo */
+ addr_2 = find_instr(usdt_2, nop1_nop10_combo, 11);
+ if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop10_combo"))
return;
skel = test_usdt__open_and_load();
@@ -309,12 +309,12 @@ static void subtest_optimized_attach(void)
bpf_link__destroy(skel->links.usdt_executed);
- /* we expect the nop5 ip */
+ /* we expect the nop10 ip */
skel->bss->expected_ip = (unsigned long) addr_2 + 1;
/*
* Attach program on top of usdt_2 which is probe defined on top
- * of nop1,nop5 combo, so the probe gets optimized on top of nop5.
+ * of nop1,nop10 combo, so the probe gets optimized on top of nop10.
*/
skel->links.usdt_executed = bpf_program__attach_usdt(skel->progs.usdt_executed,
0 /*self*/, "/proc/self/exe",
@@ -328,8 +328,13 @@ static void subtest_optimized_attach(void)
/* nop stays on addr_2 address */
ASSERT_EQ(*addr_2, 0x90, "nop");
- /* call is on addr_2 + 1 address */
- ASSERT_EQ(*(addr_2 + 1), 0xe8, "call");
+ /*
+ * lea -0x80(%rsp), %rsp
+ * call ...
+ */
+ static unsigned char expected[] = { 0x48, 0x8d, 0x64, 0x24, 0x80, 0xe8 };
+
+ ASSERT_MEMEQ(addr_2 + 1, expected, sizeof(expected), "lea_and_call");
ASSERT_EQ(skel->bss->executed, 4, "executed");
cleanup:
diff --git a/tools/testing/selftests/bpf/usdt_2.c b/tools/testing/selftests/bpf/usdt_2.c
index 789883aaca4c..b359b389f6c0 100644
--- a/tools/testing/selftests/bpf/usdt_2.c
+++ b/tools/testing/selftests/bpf/usdt_2.c
@@ -3,7 +3,7 @@
#if defined(__x86_64__)
/*
- * Include usdt.h with default nop,nop5 instructions combo.
+ * Include usdt.h with default nop,nop10 instructions combo.
*/
#include "usdt.h"
--
2.53.0
^ permalink raw reply related
* [PATCHv3 07/12] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
Syncing latest usdt.h change [1].
Now that we have nop10 optimization support in kernel, let's emit
nop,nop10 for usdt probe. We leave it up to the library to use
desirable nop instruction.
[1] TBD
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/usdt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
index c71e21df38b3..75687f50f4e2 100644
--- a/tools/testing/selftests/bpf/usdt.h
+++ b/tools/testing/selftests/bpf/usdt.h
@@ -313,7 +313,7 @@ struct usdt_sema { volatile unsigned short active; };
#if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
#define USDT_NOP nop 0
#elif defined(__x86_64__)
-#define USDT_NOP .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
+#define USDT_NOP .byte 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 /* nop, nop10 */
#else
#define USDT_NOP nop
#endif
--
2.53.0
^ permalink raw reply related
* [PATCHv3 06/12] libbpf: Detect uprobe syscall with new error
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
In the previous optimized uprobe fix we changed the syscall
error used for its detection from ENXIO to EPROTO.
Changing related probe_uprobe_syscall detection check.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Fixes: 05738da0efa1 ("libbpf: Add uprobe syscall feature detection")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/bpf/features.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index b7e388f99d0b..e5641fa60163 100644
--- a/tools/lib/bpf/features.c
+++ b/tools/lib/bpf/features.c
@@ -577,10 +577,10 @@ static int probe_ldimm64_full_range_off(int token_fd)
static int probe_uprobe_syscall(int token_fd)
{
/*
- * If kernel supports uprobe() syscall, it will return -ENXIO when called
+ * If kernel supports uprobe() syscall, it will return -EPROTO when called
* from the outside of a kernel-generated uprobe trampoline.
*/
- return syscall(__NR_uprobe) < 0 && errno == ENXIO;
+ return syscall(__NR_uprobe) < 0 && errno == EPROTO;
}
#else
static int probe_uprobe_syscall(int token_fd)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 955a37751b52..c944136252c6 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -762,7 +762,7 @@ static void test_uprobe_error(void)
long err = syscall(__NR_uprobe);
ASSERT_EQ(err, -1, "error");
- ASSERT_EQ(errno, ENXIO, "errno");
+ ASSERT_EQ(errno, EPROTO, "errno");
}
static void __test_uprobe_syscall(void)
--
2.53.0
^ permalink raw reply related
* [PATCHv3 05/12] libbpf: Change has_nop_combo to work on top of nop10
From: Jiri Olsa @ 2026-05-21 12:44 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: Jakub Sitnicki, bpf, linux-trace-kernel
In-Reply-To: <20260521124411.31133-1-jolsa@kernel.org>
We now expect nop combo with 10 bytes nop instead of 5 bytes nop,
fixing has_nop_combo to reflect that.
Fixes: 41a5c7df4466 ("libbpf: Add support to detect nop,nop5 instructions combo for usdt probe")
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/bpf/usdt.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
index e3710933fd52..484a4354e82b 100644
--- a/tools/lib/bpf/usdt.c
+++ b/tools/lib/bpf/usdt.c
@@ -305,7 +305,7 @@ struct usdt_manager *usdt_manager_new(struct bpf_object *obj)
/*
* Detect kernel support for uprobe() syscall, it's presence means we can
- * take advantage of faster nop5 uprobe handling.
+ * take advantage of faster nop10 uprobe handling.
* Added in: 56101b69c919 ("uprobes/x86: Add uprobe syscall to speed up uprobe")
*/
man->has_uprobe_syscall = kernel_supports(obj, FEAT_UPROBE_SYSCALL);
@@ -596,14 +596,14 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
#if defined(__x86_64__)
static bool has_nop_combo(int fd, long off)
{
- unsigned char nop_combo[6] = {
- 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 /* nop,nop5 */
+ unsigned char nop_combo[11] = {
+ 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00,
};
- unsigned char buf[6];
+ unsigned char buf[11];
- if (pread(fd, buf, 6, off) != 6)
+ if (pread(fd, buf, 11, off) != 11)
return false;
- return memcmp(buf, nop_combo, 6) == 0;
+ return memcmp(buf, nop_combo, 11) == 0;
}
#else
static bool has_nop_combo(int fd, long off)
@@ -814,8 +814,8 @@ static int collect_usdt_targets(struct usdt_manager *man, struct elf_fd *elf_fd,
memset(target, 0, sizeof(*target));
/*
- * We have uprobe syscall and usdt with nop,nop5 instructions combo,
- * so we can place the uprobe directly on nop5 (+1) and get this probe
+ * We have uprobe syscall and usdt with nop,nop10 instructions combo,
+ * so we can place the uprobe directly on nop10 (+1) and get this probe
* optimized.
*/
if (man->has_uprobe_syscall && has_nop_combo(elf_fd->fd, usdt_rel_ip)) {
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox