* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Sean Christopherson @ 2026-06-25 14:36 UTC (permalink / raw)
To: Yan Zhao
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <aj0Jf30PS2f7x1nt@yzhao56-desk.sh.intel.com>
On Thu, Jun 25, 2026, Yan Zhao wrote:
> On Thu, Jun 25, 2026 at 09:51:01AM +0800, Yan Zhao wrote:
> > On Wed, Jun 24, 2026 at 05:41:58PM -0700, Sean Christopherson wrote:
> > > On Wed, Jun 24, 2026, Ackerley Tng wrote:
> > > > Yan Zhao <yan.y.zhao@intel.com> writes:
> > > > > With gmem_in_place_conversion=true, userspace can create guest_memfd without the
> > > > > MMAP flag. In such cases, shared memory is allocated from different backends.
> > > > > This means this module parameter only enables per-gmem memory attribute and does
> > > > > not guarantee that gmem in-place conversion will actually occur.
> > >
> > > KVM module params are pretty much always about what KVM supports, not what is
> > > guaranteed to happen.
> > >
> > > - enable_mmio_caching doesn't guarantee there will actually be MMIO SPTEs,
> > > because maybe the guest never accesses emulated MMIO.
> > > - enable_pmu doesn't guarantee VMs will get a PMU, because userspace may elect
> > > not to advertise one.
> > > - and so on and so forth...
> > >
> > > Yes, there's a small mental jump to get from "KVM supports in-place conversion"
> > > to "I need to set memory attributes on the guest_memfd instance, not the VM",
> > > but I don't see that as a big hurdle, certainly not in the long term. And once
> > > the VMM code is written, I really do think most people are going to care about
> > > whether or not KVM supports in-place conversion, not where PRIVATE is tracked.
> > Sorry, I just saw this mail after posting my reply in [1].
> >
> > I'm ok with gmem_in_place_conversion=true just means KVM supports in-place
> > conversion, while we can still create VMs with shared memory not from gmem.
> Or what about "allow_gmem_in_place_conversion" ?
No, because turning on the param also disallows setting PRIVATE in the VM-scoped
KVM_SET_MEMORY_ATTRIBUTES ioctl.
> > Though it still feels a bit odd to require TDX huge pages to depend on
> > gmem_in_place_conversion=true when shared memory is not currently allocated
> > from gmem,
I fully expect that to be a transient state, and in all likelihood not something
that is *ever* shipped in production. Landing TDX hugepages without guest_memfd
hugepage support is all about avoiding unnecessary serialization of series and
features that aren't strictly dependent on each other.
> > it should become more natural over time once gmem supports in-place
> > conversions for huge page.
Yes, and I want to prioritize the steady state for end users, not the in-progress
state for developers. Once all of this settles out, I fully expect the majority
of deployments to only support in-place conversion, at which point the end user
is only going to care whether or not in-place conversion is enabled in KVM, not
the subtle detail that it's still possible to do out-of-place conversions (and
that will always hold true, it's not like VMA-based memslots are being deprecated).
> > Besides my current usage, there may be other scenarios where gmem memory
> > attributes is preferred without allocating shared memory from gmem.
> > (e.g., PAGE.ADD from a temp extra shared source memory).
> >
> > For such use cases, I'm concerns that the admins may find it confusing if they
> > enable gmem_in_place_conversion but still observe extra memory consumptions for
> > shared memory.
KVM can help with documentation, but beyond that, it's not KVM's problem to solve.
If a VMM *and* platform owner chooses to deploy a setup that utilizes out-of-place
conversions, then it's on the VMM and/or plaform owner to understand and communicate
the implications to the end user.
And I'm not remotely convinced that prepending allow_ to the param will help
end users diagnose "unexpected" memory consumption, in quotes because anyone that
is deploying a stack that utilizes out-of-place conversion absolutely needs to
understand and plan for the additional memory consumption. I.e. if the memory
consumption is "unexpected" to the end user, they likely have far bigger problems.
^ permalink raw reply
* Re: [PATCH v8 18/46] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Sean Christopherson @ 2026-06-25 15:40 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <6ed7d12a-c3a1-4572-8385-754e6d5b8b44@kernel.org>
On Thu, Jun 25, 2026, David Hildenbrand (Arm) wrote:
> On 6/25/26 02:35, Sean Christopherson wrote:
> > One thought I had, to avoid the IPIs that draining all per-CPU caches requires,
> > was to disallow putting guest_memfd pages in folio batches, e.g. by hacking
> > something into folio_may_be_lru_cached(). But due to taking a per-lru lock,
> > that would penalize the relatively hot path and definitely common operation of
> > faulting in guest memory. On the other hand, memory conversion is already a
> > relatively slow operation and is relatively uncommon compared to page faults,
> > (and likely very uncommon for real world setups). I.e. having to drain all
> > caches if conversion isn't safe penalizes a relatively slow, relatively uncommon
> > path.
>
> Yeah, the lru_add_drain_all is rather messy.
>
> We have similar code in
>
> collect_longterm_unpinnable_folios(), where we first try a lru_add_drain(), to
> then escalate to a lru_add_drain_all().
>
> Maybe we could factor that (suboptimal code) out to not have to reinvent the
> same thing multiple times?
As discussed in the guest_memfd call, we should do this straightaway, i.e. instead
of merging this series as-is, so that we don't export lru_add_drain_all() only to
drop the export a kernel or two later, and can instead export the helper to drain
any batches for a folio (or set of folios/pages).
^ permalink raw reply
* [PATCH] tracing/user_events: Use kfree_rcu for enabler cleanup
From: Tristan Madani @ 2026-06-25 18:02 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Beau Belgrave, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, stable, Tristan Madani
From: Tristan Madani <tristan@talencesecurity.com>
user_event_enabler_destroy() removes the enabler from an RCU-protected
list via list_del_rcu() and then immediately frees it with kfree(). This
can result in a concurrent reader in user_event_enabler_dup() accessing
stale memory during fork, since the enabler list is traversed under
rcu_read_lock().
The ENABLE_VAL_FREEING_BIT check in user_event_enabler_dup() is not
sufficient to prevent this, as the enabler can be freed between the bit
test and the subsequent pointer dereference.
Use kfree_rcu() to defer the free until after all RCU read-side critical
sections complete.
Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
kernel/trace/trace_events_user.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index c4ba484f7b38b..72bcb429eb4f3 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -109,6 +109,7 @@ struct user_event_enabler {
/* Track enable bit, flags, etc. Aligned for bitops. */
unsigned long values;
+ struct rcu_head rcu;
};
/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
@@ -404,7 +405,7 @@ static void user_event_enabler_destroy(struct user_event_enabler *enabler,
/* No longer tracking the event via the enabler */
user_event_put(enabler->event, locked);
- kfree(enabler);
+ kfree_rcu(enabler, rcu);
}
static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default
From: Ackerley Tng @ 2026-06-25 18:20 UTC (permalink / raw)
To: Yan Zhao
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajyCn0PnFtQK+Nka@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Wed, Jun 24, 2026 at 05:05:44PM -0700, Ackerley Tng wrote:
>> Yan Zhao <yan.y.zhao@intel.com> writes:
>>
>> >
>> > [...snip...]
>> >
>> >>
>> >> #ifdef kvm_arch_has_private_mem
>> >> -bool __ro_after_init gmem_in_place_conversion = false;
>> >> +bool __ro_after_init gmem_in_place_conversion = !IS_ENABLED(CONFIG_KVM_VM_MEMORY_ATTRIBUTES);
>> >> +module_param(gmem_in_place_conversion, bool, 0444);
>> >
>> > With gmem_in_place_conversion=true, userspace can create guest_memfd without the
>> > MMAP flag. In such cases, shared memory is allocated from different backends.
>> > This means this module parameter only enables per-gmem memory attribute and does
>> > not guarantee that gmem in-place conversion will actually occur.
>> >
>> > To avoid confusion, could we rename this module parameter to something more
>> > accurate, such as gmem_memory_attribute?
>> >
>>
>> I asked Sean about this after getting some fixes off list. Sean said
>> gmem_in_place_conversion is named for a host admin to use, and something
>> like gmem_memory_attributes is too much implementation details for the
>> admin.
> Thanks for this background.
>
> Some more context on why I'm asking:
>
> Currently, I'm testing TDX huge pages with the following two gmem components:
> 1. The gmem memory attribute in this gmem in-place conversion v8.
> 2. The gmem 2MB from buddy allocator. (for development/testing only).
>
> The gmem 2MB from buddy allocator allocates 2MB folios from buddy for private
> memory, while shared memory is allocated from a different backend.
> (To avoid fragmentation, only private mappings are split during private-to-shared
> conversions. In this approach, the 2MB folios are always retained in the gmem
> inode filemap cache without splitting.)
>
> Since shared memory is not allocated from gmem, there're no in-place conversions.
> The reason I'm using "gmem memory attribute" is that the per-VM attribute is
> being deprecated, as suggested by Sean [1].
>
v8 of conversions series changed that slightly, per-VM attributes is
going to stay around (because of work on RWX attributes, coming up) and
RWX will stay tracked at the VM level.
For v8 and beyond, only tracking of private/shared in per-VM attributes
is being deprecated.
By extension the entire thing about using guest_memfd for private memory
and a different backing memory for shared memory is being deprecated.
> Besides my current usage,
I think you can set up guest_memfd+2M for private memory and shared
memory from some other source, and that's the deprecated usage pattern.
> there may be other scenarios where gmem memory
> attributes is preferred without allocating shared memory from gmem.
> (e.g., PAGE.ADD from a temp extra shared source memory).
>
Is this TDH.MEM.PAGE.ADD, used indirectly from
tdx_gmem_post_populate()? This use case isn't blocked. Even if
gmem_in_place_conversion=true, you can still set src_address to
non-guest_memfd memory and load from anywhere you like.
Please let me know if that is broken! I think I accidentally used that
setup in selftests and it worked. The selftests are now defaulting to
in-place conversion.
> For such use cases, I'm concerns that the admins may find it confusing if they
> enable gmem_in_place_conversion but still observe extra memory consumptions for
> shared memory.
>
Hmm but I guess if someone enables gmem_in_place_conversion but still
allocates from elsewhere, they'd have to figure it out?
> [1] https://lore.kernel.org/kvm/aWmEegVP_A613WIr@google.com/
>
>> Sean, would you reconsider since Yan also asked? If the admin compiled
>> the kernel knowing what CONFIG_KVM_VM_MEMORY_ATTRIBUTES means, then the
>> admin would also be able to use a param like gmem_memory_attributes?
>>
>> There's the additional benefit that the similar naming aids in
>> understanding for both the admin and software engineers.
>>
>> Either way, in the next revision, I'll also add this documentation for
>> this module_param:
>>
>> Setting the module parameter gmem_in_place_conversion to true will
>> enable the KVM_SET_MEMORY_ATTRIBUTES2 guest_memfd ioctl and disables
>> the KVM_SET_MEMORY_ATTRIBUTES VM ioctl. If gmem_in_place_conversion is
>> true, the private/shared attribute will be tracked per-guest_memfd
>> instead of per-VM.
>>
>> Let me know what y'all think of the wording!
>>
>> >>
>> >> [...snip...]
>> >>
^ permalink raw reply
* Re: [PATCH] tracing/user_events: Use kfree_rcu for enabler cleanup
From: Beau Belgrave @ 2026-06-25 18:48 UTC (permalink / raw)
To: Tristan Madani
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, stable, Tristan Madani
In-Reply-To: <20260625180203.3343545-1-tristmd@gmail.com>
On Thu, Jun 25, 2026 at 06:02:03PM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
>
> user_event_enabler_destroy() removes the enabler from an RCU-protected
> list via list_del_rcu() and then immediately frees it with kfree(). This
> can result in a concurrent reader in user_event_enabler_dup() accessing
> stale memory during fork, since the enabler list is traversed under
> rcu_read_lock().
>
> The ENABLE_VAL_FREEING_BIT check in user_event_enabler_dup() is not
> sufficient to prevent this, as the enabler can be freed between the bit
> test and the subsequent pointer dereference.
>
> Use kfree_rcu() to defer the free until after all RCU read-side critical
> sections complete.
>
> Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> kernel/trace/trace_events_user.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index c4ba484f7b38b..72bcb429eb4f3 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -109,6 +109,7 @@ struct user_event_enabler {
>
> /* Track enable bit, flags, etc. Aligned for bitops. */
> unsigned long values;
> + struct rcu_head rcu;
> };
>
> /* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
> @@ -404,7 +405,7 @@ static void user_event_enabler_destroy(struct user_event_enabler *enabler,
> /* No longer tracking the event via the enabler */
> user_event_put(enabler->event, locked);
>
> - kfree(enabler);
> + kfree_rcu(enabler, rcu);
> }
>
> static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
> --
> 2.47.3
See [1] as there are more issues than simply the enabler being freed via
RCU, there are lifetime aspects of the underlying user_event.
Thanks,
-Beau
1. https://lore.kernel.org/linux-trace-kernel/20260618222743.538915-1-michael.bommarito@gmail.com/
^ permalink raw reply
* Re: [PATCHv4 05/13] uprobes/x86: Move optimized uprobe from nop5 to nop10
From: Oleg Nesterov @ 2026-06-25 18:53 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Andrii Nakryiko,
bpf, linux-trace-kernel
In-Reply-To: <20260526205840.173790-6-jolsa@kernel.org>
On 05/26, Jiri Olsa wrote:
>
> + * Note that unoptimization deliberately keeps the call opcode and displacement
> + * in bytes 5..9. Those bytes become operands of the restored 10-byte NOP.
> + *
> + * Since there is only a single target uprobe-trampoline for the given nop10
> + * instruction address, the CALL instruction will not be changed across
> + * unoptimization/optimization cycles.
> + * Therefore, any task that is preempted at the CALL instruction is guaranteed
> + * to observe that CALL and not anything else.
Understand... and I guess synchronize_rcu_tasks() is too heavy.
But this means that unregister/unapply will never discard the COW'ed anonymous page
with optimized up; __uprobe_write() -> orig_page_is_identical() will never be true...
Plus this means that we can never "gc" the unused tramp vma's, but this is minor.
OK. This is not critical, and other than that I don't see any problems in yout patch.
(but I am sure this is only because I don't understand this code/patch enough ;)
So, FWIW
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply
* Re: [PATCH v3 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Paul Walmsley @ 2026-06-25 19:22 UTC (permalink / raw)
To: Wang Han
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Steven Rostedt,
Alexandre Ghiti, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Chen Pei, Andy Chiu, Björn Töpel, Deepak Gupta,
Puranjay Mohan, Conor Dooley, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence, Shuah Khan,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <20260609063002.3943001-1-wanghan@linux.alibaba.com>
Hi,
On Tue, 9 Jun 2026, Wang Han wrote:
> RISC-V uses -fpatchable-function-entry=8,4 when the compressed ISA is
> enabled and -fpatchable-function-entry=4,2 otherwise. In both cases, the
> patchable NOP area starts 8 bytes before the function symbol address.
> The __mcount_loc entries therefore point at the patchable NOP area
> associated with a function, while nm reports the function symbol at the
> entry address used for the function range check.
>
> After RISC-V selected HAVE_BUILDTIME_MCOUNT_SORT, sorttable started
> applying that range check at build time. Without allowing entries just
> before the reported function address, the mcount sorter treats valid
> RISC-V ftrace callsites as invalid weak-function entries and writes
> them back as zero. The resulting kernel boots with no ftrace entries,
> breaking dynamic ftrace and users such as livepatch.
>
> The failure is silent during the final link because zeroing weak-function
> entries is an expected sorttable operation. At boot, those zero entries
> are skipped by ftrace_process_locs(), so the only obvious symptom is that
> the vmlinux ftrace table has lost valid callsites and ftrace users cannot
> attach to them.
>
> CONFIG_FTRACE_SORT_STARTUP_TEST also reports the table as sorted in this
> state: it only checks that the __mcount_loc entries are in ascending
> order, which a fully zeroed table trivially satisfies. The original
> commit relied on this check and did not see the regression.
>
> On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
> and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
> while ftrace reports zero usable entries and the early selftests fail:
>
> [ 0.000000] ftrace section at ffffffff8101da98 sorted properly
> [ 0.000000] ftrace: allocating 0 entries in 128 pages
> [ 0.054999] Testing tracer function: .. no entries found ..FAILED!
> [ 0.172407] tracer: function failed selftest, disabling
> [ 0.178186] Failed to init function_graph tracer, init returned -19
>
> Handle RISC-V like arm64 for the function-range check and allow
> patchable entries up to 8 bytes before the function address.
>
> With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
> the vmlinux ftrace table is populated and dynamic ftrace still works:
>
> [ 0.000000] ftrace: allocating 46749 entries in 184 pages
> [ 0.051115] Testing tracer function: PASSED
> [ 1.283782] Testing dynamic ftrace: PASSED
> [ 6.275456] Testing tracer function_graph: PASSED
>
> Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
> Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
> Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
Thanks, I'm going to pull this one out of the rest of your series since
this is clearly a fix and needs to go in sooner rather than later. Queued
for v7.2-rc.
- Paul
^ permalink raw reply
* Re: [PATCH 5.15.y] ring-buffer: Remove ring_buffer_read_prepare_sync()
From: Bjoern Doebel @ 2026-06-25 22:19 UTC (permalink / raw)
To: Sasha Levin
Cc: Bjoern Doebel, stable, Steven Rostedt, Masami Hiramatsu,
linux-trace-kernel, linux-kernel, Mathieu Desnoyers,
David Howells
In-Reply-To: <20260625054005.0014.ringbuf-515@kernel.org>
On Thu, Jun 25, 2026 at 06:41:58AM -0400, Sasha Levin wrote:
> > [PATCH 5.15.y] ring-buffer: Remove ring_buffer_read_prepare_sync()
>
> I had to drop this one for 5.15. The upstream guard(raw_spinlock_irqsave)
> conversion in ring_buffer_read_start() introduces a new
> -Wdeclaration-after-statement warning on 5.15 (the guard variable ends up after
> a statement), which the build flags as an
> error there.
>
> Could you respin a warning-free version for 5.15 (and 5.10, which has the same
> problem)? E.g. hoisting the declaration or keeping the explicit
> raw_spin_lock/unlock instead of guard() on these older trees. 6.6 and 6.1 are
> already queued.
Absolutely, I'll send a v2.
Best,
Bjoern
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply
* Re: [PATCH v4 2/2] tracing: Remove trace_printk.h from kernel.h
From: Nathan Chancellor @ 2026-06-25 23:41 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Linus Torvalds,
Sebastian Andrzej Siewior, John Ogness, Thomas Gleixner,
Peter Zijlstra, Julia Lawall, Yury Norov, linux-doc, linux-kbuild,
linuxppc-dev, dri-devel, linux-stm32, linux-arm-kernel,
linux-rdma, linux-usb, linux-ext4, linux-nfs, kvm, intel-gfx
In-Reply-To: <20260625104402.210473477@kernel.org>
Hi Steve,
On Thu, Jun 25, 2026 at 06:40:09AM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> There have been complaints about trace_printk.h causing more build time
> for being in kernel.h if it changes. There is also an effort to clean up
> kernel.h to have it not include unneeded header files. Move trace_printk.h
> out of kernel.h and place it in the headers and C files that use it.
>
> Link: https://lore.kernel.org/all/CAHk-=wikCBeVFjVXiY4o-oepdbjAoir5+TcAgtL12c4u1TpZLQ@mail.gmail.com/
>
> Suggested-by: Yury Norov <yury.norov@gmail.com>
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
This patch breaks lib/test_context-analysis.c for me in several
configurations:
In file included from lib/test_context-analysis.c:9:
In file included from include/linux/local_lock.h:5:
include/linux/local_lock_internal.h:46:2: error: use of undeclared identifier '_THIS_IP_'
46 | lock_map_acquire(&l->dep_map);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/lockdep.h:541:69: note: expanded from macro 'lock_map_acquire'
541 | #define lock_map_acquire(l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
| ^~~~~~~~~
In file included from lib/test_context-analysis.c:9:
In file included from include/linux/local_lock.h:5:
include/linux/local_lock_internal.h:53:2: error: use of undeclared identifier '_THIS_IP_'
53 | lock_map_acquire_try(&l->dep_map);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/lockdep.h:542:73: note: expanded from macro 'lock_map_acquire_try'
542 | #define lock_map_acquire_try(l) lock_acquire_exclusive(l, 0, 1, NULL, _THIS_IP_)
| ^~~~~~~~~
In file included from lib/test_context-analysis.c:9:
In file included from include/linux/local_lock.h:5:
include/linux/local_lock_internal.h:62:2: error: use of undeclared identifier '_THIS_IP_'
62 | lock_map_release(&l->dep_map);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/lockdep.h:545:47: note: expanded from macro 'lock_map_release'
545 | #define lock_map_release(l) lock_release(l, _THIS_IP_)
| ^~~~~~~~~
3 errors generated.
The following diff resolves it for me, should I send it as a separate
patch or do you want to just fold it in with a note?
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 621566345406..2301a701ffbb 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -10,6 +10,7 @@
#ifndef __LINUX_LOCKDEP_H
#define __LINUX_LOCKDEP_H
+#include <linux/instruction_pointer.h>
#include <linux/lockdep_types.h>
#include <linux/smp.h>
#include <asm/percpu.h>
--
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default
From: Yan Zhao @ 2026-06-26 0:04 UTC (permalink / raw)
To: Ackerley Tng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CAEvNRgFfgV0FbQLzP8hhNH5hMGaQao6OFQin4cb3TAmC7SVhfA@mail.gmail.com>
On Thu, Jun 25, 2026 at 11:20:30AM -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> > On Wed, Jun 24, 2026 at 05:05:44PM -0700, Ackerley Tng wrote:
> >> Yan Zhao <yan.y.zhao@intel.com> writes:
> >>
> >> >
> >> > [...snip...]
> >> >
> >> >>
> >> >> #ifdef kvm_arch_has_private_mem
> >> >> -bool __ro_after_init gmem_in_place_conversion = false;
> >> >> +bool __ro_after_init gmem_in_place_conversion = !IS_ENABLED(CONFIG_KVM_VM_MEMORY_ATTRIBUTES);
> >> >> +module_param(gmem_in_place_conversion, bool, 0444);
> >> >
> >> > With gmem_in_place_conversion=true, userspace can create guest_memfd without the
> >> > MMAP flag. In such cases, shared memory is allocated from different backends.
> >> > This means this module parameter only enables per-gmem memory attribute and does
> >> > not guarantee that gmem in-place conversion will actually occur.
> >> >
> >> > To avoid confusion, could we rename this module parameter to something more
> >> > accurate, such as gmem_memory_attribute?
> >> >
> >>
> >> I asked Sean about this after getting some fixes off list. Sean said
> >> gmem_in_place_conversion is named for a host admin to use, and something
> >> like gmem_memory_attributes is too much implementation details for the
> >> admin.
> > Thanks for this background.
> >
> > Some more context on why I'm asking:
> >
> > Currently, I'm testing TDX huge pages with the following two gmem components:
> > 1. The gmem memory attribute in this gmem in-place conversion v8.
> > 2. The gmem 2MB from buddy allocator. (for development/testing only).
> >
> > The gmem 2MB from buddy allocator allocates 2MB folios from buddy for private
> > memory, while shared memory is allocated from a different backend.
> > (To avoid fragmentation, only private mappings are split during private-to-shared
> > conversions. In this approach, the 2MB folios are always retained in the gmem
> > inode filemap cache without splitting.)
> >
> > Since shared memory is not allocated from gmem, there're no in-place conversions.
> > The reason I'm using "gmem memory attribute" is that the per-VM attribute is
> > being deprecated, as suggested by Sean [1].
> >
>
> v8 of conversions series changed that slightly, per-VM attributes is
> going to stay around (because of work on RWX attributes, coming up) and
> RWX will stay tracked at the VM level.
>
> For v8 and beyond, only tracking of private/shared in per-VM attributes
> is being deprecated.
>
> By extension the entire thing about using guest_memfd for private memory
> and a different backing memory for shared memory is being deprecated.
Thanks for the info. I was actually referring to the per-VM shared/private
attribute, which is being deprecated. Sean hoped TDX huge page would be the
first mandated user of the per-gmem shared/private attribute.
> > Besides my current usage,
>
> I think you can set up guest_memfd+2M for private memory and shared
> memory from some other source, and that's the deprecated usage pattern.
Yes, though this is the deprecated usage pattern, gmem_in_place_conversion=true
allows it.
In fact, even without huge pages, v8 allows userspace to have shared memory
allocated from other source when gmem_in_place_conversion=true.
(My default testing of this series for the 4KB setting is with this
configuration).
> > there may be other scenarios where gmem memory
> > attributes is preferred without allocating shared memory from gmem.
> > (e.g., PAGE.ADD from a temp extra shared source memory).
> >
>
> Is this TDH.MEM.PAGE.ADD, used indirectly from
> tdx_gmem_post_populate()? This use case isn't blocked. Even if
> gmem_in_place_conversion=true, you can still set src_address to
> non-guest_memfd memory and load from anywhere you like.
>
> Please let me know if that is broken! I think I accidentally used that
It's not broken. I tested it with my hacked-up QEMU.
> setup in selftests and it worked. The selftests are now defaulting to
> in-place conversion.
>
> > For such use cases, I'm concerns that the admins may find it confusing if they
> > enable gmem_in_place_conversion but still observe extra memory consumptions for
> > shared memory.
> >
>
> Hmm but I guess if someone enables gmem_in_place_conversion but still
> allocates from elsewhere, they'd have to figure it out?
If gmem_in_place_conversion=true means gmem in place conversion is allowed (but
not enforced), I agree.
I'm wondering if we could rename it to "allow_gmem_in_place_conversion":)
> > [1] https://lore.kernel.org/kvm/aWmEegVP_A613WIr@google.com/
> >
> >> Sean, would you reconsider since Yan also asked? If the admin compiled
> >> the kernel knowing what CONFIG_KVM_VM_MEMORY_ATTRIBUTES means, then the
> >> admin would also be able to use a param like gmem_memory_attributes?
> >>
> >> There's the additional benefit that the similar naming aids in
> >> understanding for both the admin and software engineers.
> >>
> >> Either way, in the next revision, I'll also add this documentation for
> >> this module_param:
> >>
> >> Setting the module parameter gmem_in_place_conversion to true will
> >> enable the KVM_SET_MEMORY_ATTRIBUTES2 guest_memfd ioctl and disables
> >> the KVM_SET_MEMORY_ATTRIBUTES VM ioctl. If gmem_in_place_conversion is
> >> true, the private/shared attribute will be tracked per-guest_memfd
> >> instead of per-VM.
> >>
> >> Let me know what y'all think of the wording!
> >>
> >> >>
> >> >> [...snip...]
> >> >>
^ permalink raw reply
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Ackerley Tng @ 2026-06-26 0:07 UTC (permalink / raw)
To: Yan Zhao
Cc: Sean Christopherson, aik, andrew.jones, binbin.wu, brauner,
chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <ajyRg3BwGu5dCfOn@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Wed, Jun 24, 2026 at 04:00:32PM -0700, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>
>> > On Tue, Jun 23, 2026, Yan Zhao wrote:
>> >> On Tue, Jun 23, 2026 at 01:16:14PM +0800, Yan Zhao wrote:
>> >> > On Mon, Jun 22, 2026 at 06:22:45PM -0700, Sean Christopherson wrote:
>> >> > > On Mon, Jun 22, 2026, Yan Zhao wrote:
>> >> > > > On Thu, Jun 18, 2026 at 05:32:00PM -0700, Ackerley Tng via B4 Relay wrote:
>> >> > > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> >> > > > > index ffe9d0db58c59..56d10333c61a7 100644
>> >> > > > > --- a/arch/x86/kvm/vmx/tdx.c
>> >> > > > > +++ b/arch/x86/kvm/vmx/tdx.c
>> >> > > > > @@ -3198,8 +3198,12 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>> >> > > > > if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
>> >> > > > > return -EIO;
>> >> > > > >
>> >> > > > > - if (!src_page)
>> >> > > > > - return -EOPNOTSUPP;
>> >> > > > > + if (!src_page) {
>> >> > > > > + if (!gmem_in_place_conversion)
>> >> > > > When userspace turns on gmem_in_place_conversion while creating guest_memfd
>> >> > > > without the MMAP flag, the absence of src_page should still be treated as an
>> >> > > > error.
>> >> > >
>> >> > > Why MMAP?
>> >> > Hmm, I was showing a scenario that in-place conversion couldn't occur.
>> >> > I didn't mean that with the MMAP flag, mmap() and user write must occur.
>> >> >
>> >> > > Shouldn't this be a general "if (!src_page && !up-to-date)"? Just
>> >> > > because userspace _can_ mmap() the memory doesn't mean userspace _has_ mmap()'d
>> >> > > and written memory. And when write() lands, MMAP wouldn't be necessary to
>> >> > > initialize the memory.
>> >> > Do you mean using up-to-date flag as below?
>> >
>> > Yes? I didn't actually look at the implementation details.
>> >
>> >> > if (!src_page) {
>> >> > src_page = pfn_to_page(pfn);
>> >> > if (!folio_test_uptodate(page_folio(src_page)))
>> >> > return -EOPNOTSUPP;
>> >> > }
>>
>> Yan is right that with the earlier patch "Zero page while getting pfn",
>> folio_test_uptodate() here will always return true.
>>
>> Actually, this is an alternative fix for the issue Sashiko pointed out
>> on v7 where userspace can do a populate() (either TDX or SNP) without
>> first allocating the page, with src_address == NULL, and leak
>> uninitialized memory into the guest.
>>
>> Advantage of using the uptodate check in populate: if the host never
>> allocates the page, populate doesn't incur zeroing before writing the
>> page anyway in populate().
>>
>> Disadvantage: Both TDX and SNP will have to implement this uptodate
>> check. guest_memfd can't check centrally because for SNP, for a
>> PAGE_TYPE_ZERO, !src_page should be allowed with a !uptodate page since
>> firmware will zero and there's no leakage of uninitialized host memory?
> Another disadvantage: the uptodate flag is per-folio. What if the folio
> is only partially initialized by the userspace especially after huge page is
> supported?
>
Good point on huge pages!
The uptodate flag on the folio in guest_memfd means "this folio has been
written to". As of now (before patch at [1]), this happens when
+ folio is zeroed on first use by userspace
+ folio is zeroed on first use of the guest
+ folio is populated
When huge pages are supported, the folio can't partially be initialized?
On allocation, if any part is shared, we split the page. The parts are
separate folios that have their own uptodate flags.
On splitting, if the huge page is uptodate, the split pages will also be
uptodate. If the huge page is not uptodate, the split pages won't be
uptodate, but that's ok since they will be marked uptodate on first use.
On merging, the non-uptodate parts have to be zeroed and then marked
uptodate. Any parts that are in use would have been marked uptodate
already, so there's no overwriting data that is in use. I'll need to
think more about when it's safe to zero.
I'm still on the fence between the two options
1. Using uptodate check in populate to reject src_pages that have never
been written to or
2. Always zero before populate
but whether the uptodate flag is per-folio or not doesn't affect these
two options in terms of fixing the leak of uninitialized host memory,
right?
>
>> >> Another concern with this fix is that:
>> >> commit "KVM: guest_memfd: Zero page while getting pfn" [1] always marks the
>> >> folio uptodate before reaching post_populate().
>> >>
>> >> [1] https://lore.kernel.org/all/20260618-gmem-inplace-conversion-v8-21-9d2959357853@google.com/
>> >>
>> >> > One concern is that TDX now does not much care about the up-to-date flag since
>> >> > TDX doesn't rely on the flag to clear pages on conversions.
>> >> > I'm not sure if the flag can be reliably checked in this case. e.g.,
>> >> > now the whole folio is marked up-to-date even if only part of it is faulted by
>> >> > user access.
>> >> > Ensuring that the up-to-date flag works correctly with huge page support seems
>> >> > to have more effort than introducing a dedicated flag for TDX.
>> >> >
>> >> > > > Additionally, to properly enable in-place copying for the TDX initial memory
>> >> > > > region, userspace must not only specify source_addr to NULL, but also follow
>> >> > > > a specific sequence (where steps 1/2/3/7 are required only for in-place copy):
>> >> > > > 1. create guest_memfd with MMAP flag
>> >> > > > 2. mmap the guest_memfd.
>> >> > > > 3. convert the initial memory range to shared.
>> >> > > > 4. copy initial content to the source page.
>> >> > > > 5. convert the initial memory range to private
>> >> > > > 6. invoke ioctl KVM_TDX_INIT_MEM_REGION.
>> >> > > > 7. do not unmap the source backend.
>> >> > > >
>> >> > > > So, would it be reasonable to introduce a dedicated flag that allows userspace
>> >> > > > to explicitly opt into the in-place copy functionality? e.g.,
>> >> > >
>> >> > > Why? It's userspace's responsibility to get the above right. If userspace fails
>> >> > > to provide a src_page when it doesn't want in-place copy, that's a userspace bug.
>>
>> Yan, is your concern that userspace forgot to update the code and
>> forgets to provide a src_page, and if we keep the "Zero page while
> Yes. Previously, it would be rejected after GUP fails.
>
I see, didn't realize previously it would be rejected because GUP
fails. GUP failed because it wasn't faulted into the host?
That's kind of orthogonal, I don't think GUP fail leading to rejecting
populate was meant to help userspace catch these issues. GUP would also
fail if the user did mmap(), write to it, unmap using
madvise(MADV_DONTNEED), then forget and pass 0 as src_address.
>> getting pfn" patch, ends up with the guest silently having a zero page?
>> I think that would be found quite early in userspace VMM testing...
> I actually encountered this during testing this patch.
> I update most code path to follow this sequence. However, still some corner ones
> for TDVF HOB, which are less obvious and harder to update.
> The TD just booted up and hang silently.
>
I think this is just the life of a close-to-hardware software engineer
:P no errors, got stuck somewhere, root cause is some unitialized
thing.
>> >> > I mean if userspace specifies a NULL source_addr by mistake, it's better for
>> >> > kernel to detect this mistake, similar to how it validates whether source_addr
>> >> > is PAGE_ALIGNED.
>> >
>> > The alignment case is different. If userspace provides an unaligned value, KVM
>> > *can't* do what userspace is asking because hardware and thus KVM only supports
>> > converting on page boundaries.
>> >
>> > For a NULL source, KVM can still do what userspace is asking. Rejecting userspace's
>> > request would then be making assumptions about what userspace wants.
>> >
>>
>> Also, +1 on this, what if userspace, knowing that pages are zeroed on
>> allocation, actually wants to rely on that to get a zero page in the guest?
> What if 0 uaddr is a valid address? :)
>
>> >> > Since userspace already needs to perform additional steps to enable in-place
>> >> > copy, specifying a dedicated flag to indicate that the NULL source_addr is
>> >> > intentional seems like a reasonable burden.
>> >
>> > I don't see how it adds any value. I wouldn't be at all surprised if most VMMs
>> > just wen up with code that does:
>> >
>> > if (in-place) {
>> > src = NULL;
>> > flags |= KVM_TDX_IN_PLACE_COPY_INITIAL_MEMORY_REGION;
>> > }
>>
^ permalink raw reply
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Yan Zhao @ 2026-06-26 0:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <aj087H1UWSFxbShR@google.com>
On Thu, Jun 25, 2026 at 07:36:28AM -0700, Sean Christopherson wrote:
> On Thu, Jun 25, 2026, Yan Zhao wrote:
> > On Thu, Jun 25, 2026 at 09:51:01AM +0800, Yan Zhao wrote:
> > > On Wed, Jun 24, 2026 at 05:41:58PM -0700, Sean Christopherson wrote:
> > > > On Wed, Jun 24, 2026, Ackerley Tng wrote:
> > > > > Yan Zhao <yan.y.zhao@intel.com> writes:
> > > > > > With gmem_in_place_conversion=true, userspace can create guest_memfd without the
> > > > > > MMAP flag. In such cases, shared memory is allocated from different backends.
> > > > > > This means this module parameter only enables per-gmem memory attribute and does
> > > > > > not guarantee that gmem in-place conversion will actually occur.
> > > >
> > > > KVM module params are pretty much always about what KVM supports, not what is
> > > > guaranteed to happen.
> > > >
> > > > - enable_mmio_caching doesn't guarantee there will actually be MMIO SPTEs,
> > > > because maybe the guest never accesses emulated MMIO.
> > > > - enable_pmu doesn't guarantee VMs will get a PMU, because userspace may elect
> > > > not to advertise one.
> > > > - and so on and so forth...
> > > >
> > > > Yes, there's a small mental jump to get from "KVM supports in-place conversion"
> > > > to "I need to set memory attributes on the guest_memfd instance, not the VM",
> > > > but I don't see that as a big hurdle, certainly not in the long term. And once
> > > > the VMM code is written, I really do think most people are going to care about
> > > > whether or not KVM supports in-place conversion, not where PRIVATE is tracked.
> > > Sorry, I just saw this mail after posting my reply in [1].
> > >
> > > I'm ok with gmem_in_place_conversion=true just means KVM supports in-place
> > > conversion, while we can still create VMs with shared memory not from gmem.
> > Or what about "allow_gmem_in_place_conversion" ?
>
> No, because turning on the param also disallows setting PRIVATE in the VM-scoped
> KVM_SET_MEMORY_ATTRIBUTES ioctl.
>
> > > Though it still feels a bit odd to require TDX huge pages to depend on
> > > gmem_in_place_conversion=true when shared memory is not currently allocated
> > > from gmem,
>
> I fully expect that to be a transient state, and in all likelihood not something
> that is *ever* shipped in production. Landing TDX hugepages without guest_memfd
> hugepage support is all about avoiding unnecessary serialization of series and
> features that aren't strictly dependent on each other.
>
> > > it should become more natural over time once gmem supports in-place
> > > conversions for huge page.
>
> Yes, and I want to prioritize the steady state for end users, not the in-progress
> state for developers. Once all of this settles out, I fully expect the majority
> of deployments to only support in-place conversion, at which point the end user
> is only going to care whether or not in-place conversion is enabled in KVM, not
> the subtle detail that it's still possible to do out-of-place conversions (and
> that will always hold true, it's not like VMA-based memslots are being deprecated).
>
> > > Besides my current usage, there may be other scenarios where gmem memory
> > > attributes is preferred without allocating shared memory from gmem.
> > > (e.g., PAGE.ADD from a temp extra shared source memory).
> > >
> > > For such use cases, I'm concerns that the admins may find it confusing if they
> > > enable gmem_in_place_conversion but still observe extra memory consumptions for
> > > shared memory.
>
> KVM can help with documentation, but beyond that, it's not KVM's problem to solve.
> If a VMM *and* platform owner chooses to deploy a setup that utilizes out-of-place
> conversions, then it's on the VMM and/or plaform owner to understand and communicate
> the implications to the end user.
Thanks for all the explanations!
Documentation that choosing a different source after enabling
gmem_in_place_conversion is deprecated looks good to me.
> And I'm not remotely convinced that prepending allow_ to the param will help
> end users diagnose "unexpected" memory consumption, in quotes because anyone that
> is deploying a stack that utilizes out-of-place conversion absolutely needs to
> understand and plan for the additional memory consumption. I.e. if the memory
> consumption is "unexpected" to the end user, they likely have far bigger problems.
My first impression of gmem_in_place_conversion=true was that it enforces gmem
in-place conversion. However, it actually only enforces per-gmem private/shared
attribute.
My worry was that people might think it's a kernel bug if userspace can still
have shared memory from other sources after they configured
gmem_in_place_conversion=true.
However, I have no strong opinion if you think gmem_in_place_conversion is good,
and with the above documentation. :)
^ permalink raw reply
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Yan Zhao @ 2026-06-26 1:17 UTC (permalink / raw)
To: Ackerley Tng
Cc: Sean Christopherson, aik, andrew.jones, binbin.wu, brauner,
chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <CAEvNRgH5KOHoemnC9QOn_oK97=KeAH1XuX3ps36-pJ0Fn0aBHQ@mail.gmail.com>
On Thu, Jun 25, 2026 at 05:07:23PM -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> > On Wed, Jun 24, 2026 at 04:00:32PM -0700, Ackerley Tng wrote:
> >> Sean Christopherson <seanjc@google.com> writes:
> >>
> >> > On Tue, Jun 23, 2026, Yan Zhao wrote:
> >> >> On Tue, Jun 23, 2026 at 01:16:14PM +0800, Yan Zhao wrote:
> >> >> > On Mon, Jun 22, 2026 at 06:22:45PM -0700, Sean Christopherson wrote:
> >> >> > > On Mon, Jun 22, 2026, Yan Zhao wrote:
> >> >> > > > On Thu, Jun 18, 2026 at 05:32:00PM -0700, Ackerley Tng via B4 Relay wrote:
> >> >> > > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >> >> > > > > index ffe9d0db58c59..56d10333c61a7 100644
> >> >> > > > > --- a/arch/x86/kvm/vmx/tdx.c
> >> >> > > > > +++ b/arch/x86/kvm/vmx/tdx.c
> >> >> > > > > @@ -3198,8 +3198,12 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> >> >> > > > > if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
> >> >> > > > > return -EIO;
> >> >> > > > >
> >> >> > > > > - if (!src_page)
> >> >> > > > > - return -EOPNOTSUPP;
> >> >> > > > > + if (!src_page) {
> >> >> > > > > + if (!gmem_in_place_conversion)
> >> >> > > > When userspace turns on gmem_in_place_conversion while creating guest_memfd
> >> >> > > > without the MMAP flag, the absence of src_page should still be treated as an
> >> >> > > > error.
> >> >> > >
> >> >> > > Why MMAP?
> >> >> > Hmm, I was showing a scenario that in-place conversion couldn't occur.
> >> >> > I didn't mean that with the MMAP flag, mmap() and user write must occur.
> >> >> >
> >> >> > > Shouldn't this be a general "if (!src_page && !up-to-date)"? Just
> >> >> > > because userspace _can_ mmap() the memory doesn't mean userspace _has_ mmap()'d
> >> >> > > and written memory. And when write() lands, MMAP wouldn't be necessary to
> >> >> > > initialize the memory.
> >> >> > Do you mean using up-to-date flag as below?
> >> >
> >> > Yes? I didn't actually look at the implementation details.
> >> >
> >> >> > if (!src_page) {
> >> >> > src_page = pfn_to_page(pfn);
> >> >> > if (!folio_test_uptodate(page_folio(src_page)))
> >> >> > return -EOPNOTSUPP;
> >> >> > }
> >>
> >> Yan is right that with the earlier patch "Zero page while getting pfn",
> >> folio_test_uptodate() here will always return true.
> >>
> >> Actually, this is an alternative fix for the issue Sashiko pointed out
> >> on v7 where userspace can do a populate() (either TDX or SNP) without
> >> first allocating the page, with src_address == NULL, and leak
> >> uninitialized memory into the guest.
> >>
> >> Advantage of using the uptodate check in populate: if the host never
> >> allocates the page, populate doesn't incur zeroing before writing the
> >> page anyway in populate().
> >>
> >> Disadvantage: Both TDX and SNP will have to implement this uptodate
> >> check. guest_memfd can't check centrally because for SNP, for a
> >> PAGE_TYPE_ZERO, !src_page should be allowed with a !uptodate page since
> >> firmware will zero and there's no leakage of uninitialized host memory?
> > Another disadvantage: the uptodate flag is per-folio. What if the folio
> > is only partially initialized by the userspace especially after huge page is
> > supported?
> >
>
> Good point on huge pages!
>
> The uptodate flag on the folio in guest_memfd means "this folio has been
> written to". As of now (before patch at [1]), this happens when
>
> + folio is zeroed on first use by userspace
> + folio is zeroed on first use of the guest
> + folio is populated
>
> When huge pages are supported, the folio can't partially be initialized?
>
> On allocation, if any part is shared, we split the page. The parts are
> separate folios that have their own uptodate flags.
>
> On splitting, if the huge page is uptodate, the split pages will also be
> uptodate. If the huge page is not uptodate, the split pages won't be
> uptodate, but that's ok since they will be marked uptodate on first use.
>
> On merging, the non-uptodate parts have to be zeroed and then marked
If that's true, it would be good.
> uptodate. Any parts that are in use would have been marked uptodate
> already, so there's no overwriting data that is in use. I'll need to
> think more about when it's safe to zero.
>
> I'm still on the fence between the two options
>
> 1. Using uptodate check in populate to reject src_pages that have never
> been written to or
> 2. Always zero before populate
2 does not work?
The flow is
1. mmap gmem_fd, make GFN shared, and write initial content.
2. convert GFN to private
3. invoke ioctl to trigger populate.
> but whether the uptodate flag is per-folio or not doesn't affect these
> two options in terms of fixing the leak of uninitialized host memory,
> right?
yes, provided "On merging, the non-uptodate parts have to be zeroed and then
marked uptodate".
> >
> >> >> Another concern with this fix is that:
> >> >> commit "KVM: guest_memfd: Zero page while getting pfn" [1] always marks the
> >> >> folio uptodate before reaching post_populate().
> >> >>
> >> >> [1] https://lore.kernel.org/all/20260618-gmem-inplace-conversion-v8-21-9d2959357853@google.com/
> >> >>
> >> >> > One concern is that TDX now does not much care about the up-to-date flag since
> >> >> > TDX doesn't rely on the flag to clear pages on conversions.
> >> >> > I'm not sure if the flag can be reliably checked in this case. e.g.,
> >> >> > now the whole folio is marked up-to-date even if only part of it is faulted by
> >> >> > user access.
> >> >> > Ensuring that the up-to-date flag works correctly with huge page support seems
> >> >> > to have more effort than introducing a dedicated flag for TDX.
> >> >> >
> >> >> > > > Additionally, to properly enable in-place copying for the TDX initial memory
> >> >> > > > region, userspace must not only specify source_addr to NULL, but also follow
> >> >> > > > a specific sequence (where steps 1/2/3/7 are required only for in-place copy):
> >> >> > > > 1. create guest_memfd with MMAP flag
> >> >> > > > 2. mmap the guest_memfd.
> >> >> > > > 3. convert the initial memory range to shared.
> >> >> > > > 4. copy initial content to the source page.
> >> >> > > > 5. convert the initial memory range to private
> >> >> > > > 6. invoke ioctl KVM_TDX_INIT_MEM_REGION.
> >> >> > > > 7. do not unmap the source backend.
> >> >> > > >
> >> >> > > > So, would it be reasonable to introduce a dedicated flag that allows userspace
> >> >> > > > to explicitly opt into the in-place copy functionality? e.g.,
> >> >> > >
> >> >> > > Why? It's userspace's responsibility to get the above right. If userspace fails
> >> >> > > to provide a src_page when it doesn't want in-place copy, that's a userspace bug.
> >>
> >> Yan, is your concern that userspace forgot to update the code and
> >> forgets to provide a src_page, and if we keep the "Zero page while
> > Yes. Previously, it would be rejected after GUP fails.
> >
>
> I see, didn't realize previously it would be rejected because GUP
> fails. GUP failed because it wasn't faulted into the host?
GUP fails if 0 is not a valid user address.
But GUP would not fail if 0 is a valid address. e.g., in below scenario:
#include <sys/mman.h>
#include <stdio.h>
int main(void)
{
void *p=mmap((void*)0,4096,PROT_READ|PROT_WRITE, MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS,-1,0);
if (p==MAP_FAILED) {
perror("mmap");
return 1;
}
*(char*)0='Y';
printf("addr0=%p val=%c\n",p,*(char*)0);
return 0;
}
> That's kind of orthogonal, I don't think GUP fail leading to rejecting
> populate was meant to help userspace catch these issues. GUP would also
> fail if the user did mmap(), write to it, unmap using
> madvise(MADV_DONTNEED), then forget and pass 0 as src_address.
The original uAPI did not explicitly define 0 as an invalid uaddr. Whether 0 was
rejected depended on whether the user mmap()'d address 0. If 0 was a valid
mapping, populate() could proceed.
commit 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating
guest memory") changed the behavior though. It would return -EOPNOTSUPP for a 0
uaddr.
But if a user configures 0 uaddr as valid, writes to it, and then passes 0 as
source_addr(not from gmem), I'm not sure if it's good for the kernel to silently
treat 0 uaddr as an identifier for in-place copy from the private PFN in gmem.
> >> getting pfn" patch, ends up with the guest silently having a zero page?
> >> I think that would be found quite early in userspace VMM testing...
> > I actually encountered this during testing this patch.
> > I update most code path to follow this sequence. However, still some corner ones
> > for TDVF HOB, which are less obvious and harder to update.
> > The TD just booted up and hang silently.
> >
>
> I think this is just the life of a close-to-hardware software engineer
> :P no errors, got stuck somewhere, root cause is some unitialized
> thing.
>
> >> >> > I mean if userspace specifies a NULL source_addr by mistake, it's better for
> >> >> > kernel to detect this mistake, similar to how it validates whether source_addr
> >> >> > is PAGE_ALIGNED.
> >> >
> >> > The alignment case is different. If userspace provides an unaligned value, KVM
> >> > *can't* do what userspace is asking because hardware and thus KVM only supports
> >> > converting on page boundaries.
> >> >
> >> > For a NULL source, KVM can still do what userspace is asking. Rejecting userspace's
> >> > request would then be making assumptions about what userspace wants.
> >> >
> >>
> >> Also, +1 on this, what if userspace, knowing that pages are zeroed on
> >> allocation, actually wants to rely on that to get a zero page in the guest?
> > What if 0 uaddr is a valid address? :)
> >
> >> >> > Since userspace already needs to perform additional steps to enable in-place
> >> >> > copy, specifying a dedicated flag to indicate that the NULL source_addr is
> >> >> > intentional seems like a reasonable burden.
> >> >
> >> > I don't see how it adds any value. I wouldn't be at all surprised if most VMMs
> >> > just wen up with code that does:
> >> >
> >> > if (in-place) {
> >> > src = NULL;
> >> > flags |= KVM_TDX_IN_PLACE_COPY_INITIAL_MEMORY_REGION;
> >> > }
> >>
^ permalink raw reply
page: | 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