* Re: [PATCH 00/32] x86/msr: Drop 32-bit MSR interfaces
From: Ingo Molnar @ 2026-07-02 10:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: Arnd Bergmann, 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, 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: <akQR9YMtMHReJTfB@google.com>
* Sean Christopherson <seanjc@google.com> wrote:
> > 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.
>
> Is the plan for subsystem maintainers to pick up the relevant patches,
> and then do the treewide change one release cycle later?
I'll try to keep the patches in a single tree (tip:x86/msr)
in the hope of not prolonging the pain two cycles - but it's
of course fine for maintainers to pick up the patches too
(most of them are standalone), we'll sort it all out in the end.
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 06/15] x86/virt/tdx: Initialize Quoting extension during bringup
From: Peter Fang @ 2026-07-02 9:48 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: djbw@kernel.org, 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: <d862b418cdbc9b55c453038184b7a438f34e4da8.camel@intel.com>
On Mon, Jun 29, 2026 at 11:11:27AM -0700, Edgecombe, Rick P wrote:
> 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.
Hm this is simpler. The "fallback" scenario is probably a stretch right
now. I'll make this change. Thanks.
^ permalink raw reply
* Re: [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path
From: Binbin Wu @ 2026-07-02 9:32 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
In-Reply-To: <20260526023515.288829-9-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> When handling an EPT violation, KVM holds a spinlock while manipulating
> the EPT. Before entering the spinlock it doesn't know how many EPT page
> tables will need to be installed or whether a huge page will be used. For
> this reason it allocates a worst case number of page tables that it might
> need as part of servicing the EPT violation.
>
> Under Dynamic PAMT these pre-allocated pages will potentially need to have
> Dynamic PAMT backing pages installed for them. KVM already has helpers to
> manage topping up page caches before taking the MMU lock, but they cannot be
> passed from KVM to arch/x86 code.
>
> The problem of how and when to install the DPAMT backing pages for the
> pages given to the TDX module during the fault path has had a lot of
> design attempts.
> - Extracting KVM's MMU caches requires too much inlined code added to
> headers.
> - A few varieties of installing Dynamic PAMT backing when allocating the
> S-EPT page tables. [0][1]
> - Using mempool_t to transfer the pages between KVM and arch/x86 doesn't
> work because it is the component is designed more around maintaining a
^^^^^
Nit: extra "it is"
> pool of pages, rather than topping up a continually drained cache.
>
> So don't do these as they all had various problems. Instead just create a
> small simple data structure to use for handing a pre-allocated list of
> pages between KVM and arch/x86 code. Model this on KVM's existing MMU
> memory caches.
>
> Add a tdx_pamt_cache arg to tdx_pamt_get() so it can draw pages from a
> cache when needed. Not all DPAMT page installations will happen under
> spinlock, for example control pages. So have tdx_pamt_get() maintain the
> existing behavior of allocating from the page allocator when NULL is
> passed for the struct tdx_pamt_cache arg. This prevents excess allocations
> for cases where it can be avoided.
>
> Export the new helpers for KVM.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Link: https://lore.kernel.org/kvm/de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com/ [0]
> Link: https://lore.kernel.org/kvm/aYprxnSHKHUtk7pt@google.com/ [1]
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Binbin Wu @ 2026-07-02 8:55 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-8-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Use control page helpers for allocating and freeing TD control structures,
> such these operations can work for Dynamic PAMT.
^
Nit:
It appears to be missing the word "that".
>
> The TDX module tracks some state for each page of physical memory that it
> might use. It calls this state the PAMT. It includes separate state for
> each page size a physical page could be utilized at within the TDX module
> (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> allocated dynamically. So the kernel must install PAMT backing for each 4KB
> page before gifting it to the TDX module, and tear it down after the page
> is reclaimed.
>
> TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> TDCX) are all handed to the TDX module at 4KB page size and are therefore
> subject to this requirement. Replace the raw alloc_page()/__free_page()
> calls for these pages with tdx_alloc/free_control_page().
>
> Switching between special Dynamic PAMT operations or normal page
> alloc/free operations is handled internally in
> tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> calls. Just call them unconditionally. Similarly, drop the NULL checks
> before freeing, as tdx_free_control_page() handles NULL internally.
>
> No functional change intended when Dynamic PAMT is not in use.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [sean: handle alloc+free+reclaim in one patch]
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [Rick: enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Binbin Wu @ 2026-07-02 7:39 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-6-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> backing for the 2MB region covering the passed pfn. However, multiple
> callers can concurrently operate on 4KB pages that fall within the same
> 2MB region. When this happens only one Dynamic PAMT page pair needs to be
> installed to cover the 2MB range. And when one page is freed, the Dynamic
> PAMT backing cannot be freed until all pages in the range are no longer in
> use. Make the helpers handle these races internally.
>
> Use the per-2MB refcounts from previous changes to track how many 4KB
> pages are in use within each region. Gate the actual Dynamic PAMT add and
> remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> check and SEAMCALL with a global spinlock so the read-decide-act sequence
> is atomic. This also avoids TDX module BUSY errors, as Dynamic PAMT add
> and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
> so simultaneous attempts on the same region would conflict.
>
> The lock is global and heavyweight. Use simple conditional logic to keep
> correctness obvious. This will be optimized in a later change.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
One nit below.
[...]
>
> @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> if (ret)
> return ret;
>
> - tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> - if (tdx_status != TDX_SUCCESS) {
> - ret = -EIO;
> - goto out_free;
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
tdx_find_pamt_refcount() is tagged as __maybe_unused in the previous patch.
I think the tag should be removed.
[...]
^ permalink raw reply
* Re: [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory
From: Binbin Wu @ 2026-07-02 7:20 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-5-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> The PAMT memory holds metadata for all possible TDX protected memory. Each
> physical address range is covered by PAMT entries at three levels (1GB,
> 2MB, 4KB). With Dynamic PAMT, the 4KB range of PAMT is allocated on
> demand. The kernel supplies the TDX module with page pairs to store the
> 4KB entries, which cover 2MB of host physical memory. The kernel must
> provide this page pair before using pages from the range for TDX. If this
> is not done, SEAMCALLs that give the pages to be protected by the TDX module
> will fail.
>
> Allocate reference counters for every 2MB range to track TDX memory usage.
> This can be used to handle concurrent get/put callers, in order to
> accurately determine when the dynamic 4KB level of Dynamic PAMT needs to
> be allocated and when it can be freed.
>
> This allocation will currently consume 2 MB for every 1 TB of address
> space from 0 to max_pfn. The allocation size will depend on how the RAM is
> physically laid out. In a worst case scenario where the entire 52-bit
> address space is covered this would be 8GB. Then the DPAMT refcount
> allocations could hypothetically cause the savings from Dynamic PAMT to go
> negative on exotic platforms with sparse, small amounts of memory.
Nit:
Do you think it make sense to print the memory size allocated for
DPAMT refcounts in the kernel log?
>
> Future changes could reduce this refcount overhead to be only allocating
> refcounts for physical ranges that contain memory that TDX can use.
> However, this is left for future work.
>
> Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Sashiko:claude-opus-4-6
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* [PATCH] KVM: x86/tdx: Do not print error message on non-present feature
From: Jiri Slaby (SUSE) @ 2026-07-02 4:32 UTC (permalink / raw)
To: seanjc
Cc: linux-kernel, Jiri Slaby (SUSE), Dan Williams, Chao Gao,
Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, linux-coco, kvm
Before commit 165e77353831 below, the kernel did not print anything when
X86_FEATURE_TDX_HOST_PLATFORM was not present in a cpu.
After the commit, this is emitted to the log, with the error level:
virt/tdx: TDX not supported by the host platform
This is disturbing as it is not an error running CPUs without the
feature.
Drop the error message completely to revert to the state before
165e77353831.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Fixes: 165e77353831 ("KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init")
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Chao Gao <chao.gao@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
--
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kiryl Shutsemau <kas@kernel.org>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-coco@lists.linux.dev
Cc: kvm@vger.kernel.org
---
arch/x86/virt/vmx/tdx/tdx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..bdfc3bf12a9c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1228,10 +1228,8 @@ static __init int tdx_enable(void)
enum cpuhp_state state;
int ret;
- if (!cpu_feature_enabled(X86_FEATURE_TDX_HOST_PLATFORM)) {
- pr_err("TDX not supported by the host platform\n");
+ if (!cpu_feature_enabled(X86_FEATURE_TDX_HOST_PLATFORM))
return -ENODEV;
- }
if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
pr_err("XSAVE is required for TDX\n");
--
2.54.0
^ permalink raw reply related
* Re: [RFC PATCH v5 30/45] x86/virt/tdx: Add API to demote a 2MB mapping to 512 4KB mappings
From: Yan Zhao @ 2026-07-02 3:34 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, Li, Xiaoyao, linux-coco@lists.linux.dev,
Huang, Kai, dave.hansen@linux.intel.com, kas@kernel.org,
seanjc@google.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
mingo@redhat.com, isaku.yamahata@intel.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
tglx@kernel.org, sagis@google.com, bp@alien8.de,
Annapurve, Vishal, x86@kernel.org
In-Reply-To: <212aa1eee0c19fe9cabc1ab8cc2b217f1ee0b7ca.camel@intel.com>
On Wed, Jul 01, 2026 at 12:35:34AM +0800, Edgecombe, Rick P wrote:
> On Tue, 2026-06-30 at 10:36 +0800, Yan Zhao wrote:
> > > 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.
>
> Oops, I thought I deleted it before I sent the mail. After thinking more, it
> seems reasonable. But I didn't check it thoroughly with the other flows yet.
The new wrapper implementation is as below.
I adjusted the pamt_refcount part and made TDX huge page rely on DPAMT.
Will add the following justification for the dependency on DPAMT:
A TDX module supporting non-interruptible DEMOTE usually also supports DPAMT.
Having TDX huge page rely on DPAMT as well simplifies the 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)
{
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 = tdx_find_pamt_refcount(pfn);
struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
u64 ret;
if (!tdx_huge_page_demote_uninterruptible(&tdx_sysinfo) ||
!tdx_supports_dynamic_pamt(&tdx_sysinfo) || level != PG_LEVEL_2M)
return TDX_SW_ERROR;
if (alloc_pamt_array(pamt_pages, pamt_cache))
return TDX_SW_ERROR;
/* Flush the new S-EPT page to be added */
tdx_clflush_page(new_sp);
/* PAMT page pair to track the 2MB guest memory */
args.r12 = page_to_phys(pamt_pages[0]);
args.r13 = page_to_phys(pamt_pages[1]);
spin_lock(&pamt_lock);
/*
* Before demotion, the 2MB guest memory range is tracked in a
* statically allocated pamt 2MB entry, so the pamt_refcount of the 2MB
* range should be 0.
*/
if (atomic_read(pamt_refcount)) {
WARN_ON_ONCE(1);
ret = TDX_SW_ERROR;
goto out;
}
ret = seamcall_saved_ret(TDH_MEM_PAGE_DEMOTE, &args);
if (ret)
goto out;
/* After a sucessful demotion from 2MB to 4KB, set the refcount to 512.
* Subsequent removal of each 4KB mapping will decrease the refcount by
* 1.
*/
atomic_set(pamt_refcount, PTRS_PER_PMD);
out:
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 v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Xiaoyao Li @ 2026-07-02 2:32 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco, linux-kernel,
Sashiko Bot, Joerg Roedel, Yan Zhao, Ackerley Tng
In-Reply-To: <20260630213711.479692-3-seanjc@google.com>
On 7/1/2026 5:37 AM, Sean Christopherson wrote:
> Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
> pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
> consistent between TDX and SNP (for LAUNCH_UPDATE). EOPNOTSUPP was chosen
> to be a forward-looking error code for when guest_memfd supports in-place
> conversion, but even when in-place conversion comes along, it's an awkward
> error code as KVM is deliberately choosing to disallow virtual address '0',
> which is technically a legal userspace address. I.e. it's not so much a
> lack of support as it is that KVM reserves address '0' to simplify KVM's
> internal implementation.
>
> Opportunistically move the check so that it's co-located with the other
> checks on the userspace address, and so that it's more obvious that a NULL
> source address is explicitly disallowed.
>
> Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/vmx/tdx.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ffe9d0db58c5..b0ec054732b9 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -3198,9 +3198,6 @@ 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;
> -
> kvm_tdx->page_add_src = src_page;
> ret = kvm_tdp_mmu_map_private_pfn(arg->vcpu, gfn, pfn);
> kvm_tdx->page_add_src = NULL;
> @@ -3247,8 +3244,8 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
> if (copy_from_user(®ion, u64_to_user_ptr(cmd->data), sizeof(region)))
> return -EFAULT;
>
> - if (!PAGE_ALIGNED(region.source_addr) || !PAGE_ALIGNED(region.gpa) ||
> - !region.nr_pages ||
> + if (!PAGE_ALIGNED(region.source_addr) || !region.source_addr ||
> + !PAGE_ALIGNED(region.gpa) || !region.nr_pages ||
> region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa ||
> !vt_is_tdx_private_gpa(kvm, region.gpa) ||
> !vt_is_tdx_private_gpa(kvm, region.gpa + (region.nr_pages << PAGE_SHIFT) - 1))
^ permalink raw reply
* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Binbin Wu @ 2026-07-02 1:12 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
Yan Zhao, Ackerley Tng
In-Reply-To: <akVKgY6JK9im5aDZ@google.com>
On 7/2/2026 1:12 AM, Sean Christopherson wrote:
> On Wed, Jul 01, 2026, Binbin Wu wrote:
>> On 7/1/2026 5:37 AM, Sean Christopherson wrote:
>>> Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
>>> pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
>>> consistent between TDX and SNP (for LAUNCH_UPDATE). EOPNOTSUPP was chosen
>>> to be a forward-looking error code for when guest_memfd supports in-place
>>> conversion, but even when in-place conversion comes along, it's an awkward
>>> error code as KVM is deliberately choosing to disallow virtual address '0',
>>> which is technically a legal userspace address. I.e. it's not so much a
>>> lack of support as it is that KVM reserves address '0' to simplify KVM's
>>> internal implementation.
>>
>> Nit:
>> Do you think it's worth calling this out in the documentation?
>
> Yes, though that can be done separate since this series doesn't change ABI.
> E.g. we can probably do it opportunistically as part of the in-place conversion
> series?
Yes, make sense.
^ permalink raw reply
* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Alexey Kardashevskiy @ 2026-07-02 1:04 UTC (permalink / raw)
To: Jason Gunthorpe, Aneesh Kumar K.V
Cc: 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, Lendacky, Thomas
In-Reply-To: <20260630174216.GK7525@ziepe.ca>
On 1/7/26 03:42, Jason Gunthorpe wrote:
> On Mon, Jun 29, 2026 at 12:16:30PM +0530, Aneesh Kumar K.V wrote:
>>>> Thinking about this more, I guess we should mark the swiotlb as
>>>> cc_shared only with CC_ATTR_GUEST_MEM_ENCRYPT instead of
>>>> CC_ATTR_MEM_ENCRYPT as we have below.
>>>
>>> The name cc_shared should be used for GUEST scenarios only.
>>>
>>> I guess there is some merit in keeping swiotlb using "decrypted" to
>>> mean it usinig pgprot_decrypted and set_memory_decyped() which AMD
>>> gives meaning to on both host and guest.
>>
>> Are you suggesting to change the struct io_tlb_mem::cc_shared back to
>> struct io_tlb_mem::unencrypted?.
>
> Yes
>
>>> IDK what AMD should do on the host by default. I guess it should setup
>>> a swiotlb pool of low dma addrs "unencrypted", but not "cc_shared"?
>>>
>>
>> If by low DMA address you mean using an address with the C-bit
>> cleared.
>
> Yes
>
>> The current code already does this and uses the swiotlb pool correctly
>> on SME.
>
> Well, through the force_dma_unencrypted() hack...
>
>> 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.
>
> Yes, because cc_shared (guest) and unencrypted (host) are very
> different things and we've mixed them:
>
>> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>
> I'm aruging force_dma_unencrypted should mean cc_shared and be
> guest_only, but the SME hack breaks this.
>
>> 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;
>> }
>
> If we do this I would like to split the force_dma_.. functions into
> guest and host, ie force_dma_cc_shared() and force_host_decrypted()
imho force_dma_unencrypted() should not look at the mask at all (the mask should tell the DMA layer to use swiotlb, encrypted or not), instead, when we set up swiotlb - we could make it unencrypted if iommu=pt, otherwise encrypted (although this means IOMMU and defeats the purpose of swiotlb). But at least this patchset has enough plumbing to have swiotlb encrypted, right?
> To make it clear there are two very different things here.
>
>> 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.
>
> The external flag name should be DMA_ATTR_CC_SHARED and only used on
> CC guest. Internally that turns into using set_memory_decrypted()
> which works on guest and host for AMD. I don't know how to make the
> host only case clearer and still keep the code efficient..
>
>>> The dma api has to detect, after the driver sets the dma limit, that
>>> none of system memory is usable when:
>>> - The direct path is being used
>>> - phys to dma for 0 is outside the dma limit
>>>
>>> Then it should assume the arch has setup a swiotlb pool for it to use
>>> to fix the high memory problem.
>>>
>>> Similar hackery would be needed in the dma alloc path to know that
>>> decrypted can be used to fix the high memory problem like for GUEST.
>>>
>>> I guess some 'dev_cannot_reach_memory(dev)' sort of test in a
>>> few key places? Setup with a static branch to be a nop on everything
>>> but AMD, compiled out on every other arch.
>>>
>>
>> If we are not able to reach the memory because of the memory encryption
>> bit, then isn't dev_cannot_reach_memory(dev) the same as
>> force_dma_unencrypted(dev)? If so, that is how it is already done.
>
> Sort of yes, but it is properly named to its purpose and not confused
> with what should be a guest-only function.
>
>> x86/dma: Disable forced SWIOTLB bouncing for SME IOMMU passthrough
>
> Maybe as a crutch to get this series merged..
feels okay but I do not really know the true meaning of "swiotlb=force" so adding Tom to the thread. Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Alexey Kardashevskiy @ 2026-07-02 0:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
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, Jiri Pirko,
Michael Kelley, Cheloha, Scott
In-Reply-To: <20260630160242.GI7525@ziepe.ca>
On 1/7/26 02:02, Jason Gunthorpe wrote:
> On Thu, Jun 18, 2026 at 12:39:21PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 18/6/26 01:41, Jason Gunthorpe wrote:
>>> On Wed, Jun 17, 2026 at 10:50:39AM +1000, Alexey Kardashevskiy wrote:
>>>>> @@ -193,16 +193,31 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>>>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>>>>> {
>>>>> bool remap = false, set_uncached = false;
>>>>> - bool mark_mem_decrypt = true;
>>>>> + bool mark_mem_decrypt = false;
>>>>> struct page *page;
>>>>> void *ret;
>>>>> + /*
>>>>> + * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
>>>>> + * attribute. The direct allocator uses it internally after it has
>>>>> + * decided that the backing pages must be shared/decrypted, so the
>>>>> + * rest of the allocation path can consistently select DMA addresses,
>>>>> + * choose compatible pools and restore encryption on free.
>>>>
>>>> Why this limit?
>>>>
>>>> Context: I am looking for a memory pool for a few shared pages (to
>>>> do some guest<->host communication), SWIOTLB seems like the right
>>>> fit but swiotlb_alloc() is not exported and
>>>> dma_direct_alloc(DMA_ATTR_CC_SHARED) is not allowed. Thanks,
>>>
>>> Then setup your struct device so that the DMA API knows the
>>> guest<->host channel requires unecrypted and it will work correctly.
>>>
>>> I think this is a reasonable API to use for that, and I was just
>>> advocating that hyperv should be using it too.
>>>
>>> But it all relies on a properly setup struct device.
>>
>> Sounds good but how do I do that in practice?
>
> I think we haven't got there yet, I understood Dan's plan was to add a
> bit in the struct device that signals if the device must be
> unencrypted or can support all memory.
>
> Currently the dma api assumes all devices must have unencrypted by
> default so it should be fine already, shouldn't it?
It assumes that DMA returns a handle for shared memory but the state of that memory is not guaranteed beforehand. I dislike 2M page mashing.
>> not externally available so I'll have to trick the DMA layer into
>> using SWIOTLB (which is still all shared, right?) as I specifically
>> want to skip page conversions. Setting low DMA mask won't guarantee
>> that the DMA layer won't allocate a page outside of SWIOTLB and
>> convert it. Manually do
>
> Why so particular?
aahhh I missed "pre-". I need a way to get pre-shared pages for my sev-guest activities.
> Any address that satisifies the constraints should
> be good enough?
Any address can be already shared (from swiotlb) or not (from outside swiotlb). The latter will may trigger 2M page smashing and this is what I want to avoid. Makes better sense now? Thanks,
--
Alexey
^ permalink raw reply
* Re: [RFC PATCH 08/15] x86/virt/tdx: Add interface to check Quoting availability
From: Peter Fang @ 2026-07-02 0:19 UTC (permalink / raw)
To: Nikolay Borisov
Cc: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <3cf04184-c34a-4d49-926f-5791f12cd74e@suse.com>
On Wed, Jul 01, 2026 at 02:25:05PM +0300, Nikolay Borisov wrote:
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 9d04293394d7..b305fa5aab5c 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1213,6 +1213,21 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
> > return page_to_phys(td->tdr_page);
> > }
> > +/**
> > + * tdx_quote_enabled() - Check whether TDX Quoting extension is available
> > + *
> > + * Return: %true if the Quoting extension is available, otherwise %false.
> > + */
> > +bool tdx_quote_enabled(void)
>
> nit: Probably rename the function to tdx_quoting_ext_enabled or
> tdx_quote_ext_enabled, so it's abundantly clear it's about an extension and
> not the quoting functionality in general.
Makes sense to me. I'll fix that. Thanks.
>
>
> > +{
> > + /*
> > + * No need for locking here. The quote buffer is initialized as part of
> > + * core TDX bringup, which comes before KVM is ready for userspace.
> > + */
> > + return !!quote_data.buf;
>
> While this works it feels a bit like a hack, perhaps have a static boolean
> variable being set by the init code which is simply returned by this
> function.
I'll fix this as well.
>
> > +}
> > +EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
> > +
> > #define HPAS_PER_PAGE (PAGE_SIZE / sizeof(u64))
> > static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)
>
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Dave Hansen @ 2026-07-01 19:56 UTC (permalink / raw)
To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
Cc: 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, seanjc
In-Reply-To: <20260618081355.3253581-9-yilun.xu@linux.intel.com>
On 6/18/26 01:13, Xu Yilun wrote:
> 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.
va-to-pa conversions are not expensive. Even for vmalloc() memory.If
I don't like the global buffer. It just generally seems like a bad idea.
It needs locking, it wastes memory when nobody is doing quotes, etc..
Is there a way to do this without vmalloc()?
Could userspace provide the memory and then the kernel just does a gup
to keep it in place while the TDX module is writing to it? Or, could it
just be allocated:
void *buffer = vmalloc(size);
mutex_lock();
// talk to TDX module
mutex_unlock();
vfree(buffer);
return ret;
That actually has some nice properties. It doesn't require locking for
the buffer. It also penalizes folks doing lots of quotes. If you hammer
on quoting, you hammer on the memory allocator and TLB flushing code and
slow yourself down more. If you hammer on it from multiple threads, it
gets even worse.
That seems like a feature not a bug.
^ permalink raw reply
* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-01 21:25 UTC (permalink / raw)
To: Jethro Beekman, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, 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: <80f3f279-d70e-44d7-a179-c52068115e46@fortanix.com>
On 7/1/2026 4:40 AM, Jethro Beekman wrote:
> Hi Ashish,
>
> I don't believe my concern has been addressed
>
> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/
>
> --
The disable tracks SNP_INIT, not "SNP" in general: SNP_INIT requires SnpEn to be set on all present CPUs, and a CPU brought online afterward wouldn't have it, so the kernel that runs SNP_INIT must keep its CPU set stable. Today the only kernel that runs SNP_INIT is the bare-metal host, so a plain L1 guest keeps full CPU hotplug.
Concretely, the path is gated by CC_ATTR_HOST_SEV_SNP, which bsp_determine_snp() sets only when X86_FEATURE_HYPERVISOR is clear and clears otherwise
(as Prateek pointed out). So a Linux L1 guest never has it set, never reaches snp_prepare()/snp_rmptable_init(), and keeps CPU hotplug —
including while running SEV/SEV-ES confidential L2 guests. Only SNP initialization disables hotplug; the other SEV variants don't. And KVM doesn't expose
SNP to L1, so an L1 can't be an SNP host today in any case.
On the nested scenario you raised: if SNP-guest-as-L2 support is added, an L1 acting as an SNP host would run a *virtualized* SNP_INIT. A faithful virtualization carries the same constraint as physical SNP_INIT — all present (v)CPUs must be SnpEn — so that L1 would have the same (v)CPU-hotplug-disable requirement, just over its virtual CPUs, and this same code would apply at that level. So the disable isn't too broad; it correctly tracks SNP_INIT. It simply doesn't apply to a plain L1 guest today, because such a guest isn't running SNP_INIT.
Thanks,
Ashish
> Jethro Beekman | CTO | Fortanix
>
> On 2026-06-30 20:11, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> While SNP is active, every memory write is checked against the RMP to
>> protect SEV-SNP guest memory. A core performs these RMP checks only once
>> SNP has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is
>> set on that core; the firmware requires the SNP-enable bit to be set on
>> every present CPU before SNP initialization. A core that is not
>> SNP-enabled and not SNP-initialized 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. SNP_INIT fails unless SnpEn is set on all CPUs, so a
>> CPU that is offline at SNP init does not have SnpEn set, SNP_INIT fails,
>> and there can be no SNP guest memory. OS CPU hotplug can thus diverge
>> from the firmware's expectations and break SNP.
>>
>> Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
>> SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
>> disabled SNP. If snp_prepare() fails before enabling SNP it re-enables
>> hotplug itself; once SNP is enabled hotplug stays disabled, including
>> across a failed SNP_INIT and across the legacy SNP_SHUTDOWN_EX path, both
>> of which leave SNP enabled. A kexec target that boots with SNP already
>> enabled disables hotplug once in snp_rmptable_init(), since snp_prepare()
>> bails when SNP is already enabled.
>>
>> This also keeps the CPU set stable for the asynchronous RMPOPT scan added
>> later in this series, and ensures cpus_read_lock() in the scan is
>> uncontended.
>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index dab6e1c290bc..04a58ac4339c 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -535,6 +535,15 @@ int snp_prepare(void)
>>
>> clear_rmp();
>>
>> + /*
>> + * Disable CPU hotplug before enabling SNP, so no CPU can come online
>> + * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
>> + * once SNP is disabled. Must be before cpus_read_lock():
>> + * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
>> + * cpu_hotplug_lock.
>> + */
>> + cpu_hotplug_disable();
>> +
>> cpus_read_lock();
>>
>> if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
>> @@ -560,6 +569,10 @@ int snp_prepare(void)
>> unlock:
>> cpus_read_unlock();
>>
>> + /* Re-enable CPU hotplug; SnpEn was never set. */
>> + if (ret)
>> + cpu_hotplug_enable();
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>> @@ -587,6 +600,13 @@ void snp_shutdown(void)
>>
>> rmpopt_cleanup();
>>
>> + /*
>> + * Re-enable CPU hotplug now that the firmware has disabled SNP; CPU
>> + * hotplug is not re-enabled for a legacy SNP shutdown. After
>> + * rmpopt_cleanup() so RMPOPT_BASE is cleared with hotplug still disabled.
>> + */
>> + cpu_hotplug_enable();
>> +
>> clear_rmp();
>> on_each_cpu(mfd_reconfigure, NULL, 1);
>> }
>> @@ -645,6 +665,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
>> */
>> int __init snp_rmptable_init(void)
>> {
>> + u64 val;
>> +
>> if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>> return -ENOSYS;
>>
>> @@ -654,6 +676,15 @@ int __init snp_rmptable_init(void)
>> if (!setup_rmptable())
>> return -ENOSYS;
>>
>> + /*
>> + * On a kexec boot SNP may already be enabled (legacy firmware leaves
>> + * SnpEn set across shutdown), in which case snp_prepare() bails without
>> + * disabling CPU hotplug, so disable it here.
>> + */
>> + rdmsrq(MSR_AMD64_SYSCFG, val);
>> + if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> + cpu_hotplug_disable();
>> +
>> /*
>> * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>> * notifier is invoked to do SNP IOMMU shutdown before kdump.
>
^ permalink raw reply
* Re: [PATCH v2 1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE
From: Sean Christopherson @ 2026-07-01 21:22 UTC (permalink / raw)
To: Ackerley Tng
Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
Yan Zhao
In-Reply-To: <CAEvNRgEK+LH+M32LrxFSbN3Nf9sYtAVkr2c2vYq5FHzJmJqL7g@mail.gmail.com>
On Wed, Jul 01, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 74fb15551e83..621a2eaa58f2 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > int level;
> > int ret;
> >
> > - if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> > - return -EINVAL;
> > -
> > ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
> > if (ret || assigned) {
> > pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
> > @@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
> > return -EINVAL;
> >
> > - src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
> > -
> > - if (!PAGE_ALIGNED(src))
> > + if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
> > + src = NULL;
> > + else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
> > return -EINVAL;
> > + else
> > + src = u64_to_user_ptr(params.uaddr);
> >
>
> I think separating validation from src assignment logic is better, like
>
> if (!PAGE_ALIGNED(params.uaddr))
> return -EINVAL;
>
> if (!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
> return -EINVAL;
>
> src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
> u64_to_user_ptr(params.uaddr);
>
> This version is also slightly stricter, since it enforces that
> params.uaddr is aligned even if params.type is ZERO.
That's technically an ABI change since KVM fully ignored params.uaddr before
this, i.e. is a potentially breaking change. As noted in the changelog, KVM
should have required the address to be '0', but unfortunately we can't do that
without breaking userspace.
I highly doubt requiring the address to be page-aligned would break userspace,
but KVM also gains nothing from such a requirement, and from an ABI perspective,
placing restrictions on a value that is completely ignored is rather bizarre.
> (unless the intention is to really not care about uaddr and permit an
> unaligned uaddr?)
I don't think there was any intention (which is the whole problem).
^ permalink raw reply
* Re: [PATCH v2 1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE
From: Ackerley Tng @ 2026-07-01 21:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco, linux-kernel,
Sashiko Bot, Joerg Roedel, Yan Zhao
In-Reply-To: <20260630213711.479692-2-seanjc@google.com>
Sean Christopherson <seanjc@google.com> writes:
> From: Joerg Roedel <joerg.roedel@amd.com>
>
> Explicitly reject a NULL userspace virtual address for the source page of
> SNP_LAUNCH_UPDATE instead of relying on the post-populate callback to do
> the check, and don't WARN on failure, as the scenario is blatantly user-
> triggerable, as reported by Sashiko. Waiting until post-populate to check
> the address "works", but makes it unnecessarily difficult to see that KVM's
> ABI is to disallow a NULL source page for non-ZERO pages.
>
> Note, several existing VMMs pass a valid userspace address for the ZERO
> case, i.e. KVM can't *require* the userspace address to be NULL for ZERO
> pages, at least not without breaking userspace.
>
> Fixes: dee5a47cc7a4 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260611125849.9ED631F00893@smtp.kernel.org
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/sev.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74fb15551e83..621a2eaa58f2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> int level;
> int ret;
>
> - if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> - return -EINVAL;
> -
> ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
> if (ret || assigned) {
> pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
> @@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
> return -EINVAL;
>
> - src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
> -
> - if (!PAGE_ALIGNED(src))
> + if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
> + src = NULL;
> + else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
> return -EINVAL;
> + else
> + src = u64_to_user_ptr(params.uaddr);
>
I think separating validation from src assignment logic is better, like
if (!PAGE_ALIGNED(params.uaddr))
return -EINVAL;
if (!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
return -EINVAL;
src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
u64_to_user_ptr(params.uaddr);
This version is also slightly stricter, since it enforces that
params.uaddr is aligned even if params.type is ZERO. (unless the
intention is to really not care about uaddr and permit an unaligned
uaddr?)
And then with conversions I'll add
if (!gmem_in_place_conversion &&
!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
Just a suggestion:
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> npages = params.len / PAGE_SIZE;
>
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply
* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-01 21:08 UTC (permalink / raw)
To: K Prateek Nayak, Jethro Beekman, tglx, mingo, bp, dave.hansen,
x86, hpa, seanjc, peterz, thomas.lendacky, 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: <8477525d-55ad-4fc4-b7c6-05bab3d7a861@amd.com>
Hi Prateek,
On 7/1/2026 11:39 AM, K Prateek Nayak wrote:
> Hello Jethro,
>
> On 7/1/2026 3:10 PM, Jethro Beekman wrote:
>> I don't believe my concern has been addressed
>>
>> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/
>
> Quoting your question:
>
>> I think this is too broad. If I have a hypervisor that supports SNP
>> virtualization, a (non-confidential) L1 guest running Linux should
>> still support CPU hotplug while also running confidential L2 guests.
>
> Ashish, Tom, correct me if I'm wrong, but I don't think KVM exposes SNP
> support to L1, at least as per
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/cpuid.c?h=v7.2-rc1#n1221
> and only SNP initialization disables hotplug - not the other variants.
>
> L1, running a confidential guest (SEV/SEV-ES) should still be able to
> support hotplug since it doesn't go through SNP init. Only the base
> hypervisor can setup the RMP tables and go through snp_prepare().
>
> Also bsp_determine_snp() should clear CC_ATTR_HOST_SEV_SNP if it
> detects X86_FEATURE_HYPERVISOR so I don't see how this can be a
> problem for hotplug in L1.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/amd.c?h=v7.2-rc1#n368
>
bsp_determine_snp() only sets CC_ATTR_HOST_SEV_SNP when X86_FEATURE_HYPERVISOR is clear:
if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
(ZEN3 || ZEN4 || RMPREAD) && snp_probe_rmptable_info())
cc_platform_set(CC_ATTR_HOST_SEV_SNP);
else {
setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
}
So Linux running as an L1 guest (HYPERVISOR set) never has CC_ATTR_HOST_SEV_SNP.
And both hotplug-disable sites sit behind that flag:
- snp_prepare() is only called from __sev_snp_init_locked(), which returns -ENODEV early if !cc_platform_has(CC_ATTR_HOST_SEV_SNP).
- snp_rmptable_init() bails (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP))) before its kexec one-shot disable.
So an L1 guest can't reach the disable at all; only the bare-metal host that programs the RMP does.
An L1 running SEV/SEV-ES guests never goes through SNP host init, so it's hotplug is unaffected and KVM doesn't expose SNP to L1.
So there's no impact on L1 hotplug currently.
Thanks,
Ashish
^ permalink raw reply
* [PATCH v5 51/51] x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
When running as a KVM guest with PV timing info provided by the host,
stuff the APIC timer period/frequency with the local APIC bus frequency
reported in CPUID.0x40000010.EBX instead of trying to calibrate/guess the
frequency.
See Documentation/virt/kvm/x86/cpuid.rst for details.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index f9a6346077b0..beea0b6aa78e 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -990,7 +990,7 @@ static void __init kvm_init_platform(void)
.mask_lo = (u32)(~(SZ_4G - tolud - 1)) | MTRR_PHYSMASK_V,
.mask_hi = (BIT_ULL(boot_cpu_data.x86_phys_bits) - 1) >> 32,
};
- u32 timing_info_leaf;
+ u32 timing_info_leaf, apic_khz;
bool tsc_is_reliable;
if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
@@ -1052,6 +1052,11 @@ static void __init kvm_init_platform(void)
x86_init.hyper.get_tsc_khz = kvm_get_tsc_khz;
x86_init.hyper.get_cpu_khz = kvm_get_tsc_khz;
}
+
+ /* The leaf also includes the local APIC bus/timer frequency.*/
+ apic_khz = cpuid_ebx(timing_info_leaf);
+ if (apic_khz)
+ apic_set_timer_period_khz(apic_khz, "KVM hypervisor");
}
/*
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 50/51] x86/paravirt: Move using_native_sched_clock() stub into timer.h
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Now that timer.h ended up with CONFIG_PARAVIRT #ifdeffery anyways, move the
PARAVIRT=n using_native_sched_clock() stub into timer.h as a "free"
optimization.
No functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 6 ++++--
arch/x86/kernel/tsc.c | 2 --
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index ca5c95d48c03..a52388af6055 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -11,9 +11,9 @@ extern void recalibrate_cpu_khz(void);
extern int no_timer_check;
-extern bool using_native_sched_clock(void);
-
#ifdef CONFIG_PARAVIRT
+extern bool using_native_sched_clock(void);
+
int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
void (*save)(void), void (*restore)(void));
@@ -23,6 +23,8 @@ static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
{
(void)__paravirt_set_sched_clock(func, true, save, restore);
}
+#else
+static inline bool using_native_sched_clock(void) { return true; }
#endif
/*
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index a146fc7b5e74..564be4faa5a0 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -302,8 +302,6 @@ int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
-
-bool using_native_sched_clock(void) { return true; }
#endif
notrace u64 sched_clock(void)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 49/51] x86/kvmclock: Plumb in AP-online and BSP-resume to kvmlock, for documentation
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Invoke kvmclock_cpu_action() with AP_ONLINE and BSP_RESUME, even though
kvmclock doesn't need to do anything in either case, so that the asymmetry
of kvmclock is a detail buried in kvmclock, and to explicitly document
that doing nothing during those phases is intentional and correct.
For all intents and purposes, no functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_para.h | 2 ++
arch/x86/kernel/kvm.c | 22 +++++++++++++-------
arch/x86/kernel/kvmclock.c | 37 ++++++++++++++++++++++++++-------
3 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 08686ff19caa..763ed017738a 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -120,6 +120,8 @@ static inline long kvm_sev_hypercall3(unsigned int nr, unsigned long p1,
#ifdef CONFIG_KVM_GUEST
enum kvm_guest_cpu_action {
KVM_GUEST_BSP_SUSPEND,
+ KVM_GUEST_BSP_RESUME,
+ KVM_GUEST_AP_ONLINE,
KVM_GUEST_AP_OFFLINE,
KVM_GUEST_SHUTDOWN,
};
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 604b52f233aa..f9a6346077b0 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -474,18 +474,24 @@ static void kvm_guest_cpu_offline(enum kvm_guest_cpu_action action)
kvmclock_cpu_action(action);
}
+static void __kvm_cpu_online(unsigned int cpu, enum kvm_guest_cpu_action action)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ kvmclock_cpu_action(action);
+ kvm_guest_cpu_init();
+ local_irq_restore(flags);
+}
+
+#ifdef CONFIG_SMP
+
static int kvm_cpu_online(unsigned int cpu)
{
- unsigned long flags;
-
- local_irq_save(flags);
- kvm_guest_cpu_init();
- local_irq_restore(flags);
+ __kvm_cpu_online(cpu, KVM_GUEST_AP_ONLINE);
return 0;
}
-#ifdef CONFIG_SMP
-
static DEFINE_PER_CPU(cpumask_var_t, __pv_cpu_mask);
static bool pv_tlb_flush_supported(void)
@@ -750,7 +756,7 @@ static int kvm_suspend(void *data)
static void kvm_resume(void *data)
{
- kvm_cpu_online(raw_smp_processor_id());
+ __kvm_cpu_online(raw_smp_processor_id(), KVM_GUEST_BSP_RESUME);
#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL) && has_guest_poll)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index bc98ebb8587d..842f38c5f6ca 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -150,7 +150,7 @@ static void kvm_save_sched_clock_state(void)
#ifdef CONFIG_SMP
static void kvm_setup_secondary_clock(void)
{
- kvm_register_clock("secondary cpu clock");
+ kvm_register_clock("secondary cpu, startup");
}
#endif
@@ -174,13 +174,34 @@ static void kvmclock_resume(struct clocksource *cs)
void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
{
- /*
- * Don't disable kvmclock on the BSP during suspend. If kvmclock is
- * being used for sched_clock, then it needs to be kept alive until the
- * last minute, and restored as quickly as possible after resume.
- */
- if (action != KVM_GUEST_BSP_SUSPEND)
+ switch (action) {
+ /*
+ * The BSP's clock is managed via clocksource suspend/resume,
+ * to ensure it's enabled/disabled when timekeeping needs it
+ * to be, e.g. before reading wallclock (which uses kvmclock).
+ */
+ case KVM_GUEST_BSP_SUSPEND:
+ case KVM_GUEST_BSP_RESUME:
+ break;
+ case KVM_GUEST_AP_ONLINE:
+ /*
+ * Secondary CPUs use a dedicated hook to enable kvmclock early
+ * during bringup, there's nothing to be done during CPU online
+ * (which runs at CPUHP_AP_ONLINE_DYN). When kvmclock is being
+ * used as sched_clock, kvmclock must be enabled *very* early,
+ * and even when kvmclock is "only" being used for the main
+ * clocksource, it still needs to be enabled long before the
+ * dynamic CPUHP calls are made.
+ */
+ break;
+ case KVM_GUEST_AP_OFFLINE:
+ case KVM_GUEST_SHUTDOWN:
kvmclock_disable();
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ break;
+ }
}
/*
@@ -382,7 +403,7 @@ void __init kvmclock_init(bool prefer_tsc)
msr_kvm_system_time, msr_kvm_wall_clock);
this_cpu_write(hv_clock_per_cpu, &hv_clock_boot[0]);
- kvm_register_clock("primary cpu clock");
+ kvm_register_clock("primary cpu, online");
pvclock_set_pvti_cpu0_va(hv_clock_boot);
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT)) {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 48/51] x86/kvmclock: Use TSC for sched_clock if it's constant and non-stop
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Prefer the TSC over kvmclock for sched_clock if the TSC is constant and
nonstop. I.e. use the same criteria as tweaking the clocksource rating so
that TSC is preferred over kvmclock. Per the below comment from
native_sched_clock(), sched_clock is more tolerant of slop than
clocksource; using TSC for clocksource but not sched_clock makes little to
no sense, especially now that KVM CoCo guests with a trusted TSC use TSC,
not kvmclock.
/*
* Fall back to jiffies if there's no TSC available:
* ( But note that we still use it if the TSC is marked
* unstable. We do this because unlike Time Of Day,
* the scheduler clock tolerates small errors and it's
* very important for it to be as fast as the platform
* can achieve it. )
*/
The only advantage of using kvmclock is that doing so allows for early
and common detection of PVCLOCK_GUEST_STOPPED, but that code has been
broken for over two years with nary a complaint, i.e. it can't be
_that_ valuable. And as above, certain types of KVM guests are losing
the functionality regardless, i.e. acknowledging PVCLOCK_GUEST_STOPPED
needs to be decoupled from sched_clock() no matter what.
Link: https://lore.kernel.org/all/Z4hDK27OV7wK572A@google.com
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 22e8855fcd4d..bc98ebb8587d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -396,7 +396,6 @@ void __init kvmclock_init(bool prefer_tsc)
PVCLOCK_TSC_STABLE_BIT;
}
- kvm_sched_clock_init(stable);
if (!x86_init.hyper.get_tsc_khz)
x86_init.hyper.get_tsc_khz = kvmclock_get_tsc_khz;
@@ -416,6 +415,8 @@ void __init kvmclock_init(bool prefer_tsc)
*/
if (prefer_tsc)
kvm_clock.rating = 299;
+ else
+ kvm_sched_clock_init(stable);
clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
pv_info.name = "KVM";
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Silently ignore attempts to switch to a paravirt sched_clock when running
as a CoCo guest with trusted TSC. In hand-wavy theory, a misbehaving
hypervisor could attack the guest by manipulating the PV clock to affect
guest scheduling in some weird and/or predictable way. More importantly,
reading TSC on such platforms is faster than any PV clock, and sched_clock
is all about speed.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/tsc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 012321fed5e5..a146fc7b5e74 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
void (*save)(void), void (*restore)(void))
{
+ /*
+ * Don't replace TSC with a PV clock when running as a CoCo guest and
+ * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
+ * which isn't in the guest's TCB.
+ */
+ if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
+ boot_cpu_has(X86_FEATURE_TDX_GUEST))
+ return -EPERM;
+
if (!stable)
clear_sched_clock_stable();
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 46/51] x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Add a return code to __paravirt_set_sched_clock() so that the kernel can
reject attempts to use a PV sched_clock without breaking the caller. E.g.
when running as a CoCo VM with a secure TSC, using a PV clock is generally
undesirable.
Note, kvmclock is the only PV clock that does anything "extra" beyond
simply registering itself as sched_clock, i.e. is the only caller that
needs to check the new return value.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 6 +++---
arch/x86/kernel/kvmclock.c | 9 ++++++---
arch/x86/kernel/tsc.c | 5 +++--
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index 96ae7feac47c..ca5c95d48c03 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -14,14 +14,14 @@ extern int no_timer_check;
extern bool using_native_sched_clock(void);
#ifdef CONFIG_PARAVIRT
-void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void));
+int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void));
static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
void (*save)(void),
void (*restore)(void))
{
- __paravirt_set_sched_clock(func, true, save, restore);
+ (void)__paravirt_set_sched_clock(func, true, save, restore);
}
#endif
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 2cc3dd2ba355..22e8855fcd4d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -332,10 +332,13 @@ static int kvmclock_setup_percpu(unsigned int cpu)
static __init void kvm_sched_clock_init(bool stable)
{
+ /* Ensure the offset is configured before making kvmclock visible! */
kvm_sched_clock_offset = kvm_clock_read();
- __paravirt_set_sched_clock(kvm_sched_clock_read, stable,
- kvm_save_sched_clock_state,
- kvm_restore_sched_clock_state);
+
+ if (__paravirt_set_sched_clock(kvm_sched_clock_read, stable,
+ kvm_save_sched_clock_state,
+ kvm_restore_sched_clock_state))
+ return;
/*
* The BSP's clock is managed via dedicated sched_clock save/restore
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 0f92b29adecc..012321fed5e5 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -280,8 +280,8 @@ bool using_native_sched_clock(void)
return static_call_query(pv_sched_clock) == native_sched_clock;
}
-void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void))
+int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void))
{
if (!stable)
clear_sched_clock_stable();
@@ -289,6 +289,7 @@ void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
static_call_update(pv_sched_clock, func);
x86_platform.save_sched_clock_state = save;
x86_platform.restore_sched_clock_state = restore;
+ return 0;
}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 45/51] x86/paravirt: Mark __paravirt_set_sched_clock() as __init
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Annotate __paravirt_set_sched_clock() as __init, and make its wrapper
__always_inline to ensure sanitizers don't result in a non-inline version
hanging around. All callers run during __init, and changing sched_clock
after boot would be all kinds of crazy.
No functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 10 +++++-----
arch/x86/kernel/tsc.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index e97cd1ae03d1..96ae7feac47c 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -14,12 +14,12 @@ extern int no_timer_check;
extern bool using_native_sched_clock(void);
#ifdef CONFIG_PARAVIRT
-void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void));
+void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void));
-static inline void paravirt_set_sched_clock(u64 (*func)(void),
- void (*save)(void),
- void (*restore)(void))
+static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
+ void (*save)(void),
+ void (*restore)(void))
{
__paravirt_set_sched_clock(func, true, save, restore);
}
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 83353d643150..0f92b29adecc 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -280,8 +280,8 @@ bool using_native_sched_clock(void)
return static_call_query(pv_sched_clock) == native_sched_clock;
}
-void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void))
+void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void))
{
if (!stable)
clear_sched_clock_stable();
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox