* Re: [PATCH v6 11/43] KVM: guest_memfd: Ensure pages are not in use before conversion
From: Fuad Tabba @ 2026-05-21 7:09 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-11-91ab5a8b19a4@google.com>
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>
>
> When converting memory to private in guest_memfd, it is necessary to ensure
> that the pages are not currently being accessed by any other part of the
> kernel or userspace to avoid any current user writing to guest private
> memory.
>
> guest_memfd checks for unexpected refcounts to determine whether a page is
> still in use. The only expected refcounts after unmapping the range
> requested for conversion are those that are held by guest_memfd itself.
>
> Update the kvm_memory_attributes2 structure to include an error_offset
> field. This allows KVM to report the exact offset where a conversion
> failed to userspace. If the safety check fails, return -EAGAIN and copy
> the error_offset back to userspace so that it can potentially retry the
> operation or handle the failure gracefully.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> ---
> include/uapi/linux/kvm.h | 3 ++-
> virt/kvm/guest_memfd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index e6bbf68a83813..0b55258573d3d 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1658,7 +1658,8 @@ struct kvm_memory_attributes2 {
> __u64 size;
> __u64 attributes;
> __u64 flags;
> - __u64 reserved[12];
> + __u64 error_offset;
> + __u64 reserved[11];
> };
>
> #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 91e89b188f583..9d82642a025e9 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -572,9 +572,42 @@ static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
> return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
> }
>
> +static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> + size_t nr_pages, pgoff_t *err_index)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + const int filemap_get_folios_refcount = 1;
> + pgoff_t last = start + nr_pages - 1;
> + struct folio_batch fbatch;
> + bool safe = true;
> + int i;
> +
> + folio_batch_init(&fbatch);
> + while (safe && filemap_get_folios(mapping, &start, last, &fbatch)) {
> +
> + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> + struct folio *folio = fbatch.folios[i];
> +
> + if (folio_ref_count(folio) !=
> + folio_nr_pages(folio) + filemap_get_folios_refcount) {
> + safe = false;
> + *err_index = folio->index;
> + break;
https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=11
Sashiko raised a few issues here, but I think this one might be
genuine. Can you look into it please?
If that's right, when huge page support lands, if start falls in the
middle of a large folio, returning folio->index as the err_index will
return an offset strictly less than the requested start. A naive
userspace retry loop resuming from error_offset would step backwards
and corrupt attributes on memory it didn't intend to convert.
err_index should be clamped to max(start, folio->index).
Cheers,
/fuad
> + }
> + }
> +
> + folio_batch_release(&fbatch);
> + cond_resched();
> + }
> +
> + return safe;
> +}
> +
> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> - size_t nr_pages, uint64_t attrs)
> + size_t nr_pages, uint64_t attrs,
> + pgoff_t *err_index)
> {
> + bool to_private = attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> struct address_space *mapping = inode->i_mapping;
> struct gmem_inode *gi = GMEM_I(inode);
> pgoff_t end = start + nr_pages;
> @@ -588,8 +621,21 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>
> mas_init(&mas, mt, start);
> r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
> - if (r)
> + if (r) {
> + *err_index = start;
> goto out;
> + }
> +
> + if (to_private) {
> + unmap_mapping_pages(mapping, start, nr_pages, false);
> +
> + if (!kvm_gmem_is_safe_for_conversion(inode, start, nr_pages,
> + err_index)) {
> + mas_destroy(&mas);
> + r = -EAGAIN;
> + goto out;
> + }
> + }
>
> /*
> * From this point on guest_memfd has performed necessary
> @@ -609,9 +655,10 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
> struct gmem_file *f = file->private_data;
> struct inode *inode = file_inode(file);
> struct kvm_memory_attributes2 attrs;
> + pgoff_t err_index;
> size_t nr_pages;
> pgoff_t index;
> - int i;
> + int i, r;
>
> if (copy_from_user(&attrs, argp, sizeof(attrs)))
> return -EFAULT;
> @@ -635,8 +682,16 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
>
> nr_pages = attrs.size >> PAGE_SHIFT;
> index = attrs.offset >> PAGE_SHIFT;
> - return __kvm_gmem_set_attributes(inode, index, nr_pages,
> - attrs.attributes);
> + r = __kvm_gmem_set_attributes(inode, index, nr_pages, attrs.attributes,
> + &err_index);
> + if (r) {
> + attrs.error_offset = ((uint64_t)err_index) << PAGE_SHIFT;
> +
> + if (copy_to_user(argp, &attrs, sizeof(attrs)))
> + return -EFAULT;
> + }
> +
> + return r;
> }
>
> static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl,
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 15/43] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Fuad Tabba @ 2026-05-21 7:13 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-15-91ab5a8b19a4@google.com>
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>
>
> When checking if a guest_memfd folio is safe for conversion, its refcount
> is examined. A folio may be present in a per-CPU lru_add fbatch, which
> temporarily increases its refcount. This can lead to a false positive,
> incorrectly indicating that the folio is in use and preventing the
> conversion, even if it is otherwise safe. The conversion process might not
> be on the same CPU that holds the folio in its fbatch, making a simple
> per-CPU check insufficient.
>
> To address this, drain all CPUs' lru_add fbatches if an unexpectedly high
> refcount is encountered during the safety check. This is performed at most
> once per conversion request. Draining only if the folio in question may be
> lru cached.
>
> guest_memfd folios are unevictable, so they can only reside in the lru_add
> fbatch. If the folio's refcount is still unsafe after draining, then the
> conversion is truly deemed unsafe.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Not an area I've worked with that much, but it seems right to me:
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> mm/swap.c | 2 ++
> virt/kvm/guest_memfd.c | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mm/swap.c b/mm/swap.c
> index 5cc44f0de9877..3134d9d3d7c30 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -37,6 +37,7 @@
> #include <linux/page_idle.h>
> #include <linux/local_lock.h>
> #include <linux/buffer_head.h>
> +#include <linux/kvm_types.h>
>
> #include "internal.h"
>
> @@ -904,6 +905,7 @@ void lru_add_drain_all(void)
> lru_add_drain();
> }
> #endif /* CONFIG_SMP */
> +EXPORT_SYMBOL_FOR_KVM(lru_add_drain_all);
>
> atomic_t lru_disable_count = ATOMIC_INIT(0);
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 034b72b4947fb..050a8c092b1a3 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -8,6 +8,7 @@
> #include <linux/mempolicy.h>
> #include <linux/pseudo_fs.h>
> #include <linux/pagemap.h>
> +#include <linux/swap.h>
>
> #include "kvm_mm.h"
>
> @@ -596,18 +597,27 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> const int filemap_get_folios_refcount = 1;
> pgoff_t last = start + nr_pages - 1;
> struct folio_batch fbatch;
> + bool lru_drained = false;
> bool safe = true;
> int i;
>
> folio_batch_init(&fbatch);
> while (safe && filemap_get_folios(mapping, &start, last, &fbatch)) {
>
> - for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> + for (i = 0; i < folio_batch_count(&fbatch);) {
> struct folio *folio = fbatch.folios[i];
>
> - if (folio_ref_count(folio) !=
> - folio_nr_pages(folio) + filemap_get_folios_refcount) {
> - safe = false;
> + safe = (folio_ref_count(folio) ==
> + folio_nr_pages(folio) +
> + filemap_get_folios_refcount);
> +
> + if (safe) {
> + ++i;
> + } else if (folio_may_be_lru_cached(folio) &&
> + !lru_drained) {
> + lru_add_drain_all();
> + lru_drained = true;
> + } else {
> *err_index = folio->index;
> break;
> }
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 05/43] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Fuad Tabba @ 2026-05-21 7:19 UTC (permalink / raw)
To: Ackerley Tng
Cc: 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,
Sean Christopherson, 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: <CAEvNRgGQvMdDmVfbk42EY_PGN0ybTp-x21Zj+pg_X1mk9iCRtA@mail.gmail.com>
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.
>
> I think the "consumption" of private vs shared here actually means
> something like "don't commit a page being faulted into page tables based
> on the result of kvm_gmem_get_memory_attributes() without checking
> kvm->mmu_invalidate_in_progress.", since a racing conversion may
> complete before you commit.
>
> kvm_mem_is_private() is used from these places:
>
> 1. Fault handling in KVM, like page_fault_can_be_fast(),
> kvm_mmu_faultin_pfn(), kvm_mmu_page_fault(): this already handles the
> entire mmu_lock and invalidation dance. No fault will be committed if
> a racing conversion happened after kvm_mem_is_private() but before
> the commit.
>
> 2. kvm_mmu_max_mapping_level() from recovering huge pages after
> disabling dirty logging: Other than that it can't be used with
> guest_memfd now since dirty logging can't be used with guest_memfd
> and guest_memfd memslots are not updatable, this holds mmu_lock
> throughout until the huge page recovery is done. invalidate_begin
> also involves zapping the pages in the range, so if the order of
> events is
>
> | Thread A | Thread B |
> |------------------------------|-------------------|
> | invalidate_begin + zap | |
> | update attributes maple_tree | recover huge page |
> | invalidate_end | |
>
> Then recovering will never see the zapped pages, nothing to
> recover, no kvm_mem_is_private() lookup.
>
> 3. kvm_arch_vcpu_pre_fault_memory()
>
> This eventually calls kvm_tdp_mmu_page_fault(), which checks
> is_page_fault_stale(), so it does check before committing.
>
> Were there any other calls I missed?
The one I was looking at was `sev_handle_rmp_fault()`, which does a lockless
read without the retry loop. But as Sean just pointed out, that path can
tolerate false positives/negatives and relies on the guest faulting again,
so the lack of synchronization there is existing behavior and considered "fine".
>
> > sev_handle_rmp_fault() calls kvm_mem_is_private() without holding
> > mmu_lock and without any retry mechanism. Is that a problem?
> >
>
> Sean already replied on your actual question separately :)
>
> > Cheers,
> > /fuad
> >
> >
> >>
> >> [...snip...]
> >>
^ 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 7:30 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-16-91ab5a8b19a4@google.com>
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) ||
+ end > i_size_read(inode))
goto err;
What do you think?
/fuad
> ---
> virt/kvm/guest_memfd.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 050a8c092b1a3..9f6eebfb68f6b 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -370,6 +370,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> struct kvm_memory_slot *slot;
> struct kvm *kvm = f->kvm;
> unsigned long index;
> + pgoff_t end;
>
> /*
> * Prevent concurrent attempts to *unbind* a memslot. This is the last
> @@ -396,9 +397,10 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> * Zap all SPTEs pointed at by this file. Do not free the backing
> * memory, as its lifetime is associated with the inode, not the file.
> */
> - __kvm_gmem_invalidate_begin(f, 0, -1ul,
> + end = i_size_read(inode) >> PAGE_SHIFT;
> + __kvm_gmem_invalidate_begin(f, 0, end,
> kvm_gmem_get_invalidate_filter(inode));
> - __kvm_gmem_invalidate_end(f, 0, -1ul);
> + __kvm_gmem_invalidate_end(f, 0, end);
>
> list_del(&f->entry);
>
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 17/43] KVM: guest_memfd: Determine invalidation filter from memory attributes
From: Fuad Tabba @ 2026-05-21 7:56 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-17-91ab5a8b19a4@google.com>
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>
>
> Before conversion, the range filter doesn't really matter:
>
> + For non-CoCo VMs that use guest_memfd, they have no mirrored tdp, so
> KVM_DIRECT_ROOTS would have been invalidated anyway.
> + CoCo VMs could not use INIT_SHARED, and there's no conversion support, so
> always using KVM_FILTER_PRIVATE would have worked.
>
> Now with conversion support, update kvm_gmem_get_invalidate_filter to
> inspect the memory attributes maple tree for a given range.
>
> Instead of determining the invalidation filter based on static inode
> flags, iterate through the attributes maple tree for the specific range
> being invalidated. This allows KVM to identify if the range contains
> private pages, shared pages, or both, and set the filter bits
> accordingly.
>
> Update kvm_gmem_invalidate_begin and kvm_gmem_release to pass the range
> parameters to the filter helper to ensure invalidation accurately
> targets the memory types present in the affected range.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> virt/kvm/guest_memfd.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 9f6eebfb68f6b..c9f155c2dc5c5 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -193,12 +193,24 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> return folio;
> }
>
> -static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *inode)
> +static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(
> + struct inode *inode, pgoff_t start, pgoff_t end)
> {
> - if (GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED)
> - return KVM_FILTER_SHARED;
> + struct gmem_inode *gi = GMEM_I(inode);
> + enum kvm_gfn_range_filter filter = 0;
> + void *entry;
> +
> + lockdep_assert(mt_lock_is_held(&gi->attributes));
> +
> + mt_for_each(&gi->attributes, entry, start, end - 1) {
> + filter |= (xa_to_value(entry) & KVM_MEMORY_ATTRIBUTE_PRIVATE) ?
> + KVM_FILTER_PRIVATE : KVM_FILTER_SHARED;
> +
> + if (filter == (KVM_FILTER_PRIVATE | KVM_FILTER_SHARED))
> + break;
> + }
>
> - return KVM_FILTER_PRIVATE;
> + return filter;
> }
>
> static void __kvm_gmem_invalidate_begin(struct gmem_file *f, pgoff_t start,
> @@ -244,7 +256,7 @@ static void kvm_gmem_invalidate_begin(struct inode *inode, pgoff_t start,
> enum kvm_gfn_range_filter attr_filter;
> struct gmem_file *f;
>
> - attr_filter = kvm_gmem_get_invalidate_filter(inode);
> + attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
>
> kvm_gmem_for_each_file(f, inode)
> __kvm_gmem_invalidate_begin(f, start, end, attr_filter);
> @@ -367,6 +379,7 @@ static long kvm_gmem_fallocate(struct file *file, int mode, loff_t offset,
> static int kvm_gmem_release(struct inode *inode, struct file *file)
> {
> struct gmem_file *f = file->private_data;
> + enum kvm_gfn_range_filter filter;
> struct kvm_memory_slot *slot;
> struct kvm *kvm = f->kvm;
> unsigned long index;
> @@ -398,8 +411,8 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> * memory, as its lifetime is associated with the inode, not the file.
> */
> end = i_size_read(inode) >> PAGE_SHIFT;
> - __kvm_gmem_invalidate_begin(f, 0, end,
> - kvm_gmem_get_invalidate_filter(inode));
> + filter = kvm_gmem_get_invalidate_filter(inode, 0, end);
> + __kvm_gmem_invalidate_begin(f, 0, end, filter);
> __kvm_gmem_invalidate_end(f, 0, end);
>
> list_del(&f->entry);
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH] gpu: host1x: trace: fix string fields in host1x traces
From: Thierry Reding @ 2026-05-21 8:02 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Steven Rostedt, Artur Kowalski, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-trace-kernel, linux-tegra,
Thierry Reding, David Airlie, Simona Vetter
In-Reply-To: <pQKtL-t0RKO7KsvRTAIQ_w@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]
On Thu, May 21, 2026 at 01:33:24PM +0900, Mikko Perttunen wrote:
> On Wednesday, May 20, 2026 9:03 PM Thierry Reding wrote:
> > On Tue, May 19, 2026 at 02:10:59PM -0400, Steven Rostedt wrote:
> > > On Tue, 19 May 2026 12:16:43 +0200
> > > Artur Kowalski <arturkow2000@gmail.com> wrote:
> > >
> > > > Use __assign_str and __get_str as required by tracing subsystem. Fixes
> > > > string fields being rejected by the verifier and unreadable from
> > > > userspace.
> > >
> > > Does anyone use these tracepoints? The fact that they have been broken
> > > for 5 years and nobody noticed makes me think they are useless.
> > >
> > > I rather remove them than fix them, but if someone thinks that these
> > > are still useful then by all means apply this patch.
> > >
> > > Acked-by: Steven Rostedt <rostedt@goodmis.org>
> >
> > I know that Mikko used them a lot early on, but this driver is pretty
> > mature now, so we rarely need this low level of tracing. I'll defer to
> > Mikko on whether we still need these.
> >
> > Thierry
>
> Yeah, these have been quite useful in the past when debugging why a job
> is failing. Without the cdma traces it can be cumbersome to find out
> exactly what is being sent to the hardware in some cases.
>
> My preference is for keeping them for now.
Thanks, I'll pick this up then.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v6 18/43] KVM: Move KVM_VM_MEMORY_ATTRIBUTES config definition to x86
From: Fuad Tabba @ 2026-05-21 8:07 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-18-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Bury KVM_VM_MEMORY_ATTRIBUTES in x86 to discourage other architectures
> from adding support for per-VM memory attributes, because tracking private
> vs. shared memory on a per-VM basis is now deprecated in favor of tracking
> on a per-guest_memfd basis, and no other memory attributes are on the
> horizon.
>
> This will also allow modifying KVM_VM_MEMORY_ATTRIBUTES to be
> user-selectable (in x86) without creating weirdness in KVM's Kconfigs.
> Now that guest_memfd support memory attributes, it's entirely possible to
> run x86 CoCo VMs without support for KVM_VM_MEMORY_ATTRIBUTES.
>
> Leave the code itself in common KVM so that it's trivial to undo this
> change if new per-VM attributes do come along.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> arch/x86/kvm/Kconfig | 4 ++++
> virt/kvm/Kconfig | 4 ----
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 26f6afd51bbdc..b6d65ee664d0f 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -80,6 +80,10 @@ config KVM_WERROR
>
> If in doubt, say "N".
>
> +config KVM_VM_MEMORY_ATTRIBUTES
> + select KVM_MEMORY_ATTRIBUTES
> + bool
> +
> config KVM_SW_PROTECTED_VM
> bool "Enable support for KVM software-protected VMs"
> depends on EXPERT
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index e371e079e2c50..663de6421eda2 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -103,10 +103,6 @@ config KVM_MMU_LOCKLESS_AGING
> config KVM_MEMORY_ATTRIBUTES
> bool
>
> -config KVM_VM_MEMORY_ATTRIBUTES
> - select KVM_MEMORY_ATTRIBUTES
> - bool
> -
> config KVM_GUEST_MEMFD
> select XARRAY_MULTI
> select KVM_MEMORY_ATTRIBUTES
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v20 00/10] ring-buffer: Making persistent ring buffers robust
From: Masami Hiramatsu @ 2026-05-21 8:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers, Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260520184938.749337513@kernel.org>
On Wed, 20 May 2026 14:49:38 -0400
Steven Rostedt <rostedt@kernel.org> wrote:
>
> Here is the 20th version of improvement patches for making persistent
> ring buffers robust to failures.
> The previous version is here:
>
> https://lore.kernel.org/all/177751968499.2136606.17388366710182662849.stgit@mhiramat.tok.corp.google.com/
>
> None of the patches from the 19th version was changed. Only patches were
> added to it in this version. All of Masami's patches were in version 19,
> and all my patches are new to version 20. The reason I'm including
> Masami's patches with mine is so that Sashiko can handle all of them
> in one go.
Thanks for updating.
BTW, it seems Sashiko is stopping review this series.
https://sashiko.dev/#/patchset/20260520184938.749337513%40kernel.org
Not sure why.
>
> I moved patch 1 from v19 to my urgent branch as it was marked as
> fix for stable.
>
> The patches I added:
>
> - Fix an invalid sub-buffer in the iterator (added TBD fixes tag)
> I didn't want to fold the patch into the patch that was fixed
> as it was written by Masami.
>
> - Have the dropped buffers be persistent across boots. Masami's patches
> cleared the detection of lost subbuffers on boot up and the next
> boot would not show them. If the persistent ring buffer isn't cleared
> it should report the same info on subsequent boots.
>
> - Show [LOST EVENTS] in persistent trace file where a subbuffer was
> reset due to being invalid.
>
> - Show [LOST EVENTS] in the persistent trace_pipe file as well.
>
> Masami Hiramatsu (Google) (6):
> ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
> ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
> ring-buffer: Add persistent ring buffer invalid-page inject test
> ring-buffer: Show commit numbers in buffer_meta file
> ring-buffer: Cleanup persistent ring buffer validation
> ring-buffer: Cleanup buffer_data_page related code
>
> Steven Rostedt (4):
> ring-buffer: Skip invalid sub-buffers for iterator
> ring-buffer: Have dropped subbuffers be persistent across reboots
> ring-buffer: Show persistent buffer dropped events in trace file
> ring-buffer: Show persistent buffer dropped events in trace_pipe file
>
> ----
> include/linux/ring_buffer.h | 1 +
> kernel/trace/Kconfig | 34 +++
> kernel/trace/ring_buffer.c | 538 +++++++++++++++++++++++++++++---------------
> kernel/trace/trace.c | 4 +
> 4 files changed, 397 insertions(+), 180 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v20 10/10] ring-buffer: Show persistent buffer dropped events in trace_pipe file
From: Masami Hiramatsu @ 2026-05-21 8:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers, Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260520185018.470465795@kernel.org>
On Wed, 20 May 2026 14:49:48 -0400
Steven Rostedt <rostedt@kernel.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When the persistent ring buffer is validated on boot up, if a subbuffer is
> deemed invalid, it resets the buffer and continues. Have the code preserve
> the RB_MISSED_EVENTS flag in the commit portion of the subbuffer header
> and pass that back so that the trace_pipe file can show the missed events
> like the trace file does.
>
> For example:
>
> <...>-1242 [005] d.... 4429.120116: page_fault_user: address=0x7ffaebb6e728 ip=0x7ffaeb9d4960 error_code=0x7
> <...>-1242 [005] ..... 4429.120124: mm_page_alloc: page=00000000055254f3 pfn=0x1373bd order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE|__GFP_COMP
> <...>-1242 [005] d..2. 4429.120132: tlb_flush: pages:1 reason:local MM shootdown (3)
> CPU:5 [LOST EVENTS]
> <...>-1242 [005] d.... 4429.120661: page_fault_user: address=0x55ba7c2d0944 ip=0x55ba7c20cd02 error_code=0x7
> <...>-1242 [005] ..... 4429.120669: mm_page_alloc: page=0000000005a02500 pfn=0x12b6e4 order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE|__GFP_COMP
> <...>-1242 [005] d..2. 4429.120680: tlb_flush: pages:1 reason:local MM shootdown (3)
OK, this looks good, but I have a comment below.
[...]
> @@ -7123,19 +7132,23 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> * the reader page.
> */
> if (full &&
> - (!read || (len < (commit - read)) ||
> + (!read || (len < (size - read)) ||
> cpu_buffer->reader_page == cpu_buffer->commit_page))
> return -1;
>
> - if (len > (commit - read))
> - len = (commit - read);
> + if (len > (size - read))
> + len = (size - read);
>
> /* Always keep the time extend and data together */
> - size = rb_event_ts_length(event);
> + event_size = rb_event_ts_length(event);
>
> - if (len < size)
> + if (len < event_size)
> return -1;
>
> + if (commit & RB_MISSED_EVENTS) {
> + printk("MISSED\n");
Is it for debug?
> + flags = RB_MISSED_EVENTS; }
nit: block closing brace is in the previous line. Maybe typo?
Thanks,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v6 19/43] KVM: Let userspace disable per-VM mem attributes, enable per-gmem attributes
From: Fuad Tabba @ 2026-05-21 8:44 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-19-91ab5a8b19a4@google.com>
Hi Ackerley,
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Make vm_memory_attributes a module parameter so that userspace can disable
> the use of memory attributes on the VM level.
>
> To avoid inconsistencies in the way memory attributes are tracked in KVM
> and guest_memfd, the vm_memory_attributes module_param is made
> read-only (0444).
>
> Make CONFIG_KVM_VM_MEMORY_ATTRIBUTES selectable, only for (CoCo) VM types
> that might use vm_memory_attributes.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Config files always confuse me, but Sashiko might be onto something:
https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=19
I think this partially goes back to commit 6, the one I flagged
yesterday. But also adding "default y" to KVM_VM_MEMORY_ATTRIBUTES?
The default value should at least fix this issue, but I'm not sure if
it would cause other problems...
Cheers,
/fuad
> ---
> arch/x86/kvm/Kconfig | 13 +++++++++----
> virt/kvm/kvm_main.c | 1 +
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index b6d65ee664d0f..8b97d341bd33f 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -82,13 +82,20 @@ config KVM_WERROR
>
> config KVM_VM_MEMORY_ATTRIBUTES
> select KVM_MEMORY_ATTRIBUTES
> - bool
> + depends on KVM_SW_PROTECTED_VM || KVM_INTEL_TDX || KVM_AMD_SEV
> + bool "Enable per-VM memory attributes (for CoCo VMs)"
> + help
> + Enable support for per-VM memory attributes, which are deprecated in
> + favor of tracking memory attributes in guest_memfd. Select this if
> + you need to run CoCo VMs using a VMM that doesn't support guest_memfd
> + memory attributes.
> +
> + If unsure, say N.
>
> config KVM_SW_PROTECTED_VM
> bool "Enable support for KVM software-protected VMs"
> depends on EXPERT
> depends on KVM_X86 && X86_64
> - select KVM_VM_MEMORY_ATTRIBUTES
> help
> Enable support for KVM software-protected VMs. Currently, software-
> protected VMs are purely a development and testing vehicle for
> @@ -139,7 +146,6 @@ config KVM_INTEL_TDX
> bool "Intel Trust Domain Extensions (TDX) support"
> default y
> depends on INTEL_TDX_HOST
> - select KVM_VM_MEMORY_ATTRIBUTES
> select HAVE_KVM_ARCH_GMEM_POPULATE
> help
> Provides support for launching Intel Trust Domain Extensions (TDX)
> @@ -163,7 +169,6 @@ config KVM_AMD_SEV
> depends on KVM_AMD && X86_64
> depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
> select ARCH_HAS_CC_PLATFORM
> - select KVM_VM_MEMORY_ATTRIBUTES
> select HAVE_KVM_ARCH_GMEM_PREPARE
> select HAVE_KVM_ARCH_GMEM_INVALIDATE
> select HAVE_KVM_ARCH_GMEM_POPULATE
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cec02d68d7039..ba195bb239aaa 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -104,6 +104,7 @@ module_param(allow_unsafe_mappings, bool, 0444);
> #ifdef CONFIG_KVM_MEMORY_ATTRIBUTES
> #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> bool vm_memory_attributes = true;
> +module_param(vm_memory_attributes, bool, 0444);
> #endif
> DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, kvm_get_memory_attributes_t);
> EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes));
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 20/43] KVM: guest_memfd: Enable INIT_SHARED on guest_memfd for x86 Coco VMs
From: Fuad Tabba @ 2026-05-21 8:54 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-20-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Now that guest_memfd supports tracking private vs. shared within gmem
> itself, allow userspace to specify INIT_SHARED on a guest_memfd instance
> for x86 Confidential Computing (CoCo) VMs, so long as per-VM attributes
> are disabled, i.e. when it's actually possible for a guest_memfd instance
> to contain shared memory.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> arch/x86/kvm/x86.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1560de1e95be0..6609957ecfea3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -14172,14 +14172,13 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
> }
>
> #ifdef CONFIG_KVM_GUEST_MEMFD
> -/*
> - * KVM doesn't yet support initializing guest_memfd memory as shared for VMs
> - * with private memory (the private vs. shared tracking needs to be moved into
> - * guest_memfd).
> - */
> bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
> {
> - return !kvm_arch_has_private_mem(kvm);
> + /*
> + * INIT_SHARED isn't supported if the memory attributes are per-VM,
> + * in which case guest_memfd can _only_ be used for private memory.
> + */
> + return !vm_memory_attributes || !kvm_arch_has_private_mem(kvm);
> }
>
> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 21/43] KVM: SEV: Make 'uaddr' parameter optional for KVM_SEV_SNP_LAUNCH_UPDATE
From: Fuad Tabba @ 2026-05-21 9:55 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-21-91ab5a8b19a4@google.com>
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
- an ordering violation with the kunmap_local() calls
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.
Cheers,
/fuad
> ---
> Documentation/virt/kvm/x86/amd-memory-encryption.rst | 15 +++++++++++----
> arch/x86/kvm/svm/sev.c | 18 +++++++++++++-----
> virt/kvm/kvm_main.c | 1 +
> 3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> index b2395dd4769de..43085f65b2d85 100644
> --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> @@ -503,7 +503,8 @@ secrets.
>
> It is required that the GPA ranges initialized by this command have had the
> KVM_MEMORY_ATTRIBUTE_PRIVATE attribute set in advance. See the documentation
> -for KVM_SET_MEMORY_ATTRIBUTES for more details on this aspect.
> +for KVM_SET_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES2 for more details on
> +this aspect.
>
> Upon success, this command is not guaranteed to have processed the entire
> range requested. Instead, the ``gfn_start``, ``uaddr``, and ``len`` fields of
> @@ -511,9 +512,15 @@ range requested. Instead, the ``gfn_start``, ``uaddr``, and ``len`` fields of
> remaining range that has yet to be processed. The caller should continue
> calling this command until those fields indicate the entire range has been
> processed, e.g. ``len`` is 0, ``gfn_start`` is equal to the last GFN in the
> -range plus 1, and ``uaddr`` is the last byte of the userspace-provided source
> -buffer address plus 1. In the case where ``type`` is KVM_SEV_SNP_PAGE_TYPE_ZERO,
> -``uaddr`` will be ignored completely.
> +range plus 1, and ``uaddr`` (if specified) is the last byte of the
> +userspace-provided source buffer address plus 1.
> +
> +In the case where ``type`` is KVM_SEV_SNP_PAGE_TYPE_ZERO, ``uaddr`` will be
> +ignored completely. Otherwise, ``uaddr`` is required if
> +kvm.vm_memory_attributes=1 and optional if kvm.vm_memory_attributes=0, since
> +in the latter case guest memory can be initialized directly from userspace
> +prior to converting it to private and passing the GPA range on to this
> +interface.
>
> Parameters (in): struct kvm_sev_snp_launch_update
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c2126b3c30724..bf10d24907a00 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2343,7 +2343,15 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> int level;
> int ret;
>
> - if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> + /*
> + * For vm_memory_attributes=1, in-place conversion/population is not
> + * supported, so the initial contents necessarily need to come from a
> + * separate src address. For vm_memory_attributes=0, this isn't
> + * necessarily the case, since the pages may have been populated
> + * directly from userspace before calling KVM_SEV_SNP_LAUNCH_UPDATE.
> + */
> + if (vm_memory_attributes &&
> + sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page)
> return -EINVAL;
>
> ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
> @@ -2390,7 +2398,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> */
> if (ret && !snp_page_reclaim(kvm, pfn) &&
> sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
> - sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
> + sev_populate_args->fw_error == SEV_RET_INVALID_PARAM && src_page) {
> void *src_vaddr = kmap_local_page(src_page);
> void *dst_vaddr = kmap_local_pfn(pfn);
>
> @@ -2422,8 +2430,8 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params)))
> return -EFAULT;
>
> - pr_debug("%s: GFN start 0x%llx length 0x%llx type %d flags %d\n", __func__,
> - params.gfn_start, params.len, params.type, params.flags);
> + pr_debug("%s: GFN start 0x%llx length 0x%llx type %d flags %d src %llx\n", __func__,
> + params.gfn_start, params.len, params.type, params.flags, params.uaddr);
>
> if (!params.len || !PAGE_ALIGNED(params.len) || params.flags ||
> (params.type != KVM_SEV_SNP_PAGE_TYPE_NORMAL &&
> @@ -2479,7 +2487,7 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>
> params.gfn_start += count;
> params.len -= count * PAGE_SIZE;
> - if (params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
> + if (src && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
> params.uaddr += count * PAGE_SIZE;
>
> if (copy_to_user(u64_to_user_ptr(argp->data), ¶ms, sizeof(params)))
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ba195bb239aaa..3bf212fd99193 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -105,6 +105,7 @@ module_param(allow_unsafe_mappings, bool, 0444);
> #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> bool vm_memory_attributes = true;
> module_param(vm_memory_attributes, bool, 0444);
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(vm_memory_attributes);
> #endif
> DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, kvm_get_memory_attributes_t);
> EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes));
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 23/43] KVM: selftests: Create gmem fd before "regular" fd when adding memslot
From: Fuad Tabba @ 2026-05-21 12:11 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-23-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:23, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> When adding a memslot associated a guest_memfd instance, create/dup the
> guest_memfd before creating the "normal" backing file. This will allow
> dup'ing the gmem fd as the normal fd when guest_memfd supports mmap(),
> i.e. to make guest_memfd the _only_ backing source for the memslot.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/lib/kvm_util.c | 45 +++++++++++++++---------------
> 1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 2a76eca7029d3..df73b23a4c66a 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1054,6 +1054,29 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> if (alignment > 1)
> region->mmap_size += alignment;
>
> + if (flags & KVM_MEM_GUEST_MEMFD) {
> + if (guest_memfd < 0) {
> + u32 guest_memfd_flags = 0;
> +
> + TEST_ASSERT(!guest_memfd_offset,
> + "Offset must be zero when creating new guest_memfd");
> + guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags);
> + } else {
> + /*
> + * Install a unique fd for each memslot so that the fd
> + * can be closed when the region is deleted without
> + * needing to track if the fd is owned by the framework
> + * or by the caller.
> + */
> + guest_memfd = kvm_dup(guest_memfd);
> + }
> +
> + region->region.guest_memfd = guest_memfd;
> + region->region.guest_memfd_offset = guest_memfd_offset;
> + } else {
> + region->region.guest_memfd = -1;
> + }
> +
> region->fd = -1;
> if (backing_src_is_shared(src_type))
> region->fd = kvm_memfd_alloc(region->mmap_size,
> @@ -1083,28 +1106,6 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
>
> region->backing_src_type = src_type;
>
> - if (flags & KVM_MEM_GUEST_MEMFD) {
> - if (guest_memfd < 0) {
> - u32 guest_memfd_flags = 0;
> - TEST_ASSERT(!guest_memfd_offset,
> - "Offset must be zero when creating new guest_memfd");
> - guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags);
> - } else {
> - /*
> - * Install a unique fd for each memslot so that the fd
> - * can be closed when the region is deleted without
> - * needing to track if the fd is owned by the framework
> - * or by the caller.
> - */
> - guest_memfd = kvm_dup(guest_memfd);
> - }
> -
> - region->region.guest_memfd = guest_memfd;
> - region->region.guest_memfd_offset = guest_memfd_offset;
> - } else {
> - region->region.guest_memfd = -1;
> - }
> -
> region->unused_phy_pages = sparsebit_alloc();
> if (vm_arch_has_protected_memory(vm))
> region->protected_phy_pages = sparsebit_alloc();
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 24/43] KVM: selftests: Rename guest_memfd{,_offset} to gmem_{fd,offset}
From: Fuad Tabba @ 2026-05-21 12:13 UTC (permalink / raw)
To: ackerleytng
Cc: 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,
Sean Christopherson, 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: <20260507-gmem-inplace-conversion-v6-24-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:23, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Rename local variables and function parameters for the guest memory file
> descriptor and its offset to use a "gmem_" prefix instead of
> "guest_memfd_".
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/include/kvm_util.h | 6 +++---
> tools/testing/selftests/kvm/lib/kvm_util.c | 26 +++++++++++++-------------
> 2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 2ecaaa0e99654..f19383376ee8e 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -690,17 +690,17 @@ int __vm_set_user_memory_region(struct kvm_vm *vm, u32 slot, u32 flags,
> gpa_t gpa, u64 size, void *hva);
> void vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> gpa_t gpa, u64 size, void *hva,
> - u32 guest_memfd, u64 guest_memfd_offset);
> + u32 gmem_fd, u64 gmem_offset);
> int __vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> gpa_t gpa, u64 size, void *hva,
> - u32 guest_memfd, u64 guest_memfd_offset);
> + u32 gmem_fd, u64 gmem_offset);
>
> void vm_userspace_mem_region_add(struct kvm_vm *vm,
> enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags);
> void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags,
> - int guest_memfd_fd, u64 guest_memfd_offset);
> + int gmem_fd, u64 gmem_offset);
>
> #ifndef vm_arch_has_protected_memory
> static inline bool vm_arch_has_protected_memory(struct kvm_vm *vm)
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index df73b23a4c66a..11da9b7546d03 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -947,7 +947,7 @@ void vm_set_user_memory_region(struct kvm_vm *vm, u32 slot, u32 flags,
>
> int __vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> gpa_t gpa, u64 size, void *hva,
> - u32 guest_memfd, u64 guest_memfd_offset)
> + u32 gmem_fd, u64 gmem_offset)
> {
> struct kvm_userspace_memory_region2 region = {
> .slot = slot,
> @@ -955,8 +955,8 @@ int __vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> .guest_phys_addr = gpa,
> .memory_size = size,
> .userspace_addr = (uintptr_t)hva,
> - .guest_memfd = guest_memfd,
> - .guest_memfd_offset = guest_memfd_offset,
> + .guest_memfd = gmem_fd,
> + .guest_memfd_offset = gmem_offset,
> };
>
> TEST_REQUIRE_SET_USER_MEMORY_REGION2();
> @@ -966,10 +966,10 @@ int __vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
>
> void vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> gpa_t gpa, u64 size, void *hva,
> - u32 guest_memfd, u64 guest_memfd_offset)
> + u32 gmem_fd, u64 gmem_offset)
> {
> int ret = __vm_set_user_memory_region2(vm, slot, flags, gpa, size, hva,
> - guest_memfd, guest_memfd_offset);
> + gmem_fd, gmem_offset);
>
> TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION2 failed, errno = %d (%s)",
> errno, strerror(errno));
> @@ -979,7 +979,7 @@ void vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> /* FIXME: This thing needs to be ripped apart and rewritten. */
> void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags,
> - int guest_memfd, u64 guest_memfd_offset)
> + int gmem_fd, u64 gmem_offset)
> {
> int ret;
> struct userspace_mem_region *region;
> @@ -1055,12 +1055,12 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> region->mmap_size += alignment;
>
> if (flags & KVM_MEM_GUEST_MEMFD) {
> - if (guest_memfd < 0) {
> - u32 guest_memfd_flags = 0;
> + if (gmem_fd < 0) {
> + u32 gmem_flags = 0;
>
> - TEST_ASSERT(!guest_memfd_offset,
> + TEST_ASSERT(!gmem_offset,
> "Offset must be zero when creating new guest_memfd");
> - guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags);
> + gmem_fd = vm_create_guest_memfd(vm, mem_size, gmem_flags);
> } else {
> /*
> * Install a unique fd for each memslot so that the fd
> @@ -1068,11 +1068,11 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> * needing to track if the fd is owned by the framework
> * or by the caller.
> */
> - guest_memfd = kvm_dup(guest_memfd);
> + gmem_fd = kvm_dup(gmem_fd);
> }
>
> - region->region.guest_memfd = guest_memfd;
> - region->region.guest_memfd_offset = guest_memfd_offset;
> + region->region.guest_memfd = gmem_fd;
> + region->region.guest_memfd_offset = gmem_offset;
> } else {
> region->region.guest_memfd = -1;
> }
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v20 09/10] ring-buffer: Show persistent buffer dropped events in trace file
From: Steven Rostedt @ 2026-05-21 12:14 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, linux-trace-kernel, Mathieu Desnoyers,
Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260521153416.71be1cd6a42be89a12e0bc62@kernel.org>
On Thu, 21 May 2026 15:34:16 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> I think it is good to show the dropped events. BTW, is it better to
> comment out the line, just for parser?
> For example, add a '#' at the like.
>
> # CPU:5 [LOST EVENTS]
>
> Ah, but it is already done...
Yeah, can't modify that. The "[LOST EVENTS]" output has been around since
2020 and changing that now will likely break user API.
-- Steve
^ permalink raw reply
* Re: [PATCH v20 10/10] ring-buffer: Show persistent buffer dropped events in trace_pipe file
From: Steven Rostedt @ 2026-05-21 12:17 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, linux-trace-kernel, Mathieu Desnoyers,
Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260521171859.dec135e45fb443b7bf5fb964@kernel.org>
On Thu, 21 May 2026 17:18:59 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> >
> > + if (commit & RB_MISSED_EVENTS) {
> > + printk("MISSED\n");
>
> Is it for debug?
Yes, that was left over. I thought I got rid of all of them. :-p
>
> > + flags = RB_MISSED_EVENTS; }
>
> nit: block closing brace is in the previous line. Maybe typo?
When I do debug statements, I end block statements like this to let me know
that the if statement is supposed to be one line when I remove the print.
But since I missed removing this one, I kept the above formatting.
I'll remove this in v21.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v20 00/10] ring-buffer: Making persistent ring buffers robust
From: Steven Rostedt @ 2026-05-21 12:17 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, linux-trace-kernel, Mathieu Desnoyers,
Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260521171353.74e66d117b0809418c8822d4@kernel.org>
On Thu, 21 May 2026 17:13:53 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Thanks for updating.
>
> BTW, it seems Sashiko is stopping review this series.
> https://sashiko.dev/#/patchset/20260520184938.749337513%40kernel.org
>
> Not sure why.
Yeah, I noticed that too. And it never did your v19 either.
-- Steve
^ permalink raw reply
* Re: [PATCH v20 00/10] ring-buffer: Making persistent ring buffers robust
From: Steven Rostedt @ 2026-05-21 12:24 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, linux-trace-kernel, Mathieu Desnoyers,
Catalin Marinas, Will Deacon, Ian Rogers
In-Reply-To: <20260521081740.31194131@gandalf.local.home>
On Thu, 21 May 2026 08:17:40 -0400
Steven Rostedt <rostedt@kernel.org> wrote:
> On Thu, 21 May 2026 17:13:53 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > Thanks for updating.
> >
> > BTW, it seems Sashiko is stopping review this series.
> > https://sashiko.dev/#/patchset/20260520184938.749337513%40kernel.org
> >
> > Not sure why.
>
> Yeah, I noticed that too. And it never did your v19 either.
Looking now, it did finish v19. It wasn't finished yesterday. Maybe it just
took a lot longer for this series.
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: Fix unload_page for simple_ring_buffer init rollback
From: Steven Rostedt @ 2026-05-21 12:31 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, kernel-team,
linux-kernel
In-Reply-To: <20260512141614.1759430-1-vdonnefort@google.com>
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>
-- Steve
^ permalink raw reply
* [PATCHv3 00/12] uprobes/x86: Fix red zone issue for optimized uprobes
From: Jiri Olsa @ 2026-05-21 12:43 UTC (permalink / raw)
To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
Andrii Nakryiko
Cc: bpf, linux-trace-kernel
hi,
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.
Note we need upstream update first for patch 3 (github.com/libbpf/usdt),
if we decide to take this change.
thanks,
jirka
v1: https://lore.kernel.org/bpf/20260514135342.22130-1-jolsa@kernel.org/
v2: https://lore.kernel.org/bpf/20260518105957.123445-1-jolsa@kernel.org/
v3 changes:
- use nop10 update suggested by Peter in [2]
- remove struct uprobe_trampoline object, use vma objects directly instead
- selftests fixes [sashiko]
- ack from Andrii
v2 changes:
- several selftest fixes [sashiko]
- consolidate is_lea_insn and is_call_insn insto single check [Jakub Sitnicki]
- use proper mm_struct object in __in_uprobe_trampoline check [sashiko]
- allow to copy uprobe trampolines vma objects on fork [sashiko]
- change uprobe syscall detection error from -ENXIO to -EPROTO [Andrii]
- added fork/clone tests
- I kept the selftest changes and nop5->nop10 changes in separate
commits for easier review, we can squash them later if we want to keep
bisect working properly
[1] https://lore.kernel.org/bpf/20260509003146.976844-1-andrii@kernel.org/
[2] https://lore.kernel.org/bpf/20260518104306.GU3102624@noisy.programming.kicks-ass.net/#t
---
Andrii Nakryiko (1):
selftests/bpf: Add tests for uprobe nop10 red zone clobbering
Jiri Olsa (11):
uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline
uprobes/x86: Remove struct uprobe_trampoline object
uprobes/x86: Allow to copy uprobe trampolines on fork
uprobes/x86: Move optimized uprobe from nop5 to nop10
libbpf: Change has_nop_combo to work on top of nop10
libbpf: Detect uprobe syscall with new error
selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch
selftests/bpf: Change uprobe syscall tests to use nop10
selftests/bpf: Change uprobe/usdt trigger bench code to use nop10
selftests/bpf: Add reattach tests for uprobe syscall
selftests/bpf: Add tests for forked/cloned optimized uprobes
arch/x86/kernel/uprobes.c | 393 ++++++++++++++++++++++++++++++++++++++++++++----------------------------
include/linux/uprobes.h | 5 -
kernel/events/uprobes.c | 10 --
kernel/fork.c | 1 -
tools/lib/bpf/features.c | 4 +-
tools/lib/bpf/usdt.c | 16 +--
tools/testing/selftests/bpf/bench.c | 20 ++--
tools/testing/selftests/bpf/benchs/bench_trigger.c | 38 +++----
tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh | 2 +-
tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 307 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
tools/testing/selftests/bpf/prog_tests/usdt.c | 74 ++++++++++++--
tools/testing/selftests/bpf/progs/test_usdt.c | 25 +++++
tools/testing/selftests/bpf/usdt.h | 2 +-
tools/testing/selftests/bpf/usdt_2.c | 15 ++-
14 files changed, 670 insertions(+), 242 deletions(-)
^ permalink raw reply
* [PATCHv3 01/12] uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline
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 unregister path we use __in_uprobe_trampoline check with
current->mm for the VMA lookup, which is wrong, because we are
in the tracer context, not the traced process.
Add mm_struct pointer argument to __in_uprobe_trampoline and
changing related callers to pass proper mm_struct pointer.
Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/kernel/uprobes.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index ebb1baf1eb1d..2be6707e3320 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -761,9 +761,9 @@ void arch_uprobe_clear_state(struct mm_struct *mm)
destroy_uprobe_trampoline(tramp);
}
-static bool __in_uprobe_trampoline(unsigned long ip)
+static bool __in_uprobe_trampoline(struct mm_struct *mm, unsigned long ip)
{
- struct vm_area_struct *vma = vma_lookup(current->mm, ip);
+ struct vm_area_struct *vma = vma_lookup(mm, ip);
return vma && vma_is_special_mapping(vma, &tramp_mapping);
}
@@ -776,14 +776,14 @@ static bool in_uprobe_trampoline(unsigned long ip)
rcu_read_lock();
if (mmap_lock_speculate_try_begin(mm, &seq)) {
- found = __in_uprobe_trampoline(ip);
+ found = __in_uprobe_trampoline(mm, ip);
retry = mmap_lock_speculate_retry(mm, seq);
}
rcu_read_unlock();
if (retry) {
mmap_read_lock(mm);
- found = __in_uprobe_trampoline(ip);
+ found = __in_uprobe_trampoline(mm, ip);
mmap_read_unlock(mm);
}
return found;
@@ -1044,7 +1044,7 @@ static int copy_from_vaddr(struct mm_struct *mm, unsigned long vaddr, void *dst,
return 0;
}
-static bool __is_optimized(uprobe_opcode_t *insn, unsigned long vaddr)
+static bool __is_optimized(struct mm_struct *mm, uprobe_opcode_t *insn, unsigned long vaddr)
{
struct __packed __arch_relative_insn {
u8 op;
@@ -1053,7 +1053,7 @@ static bool __is_optimized(uprobe_opcode_t *insn, unsigned long vaddr)
if (!is_call_insn(insn))
return false;
- return __in_uprobe_trampoline(vaddr + 5 + call->raddr);
+ return __in_uprobe_trampoline(mm, vaddr + 5 + call->raddr);
}
static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
@@ -1064,7 +1064,7 @@ static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
err = copy_from_vaddr(mm, vaddr, &insn, 5);
if (err)
return err;
- return __is_optimized((uprobe_opcode_t *)&insn, vaddr);
+ return __is_optimized(mm, (uprobe_opcode_t *)&insn, vaddr);
}
static bool should_optimize(struct arch_uprobe *auprobe)
--
2.53.0
^ permalink raw reply related
* [PATCHv3 02/12] uprobes/x86: Remove struct uprobe_trampoline object
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>
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>
---
arch/x86/kernel/uprobes.c | 102 ++++++++------------------------------
include/linux/uprobes.h | 5 --
kernel/events/uprobes.c | 10 ----
kernel/fork.c | 1 -
4 files changed, 20 insertions(+), 98 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 2be6707e3320..6824376e253d 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -631,11 +631,6 @@ static struct vm_special_mapping tramp_mapping = {
.pages = tramp_mapping_pages,
};
-struct uprobe_trampoline {
- struct hlist_node node;
- unsigned long vaddr;
-};
-
static bool is_reachable_by_call(unsigned long vtramp, unsigned long vaddr)
{
long delta = (long)(vaddr + 5 - vtramp);
@@ -682,83 +677,32 @@ static unsigned long find_nearest_trampoline(unsigned long vaddr)
return high_tramp;
}
-static struct uprobe_trampoline *create_uprobe_trampoline(unsigned long vaddr)
+static struct vm_area_struct *get_uprobe_trampoline(unsigned long vaddr)
{
struct pt_regs *regs = task_pt_regs(current);
- struct mm_struct *mm = current->mm;
- struct uprobe_trampoline *tramp;
+ VMA_ITERATOR(vmi, current->mm, 0);
struct vm_area_struct *vma;
if (!user_64bit_mode(regs))
- return NULL;
-
- vaddr = find_nearest_trampoline(vaddr);
- if (IS_ERR_VALUE(vaddr))
- return NULL;
-
- tramp = kzalloc_obj(*tramp);
- if (unlikely(!tramp))
- return NULL;
-
- tramp->vaddr = vaddr;
- vma = _install_special_mapping(mm, tramp->vaddr, PAGE_SIZE,
- VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_DONTCOPY|VM_IO,
- &tramp_mapping);
- if (IS_ERR(vma)) {
- kfree(tramp);
- return NULL;
- }
- return tramp;
-}
-
-static struct uprobe_trampoline *get_uprobe_trampoline(unsigned long vaddr, bool *new)
-{
- struct uprobes_state *state = ¤t->mm->uprobes_state;
- struct uprobe_trampoline *tramp = NULL;
+ return ERR_PTR(-EINVAL);
if (vaddr > TASK_SIZE || vaddr < PAGE_SIZE)
- return NULL;
+ return ERR_PTR(-EINVAL);
- hlist_for_each_entry(tramp, &state->head_tramps, node) {
- if (is_reachable_by_call(tramp->vaddr, vaddr)) {
- *new = false;
- return tramp;
- }
+ for_each_vma(vmi, vma) {
+ if (!vma_is_special_mapping(vma, &tramp_mapping))
+ continue;
+ if (is_reachable_by_call(vma->vm_start, vaddr))
+ return vma;
}
- tramp = create_uprobe_trampoline(vaddr);
- if (!tramp)
- return NULL;
-
- *new = true;
- hlist_add_head(&tramp->node, &state->head_tramps);
- return tramp;
-}
-
-static void destroy_uprobe_trampoline(struct uprobe_trampoline *tramp)
-{
- /*
- * We do not unmap and release uprobe trampoline page itself,
- * because there's no easy way to make sure none of the threads
- * is still inside the trampoline.
- */
- hlist_del(&tramp->node);
- kfree(tramp);
-}
-
-void arch_uprobe_init_state(struct mm_struct *mm)
-{
- INIT_HLIST_HEAD(&mm->uprobes_state.head_tramps);
-}
-
-void arch_uprobe_clear_state(struct mm_struct *mm)
-{
- struct uprobes_state *state = &mm->uprobes_state;
- struct uprobe_trampoline *tramp;
- struct hlist_node *n;
+ vaddr = find_nearest_trampoline(vaddr);
+ if (IS_ERR_VALUE(vaddr))
+ return ERR_PTR(vaddr);
- hlist_for_each_entry_safe(tramp, n, &state->head_tramps, node)
- destroy_uprobe_trampoline(tramp);
+ return _install_special_mapping(current->mm, vaddr, PAGE_SIZE,
+ VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_DONTCOPY|VM_IO,
+ &tramp_mapping);
}
static bool __in_uprobe_trampoline(struct mm_struct *mm, unsigned long ip)
@@ -1111,21 +1055,15 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
static int __arch_uprobe_optimize(struct arch_uprobe *auprobe, struct mm_struct *mm,
unsigned long vaddr)
{
- struct uprobe_trampoline *tramp;
- struct vm_area_struct *vma;
- bool new = false;
- int err = 0;
+ struct vm_area_struct *vma, *tramp;
vma = find_vma(mm, vaddr);
if (!vma)
return -EINVAL;
- tramp = get_uprobe_trampoline(vaddr, &new);
- if (!tramp)
- return -EINVAL;
- err = swbp_optimize(auprobe, vma, vaddr, tramp->vaddr);
- if (WARN_ON_ONCE(err) && new)
- destroy_uprobe_trampoline(tramp);
- return err;
+ tramp = get_uprobe_trampoline(vaddr);
+ if (IS_ERR(tramp))
+ return PTR_ERR(tramp);
+ return WARN_ON_ONCE(swbp_optimize(auprobe, vma, vaddr, tramp->vm_start));
}
void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index f548fea2adec..18be159bbc34 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -186,9 +186,6 @@ struct xol_area;
struct uprobes_state {
struct xol_area *xol_area;
-#ifdef CONFIG_X86_64
- struct hlist_head head_tramps;
-#endif
};
typedef int (*uprobe_write_verify_t)(struct page *page, unsigned long vaddr,
@@ -238,8 +235,6 @@ extern void uprobe_handle_trampoline(struct pt_regs *regs);
extern void *arch_uretprobe_trampoline(unsigned long *psize);
extern unsigned long uprobe_get_trampoline_vaddr(void);
extern void uprobe_copy_from_page(struct page *page, unsigned long vaddr, void *dst, int len);
-extern void arch_uprobe_clear_state(struct mm_struct *mm);
-extern void arch_uprobe_init_state(struct mm_struct *mm);
extern void handle_syscall_uprobe(struct pt_regs *regs, unsigned long bp_vaddr);
extern void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr);
extern unsigned long arch_uprobe_get_xol_area(void);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4084e926e284..b5c516168f84 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1806,14 +1806,6 @@ static struct xol_area *get_xol_area(void)
return area;
}
-void __weak arch_uprobe_clear_state(struct mm_struct *mm)
-{
-}
-
-void __weak arch_uprobe_init_state(struct mm_struct *mm)
-{
-}
-
/*
* uprobe_clear_state - Free the area allocated for slots.
*/
@@ -1825,8 +1817,6 @@ void uprobe_clear_state(struct mm_struct *mm)
delayed_uprobe_remove(NULL, mm);
mutex_unlock(&delayed_uprobe_lock);
- arch_uprobe_clear_state(mm);
-
if (!area)
return;
diff --git a/kernel/fork.c b/kernel/fork.c
index 5f3fdfdb14c7..9c6baabdc961 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1059,7 +1059,6 @@ static void mm_init_uprobes_state(struct mm_struct *mm)
{
#ifdef CONFIG_UPROBES
mm->uprobes_state.xol_area = NULL;
- arch_uprobe_init_state(mm);
#endif
}
--
2.53.0
^ permalink raw reply related
* [PATCHv3 03/12] uprobes/x86: Allow to copy uprobe trampolines on fork
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>
When we do fork or clone without CLONE_VM the new process won't
have uprobe trampoline vma objects and at the same time it will
have optimized code calling that trampoline and crash.
Fixing this by allowing vma uprobe trampoline objects to be copied
on fork to the new process.
Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/kernel/uprobes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 6824376e253d..11ec6b89b135 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -701,7 +701,7 @@ static struct vm_area_struct *get_uprobe_trampoline(unsigned long vaddr)
return ERR_PTR(vaddr);
return _install_special_mapping(current->mm, vaddr, PAGE_SIZE,
- VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_DONTCOPY|VM_IO,
+ VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_IO,
&tramp_mapping);
}
--
2.53.0
^ permalink raw reply related
* [PATCHv3 04/12] uprobes/x86: Move optimized uprobe from nop5 to 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>
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
[1] https://lore.kernel.org/bpf/20260509003146.976844-1-andrii@kernel.org/
[2] https://lore.kernel.org/bpf/20260518104306.GU3102624@noisy.programming.kicks-ass.net/#t
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Closes: https://lore.kernel.org/bpf/20260509003146.976844-1-andrii@kernel.org/
Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
Assisted-by: Codex:GPT-5.5
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/kernel/uprobes.c | 281 +++++++++++++++++++++++++++++---------
1 file changed, 217 insertions(+), 64 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 11ec6b89b135..16d9c584b995 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -266,7 +266,6 @@ static bool is_prefix_bad(struct insn *insn)
attr = inat_get_opcode_attribute(p);
switch (attr) {
case INAT_MAKE_PREFIX(INAT_PFX_ES):
- case INAT_MAKE_PREFIX(INAT_PFX_CS):
case INAT_MAKE_PREFIX(INAT_PFX_DS):
case INAT_MAKE_PREFIX(INAT_PFX_SS):
case INAT_MAKE_PREFIX(INAT_PFX_LOCK):
@@ -631,9 +630,29 @@ static struct vm_special_mapping tramp_mapping = {
.pages = tramp_mapping_pages,
};
+
+#define LEA_INSN_SIZE 5
+#define OPT_INSN_SIZE (LEA_INSN_SIZE + CALL_INSN_SIZE)
+#define REDZONE_SIZE 0x80
+
+static const u8 lea_rsp[] = { 0x48, 0x8d, 0x64, 0x24, 0x80 };
+
+static bool is_opt_insns(const uprobe_opcode_t *insn)
+{
+ return !memcmp(insn, lea_rsp, LEA_INSN_SIZE) &&
+ insn[LEA_INSN_SIZE] == CALL_INSN_OPCODE;
+}
+
+static bool is_swbp_opt_insns(uprobe_opcode_t *insn)
+{
+ return is_swbp_insn(&insn[0]) &&
+ !memcmp(&insn[1], &lea_rsp[1], LEA_INSN_SIZE - 1) &&
+ insn[LEA_INSN_SIZE] == CALL_INSN_OPCODE;
+}
+
static bool is_reachable_by_call(unsigned long vtramp, unsigned long vaddr)
{
- long delta = (long)(vaddr + 5 - vtramp);
+ long delta = (long)(vaddr + OPT_INSN_SIZE - vtramp);
return delta >= INT_MIN && delta <= INT_MAX;
}
@@ -646,7 +665,7 @@ static unsigned long find_nearest_trampoline(unsigned long vaddr)
};
unsigned long low_limit, high_limit;
unsigned long low_tramp, high_tramp;
- unsigned long call_end = vaddr + 5;
+ unsigned long call_end = vaddr + OPT_INSN_SIZE;
if (check_add_overflow(call_end, INT_MIN, &low_limit))
low_limit = PAGE_SIZE;
@@ -754,7 +773,7 @@ SYSCALL_DEFINE0(uprobe)
/* Allow execution only from uprobe trampolines. */
if (!in_uprobe_trampoline(regs->ip))
- return -ENXIO;
+ return -EPROTO;
err = copy_from_user(&args, (void __user *)regs->sp, sizeof(args));
if (err)
@@ -770,8 +789,8 @@ SYSCALL_DEFINE0(uprobe)
regs->ax = args.ax;
regs->r11 = args.r11;
regs->cx = args.cx;
- regs->ip = args.retaddr - 5;
- regs->sp += sizeof(args);
+ regs->ip = args.retaddr - OPT_INSN_SIZE;
+ regs->sp += sizeof(args) + REDZONE_SIZE;
regs->orig_ax = -1;
sp = regs->sp;
@@ -788,12 +807,12 @@ SYSCALL_DEFINE0(uprobe)
*/
if (regs->sp != sp) {
/* skip the trampoline call */
- if (args.retaddr - 5 == regs->ip)
- regs->ip += 5;
+ if (args.retaddr - OPT_INSN_SIZE == regs->ip)
+ regs->ip += OPT_INSN_SIZE;
return regs->ax;
}
- regs->sp -= sizeof(args);
+ regs->sp -= sizeof(args) + REDZONE_SIZE;
/* for the case uprobe_consumer has changed ax/r11/cx */
args.ax = regs->ax;
@@ -801,7 +820,7 @@ SYSCALL_DEFINE0(uprobe)
args.cx = regs->cx;
/* keep return address unless we are instructed otherwise */
- if (args.retaddr - 5 != regs->ip)
+ if (args.retaddr - OPT_INSN_SIZE != regs->ip)
args.retaddr = regs->ip;
if (shstk_push(args.retaddr) == -EFAULT)
@@ -835,7 +854,7 @@ asm (
"pop %rax\n"
"pop %r11\n"
"pop %rcx\n"
- "ret\n"
+ "ret $" __stringify(REDZONE_SIZE) "\n"
"int3\n"
".balign " __stringify(PAGE_SIZE) "\n"
".popsection\n"
@@ -853,7 +872,9 @@ late_initcall(arch_uprobes_init);
enum {
EXPECT_SWBP,
- EXPECT_CALL,
+ EXPECT_DUAL_SWBP,
+ EXPECT_OPTIMIZED,
+ EXPECT_SWBP_OPTIMIZED,
};
struct write_opcode_ctx {
@@ -861,30 +882,34 @@ struct write_opcode_ctx {
int expect;
};
-static int is_call_insn(uprobe_opcode_t *insn)
-{
- return *insn == CALL_INSN_OPCODE;
-}
-
/*
- * Verification callback used by int3_update uprobe_write calls to make sure
- * the underlying instruction is as expected - either int3 or call.
+ * Verification callback used by uprobe_write calls to make sure the underlying
+ * instruction is in the expected stage of the INT3 update sequence.
*/
static int verify_insn(struct page *page, unsigned long vaddr, uprobe_opcode_t *new_opcode,
int nbytes, void *data)
{
struct write_opcode_ctx *ctx = data;
- uprobe_opcode_t old_opcode[5];
+ uprobe_opcode_t old_opcode[OPT_INSN_SIZE];
- uprobe_copy_from_page(page, ctx->base, (uprobe_opcode_t *) &old_opcode, 5);
+ uprobe_copy_from_page(page, ctx->base, old_opcode, OPT_INSN_SIZE);
switch (ctx->expect) {
case EXPECT_SWBP:
if (is_swbp_insn(&old_opcode[0]))
return 1;
break;
- case EXPECT_CALL:
- if (is_call_insn(&old_opcode[0]))
+ case EXPECT_DUAL_SWBP:
+ if (is_swbp_insn(&old_opcode[0]) &&
+ is_swbp_insn(&old_opcode[LEA_INSN_SIZE]))
+ return 1;
+ break;
+ case EXPECT_OPTIMIZED:
+ if (is_opt_insns(&old_opcode[0]))
+ return 1;
+ break;
+ case EXPECT_SWBP_OPTIMIZED:
+ if (is_swbp_opt_insns(&old_opcode[0]))
return 1;
break;
}
@@ -893,48 +918,134 @@ static int verify_insn(struct page *page, unsigned long vaddr, uprobe_opcode_t *
}
/*
- * Modify multi-byte instructions by using INT3 breakpoints on SMP.
+ * 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:
- * - Add an INT3 trap to the address that will be patched
- * - SMP sync all CPUs
- * - Update all but the first byte of the patched range
- * - SMP sync all CPUs
- * - Replace the first byte (INT3) by the first byte of the replacing opcode
- * - SMP sync all CPUs
+ * 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.
*/
-static int int3_update(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
- unsigned long vaddr, char *insn, bool optimize)
+static int int3_update_optimize(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
+ unsigned long vaddr, uprobe_opcode_t *insn)
{
uprobe_opcode_t int3 = UPROBE_SWBP_INSN;
+ uprobe_opcode_t opt_int3[OPT_INSN_SIZE];
struct write_opcode_ctx ctx = {
.base = vaddr,
};
int err;
/*
- * Write int3 trap.
+ * 1) Initial state after set_swbp() installed the uprobe:
+ * cc 2e 0f 1f 84 00 00 00 00 00
+ */
+ smp_text_poke_sync_each_cpu();
+
+ /*
+ * 2) Trap the call slot before rewriting the NOP tail:
+ * cc 2e 0f 1f 84 [cc] 00 00 00 00
+ */
+ ctx.expect = EXPECT_SWBP;
+ err = uprobe_write(auprobe, vma, vaddr + LEA_INSN_SIZE, &int3, 1, verify_insn,
+ true /* is_register */, false /* do_update_ref_ctr */,
+ &ctx);
+ if (err)
+ return err;
+
+ smp_text_poke_sync_each_cpu();
+
+ memcpy(opt_int3, insn, OPT_INSN_SIZE);
+ opt_int3[LEA_INSN_SIZE] = UPROBE_SWBP_INSN;
+
+ /*
+ * 3) Rewrite the LEA tail and call displacement, keeping both INT3 bytes:
+ * cc [8d 64 24 80] cc [d0 d1 d2 d3]
+ */
+ ctx.expect = EXPECT_DUAL_SWBP;
+ err = uprobe_write(auprobe, vma, vaddr + 1, opt_int3 + 1,
+ OPT_INSN_SIZE - 1, verify_insn,
+ true /* is_register */, false /* do_update_ref_ctr */,
+ &ctx);
+ if (err)
+ goto error;
+
+ smp_text_poke_sync_each_cpu();
+
+ /*
+ * 4) Restore the call opcode at offset 5.
+ * cc 8d 64 24 80 [e8] d0 d1 d2 d3
+ */
+ err = uprobe_write(auprobe, vma, vaddr + LEA_INSN_SIZE,
+ insn + LEA_INSN_SIZE, 1, verify_insn,
+ true /* is_register */, false /* do_update_ref_ctr */,
+ &ctx);
+ if (err)
+ goto error;
+
+ smp_text_poke_sync_each_cpu();
+
+ /*
+ * 5) Publish the first LEA byte:
+ * [48] 8d 64 24 80 e8 d0 d1 d2 d3
*
- * The swbp_optimize path comes with breakpoint already installed,
- * so we can skip this step for optimize == true.
+ * From offset 0 this is:
+ * lea -0x80(%rsp), %rsp
+ * call <uprobe-trampoline>
*/
- if (!optimize) {
- ctx.expect = EXPECT_CALL;
- err = uprobe_write(auprobe, vma, vaddr, &int3, 1, verify_insn,
- true /* is_register */, false /* do_update_ref_ctr */,
- &ctx);
- if (err)
- return err;
- }
+ ctx.expect = EXPECT_SWBP_OPTIMIZED;
+ err = uprobe_write(auprobe, vma, vaddr, insn, 1, verify_insn,
+ true /* is_register */, false /* do_update_ref_ctr */,
+ &ctx);
+ if (err)
+ goto error;
smp_text_poke_sync_each_cpu();
+ return 0;
- /* Write all but the first byte of the patched range. */
+error:
+ /*
+ * In all intermediate states byte 0 is INT3, so EXPECT_SWBP covers every
+ * case. Restore original NOP bytes 1..9 in one write.
+ */
ctx.expect = EXPECT_SWBP;
- err = uprobe_write(auprobe, vma, vaddr + 1, insn + 1, 4, verify_insn,
+ uprobe_write(auprobe, vma, vaddr + 1, auprobe->insn + 1, OPT_INSN_SIZE - 1,
+ verify_insn, true, false, &ctx);
+ smp_text_poke_sync_each_cpu();
+ return err;
+}
+
+static int int3_update_unoptimize(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
+ unsigned long vaddr, uprobe_opcode_t *insn)
+{
+ uprobe_opcode_t int3 = UPROBE_SWBP_INSN;
+ struct write_opcode_ctx ctx = {
+ .base = vaddr,
+ .expect = EXPECT_OPTIMIZED,
+ };
+ int err;
+
+ /*
+ * 1) Initial optimized state:
+ * 48 8d 64 24 80 e8 d0 d1 d2 d3
+ *
+ * 2) Trap new entries before restoring the NOP bytes:
+ * [cc] 8d 64 24 80 e8 d0 d1 d2 d3
+ */
+ err = uprobe_write(auprobe, vma, vaddr, &int3, 1, verify_insn,
true /* is_register */, false /* do_update_ref_ctr */,
&ctx);
if (err)
@@ -943,13 +1054,31 @@ static int int3_update(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
smp_text_poke_sync_each_cpu();
/*
- * Write first byte.
+ * 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
+ */
+ ctx.expect = EXPECT_SWBP_OPTIMIZED;
+ err = uprobe_write(auprobe, vma, vaddr + 1, insn + 1,
+ LEA_INSN_SIZE - 1, verify_insn,
+ true /* is_register */, false /* do_update_ref_ctr */,
+ &ctx);
+ if (err)
+ return err;
+
+ smp_text_poke_sync_each_cpu();
+
+ /*
+ * 4) Publish the first byte of the original NOP:
+ * [66] 2e 0f 1f 84 e8 d0 d1 d2 d3
*
- * The swbp_unoptimize needs to finish uprobe removal together
- * with ref_ctr update, using uprobe_write with proper flags.
+ * 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.
*/
+ ctx.expect = EXPECT_SWBP;
err = uprobe_write(auprobe, vma, vaddr, insn, 1, verify_insn,
- optimize /* is_register */, !optimize /* do_update_ref_ctr */,
+ false /* is_register */, true /* do_update_ref_ctr */,
&ctx);
if (err)
return err;
@@ -961,17 +1090,25 @@ static int int3_update(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
static int swbp_optimize(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
unsigned long vaddr, unsigned long tramp)
{
- u8 call[5];
+ u8 insn[OPT_INSN_SIZE], *call = &insn[LEA_INSN_SIZE];
- __text_gen_insn(call, CALL_INSN_OPCODE, (const void *) vaddr,
+ /*
+ * We have nop10 instruction (with first byte overwritten to int3),
+ * changing it to:
+ * lea -0x80(%rsp), %rsp
+ * call tramp
+ */
+ memcpy(insn, lea_rsp, LEA_INSN_SIZE);
+ __text_gen_insn(call, CALL_INSN_OPCODE,
+ (const void *) (vaddr + LEA_INSN_SIZE),
(const void *) tramp, CALL_INSN_SIZE);
- return int3_update(auprobe, vma, vaddr, call, true /* optimize */);
+ return int3_update_optimize(auprobe, vma, vaddr, insn);
}
static int swbp_unoptimize(struct arch_uprobe *auprobe, struct vm_area_struct *vma,
unsigned long vaddr)
{
- return int3_update(auprobe, vma, vaddr, auprobe->insn, false /* optimize */);
+ return int3_update_unoptimize(auprobe, vma, vaddr, auprobe->insn);
}
static int copy_from_vaddr(struct mm_struct *mm, unsigned long vaddr, void *dst, int len)
@@ -993,19 +1130,19 @@ static bool __is_optimized(struct mm_struct *mm, uprobe_opcode_t *insn, unsigned
struct __packed __arch_relative_insn {
u8 op;
s32 raddr;
- } *call = (struct __arch_relative_insn *) insn;
+ } *call = (struct __arch_relative_insn *)(insn + LEA_INSN_SIZE);
- if (!is_call_insn(insn))
+ if (!is_opt_insns(insn))
return false;
- return __in_uprobe_trampoline(mm, vaddr + 5 + call->raddr);
+ return __in_uprobe_trampoline(mm, vaddr + OPT_INSN_SIZE + call->raddr);
}
static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
{
- uprobe_opcode_t insn[5];
+ uprobe_opcode_t insn[OPT_INSN_SIZE];
int err;
- err = copy_from_vaddr(mm, vaddr, &insn, 5);
+ err = copy_from_vaddr(mm, vaddr, &insn, OPT_INSN_SIZE);
if (err)
return err;
return __is_optimized(mm, (uprobe_opcode_t *)&insn, vaddr);
@@ -1069,7 +1206,7 @@ static int __arch_uprobe_optimize(struct arch_uprobe *auprobe, struct mm_struct
void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr)
{
struct mm_struct *mm = current->mm;
- uprobe_opcode_t insn[5];
+ uprobe_opcode_t insn[OPT_INSN_SIZE];
if (!should_optimize(auprobe))
return;
@@ -1080,7 +1217,7 @@ void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr)
* Check if some other thread already optimized the uprobe for us,
* if it's the case just go away silently.
*/
- if (copy_from_vaddr(mm, vaddr, &insn, 5))
+ if (copy_from_vaddr(mm, vaddr, &insn, OPT_INSN_SIZE))
goto unlock;
if (!is_swbp_insn((uprobe_opcode_t*) &insn))
goto unlock;
@@ -1096,16 +1233,32 @@ void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr)
mmap_write_unlock(mm);
}
+static bool is_optimizable_nop10(struct insn *insn)
+{
+ static const u8 nop10_prefix[] = {
+ 0x66, 0x2e, 0x0f, 0x1f, 0x84
+ };
+
+ /*
+ * Restrict this to the 10-byte NOP form whose last 5 bytes are
+ * SIB/displacement operands. Unoptimization keeps the call opcode and
+ * displacement in those bytes, so other NOP encodings are not safe.
+ */
+ return insn->length == OPT_INSN_SIZE &&
+ insn_is_nop(insn) &&
+ !memcmp(insn->kaddr, nop10_prefix, ARRAY_SIZE(nop10_prefix));
+}
+
static bool can_optimize(struct insn *insn, unsigned long vaddr)
{
- if (!insn->x86_64 || insn->length != 5)
+ if (!insn->x86_64)
return false;
- if (!insn_is_nop(insn))
+ if (!is_optimizable_nop10(insn))
return false;
/* We can't do cross page atomic writes yet. */
- return PAGE_SIZE - (vaddr & ~PAGE_MASK) >= 5;
+ return PAGE_SIZE - (vaddr & ~PAGE_MASK) >= OPT_INSN_SIZE;
}
#else /* 32-bit: */
/*
--
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