* Re: [PATCH v2 06/17] x86/virt/tdx: Re-initialize the extensions on runtime TDX module update
From: Xu Yilun @ 2026-06-30 11:14 UTC (permalink / raw)
To: Chao Gao
Cc: x86, kvm, linux-coco, linux-kernel, djbw, kas, rick.p.edgecombe,
yilun.xu, xiaoyao.li, sohil.mehta, adrian.hunter, kishen.maloor,
tony.lindgren, peter.fang, baolu.lu, zhenzhong.duan, dave.hansen,
dave.hansen, seanjc
In-Reply-To: <akIo+E5hfQL7C30E@intel.com>
On Mon, Jun 29, 2026 at 04:12:40PM +0800, Chao Gao wrote:
> >+/*
> >+ * Mostly the same flow as init_tdx_module_extensions(), but rejects adding
> >+ * more memory.
> >+ */
> >+static int update_tdx_module_extensions(void)
> >+{
> >+ struct tdx_sys_info_ext sysinfo_ext;
> >+ int ret;
> >+
> >+ if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> >+ return 0;
> >+
> >+ ret = get_tdx_sys_info_ext(&sysinfo_ext);
> >+ if (ret)
> >+ return ret;
> >+
> >+ if (!sysinfo_ext.ext_required)
> >+ return 0;
> >+
> >+ if (sysinfo_ext.memory_pool_required_pages)
> >+ return -EFAULT;
>
> Will tdx_ext_init() return an error if more memory is needed?
Yes.
>
> If yes, we can leave this check to the module.
Yes, we can. Although we can't remove ext_required check, we can remove
memory_pool_required_pages check, making the function simpler.
^ permalink raw reply
* Re: [PATCH v2 03/17] x86/virt/tdx: Detect if the extensions initialization is required
From: Xu Yilun @ 2026-06-30 11:10 UTC (permalink / raw)
To: Chao Gao
Cc: x86, kvm, linux-coco, linux-kernel, djbw, kas, rick.p.edgecombe,
yilun.xu, xiaoyao.li, sohil.mehta, adrian.hunter, kishen.maloor,
tony.lindgren, peter.fang, baolu.lu, zhenzhong.duan, dave.hansen,
dave.hansen, seanjc
In-Reply-To: <akIR1ClnTs1bRsD9@intel.com>
On Mon, Jun 29, 2026 at 02:33:56PM +0800, Chao Gao wrote:
> >+static __init int init_tdx_module_extensions(void)
> >+{
> >+ struct tdx_sys_info_ext sysinfo_ext;
> >+ int ret;
> >+
> >+ if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> >+ return 0;
> >+
> >+ ret = get_tdx_sys_info_ext(&sysinfo_ext);
> >+ if (ret)
> >+ return ret;
> >+
> >+ /* Skip if no feature requires TDX module extensions. */
> >+ if (!sysinfo_ext.ext_required)
> >+ return 0;
>
> What would happen if the kernel doesn't do this 'ext_required' check
> and always does the extension initialization?
>
> If TDH.EXT.INIT returns success when no extension is configured, we
No, TDH.EXT.INIT fails when ext_required==false
> could drop this 'ext_required' from this series entirely.
^ permalink raw reply
* Re: [PATCH v8 05/46] KVM: Make CONFIG_KVM_VM_MEMORY_ATTRIBUTES selectable
From: Xiaoyao Li @ 2026-06-30 10:55 UTC (permalink / raw)
To: ackerleytng, 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, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-5-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
>
> Make CONFIG_KVM_VM_MEMORY_ATTRIBUTES selectable, only for (CoCo) VM types
> that might use vm_memory_attributes.
>
> Also document CONFIG_KVM_VM_MEMORY_ATTRIBUTES to specifically be about the
> private/shared attribute.
I think this patch needs to be moved later after per-gmem shared/private
attribute is implemented. Because so far, TDX/SEV indeed depend on
CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
Not to discuss if it makes sense to report TDX as supported VM TYPE when
CONFIG_KVM_VM_MEMORY_ATTRIBUTES is not enabled, this patch just fails
the compilation when
CONFIG_KVM_VM_MEMORY_ATTRIBUTES = n
and KVM_INTEL_TDX/KVM_AMD_SEV is enabled:
arch/x86/kvm/../../../virt/kvm/guest_memfd.c: In function
‘__kvm_gmem_populate’:
arch/x86/kvm/../../../virt/kvm/guest_memfd.c:918:14: error: implicit
declaration of function ‘kvm_range_has_memory_attributes’
[-Werror=implicit-function-declaration]
918 | if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/Kconfig | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 24f96396cfa1c..c28393dc664eb 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -81,13 +81,16 @@ config KVM_WERROR
> If in doubt, say "N".
>
> config KVM_VM_MEMORY_ATTRIBUTES
> - bool
> + depends on KVM_SW_PROTECTED_VM || KVM_INTEL_TDX || KVM_AMD_SEV
> + bool "Enable per-VM PRIVATE vs. SHARED attributes (for CoCo VMs)"
> + help
> + Enable support for tracking PRIVATE vs. SHARED memory using per-VM
> + memory attributes.
>
> 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
> @@ -138,7 +141,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)
> @@ -162,7 +164,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
>
^ permalink raw reply
* Re: [PATCH v8 04/46] KVM: Decouple kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTES
From: Xiaoyao Li @ 2026-06-30 10:47 UTC (permalink / raw)
To: ackerleytng, 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, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-4-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Sean Christopherson <seanjc@google.com>
>
> When memory attributes become trackable in guest_memfd, the concept of
> having private memory is no longer dependent on
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
>
> With this, on x86, kvm_arch_has_private_mem() is defined if some CoCo
> platform support (or the testing CONFIG_KVM_SW_PROTECTED_VM) is compiled
> in.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/include/asm/kvm_host.h | 4 +++-
> include/linux/kvm_host.h | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 8e8eb8a5e8a6b..1bde67cf6eb0e 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -2394,7 +2394,9 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
> int tdp_max_root_level, int tdp_huge_page_level);
>
>
> -#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> +#if defined(CONFIG_KVM_SW_PROTECTED_VM) || \
> + defined(CONFIG_KVM_INTEL_TDX) || \
> + defined(CONFIG_KVM_AMD_SEV)
Maybe we can just remove the #ifdef and make it always avaiable?
> #define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
> #endif
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 201d0f2143976..d370e834d619e 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -722,7 +722,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
> }
> #endif
>
> -#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> +#ifndef kvm_arch_has_private_mem
> static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
> {
> return false;
>
^ permalink raw reply
* Re: [PATCH v8 03/46] KVM: Move KVM_VM_MEMORY_ATTRIBUTES config definition to x86
From: Xiaoyao Li @ 2026-06-30 10:45 UTC (permalink / raw)
To: ackerleytng, 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, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-3-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay 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 while RWX memory attributes are on the
> horizon, they too are expected to be x86-only.
>
> 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 supports in-place conversions, 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>
> Reviewed-by: Fuad Tabba<tabba@google.com>
> Signed-off-by: Ackerley Tng<ackerleytng@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v8 02/46] KVM: Rename KVM_GENERIC_MEMORY_ATTRIBUTES to KVM_VM_MEMORY_ATTRIBUTES
From: Xiaoyao Li @ 2026-06-30 10:45 UTC (permalink / raw)
To: ackerleytng, 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, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-2-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Sean Christopherson <seanjc@google.com>
>
> Rename the per-VM memory attributes Kconfig to make it explicitly about
> per-VM attributes in anticipation of adding memory attributes support to
> guest_memfd, at which point it will be possible (and desirable) to have
> memory attributes without the per-VM support, even in x86.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v2 04/17] x86/virt/tdx: Add extra memory to TDX module for the extensions
From: Xu Yilun @ 2026-06-30 10:27 UTC (permalink / raw)
To: Chao Gao
Cc: x86, kvm, linux-coco, linux-kernel, djbw, kas, rick.p.edgecombe,
yilun.xu, xiaoyao.li, sohil.mehta, adrian.hunter, kishen.maloor,
tony.lindgren, peter.fang, baolu.lu, zhenzhong.duan, dave.hansen,
dave.hansen, seanjc
In-Reply-To: <akIlHqh0/Rna7aYY@intel.com>
> >+static __init int tdx_ext_mem_setup(unsigned int required_pages)
> >+{
> >+ struct tdx_hpa_list *hpa_list;
> >+ struct page *page;
> >+ unsigned int i;
> >+ int ret;
> >+
> >+ /*
> >+ * memory_pool_required_pages == 0 means no need to add pages,
> >+ * skip the memory setup.
> >+ */
>
> There is no "memory_pool_required_pages" here.
OK.
BTW, sashiko says the comment restates what the if-statement. So I
change to:
/*
* This required_pages comes from the metadata
* memory_pool_required_pages. TDX module uses it to indicate how much
* memory is still needed. This value decreases each time memory is
* added via TDH.EXT.MEM.ADD.
*
* On first time initialization, a value of 0 before any memory is
* added is unusual. But host makes no assumptions. Skip the memory
* setup and let subsequent steps catch any actual errors.
*/
>
> >+ if (!required_pages)
> >+ return 0;
Other comments are all good, included.
^ permalink raw reply
* Re: [PATCH v2 17/17] KVM: TDX: Support event-notify interrupts only with userspace Quoting
From: Peter Fang @ 2026-06-30 6:36 UTC (permalink / raw)
To: Tony Lindgren
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, baolu.lu, zhenzhong.duan,
dave.hansen, dave.hansen, seanjc
In-Reply-To: <ajzKl4Ov0jdXvH-o@tlindgre-MOBL1>
On Thu, Jun 25, 2026 at 09:28:39AM +0300, Tony Lindgren wrote:
> On Thu, Jun 18, 2026 at 04:13:55PM +0800, Xu Yilun wrote:
> > From: Peter Fang <peter.fang@intel.com>
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -202,8 +202,15 @@ static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
> >
> > caps->cpuid.nent = td_conf->num_cpuid_config;
> >
> > - caps->user_tdvmcallinfo_1_r11 =
> > - TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT;
> > + /*
> > + * Don't advertise userspace event-notify interrupt support if TDX
> > + * quoting service is enabled, as quote generation will be handled
> > + * entirely in the kernel. Support in the kernel can be added later.
> > + */
> > + if (!tdx_quote_enabled()) {
> > + caps->user_tdvmcallinfo_1_r11 |=
> > + TDVMCALLINFO_SETUP_EVENT_NOTIFY_INTERRUPT;
> > + }
>
> Can you use kvm_tdx->get_quote_in_kernel also above? Or should it maybe
> be initialized here if not used earlier?
This is a bit tricky. init_kvm_tdx_caps() is designed to be independent
of a TDX VM instance (it reports the overall TDX capabilities), so there
is no struct kvm_tdx available.
>
> > @@ -1684,9 +1691,16 @@ static int tdx_get_quote(struct kvm_vcpu *vcpu)
> >
> > static int tdx_setup_event_notify_interrupt(struct kvm_vcpu *vcpu)
> > {
> > + struct kvm_tdx *kvm_tdx = to_kvm_tdx(vcpu->kvm);
> > struct vcpu_tdx *tdx = to_tdx(vcpu);
> > u64 vector = tdx->vp_enter_args.r12;
> >
> > + /* See comment in init_kvm_tdx_caps() */
> > + if (kvm_tdx->get_quote_in_kernel) {
> > + tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_SUBFUNC_UNSUPPORTED);
> > + return 1;
> > + }
> > +
>
> Since you're using kvm_tdx->get_quote_in_kernel here.
This is in per-vCPU context, so struct kvm_tdx is available.
^ permalink raw reply
* Re: [PATCH v2 07/17] x86/virt/tdx: Initialize Quoting extension
From: Peter Fang @ 2026-06-30 5:20 UTC (permalink / raw)
To: Chao Gao
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen, seanjc
In-Reply-To: <akIt1b9jJl1crp1A@intel.com>
On Mon, Jun 29, 2026 at 04:33:25PM +0800, Chao Gao wrote:
> On Thu, Jun 18, 2026 at 04:13:45PM +0800, Xu Yilun wrote:
> >From: Peter Fang <peter.fang@intel.com>
> >
> >Initialize the Quoting extension during TDX bringup, after enabling TDX
> >module Extension.
>
> This jumps into what the patch does without explaining what the Quoting
> extension is. A brief background would help reviewers who aren't familiar
> with it.
Yep, I'll add more background in the changelog.
>
> >diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> >index 4d2940f4538a..06c42b86b05e 100644
> >--- a/arch/x86/virt/vmx/tdx/tdx.c
> >+++ b/arch/x86/virt/vmx/tdx/tdx.c
> >@@ -1167,6 +1167,32 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
> > return 0;
> > }
> >
> >+/* Initialize quoting extension */
>
> This comment isn't quite helpful.
Got it. It makes more sense to add it in patch 12.
>
> >+static __init int tdx_quote_init(void)
> >+{
> >+ struct tdx_module_args args = {};
> >+ u64 r;
> >+
> >+ do {
> >+ r = seamcall(TDH_QUOTE_INIT, &args);
> >+ } while (r == TDX_INTERRUPTED_RESUMABLE);
> >+
> >+ if (r != TDX_SUCCESS)
> >+ return -EFAULT;
> >+
> >+ return 0;
> >+}
> >+
> >+static __init void init_tdx_quoting_extension(void)
> >+{
> >+ int ret;
> >+
> >+ if (tdx_addon_feature0 & TDX_FEATURES0_QUOTE) {
> >+ ret = tdx_quote_init();
> >+ WARN_ON_ONCE(ret);
> >+ }
>
> nit: Reduce indentation of the main body.
>
> if (!(tdx_addon_feature0 & TDX_FEATURES0_QUOTE))
> return;
>
> ret = tdx_quote_init();
> WARN_ON_ONCE(ret);
That's better. I'll make this change.
>
>
> Also, why two functions? Can we inline tdx_quote_init() here, or push the
> feature check into it.
tdx_quote_init() has a second call site in patch 12
(update_tdx_quoting_extension()), during runtime TDX module update.
Adding the "if (feature)" check inside a SEAMCALL helper doesn't seem
like a common pattern. I'm a bit concerned about making this look
inconsistent.
>
^ permalink raw reply
* Re: [PATCH v2 11/17] x86/virt/tdx: Add interface to generate a Quote
From: Peter Fang @ 2026-06-30 4:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, baolu.lu, zhenzhong.duan,
dave.hansen, dave.hansen, seanjc
In-Reply-To: <ajzFKBZWpbwefE03@tlindgre-MOBL1>
On Thu, Jun 25, 2026 at 09:05:28AM +0300, Tony Lindgren wrote:
> On Thu, Jun 18, 2026 at 04:13:49PM +0800, Xu Yilun wrote:
> > From: Peter Fang <peter.fang@intel.com>
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> ...
> > +void *tdx_quote_generate(struct tdx_td *td, void *in_data, u32 in_data_len,
> > + u32 *quote_len)
> > +{
> > + struct tdx_quote_data *qdata = &tdx_quote;
> > + void *quote_dup = NULL;
> > + u64 r, out_len;
> > +
> > + if (!tdx_quote_enabled())
> > + return NULL;
> > +
> > + mutex_lock(&tdx_quote_lock);
>
> How about make the pre-generated static tdx_quote a template page that only
> gets read and copied to an allocated bufer here?
>
> If the tdx_quote template is only read for copying here, seems you're not
> going to need the mutex at all? That is assuming tdx_quote template does
> not change after init.
Hm, actually tdx_quote is an output buffer as well (in the form of a
head pointer: qdata->hpa_entries_pa). Maybe this code needs better
commenting...
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Peter Fang @ 2026-06-30 4:12 UTC (permalink / raw)
To: Tony Lindgren
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, baolu.lu, zhenzhong.duan,
dave.hansen, dave.hansen, seanjc
In-Reply-To: <ajzF3F5N8ix3SCzm@tlindgre-MOBL1>
On Thu, Jun 25, 2026 at 09:08:28AM +0300, Tony Lindgren wrote:
> On Thu, Jun 18, 2026 at 04:13:46PM +0800, Xu Yilun wrote:
> > From: Peter Fang <peter.fang@intel.com>
> > For simplicity, let all guests share a global buffer. Build the buffer's
> > HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> > va-to-pa conversions at runtime.
>
> To me it seems the pre-generated parts can be made into a template that
> can be copied to an allocated buffer looking at patch 11/17.
Thanks. I'm planning to switch to dynamic allocation in the next
revision to make this simpler [1]. So the template will be the
HPA_LINKED_LIST node pages plus the next pointers in these nodes.
[1] https://lore.kernel.org/linux-coco/20260626095833.GB1600180@pedri/
^ permalink raw reply
* Re: [RFC PATCH v5 30/45] x86/virt/tdx: Add API to demote a 2MB mapping to 512 4KB mappings
From: Yan Zhao @ 2026-06-30 2:36 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: seanjc@google.com, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, Huang, Kai,
dave.hansen@linux.intel.com, kas@kernel.org,
binbin.wu@linux.intel.com, mingo@redhat.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
isaku.yamahata@intel.com, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Annapurve, Vishal, x86@kernel.org
In-Reply-To: <9c22a8bc7f326f81effdff27fd72c3c5339e5053.camel@intel.com>
On Tue, Jun 30, 2026 at 05:07:13AM +0800, Edgecombe, Rick P wrote:
> On Thu, 2026-06-11 at 16:44 +0800, Yan Zhao wrote:
>
> Nice find.
>
> > Since this contention should occur rarely (e.g., when there's a second TD
> > invoking PAMT.ADD concurrently while the first TD is invoking DEMOTE, and the
> > DPAMT page pair to add for DEMOTE must reside in the 2MB target range as
> > PAMT.ADD), a possible optimization is to avoid holding the global pamt_lock in
> > the first invocation of tdh_mem_page_demote() (e.g., by indicating try or fast
> > mode); only acquire the global pamt_lock if the first try returns busy, ensuring
> > the second invocation must succeed.
>
> Yes, let's not do it for the initial implementation.
Ok. I can put the optimization in the TODO list.
> >
> > [1] https://lore.kernel.org/kvm/aNX6V6OSIwly1hu4@yzhao56-desk.sh.intel.com
> > [2] The contention is verified with an internal POC.
> > Error logs:
> > a.1) DEMOTE adds PAMT pages pfn1=0x19c0a0, pfn2=0x1b572f for guest pfn=0x519800
> > 2) __tdx_pamt_get() adds PAMT pages for pfn=0x19c0a1.
> > 3) DEMOTE returns error 0x800002000000000c.
> > b.1) DEMOTE adds PAMT pages pfn1=0x1b090c, pfn2=0x119b4f for guest pfn=0x511a00
> > 2) __tdx_pamt_get() adds PAMT pages for pfn=0x1b090d
> > 3) PAMT.ADD returns error 0x8000020000000001.
> >
> > [3] New implementation:
> > u64 tdh_mem_page_demote(struct tdx_td *td, u64 gpa, enum pg_level level, u64 pfn,
> > struct page *new_sp, struct tdx_pamt_cache *pamt_cache,
> > u64 *ext_err1, u64 *ext_err2)
> > {
> > bool dpamt = tdx_supports_dynamic_pamt(&tdx_sysinfo) && level == PG_LEVEL_2M;
> > struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> > struct tdx_module_args args = {
> > .rcx = gpa | pg_level_to_tdx_sept_level(level),
> > .rdx = tdx_tdr_pa(td),
> > .r8 = page_to_phys(new_sp),
> > };
> > atomic_t *pamt_refcount;
> > u64 ret;
> >
> > if (!tdx_supports_demote_nointerrupt(&tdx_sysinfo))
> > return TDX_SW_ERROR;
> >
> > /* Flush the new S-EPT page to be added */
> > tdx_clflush_page(new_sp);
> >
> > if (dpamt) {
> >
>
> Can we just not do huge pages if we don't have DPAMT? Actually, weren't we
Yes, we can.
I wasn't aware we're going to disallow huge page if we don't have DPAMT.
I thought we could have huge page with and without DPAMT.
> already going to do it like that?
Is this to avoid the "if (dpamt)" check here?
We may still need to check "level == PG_LEVEL_2M" here, since DPAMT related
code is not necessary if the demotion is from 1GB to 2MB.
(This tdh_mem_page_demote() implementation could support 1GB --> 2MB demotion
theoretically).
Or maybe for now just warn and return TDX_SW_ERROR if level != PG_LEVEL_2M ?
> >
> > if (alloc_pamt_array(pamt_pages, pamt_cache))
> > return TDX_SW_ERROR;
> >
> > args.r12 = page_to_phys(pamt_pages[0]);
> > args.r13 = page_to_phys(pamt_pages[1]);
> >
> > /*
> > * Before demotion, the 2MB guest memory range is not managed
> > * by DPAMT, so its pamt_refcount should be 0.
> > * Set it to 512 after demotion succeeds, since removing of each
> > * 4KB mapping will reduce the refcount by 1.
> > */
> > pamt_refcount = tdx_find_pamt_refcount(pfn);
>
> It feels kind of hacky. The refcount stuff is already a bit hard to follow.
After demotion, there would be 512 4KB mappings for this 2MB range.
Then, in later tdx_sept_remove_leaf_spte(), tdx_pamt_put() would be invoked
for 512 times. That's why we need to set the refcount to 512 after a successful
demotion here.
>
> >
> > spin_lock(&pamt_lock);
> > }
> > ret = seamcall_saved_ret(TDH_MEM_PAGE_DEMOTE, &args);
> >
> > if (dpamt) {
> > if (!ret)
> > WARN_ON_ONCE(atomic_cmpxchg_release(pamt_refcount, 0, PTRS_PER_PMD));
> >
> > spin_unlock(&pamt_lock);
> >
> > if (ret)
> > free_pamt_array(pamt_pages);
> > }
> >
> > *ext_err1 = args.rcx;
> > *ext_err2 = args.rdx;
> >
> > return ret;
> > }
>
^ 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-30 2:21 UTC (permalink / raw)
To: Sean Christopherson
Cc: Ackerley Tng, aik@amd.com, andrew.jones@linux.dev,
binbin.wu@linux.intel.com, brauner@kernel.org,
chao.p.peng@linux.intel.com, david@kernel.org,
jmattson@google.com, jthoughton@google.com, michael.roth@amd.com,
oupton@kernel.org, pankaj.gupta@amd.com, qperret@google.com,
Edgecombe, Rick P, rientjes@google.com, shivankg@amd.com,
steven.price@arm.com, tabba@google.com, willy@infradead.org,
wyihan@google.com, forkloop@google.com, pratyush@kernel.org,
suzuki.poulose@arm.com, aneesh.kumar@kernel.org,
liam@infradead.org, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86@kernel.org, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Annapurve, Vishal,
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@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-mm@kvack.org, linux-coco@lists.linux.dev
In-Reply-To: <akMPZePBdwQlD74H@google.com>
On Tue, Jun 30, 2026 at 08:35:49AM +0800, Sean Christopherson wrote:
> Gah, I thought I had sent this out this morning, long before Ackerley's response.
> But I got distracted by a meeting and forgot to get back to this... *sigh*
>
> Sending what I already wrote, even though there's a lot of overlap with Ackerley's
> mail.
>
> On Mon, Jun 29, 2026, Yan Zhao wrote:
> > On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:
> > > Yan Zhao <yan.y.zhao@intel.com> writes:
> > > > 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.
> > > >
> > >
> > > I'd say the original uAPI perhaps just didn't document 0 as an
> > > unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
> > > was perhaps accidentally changed and no customer complained, I think we
> > > can move forward with 0 as an invalid src_address? I wouldn't think
> > > anyone relies on 0 intentionally being a valid address.
> > >
> > > I could document that, if it helps?
> > What about just documenting that 0 is an unsupported uaddr which will be
> > re-purposed as an indicator to use the target pfn as the source, regardless of
> > whether gmem_in_place_conversion is true? i.e.,
> >
> > if (!src_page)
> > src_page = pfn_to_page(pfn);
>
> Because KVM can't generally use the target page as the source without in-place
> conversion, it's not supported today, and out-of-place conversion is being
> deprecated.
By "out-of-place conversion", do you mean using per-VM memory attribute
conversion?
> > I don't get why the two scenarios should be treated differently:
> > 1. gmem_in_place_conversion==true, shared memory is not from gmem
> > 2. gmem_in_place_conversion==false, shared memory is not from gmem
> >
> > In both case, a 0 uaddr could be mapped to a valid page not from gmem.
>
> That's immaterial. KVM's ABI (that we're solidifying) is that an address of '0'
> for the source means NULL. The fact that userspace could have a valid mapping
> at virtual address '0' is irrelevant.
So, I'm wondering if we can document that 0 uaddr could always mean using target
PFN.
i.e., for both scenarios 1 and 2, al long as 0 uaddr is specified, we always
use target PFN as source for in-place add.
> Again, just because something is technically possible doesn't mean it needs to
> be supported by every piece of KVM's uAPI.
>
> > So why not update the uAPI to handle both cases consistently? :)
>
> Because retroactively adding support for out-of-place conversion is pointless
> (requires a userspace update for a feature that's being deprecated), KVM can't
> generally support using the source for out-of-place conversion (it's effectively
> an obscure zero-page optimization), and IMO rejecting the out-of-place conversion
> scenario is valuable for KVM developers, e.g. to help newcomers understand what
> exactly is and isn't possible.
Ok. You mean per-VM memory attribute is deprecating, and source page from !gmem
backend is also deprecating, so we don't want to change uAPI for scenarios under
gmem_in_place_conversion==false. Right?
> Side topic, isn't TDX broken if target page has already been added to the TD?
> IIUC, kvm_tdp_mmu_map_private_pfn() will be a glorified nop due to the page
> already having a valid S-EPT mapping, and so KVM will incorrectly allow a double
Not sure if my understand out-of-place conversion correctly.
Given target PFNs and GFNs are not duplicated, what would cause double add? :)
> add. Ahhh, no, because KVM will return RET_PF_SPURIOUS and
> kvm_tdp_mmu_map_private_pfn() will then return -EIO.
My asking was if we could document uaddr always means using target PFN, since
TDX's in-place add does not rely on gmem in-place conversion.
^ 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-30 2:09 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: <CAEvNRgHO3T6pKDP7ye-RdqbGhAzVC7a=8uBUyaPxwbSuj9khqA@mail.gmail.com>
On Mon, Jun 29, 2026 at 05:00:02PM -0700, Ackerley Tng wrote:
> >> > 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.
> >> >
> >>
> >> I see, I only looked at this after commit 2a62345b3052.
> >>
> >> > 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.
> >> >
> >>
> >> I'd say the original uAPI perhaps just didn't document 0 as an
> >> unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
> >> was perhaps accidentally changed and no customer complained, I think we
> >> can move forward with 0 as an invalid src_address? I wouldn't think
> >> anyone relies on 0 intentionally being a valid address.
> >>
> >> I could document that, if it helps?
> > What about just documenting that 0 is an unsupported uaddr which will be
> > re-purposed as an indicator to use the target pfn as the source, regardless of
> > whether gmem_in_place_conversion is true? i.e.,
> >
> > if (!src_page)
> > src_page = pfn_to_page(pfn);
> >
> > I don't get why the two scenarios should be treated differently:
> > 1. gmem_in_place_conversion==true, shared memory is not from gmem
> > 2. gmem_in_place_conversion==false, shared memory is not from gmem
> >
> > In both case, a 0 uaddr could be mapped to a valid page not from gmem.
>
> This is true, but this check isn't about whether the page is from gmem.
Hmm. TDX's in-place add does not rely on gmem in-place conversion, which means
when gmem_in_place_conversion==false, TDX's in-place add can still be successful.
Since checking gmem_in_place_conversion==true also can't guarantee the share
memory is from gmem, it makes me feel odd to reject scenario 2 while turning
scenario 1 to in-place add.
> > So why not update the uAPI to handle both cases consistently? :)
> >
>
> Wait, but before this series, if region.src_address = 0, src_page = NULL
> and that's not supported so it returns -EOPNOTSUPP.
As in our previous discussion, no customer complaining about the previous change
to -EOPNOTSUPP means no one uses 0 uaddr today.
> If that's dropped, then suddenly if region.src_address = 0 and
> !gmem_in_place_conversion, tdx_gmem_post_populate() will now load the
> memory (zeroed) after [1] into the guest? I don't think we want to
> change that behavior.
>
> I could document that 0 is an unsupported uaddr only for TDX, and only
> when gmem_in_place_conversion = false.
>
> Since it is unsupported only when gmem_in_place_conversion = false, the
> check two lines marked with <<==== can't go away?
>
> if (!src_page) {
> if (!gmem_in_place_conversion) <<====
> return -EOPNOTSUPP; <<====
This rejecting scenario 2.
> src_page = pfn_to_page(pfn);
This turning scenario 1 to in-place add.
> }
>
> Also, for SNP, src_address == 0 is permitted (and desired, I believe, to
> avoid a pointless kernel memcpy) if the type of population is
> KVM_SEV_SNP_PAGE_TYPE_ZERO.
>
> >> >> >> 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...
> >>
> >> [...snip...]
> >>
>
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Sean Christopherson @ 2026-06-30 0:42 UTC (permalink / raw)
To: Peter Fang
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen
In-Reply-To: <20260629100301.GA1743876@pedri>
On Mon, Jun 29, 2026, Peter Fang wrote:
> On Thu, Jun 25, 2026 at 11:01:58AM -0700, Sean Christopherson wrote:
> > On Thu, Jun 18, 2026, Xu Yilun wrote:
> > > From: Peter Fang <peter.fang@intel.com>
> > >
> > > Provide an in-kernel path for Quote generation when handling
> > > TDG.VP.VMCALL<GetQuote>, without requiring an exit to userspace.
> >
> > Why?
> >
>
> Hi Sean,
>
> This is mainly to avoid a round trip to userspace for the GetQuote flow.
Again, why?
> New TDX modules can now get a Quote directly via an "extension SEAMCALL"
> instead of exiting to userspace and using an SGX enclave. Exiting to
> userspace for GetQuote no longer seems worth the overhead/complexity.
I dunno, from a kernel perspective, this is more complexity, not less:
Documentation/arch/x86/tdx.rst | 19 ++---
Documentation/virt/kvm/api.rst | 3 +
arch/x86/include/asm/tdx.h | 9 +++
arch/x86/kvm/vmx/tdx.h | 6 ++
arch/x86/kvm/vmx/tdx.c | 135 ++++++++++++++++++++++++++++++++-
virt/kvm/kvm_main.c | 1 +
6 files changed, 163 insertions(+), 10 deletions(-)
> The first half of the series enables extension SEAMCALLs. They implement
> simple APIs for higher-order security protocols that would otherwise need
> to be broken into smaller routines. For Quoting, this allows KVM to get
> a Quote directly through TDH.QUOTE.GET. The TDX module needs only the
> input data from TDG.VP.VMCALL<GetQuote> for that call.
Answering my own question (though probably poorly), IIUC the answer is that
DICE-based quoting is done through the TDX Module, whereas existing quoting is
done through an SGX enclave and so was routed through userspace.
If that's all there is too this, then why is KVM involved? I.e. why doesn't the
TDX Module provide the quote directly to the guest?
^ permalink raw reply
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Sean Christopherson @ 2026-06-30 0:35 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: <akI9m02jgKAdi4gX@yzhao56-desk.sh.intel.com>
Gah, I thought I had sent this out this morning, long before Ackerley's response.
But I got distracted by a meeting and forgot to get back to this... *sigh*
Sending what I already wrote, even though there's a lot of overlap with Ackerley's
mail.
On Mon, Jun 29, 2026, Yan Zhao wrote:
> On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:
> > Yan Zhao <yan.y.zhao@intel.com> writes:
> > > 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.
> > >
> >
> > I'd say the original uAPI perhaps just didn't document 0 as an
> > unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
> > was perhaps accidentally changed and no customer complained, I think we
> > can move forward with 0 as an invalid src_address? I wouldn't think
> > anyone relies on 0 intentionally being a valid address.
> >
> > I could document that, if it helps?
> What about just documenting that 0 is an unsupported uaddr which will be
> re-purposed as an indicator to use the target pfn as the source, regardless of
> whether gmem_in_place_conversion is true? i.e.,
>
> if (!src_page)
> src_page = pfn_to_page(pfn);
Because KVM can't generally use the target page as the source without in-place
conversion, it's not supported today, and out-of-place conversion is being
deprecated.
> I don't get why the two scenarios should be treated differently:
> 1. gmem_in_place_conversion==true, shared memory is not from gmem
> 2. gmem_in_place_conversion==false, shared memory is not from gmem
>
> In both case, a 0 uaddr could be mapped to a valid page not from gmem.
That's immaterial. KVM's ABI (that we're solidifying) is that an address of '0'
for the source means NULL. The fact that userspace could have a valid mapping
at virtual address '0' is irrelevant.
Again, just because something is technically possible doesn't mean it needs to
be supported by every piece of KVM's uAPI.
> So why not update the uAPI to handle both cases consistently? :)
Because retroactively adding support for out-of-place conversion is pointless
(requires a userspace update for a feature that's being deprecated), KVM can't
generally support using the source for out-of-place conversion (it's effectively
an obscure zero-page optimization), and IMO rejecting the out-of-place conversion
scenario is valuable for KVM developers, e.g. to help newcomers understand what
exactly is and isn't possible.
Side topic, isn't TDX broken if target page has already been added to the TD?
IIUC, kvm_tdp_mmu_map_private_pfn() will be a glorified nop due to the page
already having a valid S-EPT mapping, and so KVM will incorrectly allow a double
add. Ahhh, no, because KVM will return RET_PF_SPURIOUS and
kvm_tdp_mmu_map_private_pfn() will then return -EIO.
^ 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-30 0:00 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: <akI9m02jgKAdi4gX@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:
>> Yan Zhao <yan.y.zhao@intel.com> writes:
>>
>> > 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.
>> >
>>
>> This flow is correct, is what users of in-place conversion should do.
>>
>> "Always" is the wrong word, I should have said "zero if not uptodate
>> before populate", as in, with patch at [1].
>>
>> By doing the zeroing in __kvm_gmem_get_pfn instead, by the time populate
>> gets the pfn, the page would be zeroed, either because userspace faulted
>> it in, and the zeroing happened in kvm_gmem_fault_user_mapping(), or if
>> userspace never faulted it in, the zeroing would happen because
>> populate() allocated the page.
>
> I see.
>
>> >> 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".
>> >
>>
>> Thank you so much for bringing this up, I hadn't considered this
>> before. I'll do that when I get to guest_memfd hugepage restructuring.
>>
>> >> >
>> >> >> >> 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.
>> >
>>
>> I see, I only looked at this after commit 2a62345b3052.
>>
>> > 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.
>> >
>>
>> I'd say the original uAPI perhaps just didn't document 0 as an
>> unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
>> was perhaps accidentally changed and no customer complained, I think we
>> can move forward with 0 as an invalid src_address? I wouldn't think
>> anyone relies on 0 intentionally being a valid address.
>>
>> I could document that, if it helps?
> What about just documenting that 0 is an unsupported uaddr which will be
> re-purposed as an indicator to use the target pfn as the source, regardless of
> whether gmem_in_place_conversion is true? i.e.,
>
> if (!src_page)
> src_page = pfn_to_page(pfn);
>
> I don't get why the two scenarios should be treated differently:
> 1. gmem_in_place_conversion==true, shared memory is not from gmem
> 2. gmem_in_place_conversion==false, shared memory is not from gmem
>
> In both case, a 0 uaddr could be mapped to a valid page not from gmem.
This is true, but this check isn't about whether the page is from gmem.
> So why not update the uAPI to handle both cases consistently? :)
>
Wait, but before this series, if region.src_address = 0, src_page = NULL
and that's not supported so it returns -EOPNOTSUPP.
If that's dropped, then suddenly if region.src_address = 0 and
!gmem_in_place_conversion, tdx_gmem_post_populate() will now load the
memory (zeroed) after [1] into the guest? I don't think we want to
change that behavior.
I could document that 0 is an unsupported uaddr only for TDX, and only
when gmem_in_place_conversion = false.
Since it is unsupported only when gmem_in_place_conversion = false, the
check two lines marked with <<==== can't go away?
if (!src_page) {
if (!gmem_in_place_conversion) <<====
return -EOPNOTSUPP; <<====
src_page = pfn_to_page(pfn);
}
Also, for SNP, src_address == 0 is permitted (and desired, I believe, to
avoid a pointless kernel memcpy) if the type of population is
KVM_SEV_SNP_PAGE_TYPE_ZERO.
>> >> >> 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...
>>
>> [...snip...]
>>
^ permalink raw reply
* Re: [RFC PATCH v5 30/45] x86/virt/tdx: Add API to demote a 2MB mapping to 512 4KB mappings
From: Edgecombe, Rick P @ 2026-06-29 21:07 UTC (permalink / raw)
To: seanjc@google.com, Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Huang, Kai, dave.hansen@linux.intel.com, kas@kernel.org,
binbin.wu@linux.intel.com, mingo@redhat.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
isaku.yamahata@intel.com, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Annapurve, Vishal, x86@kernel.org
In-Reply-To: <aip1eO7wvJDxKtBX@yzhao56-desk.sh.intel.com>
On Thu, 2026-06-11 at 16:44 +0800, Yan Zhao wrote:
Nice find.
> Since this contention should occur rarely (e.g., when there's a second TD
> invoking PAMT.ADD concurrently while the first TD is invoking DEMOTE, and the
> DPAMT page pair to add for DEMOTE must reside in the 2MB target range as
> PAMT.ADD), a possible optimization is to avoid holding the global pamt_lock in
> the first invocation of tdh_mem_page_demote() (e.g., by indicating try or fast
> mode); only acquire the global pamt_lock if the first try returns busy, ensuring
> the second invocation must succeed.
Yes, let's not do it for the initial implementation.
>
> [1] https://lore.kernel.org/kvm/aNX6V6OSIwly1hu4@yzhao56-desk.sh.intel.com
> [2] The contention is verified with an internal POC.
> Error logs:
> a.1) DEMOTE adds PAMT pages pfn1=0x19c0a0, pfn2=0x1b572f for guest pfn=0x519800
> 2) __tdx_pamt_get() adds PAMT pages for pfn=0x19c0a1.
> 3) DEMOTE returns error 0x800002000000000c.
> b.1) DEMOTE adds PAMT pages pfn1=0x1b090c, pfn2=0x119b4f for guest pfn=0x511a00
> 2) __tdx_pamt_get() adds PAMT pages for pfn=0x1b090d
> 3) PAMT.ADD returns error 0x8000020000000001.
>
> [3] New implementation:
> u64 tdh_mem_page_demote(struct tdx_td *td, u64 gpa, enum pg_level level, u64 pfn,
> struct page *new_sp, struct tdx_pamt_cache *pamt_cache,
> u64 *ext_err1, u64 *ext_err2)
> {
> bool dpamt = tdx_supports_dynamic_pamt(&tdx_sysinfo) && level == PG_LEVEL_2M;
> struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> struct tdx_module_args args = {
> .rcx = gpa | pg_level_to_tdx_sept_level(level),
> .rdx = tdx_tdr_pa(td),
> .r8 = page_to_phys(new_sp),
> };
> atomic_t *pamt_refcount;
> u64 ret;
>
> if (!tdx_supports_demote_nointerrupt(&tdx_sysinfo))
> return TDX_SW_ERROR;
>
> /* Flush the new S-EPT page to be added */
> tdx_clflush_page(new_sp);
>
> if (dpamt) {
>
Can we just not do huge pages if we don't have DPAMT? Actually, weren't we
already going to do it like that?
>
> if (alloc_pamt_array(pamt_pages, pamt_cache))
> return TDX_SW_ERROR;
>
> args.r12 = page_to_phys(pamt_pages[0]);
> args.r13 = page_to_phys(pamt_pages[1]);
>
> /*
> * Before demotion, the 2MB guest memory range is not managed
> * by DPAMT, so its pamt_refcount should be 0.
> * Set it to 512 after demotion succeeds, since removing of each
> * 4KB mapping will reduce the refcount by 1.
> */
> pamt_refcount = tdx_find_pamt_refcount(pfn);
It feels kind of hacky. The refcount stuff is already a bit hard to follow.
>
> spin_lock(&pamt_lock);
> }
> ret = seamcall_saved_ret(TDH_MEM_PAGE_DEMOTE, &args);
>
> if (dpamt) {
> if (!ret)
> WARN_ON_ONCE(atomic_cmpxchg_release(pamt_refcount, 0, PTRS_PER_PMD));
>
> spin_unlock(&pamt_lock);
>
> if (ret)
> free_pamt_array(pamt_pages);
> }
>
> *ext_err1 = args.rcx;
> *ext_err2 = args.rdx;
>
> return ret;
> }
^ permalink raw reply
* Re: [RFC PATCH 06/15] x86/virt/tdx: Initialize Quoting extension during bringup
From: Edgecombe, Rick P @ 2026-06-29 18:11 UTC (permalink / raw)
To: djbw@kernel.org, Fang, Peter
Cc: Xu, Yilun, x86@kernel.org, kas@kernel.org,
yilun.xu@linux.intel.com, Duan, Zhenzhong,
linux-kernel@vger.kernel.org, Li, Xiaoyao, Mehta, Sohil,
kvm@vger.kernel.org, linux-coco@lists.linux.dev,
baolu.lu@linux.intel.com
In-Reply-To: <20260614075026.GE3200182@pedri>
On Sun, 2026-06-14 at 00:50 -0700, Peter Fang wrote:
> > Is this micro-optimization worth it? What are the classes of quote-init
> > failures vs just make the policy be anything in the module must init.
>
> Since there is a fallback option to do the Quoting in userspace, I think
> it is probably not worth shooting down TDX entirely over quote-init
> failures.
>
> The quote-init failures can come from:
>
> 1. Quoting init SEAMCALL failures, which look pretty opaque to the
> kernel and there's not much it can do about it.
> 2. Quoting buffer allocation failures, which *are* understood by the
> kernel, and it could maybe try something else. Right now, we just
> treat it the same as 1.
>
> This is helpful because I think the question of "what if the Quoting
> extension fails" has come up enough times that it warrants some
> explanation in the patch log. Thanks.
I thought we were going to do:
- If quoting is not supported, don't try to init quoting, and don't fail init
over it.
- If quoting is supported, but fails to init. Just fail TDX initialization for
simplicity.
This is what we are doing for the other features.
^ permalink raw reply
* COCONUT-SVSM Development Release v2026.06-devel
From: Jörg Rödel @ 2026-06-29 11:47 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi all,
The end of the month is approaching quickly and it is time for another exciting
development release of COCONUT-SVSM. Exciting because this release brings some
new features, e.g. support for the UEFI variable store protocol. But there is
more, list below:
- Added UEFI variable-store protocol.
- Replaced the in-tree VirtIO driver fork with upstream virtio-drivers;
integrated safe-mmio and fixed unaligned MMIO and resource-drop handling.
- Major scheduler/task rework: fixed cross-CPU races, simplified affinity and
context switching, added task termination waiting and a PENDING state, and
corrected register/reference handling.
- Expanded xbuild to build test kernels and support recipe-specific
environment, toolchain, and feature settings.
- Moved SVSM to 128 MiB on QEMU and Hyper-V platforms.
- Improved APIC emulation correctness, including ICR/EOI validation, LDR
reads, NMI self-IPIs, PPR calculation, and protocol error reporting.
- Added libtcgtpm cross-build support and switched to Rust’s default linker
behavior. COCONUT-SVSM can now be cross-compiled on Apple Silicon with MacOS.
- Hardened CPUID/XSAVE handling.
- Improved per-CPU TLB flushing.
These changes include the work of 9 contributors with a total of 99 non-merge
commits in 42 merges. The full shortlog is included below.
Happy testing!
-Joerg
Carlos López (30):
kernel: cpu/tlb: add TlbFlushScope constructor methods
kernel: cpu/tlb: homogenize per-CPU TLB flushes
kernel: mm/vm/range: perform partial TLB flush for per-CPU mappings
igvmbuilder: add CPUID leaf for YMM XSAVE size
cpuarch: remove packed qualifier from CPUID structures
kernel: cpu/cpuid: filter FPU/SSE features if size subleaf is not present
kernel: cpu/sse: retrieve XSAVE area size from 0xD CPUID subleaves
kernel: platform/snp: ignore XCR0/XSS for CPUID lookups
Merge pull request #1111 from kraxel/edk2-docs
xbuild: build TDX stage1 directly
xbuild: introduce no_default_features recipe field
xbuild: introduce toolchain recipe field
xbuild: introduce env recipe field
xbuild: introduce test kernel build infrastructure
Makefile: move test kernel builds into xbuild recipes
Makefile: respect RELEASE for test kernels
kernel: mm/alloc: convert linear bitscan to trailing bit count
kernel: cpu/apic: truly ignore host EOI errors
kernel: cpu/apic: verify value written to EOI register
kernel: cpu/apic: verify MBZ fields for ICR writes
kernel: cpu/apic: fix get_ppr_with_tpr() ISR indexing
kernel: cpu/apic: fix NMI delivery for physical-mode self-IPI
kernel: cpu/apic: handle LDR register reads
kernel: protocols/apic: correct error on invalid register access
Merge pull request #1125 from joergroedel/gpa-map
kernel: cpu/apic: reject more fields on ICR writes
Merge pull request #1135 from n-ramacciotti/protocols/core_query_add_vtpm
Documentation: add release process to doc website
Documentation: add object handle documentation to doc website
packit: update to latest version
Gerd Hoffmann (20):
stage1: tag code sections as 'ax'
stage1: rework linker script
kernel: add .got to linker script
bldr: add .got to linker script
cargo: switch to rust default linker
libtcgtpm: cross compiler setup
libtcgtpm: libcrt cross build
libtcgtpm: openssl cross build
libtcgtpm: tcgtpm cross build
Documentation: update build instructions
Documentation: remove PcdUninstallMemAttrProtocol=TRUE
Documentation: recommend the upstream edk2 repo
uefivars: write up uefi mm protocol spec draft
uefivars: add uefi mm protocol
uefivars: announce via query protocol
uefivars: add secure boot support
uefivars: add uefi_mm_get_manifest
uefivars: wire up attestation manifest
uefivars: ensure test build coverage.
Documentation: document edk2 build options
Joerg Roedel (6):
CODEOWNERS: Add Carlos López
igvmbuilder: Move COCONUT-SVSM to 128MB physical address
Documentation: Update installation documentation for KVM planes
launch_guest.sh: Update for KVM planes support
Update Cargo.lock for release
COCONUT-SVSM Release 2026.06-devel
Jon Lange (25):
task: always start the scheduler in the idle task
task: change context switch to a true function
gdbstub: use common exception context structure
task: streamline register preservation during context switch
cpu/percpu: store certain address fields as atomics
task: update task per-CPU state when task is locked active
cpu/vc: remove CAA MSR handling
platform: rename `validate_fw`
svsm: create transition page tables only when required
svsm: accept low memory in the kernel
bldr: relocate from 8 MB down to 64 KB
igvmbuilder: specify a single VTL for native IGVM platforms
bldr: move SIPI stub into bldr
cpu/idt: define scheduler interrupt vector
cpu: move per-CPU run queue into `PerCpuShared`
task: simplify `set_affinity
Merge pull request #1061 from msft-jlange/bldr_cleanup
tasks/exec_user: return `TaskPointer` instead of task ID
igvmbuilder: use correct VTL for VBS VP context
Merge pull request #1099 from msft-jlange/igvm_vtl
task: implement `wait_for_termination`
task: streamline reference count management during task termination
SVSM: require SNP restricted injection
task: introduce `PENDING` state
Merge pull request #1134 from 00xc/apic/misc-fixes-v2
Jörg Rödel (28):
Merge pull request #1060 from msft-jlange/task_work
Merge pull request #1086 from msft-jlange/caa_msr
Merge pull request #1083 from luigix25/tcg_doc
Merge pull request #1096 from msft-jlange/igvm_vtl
Merge pull request #1063 from 00xc/sse/xsave-size
Merge pull request #1098 from n-ramacciotti/scripts/fix_signed_off
Merge pull request #1084 from stefano-garzarella/script-qemu-avoid-sig-remap
Merge pull request #1044 from msft-jlange/wait_for_task
Merge branch 'main' into affinity
Merge pull request #1045 from msft-jlange/affinity
Merge pull request #1028 from 00xc/cpu/tlb/percpu
Merge pull request #1106 from kraxel/vtpm-cross-build
Merge pull request #1109 from joergroedel/codeowners
Merge pull request #981 from kraxel/rust-linker
Merge pull request #1107 from kraxel/cross-build-docs
Merge pull request #1103 from msft-jlange/task_terminate
Merge pull request #694 from kraxel/uefivars
Merge pull request #1119 from luigix25/fix_fpu
Merge pull request #1112 from stefano-garzarella/contributing-capital-letter
Merge pull request #1131 from 00xc/mm/alloc/bitscan
Merge pull request #1117 from joergroedel/planes-update
Merge pull request #1126 from 00xc/apic/misc-fixes
Merge pull request #1129 from msft-jlange/task_pending
Merge pull request #1127 from 00xc/xbuild/test-kernels
Merge pull request #259 from msft-jlange/restr_inj
Merge pull request #1141 from n-ramacciotti/xbuild/fix_path
Merge pull request #1140 from 00xc/docs/fix-missing
Merge pull request #1145 from 00xc/packit/deps
Luigi Leonardi (7):
Documentation/INSTALL: document all missing launch script options
Documentation/INSTALL: remove trailing whitespace
scripts/launch_guest: use `memory-backend-ram` for nocc mode
scripts/launch_guest: replace grep -P with portable sed
virtio/mmio: handle non-page-aligned MMIO device addresses
virtio/mmio: fix drop order in MmioSlot
kernel/task: preserve r12-r15 across context switch
Nicola Ramacciotti (6):
scripts/signed-off: Fix check for empty body
scripts/signed-off: Avoid exiting on the first failure
kernel/tests: Improve accuracy of fpu test utilities
kernel/tests: Make sure that fpu test tasks terminate
kernel/protocols: Add vtpm to core query protocol
xbuild/fs: Handle multi-level path
Oliver Steffen (5):
virtio-drivers: Use forked repo
virtio-drivers: Remove in-repo copy
virtio: Add safe-mmio crate dependency
virtio: Use upstream virtio-drivers
virtio: Fix typo in safety comment
Stefano Garzarella (13):
scripts/launch_guest: avoid signal remapping
Merge pull request #1101 from luigix25/macos_support
Merge pull request #1092 from osteffenrh/virtio-from-forked-repo
Merge pull request #1102 from luigix25/virtio_alignment
Documentation: remove capital letter requirement from commit subject
Merge pull request #1110 from luigix25/fix_drop
Merge pull request #1115 from n-ramacciotti/tests/wait_for_termination
docs/attestation: fix attestation test steps ordering
docs/attestation: document kbs-test --secret argument
Merge pull request #1097 from tanish111/secretbox-zeroize
Merge pull request #1108 from osteffenrh/virtio_safe-mmio_custom_backend
docs/attestation: link to INSTALL.md in the build step
Merge pull request #1132 from stefano-garzarella/doc-attestation-fix
tanish111 (1):
kernel/attest: add SecretSlice for zeroed attestation secrets
^ permalink raw reply
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Yan Zhao @ 2026-06-29 11:39 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: <aj7NwCRwWEfLK-gQ@google.com>
On Fri, Jun 26, 2026 at 12:06:40PM -0700, Sean Christopherson wrote:
> On Fri, Jun 26, 2026, Yan Zhao wrote:
> > On Thu, Jun 25, 2026 at 07:36:28AM -0700, Sean Christopherson wrote:
> > > On Thu, Jun 25, 2026, Yan Zhao wrote:
> > > 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.
>
> Ah, I see where you're coming from. FWIW, truly enforcing in-place conversion
> is flat out impossible. E.g. userspace can simply replace the memslot, at which
> point the memory effectively reverts to shared.
>
> > However, I have no strong opinion if you think gmem_in_place_conversion is good,
> > and with the above documentation. :)
>
> Ya, I think this largely a documentation problem. I agree that a param name
> like gmem_private_memory_attributes would be more precise, but I think it'd be
> far less informative for the vast majority of users that only care whether or
> not KVM can do in-place conversion, and don't care about how that is done.
Ok.
^ permalink raw reply
* Re: [PATCH 00/32] x86/msr: Drop 32-bit MSR interfaces
From: Ingo Molnar @ 2026-06-29 11:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Juergen Gross, linux-kernel, linux-pm, linux-edac@vger.kernel.org,
x86, linux-acpi, kvm, linux-coco, linux-pci, virtualization,
linux-ide, dri-devel, linux-fbdev, linux-crypto,
open list:GPIO SUBSYSTEM, linux-hyperv, linux-hwmon,
linux-perf-users, linux-mtd, platform-driver-x86,
Rafael J . Wysocki, Daniel Lezcano, Zhang Rui,
lukasz.luba@arm.com, Jason Baron, Borislav Petkov, Tony Luck,
Yazen Ghannam, Len Brown, Pavel Machek, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Sean Christopherson,
Paolo Bonzini, Kirill A. Shutemov, Rick Edgecombe, Pu Wen,
Bjorn Helgaas, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Viresh Kumar,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Tony W Wang-oc, Damien Le Moal, Niklas Cassel, Dave Airlie,
Helge Deller, linux-geode, Olivia Mackall, Herbert Xu,
Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Guenter Roeck, Peter Zijlstra, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Josh Poimboeuf,
Pawan Gupta, Vitaly Kuznetsov, Andy Lutomirski, Boris Ostrovsky,
Huang Rui, Mario Limonciello, Perry Yuan, K Prateek Nayak,
srinivas.pandruvada@linux.intel.com, Artem Bityutskiy,
Artem Bityutskiy, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Ashok Raj, Hans de Goede, Ilpo Järvinen,
Rajneesh Bhardwaj, David E Box, xen-devel
In-Reply-To: <7332feff-2649-496c-8e49-b0a19eb54a32@app.fastmail.com>
* Arnd Bergmann <arnd@arndb.de> wrote:
> >>> Note that most patches of this series are independent from each other.
> >>> Only the patches removing a specific interface (patches 7, 15, 26 and
> >>> 30) and the last two patches of the series depend on all previous
> >>> patches.
> >>
> >> It looks like you are touching most files twice or more here, to
> >> first convert from rdmsr to rdmsrq and then to change the
> >> two-argument rdmsrq() macro to a single-argument inline. If you
> >> introduce the inline version of rdmsrq() first, you should be
> >> able to skip the second step (patch 31) as they could be able
> >> to coexist.
> >
> > I've discussed how to structure the series with Ingo Molnar before [1]. The
> > current approach was his preference.
>
> Ok.
Note that the individual patches are IMO significantly easier to review
through the actual 32-bit => 64-bit variable assignment changes done
in isolation (which sometimes include minor cleanups), while
the Coccinelle semantic patch:
{ a(b,c) => c = a(b) }
which changes both the function signature and the order of terms as
well, is just a single add-on treewide patch.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Peter Fang @ 2026-06-29 10:03 UTC (permalink / raw)
To: Sean Christopherson
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen
In-Reply-To: <aj1tFuwZWdPGfUoG@google.com>
On Thu, Jun 25, 2026 at 11:01:58AM -0700, Sean Christopherson wrote:
> On Thu, Jun 18, 2026, Xu Yilun wrote:
> > From: Peter Fang <peter.fang@intel.com>
> >
> > Provide an in-kernel path for Quote generation when handling
> > TDG.VP.VMCALL<GetQuote>, without requiring an exit to userspace.
>
> Why?
>
Hi Sean,
This is mainly to avoid a round trip to userspace for the GetQuote flow.
New TDX modules can now get a Quote directly via an "extension SEAMCALL"
instead of exiting to userspace and using an SGX enclave. Exiting to
userspace for GetQuote no longer seems worth the overhead/complexity.
The first half of the series enables extension SEAMCALLs. They implement
simple APIs for higher-order security protocols that would otherwise need
to be broken into smaller routines. For Quoting, this allows KVM to get
a Quote directly through TDH.QUOTE.GET. The TDX module needs only the
input data from TDG.VP.VMCALL<GetQuote> for that call.
Thanks,
Peter
^ permalink raw reply
* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Aneesh Kumar K.V @ 2026-06-29 9:51 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alexey Kardashevskiy, Catalin Marinas, iommu, linux-arm-kernel,
linux-kernel, linux-coco, Robin Murphy, Marek Szyprowski,
Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5ao6gtoncp.fsf@kernel.org>
Alexey,
Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
> The current code already does this and uses the swiotlb pool correctly
> on SME. The challenge arises when we want to force SWIOTLB
> bouncing even for devices that can handle encrypted DMA addresses (more
> on that below). For such a config force_dma_uencrypted(dev) will return
> false and swiotlb will be marked cc_shared/decrypted = true; This trip
> the new check we added.
>
> /* swiotlb pool is incorrect for this device */
> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
> return (phys_addr_t)DMA_MAPPING_ERROR;
>
> We can also do
>
> if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> /* swiotlb pool is incorrect for this device */
> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
> return (phys_addr_t)DMA_MAPPING_ERROR;
>
> /* Force attrs to match the kind of memory in the pool */
> if (mem->cc_shared)
> *attrs |= DMA_ATTR_CC_SHARED;
> else
> *attrs &= ~DMA_ATTR_CC_SHARED;
> } else {
> /*
> * Host memory encryption where device requires an
> * unencrypted dma_addr_t due to dma mask limit
> */
> if (force_dma_unencrypted(dev))
> *attrs |= DMA_ATTR_CC_SHARED;
> else
> *attrs &= ~DMA_ATTR_CC_SHARED;
> }
>
>
> Here I see value in having DMA_ATTR_UNENCRYPTED. The question is do we
> need to split this into two flags and introduce the resulting code
> duplication.
>
Can you help to test this patch?
commit 0275ed870ff8dadb4890fe8342e84b294f657c43
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Date: Mon Jun 29 11:55:08 2026 +0530
swiotlb: Return unencrypted DMA addresses for SME bounce buffers
On hosts with memory encryption, the default swiotlb pool is marked shared
and decrypted when memory encryption is active.
Make host-memory-encryption swiotlb mappings consistently return
unencrypted DMA addresses. This applies regardless of whether the device
itself requires unencrypted DMA due to its DMA mask, because the bounce
buffer memory backing the mapping is already unencrypted. It also preserves
swiotlb=force for devices that can address encrypted memory: forced bounce
mappings use the unencrypted swiotlb pool and receive unencrypted DMA
addresses.
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index a62e1571ec95..9ba23cf8d97b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -1514,15 +1514,26 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
- /* swiotlb pool is incorrect for this device */
- if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
- return (phys_addr_t)DMA_MAPPING_ERROR;
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+ /* swiotlb pool is incorrect for this device */
+ if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
+ return (phys_addr_t)DMA_MAPPING_ERROR;
- /* Force attrs to match the kind of memory in the pool */
- if (mem->cc_shared)
+ /* Force attrs to match the kind of memory in the pool */
+ if (mem->cc_shared)
+ *attrs |= DMA_ATTR_CC_SHARED;
+ else
+ *attrs &= ~DMA_ATTR_CC_SHARED;
+ } else if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
+ /*
+ * On hosts with memory encryption, SWIOTLB-backed memory
+ * is unencrypted memory. DMA addresses returned for bounce
+ * buffers must therefore have the C-bit cleared, even for
+ * devices that can address encrypted memory. This also
+ * preserves swiotlb=force for those devices.
+ */
*attrs |= DMA_ATTR_CC_SHARED;
- else
- *attrs &= ~DMA_ATTR_CC_SHARED;
+ }
/*
* The default swiotlb memory pool is allocated with PAGE_SIZE
^ permalink raw reply related
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Yan Zhao @ 2026-06-29 9:40 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: <CAEvNRgHb6WmOha6Pct_Tn8Ucuov95L=fj5=2R9gcHfx=b2V_+A@mail.gmail.com>
On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> > 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.
> >
>
> This flow is correct, is what users of in-place conversion should do.
>
> "Always" is the wrong word, I should have said "zero if not uptodate
> before populate", as in, with patch at [1].
>
> By doing the zeroing in __kvm_gmem_get_pfn instead, by the time populate
> gets the pfn, the page would be zeroed, either because userspace faulted
> it in, and the zeroing happened in kvm_gmem_fault_user_mapping(), or if
> userspace never faulted it in, the zeroing would happen because
> populate() allocated the page.
I see.
> >> 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".
> >
>
> Thank you so much for bringing this up, I hadn't considered this
> before. I'll do that when I get to guest_memfd hugepage restructuring.
>
> >> >
> >> >> >> 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.
> >
>
> I see, I only looked at this after commit 2a62345b3052.
>
> > 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.
> >
>
> I'd say the original uAPI perhaps just didn't document 0 as an
> unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
> was perhaps accidentally changed and no customer complained, I think we
> can move forward with 0 as an invalid src_address? I wouldn't think
> anyone relies on 0 intentionally being a valid address.
>
> I could document that, if it helps?
What about just documenting that 0 is an unsupported uaddr which will be
re-purposed as an indicator to use the target pfn as the source, regardless of
whether gmem_in_place_conversion is true? i.e.,
if (!src_page)
src_page = pfn_to_page(pfn);
I don't get why the two scenarios should be treated differently:
1. gmem_in_place_conversion==true, shared memory is not from gmem
2. gmem_in_place_conversion==false, shared memory is not from gmem
In both case, a 0 uaddr could be mapped to a valid page not from gmem.
So why not update the uAPI to handle both cases consistently? :)
> >> >> 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: 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