* Re: [PATCH v8 07/46] KVM: Rename memory attribute APIs to prepare for in-place gmem conversion
From: Xiaoyao Li @ 2026-06-30 15:22 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-7-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> -bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> - unsigned long mask, unsigned long attrs);
> +bool kvm_range_has_vm_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> + unsigned long mask, unsigned long attrs);
> bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> struct kvm_gfn_range *range);
> bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
We have
- kvm_pre_set_memory_attributes()
- kvm_arch_pre_set_memory_attributes()
- kvm_arch_post_set_memory_attributes()
left, do they need to be renamed as well?
then the interesting one is kvm_vm_set_mem_attributes(), which contains
"vm" already while it means "vm ioctl". Do we need to rename it to
kvm_vm_set_vm_mem_attributes()?
^ permalink raw reply
* SVSM Development Call July 1st, 2026
From: Jörg Rödel @ 2026-06-30 14:51 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH v9 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Tom Lendacky @ 2026-06-30 14:18 UTC (permalink / raw)
To: K Prateek Nayak, Ashish Kalra, tglx, mingo, bp, dave.hansen, x86,
hpa, seanjc, peterz, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <fe9927ad-a06a-4a4b-8122-12644513ed14@amd.com>
On 6/24/26 22:45, K Prateek Nayak wrote:
> Hello Ashish,
>
> On 6/25/2026 3:26 AM, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> While SNP is active, every memory write is checked against the RMP to
>> protect the integrity of SEV-SNP guest memory. By the SNP architecture
>> these checks cannot be disabled on a subset of CPUs: they are gated
>> per-core by SYSCFG[SNP_EN], which the SEV firmware requires to be set on
>> every present CPU before SNP initialization. A CPU that does not have
>> SNP_EN set and was not initialized via SNP_INIT performs no RMP checks at
>> all, so there is no valid configuration with SNP active and any CPU exempt
>> from RMP checks.
>>
>> The firmware determines which CPUs are present from the processor and the
>> BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates
>> them at SNP init; it is not aware of the OS bringing CPUs online or
>> offline afterwards. A CPU brought online after SNP init was not
>> enumerated at SNP_INIT and does not have SNP_EN set, so writes from it are
SNP_INIT will fail if not all of the CPUs have SnpEn set. So if the CPU
was offline and didn't have SnpEn set, SNP_INIT will fail and so you can't
have SNP guest memory.
Thanks,
Tom
>> not RMP-checked and could corrupt SEV-SNP guest memory, and there is no
>> way to keep work off such a CPU once it is online. OS CPU hotplug can thus
>> diverge from the firmware's expectations and break SNP.
>
> If this is true ...
>
> [..snip..]
>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index 217b6b19802e..66475145b3fa 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1479,6 +1479,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>
>> snp_hv_fixed_pages_state_update(sev, HV_FIXED);
>>
>> + /* Disable CPU hotplug while SNP is active (see snp_disable_cpu_hotplug). */
>> + snp_disable_cpu_hotplug();
>
> ... then this should be done at snp_prepare() before
> on_each_cpu(snp_enable) right?
>
> If not, then any CPU hotplug between the cpus_read_unlock() there and
> the snp_disable_cpu_hotplug() here will not have the SNP_EN set.
>
> Isn't that a concern?
>
> Also, this patch can probably go first since the FW assumptions on
> hotplug exists independent of RMPOPT bits.
>
>> +
>> snp_setup_rmpopt();
>>
>> sev->snp_initialized = true;
>
^ permalink raw reply
* Re: [PATCH v8 11/46] KVM: Consolidate private memory and guest_memfd ifdeffery in kvm_host.h
From: Xiaoyao Li @ 2026-06-30 13:59 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-11-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Sean Christopherson <seanjc@google.com>
>
> Move the kvm_arch_has_private_mem() stub and a few guest_memfd function
> definitions/declarations "down" in kvm_host.h to utilize existing #ifdefs,
> and so that related code is clustered together.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ 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 13:27 UTC (permalink / raw)
To: Yan Zhao
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,
Rick P Edgecombe, 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, 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@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: <akMoFqj/8Af2i/Al@yzhao56-desk.sh.intel.com>
On Tue, Jun 30, 2026, Yan Zhao wrote:
> 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?
Yep, I couldn't come up with a better description.
> > > 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 would document it as saying "no source page", and then state that a source page
is required if in-place conversion isn't enabled/supported/allowed.
> 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?
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? :)
I was working through what would happen if userspace did KVM_TDX_INIT_MEM_REGION
on the same target page multiple times.
>
> > 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.
Yeah, I was on a tangent, ignore everything from "Side topic" on.
^ permalink raw reply
* Re: [PATCH v8 04/46] KVM: Decouple kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTES
From: Sean Christopherson @ 2026-06-30 13:06 UTC (permalink / raw)
To: Xiaoyao Li
Cc: 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,
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: <6b1f0c77-f059-4f8d-8f46-443b944c59a0@intel.com>
On Tue, Jun 30, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > 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?
No, because common KVM keys off the macro to determine whether or not PRIVATE is
a supported attribute:
#ifdef kvm_arch_has_private_mem
static u64 kvm_supports_private_mem(struct kvm *kvm)
{
return !kvm || kvm_arch_has_private_mem(kvm);
}
#else
#define kvm_supports_private_mem(kvm) false
#endif
And also whether or not to provide the in-place conversion param (without PRIVATE,
conversions aren't supported in general):
#ifdef kvm_arch_has_private_mem
bool __ro_after_init gmem_in_place_conversion = !IS_ENABLED(CONFIG_KVM_VM_MEMORY_ATTRIBUTES);
module_param(gmem_in_place_conversion, bool, 0444);
EXPORT_SYMBOL_FOR_KVM_INTERNAL(gmem_in_place_conversion);
#endif
I agree the #ifdeffery is ugly, but kvm_supports_private_mem() in particular
needs to evaluate to false if PRIVATE memory isn't supported.
^ permalink raw reply
* Re: [PATCH v8 09/46] KVM: guest_memfd: Introduce function to check GFN private/shared status
From: Xiaoyao Li @ 2026-06-30 12:19 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-9-9d2959357853@google.com>
On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
>
> Introduce function for KVM to check the private/shared status of guest
> memory at a given GFN.
>
> This will be used in a later patch.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
With binbin's comment resolved,
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
BTW, it looks better to move the patch 01 just behind this one.
> ---
> include/linux/kvm_host.h | 2 ++
> virt/kvm/guest_memfd.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 3915da2a61778..27687fb9d5201 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2575,6 +2575,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
> #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
>
> #ifdef CONFIG_KVM_GUEST_MEMFD
> +bool kvm_gmem_is_private(struct kvm *kvm, gfn_t gfn);
> +
> int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
> int *max_order);
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 8101f64e0366f..bca912db5be6e 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -510,6 +510,37 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
> return 0;
> }
>
> +bool kvm_gmem_is_private(struct kvm *kvm, gfn_t gfn)
> +{
> + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> + struct inode *inode;
> +
> + /*
> + * If this gfn has no associated memslot, there's no chance of the gfn
> + * being backed by private memory, since guest_memfd must be used for
> + * private memory, and guest_memfd must be associated with some memslot.
> + */
> + if (!slot)
> + return 0;
> +
> + CLASS(gmem_get_file, file)(slot);
> + if (!file)
> + return 0;
> +
> + inode = file_inode(file);
> +
> + /*
> + * Rely on the maple tree's internal RCU lock to ensure a
> + * stable result. This result can become stale as soon as the
> + * lock is dropped, so the caller _must_ still protect
> + * consumption of private vs. shared by checking
> + * mmu_invalidate_retry_gfn() under mmu_lock to serialize
> + * against ongoing attribute updates.
> + */
> + return kvm_gmem_is_private_mem(inode, kvm_gmem_get_index(slot, gfn));
> +}
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_is_private);
> +
> static struct file_operations kvm_gmem_fops = {
> .mmap = kvm_gmem_mmap,
> .open = generic_file_open,
>
^ permalink raw reply
* 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
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