* Re: [PATCH v5 2/2] dma-buf: heaps: system: add system_cc_shared heap for explicitly shared memory
From: Maxime Ripard @ 2026-04-02 12:23 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260325192352.437608-3-jiri@resnulli.us>
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
Hi Jiri,
On Wed, Mar 25, 2026 at 08:23:52PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Add a new "system_cc_shared" dma-buf heap to allow userspace to
> allocate shared (decrypted) memory for confidential computing (CoCo)
> VMs.
>
> On CoCo VMs, guest memory is private by default. The hardware uses an
> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
> Intel TDX) to control whether a given memory access is private or
> shared. The kernel's direct map is set up as private,
> so pages returned by alloc_pages() are private in the direct map
> by default. To make this memory usable for devices that do not support
> DMA to private memory (no TDISP support), it has to be explicitly
> shared. A couple of things are needed to properly handle
> shared memory for the dma-buf use case:
>
> - set_memory_decrypted() on the direct map after allocation:
> Besides clearing the encryption bit in the direct map PTEs, this
> also notifies the hypervisor about the page state change. On free,
> the inverse set_memory_encrypted() must be called before returning
> pages to the allocator. If re-encryption fails, pages
> are intentionally leaked to prevent shared memory from being
> reused as private.
>
> - pgprot_decrypted() for userspace and kernel virtual mappings:
> Any new mapping of the shared pages, be it to userspace via
> mmap or to kernel vmalloc space via vmap, creates PTEs independent
> of the direct map. These must also have the encryption bit cleared,
> otherwise accesses through them would see encrypted (garbage) data.
>
> - DMA_ATTR_CC_SHARED for DMA mapping:
> Since the pages are already shared, the DMA API needs to be
> informed via DMA_ATTR_CC_SHARED so it can map them correctly
> as unencrypted for device access.
>
> On non-CoCo VMs, the system_cc_shared heap is not registered
> to prevent misuse by userspace that does not understand
> the security implications of explicitly shared memory.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
I'm a bit late to the party, sorry.
This new heap must be documented in
Documentation/userspace-api/dma-buf-heaps.rst, but (and especially since
it seems like it was merged already) it can be done as a follow-up
patch.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/2] dma-buf: heaps: system: add system_cc_shared heap for explicitly shared memory
From: Jiri Pirko @ 2026-04-02 12:56 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260402-discreet-glossy-perch-bda4f9@houat>
Thu, Apr 02, 2026 at 02:23:12PM +0200, mripard@redhat.com wrote:
>Hi Jiri,
>
>On Wed, Mar 25, 2026 at 08:23:52PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Add a new "system_cc_shared" dma-buf heap to allow userspace to
>> allocate shared (decrypted) memory for confidential computing (CoCo)
>> VMs.
>>
>> On CoCo VMs, guest memory is private by default. The hardware uses an
>> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
>> Intel TDX) to control whether a given memory access is private or
>> shared. The kernel's direct map is set up as private,
>> so pages returned by alloc_pages() are private in the direct map
>> by default. To make this memory usable for devices that do not support
>> DMA to private memory (no TDISP support), it has to be explicitly
>> shared. A couple of things are needed to properly handle
>> shared memory for the dma-buf use case:
>>
>> - set_memory_decrypted() on the direct map after allocation:
>> Besides clearing the encryption bit in the direct map PTEs, this
>> also notifies the hypervisor about the page state change. On free,
>> the inverse set_memory_encrypted() must be called before returning
>> pages to the allocator. If re-encryption fails, pages
>> are intentionally leaked to prevent shared memory from being
>> reused as private.
>>
>> - pgprot_decrypted() for userspace and kernel virtual mappings:
>> Any new mapping of the shared pages, be it to userspace via
>> mmap or to kernel vmalloc space via vmap, creates PTEs independent
>> of the direct map. These must also have the encryption bit cleared,
>> otherwise accesses through them would see encrypted (garbage) data.
>>
>> - DMA_ATTR_CC_SHARED for DMA mapping:
>> Since the pages are already shared, the DMA API needs to be
>> informed via DMA_ATTR_CC_SHARED so it can map them correctly
>> as unencrypted for device access.
>>
>> On non-CoCo VMs, the system_cc_shared heap is not registered
>> to prevent misuse by userspace that does not understand
>> the security implications of explicitly shared memory.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>I'm a bit late to the party, sorry.
>
>This new heap must be documented in
>Documentation/userspace-api/dma-buf-heaps.rst, but (and especially since
>it seems like it was merged already) it can be done as a follow-up
>patch.
Okay, will send a follow-up. Thanks!
>
>Maxime
^ permalink raw reply
* Re: [PATCH v5 0/2] dma-buf: heaps: system: add an option to allocate explicitly shared/decrypted memory
From: Jiri Pirko @ 2026-04-02 12:58 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Brian Starkey, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, jstultz, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco, nd
In-Reply-To: <20260402120254.GB2551565@ziepe.ca>
Thu, Apr 02, 2026 at 02:02:54PM +0200, jgg@ziepe.ca wrote:
>On Thu, Apr 02, 2026 at 10:52:34AM +0100, Brian Starkey wrote:
>> I'm not clear why the heap depends on !CONFIG_HIGHMEM, but I also
>> don't know anything about SEV/TDX.
>
>It is because the CC apis, set_memory_decrypted()/etc are slightly
>mis-designed. They take in a vaddr to represent the address instead of
>a phys_addr_t or a page *
>
>This means the user has to use page_address() and then the whole thing
>is incompatible with highmem.
>
>Which is fine, highmem and CC are never turned on together.
Yeah, I was wondering if it is worth sanitizing it, but decided to be on
the safe side, for unlikely oddities future may bring sake :)
^ permalink raw reply
* [PATCH] dma-buf: heaps: system: document system_cc_shared heap
From: Jiri Pirko @ 2026-04-02 14:11 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
From: Jiri Pirko <jiri@nvidia.com>
Document the system_cc_shared dma-buf heap that was introduced
recently. Describe its purpose, availability conditions and
relation to confidential computing VMs.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
Documentation/userspace-api/dma-buf-heaps.rst | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 05445c83b79a..591732393e7d 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -16,6 +16,14 @@ following heaps:
- The ``system`` heap allocates virtually contiguous, cacheable, buffers.
+ - The ``system_cc_shared`` heap allocates virtually contiguous, cacheable,
+ buffers using shared (decrypted) memory. It is only present on
+ confidential computing (CoCo) VMs where memory encryption is active
+ (e.g., AMD SEV, Intel TDX). The allocated pages have the encryption
+ bit cleared, making them accessible for device DMA without TDISP
+ support. On non-CoCo VMs configurations, this heap is
+ not registered.
+
- The ``default_cma_region`` heap allocates physically contiguous,
cacheable, buffers. Only present if a CMA region is present. Such a
region is usually created either through the kernel commandline
--
2.51.1
^ permalink raw reply related
* Re: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Edgecombe, Rick P @ 2026-04-02 17:06 UTC (permalink / raw)
To: Reshetova, Elena, pbonzini@redhat.com, prsampat@amd.com
Cc: bp@alien8.de, marcandre.lureau@redhat.com, kas@kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
dave.hansen@linux.intel.com, Qiang, Chenyi, tglx@kernel.org,
hpa@zytor.com, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <IA1PR11MB949557E04741169BA209A69EE751A@IA1PR11MB9495.namprd11.prod.outlook.com>
On Thu, 2026-04-02 at 08:18 +0000, Reshetova, Elena wrote:
> > Oh, I was just wondering if we could just zero the page on accept
> > failure for the case of already accepted. Handle the issue
> > internally
> > and actually go back to something like patch 1. Will it work for
> > SNP?
>
> I don't know about SNP, but if you are proposing to zero the page on
> double acceptance, this is not great from security pov.
Accept does zero the memory already. So the guest side operation is
doing an operation that says "make this memory usable in an known state
of zeros". And the operation complies. What is the difference?
> It creates a
> predictable behaviour primitive for the host to zero any data inside
> the confidential guest and it can be misused (think of zeroing out a
> page containing a cryptographic key).
If the host can trigger an accept somehow in the guest (via something
like this or other issue), then the host can also remove, then AUG the
page from the S-EPT. This will result in a normal accept which also
zeros the page.
So the part about whether a triggered accept succeeds or returns an
already accepted error is already under the control of the host. I.e.,
if we don't have the zeroing behavior, the host can already cause the
page to get zeroed. So I don't think anything is regressed. Both come
down to how careful the guest is about what it accepts.
^ permalink raw reply
* Re: [PATCH v3 1/5] x86/tdx: Move TDX architectural error codes into <asm/shared/tdx_errno.h>
From: Sean Christopherson @ 2026-04-02 17:47 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, Paolo Bonzini,
linux-kernel, linux-coco, kvm, Chao Gao
In-Reply-To: <20260402-fuller_tdx_kexec_support-v3-1-34438d7094bf@intel.com>
On Thu, Apr 02, 2026, Vishal Verma wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Today there are two separate locations where TDX error codes are defined:
>
> arch/x86/include/asm/tdx.h
> arch/x86/kvm/vmx/tdx_errno.h
>
> They have some overlap that is already defined similarly. Reduce the
> duplication by unifying the architectural error codes at:
>
> asm/shared/tdx_errno.h
>
> ...and update the headers that contained the duplicated definitions to
> include the new unified header.
>
> "asm/shared" is used for sharing TDX code between the early compressed
> code and the normal kernel code. While the compressed code for the guest
> doesn't use these error code header definitions today, it does make the
> types of calls that return the values they define. So place the defines in
> "shared" location so that it can, but leave such cleanups for future
> changes.
>
> [Rick: enhance log]
> [Vishal: reduce to a simple move of architectural defines only]
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Nit, when calling out minor amendments, IMO the blurb in the square braces should
be after the previous SoB so that there's a clear, consistent chain of handling
and ordering. I.e.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
[Rick: enhance log]
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
[Vishal: reduce to a simple move of architectural defines only]
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
That makes it clear the Kirill signed off on something, then Rick tweaked the
changelog and signed off on _that_, and Vishal came along and simplified the
path.
Acked-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
> ---
> arch/x86/include/asm/shared/tdx.h | 1 +
> arch/x86/{kvm/vmx => include/asm/shared}/tdx_errno.h | 7 +++----
> arch/x86/kvm/vmx/tdx.h | 1 -
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 8bc074c8d7c6..6a1646fc2b2f 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -4,6 +4,7 @@
>
> #include <linux/bits.h>
> #include <linux/types.h>
> +#include <asm/shared/tdx_errno.h>
>
> #define TDX_HYPERCALL_STANDARD 0
>
> diff --git a/arch/x86/kvm/vmx/tdx_errno.h b/arch/x86/include/asm/shared/tdx_errno.h
> similarity index 92%
> rename from arch/x86/kvm/vmx/tdx_errno.h
> rename to arch/x86/include/asm/shared/tdx_errno.h
> index 6ff4672c4181..3c1e8ce716e3 100644
> --- a/arch/x86/kvm/vmx/tdx_errno.h
> +++ b/arch/x86/include/asm/shared/tdx_errno.h
> @@ -1,8 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /* architectural status code for SEAMCALL */
> -
> -#ifndef __KVM_X86_TDX_ERRNO_H
> -#define __KVM_X86_TDX_ERRNO_H
> +#ifndef _ASM_X86_SHARED_TDX_ERRNO_H
> +#define _ASM_X86_SHARED_TDX_ERRNO_H
>
> #define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
>
> @@ -37,4 +36,4 @@
> #define TDX_OPERAND_ID_SEPT 0x92
> #define TDX_OPERAND_ID_TD_EPOCH 0xa9
>
> -#endif /* __KVM_X86_TDX_ERRNO_H */
> +#endif /* _ASM_X86_SHARED_TDX_ERRNO_H */
> diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
> index b5cd2ffb303e..ac8323a68b16 100644
> --- a/arch/x86/kvm/vmx/tdx.h
> +++ b/arch/x86/kvm/vmx/tdx.h
> @@ -3,7 +3,6 @@
> #define __KVM_X86_VMX_TDX_H
>
> #include "tdx_arch.h"
> -#include "tdx_errno.h"
For the record, the defines in tdx_errno.h are only ever used by arch/x86/kvm/vmx/tdx.c,
and that file already included asm/shared/tdx.h by way of arch/x86/include/asm/tdx.h.
> #ifdef CONFIG_KVM_INTEL_TDX
> #include "common.h"
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v3 1/5] x86/tdx: Move TDX architectural error codes into <asm/shared/tdx_errno.h>
From: Verma, Vishal L @ 2026-04-02 18:18 UTC (permalink / raw)
To: seanjc@google.com
Cc: Gao, Chao, Edgecombe, Rick P, bp@alien8.de, x86@kernel.org,
kas@kernel.org, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
tglx@kernel.org, pbonzini@redhat.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org
In-Reply-To: <ac6rtq-n0JNa_Oc0@google.com>
On Thu, 2026-04-02 at 10:47 -0700, Sean Christopherson wrote:
> On Thu, Apr 02, 2026, Vishal Verma wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Today there are two separate locations where TDX error codes are defined:
> >
> > arch/x86/include/asm/tdx.h
> > arch/x86/kvm/vmx/tdx_errno.h
> >
> > They have some overlap that is already defined similarly. Reduce the
> > duplication by unifying the architectural error codes at:
> >
> > asm/shared/tdx_errno.h
> >
> > ...and update the headers that contained the duplicated definitions to
> > include the new unified header.
> >
> > "asm/shared" is used for sharing TDX code between the early compressed
> > code and the normal kernel code. While the compressed code for the guest
> > doesn't use these error code header definitions today, it does make the
> > types of calls that return the values they define. So place the defines in
> > "shared" location so that it can, but leave such cleanups for future
> > changes.
> >
> > [Rick: enhance log]
> > [Vishal: reduce to a simple move of architectural defines only]
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
>
> Nit, when calling out minor amendments, IMO the blurb in the square braces should
> be after the previous SoB so that there's a clear, consistent chain of handling
> and ordering. I.e.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [Rick: enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> [Vishal: reduce to a simple move of architectural defines only]
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
>
> That makes it clear the Kirill signed off on something, then Rick tweaked the
> changelog and signed off on _that_, and Vishal came along and simplified the
> path.
>
That is indeed what I'm used to seeing/doing as well, I changed it to
this style after re-reading the tag ordering guidelines in maintainer-
tip.rst:
https://docs.kernel.org/process/maintainer-tip.html#ordering-of-commit-tags
If the handler made modifications to the patch or the changelog,
then this should be mentioned after the changelog text and above all
commit tags in the following format:
... changelog text ends.
[ handler: Replaced foo by bar and updated changelog ]
First-tag: .....
Although now I see I screwed even that up slightly - it calls for a blank
line after the [ ... ] notes.
^ permalink raw reply
* [PATCH v3 0/6] KVM: SEV: Add support for IBPB-on-Entry and BTB Isolation
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
IBPB-on-Entry and BTB Isolation are supplemental Spectre V2 mitigations
available to SNP guests.
Patch 1 fixes a longstanding bug where users weren't able
to force Automatic IBRS on SNP enabled machines using spectre_v2=eibrs.
Patch 2 fixes another longstanding bug where users couldn't
select legacy / toggling SPEC_CTRL[IBRS] on AMD systems. Users of
the BTB Isolation feature may use IBRS to mitigate possible
performance degradation caused by BTB Isolation.
Patches 3 and 4 deal with a minor code refactoring as a result of
Sean's review of the v2 IBPB-on-Entry series.
Patch 5 adds support for IBPB-on-Entry.
Patch 6 adds support for BTB Isolation.
Based on current tip/master v7.0-rc6-423-g8726fc6dc93c
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
This v3 series now also available here:
https://github.com/AMDESE/linux/tree/btb-isol-latest
Advance qemu bits (to add feature on/off switches) available here:
https://github.com/AMDESE/qemu/tree/btb-isol-latest
Qemu bits will be posted upstream once kernel bits are merged.
They depend on Naveen Rao's "target/i386: SEV: Add support for
enabling VMSA SEV features":
https://lore.kernel.org/qemu-devel/cover.1761648149.git.naveen@kernel.org/
v3:
- Merged IBPB-on-Entry and BTB Isolation into single patchseries
- Addressed comments from Sean Christopherson, Pawan Gupta, kernel test robot
- Simplified unnecessarily complicated logic in spectre_v2=eibrs-with-SNP fix
- Reworded, rebased features on top of new SNP_ONLY_MASK etc. changes
v2:
[IBPB-on-Entry]
- https://lore.kernel.org/kvm/20260203222405.4065706-1-kim.phillips@amd.com/
- Change first patch's title (Nikunj)
- Add reviews-by (Nikunj, Tom)
- Change second patch's description to more generally explain what the patch does (Boris)
- Add new, third patch renaming SNP_FEATURES_PRESENT->SNP_FEATURES_IMPL
[BTB Isolation]
- https://lore.kernel.org/kvm/20260311130611.2201214-1-kim.phillips@amd.com/
- Patch 1/3:
- Address Dave Hansen's comment to adhere to using the IBRS_ENHANCED
Intel feature flag also for AutoIBRS.
v1:
[IBPB-on-Entry] https://lore.kernel.org/kvm/20260126224205.1442196-1-kim.phillips@amd.com/
[BTB Isolation] https://lore.kernel.org/kvm/20260224180157.725159-1-kim.phillips@amd.com/
Kim Phillips (6):
cpu/bugs: Allow forcing Automatic IBRS with SNP active using
spectre_v2=eibrs
cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
KVM: SEV: Disallow setting SNP-only features for non-SNP guests via a
single mask
KVM: SEV: Advertise SVM_SEV_FEAT_SNP_ACTIVE
KVM: SEV: Add support for IBPB-on-Entry
KVM: SEV: Add support for SNP BTB Isolation
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/svm.h | 7 +++++++
arch/x86/kernel/cpu/bugs.c | 18 +++++++++++-------
arch/x86/kernel/cpu/common.c | 6 +-----
arch/x86/kvm/svm/sev.c | 18 +++++++++++++++---
5 files changed, 35 insertions(+), 15 deletions(-)
base-commit: 8726fc6dc93c62232fa625c1c91b97e21fff02b6
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/6] cpu/bugs: Allow forcing Automatic IBRS with SNP active using spectre_v2=eibrs
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, Dave Hansen, stable, kernel test robot
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
spectre_v2=eibrs currently enables retpolines when SNP is enabled,
instead of AutoIBRS (EIBRS) because the commit that disabled
AutoIBRS if SNP is enabled stopped short of enabling
X86_FEATURE_IBRS_ENHANCED.
Change the logic to enable X86_FEATURE_IBRS_ENHANCED, and move the
decision to switch to retpolines in the default/"auto" case in
spectre_v2_select_mitigation(). This allows the existing
spectre_v2=eibrs logic to work as intended.
Also emit a performance loss warning for using AutoIBRS with
SNP enabled.
Fixes: acaa4b5c4c85 ("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: stable@kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603121136.bc8zNsHS-lkp@intel.com/
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v3:
- Addressed Pawan Gupta's comment and remove wrong SPECTRE_V2_CMD_FORCE ("=on") check
- Addressed kernel test robot's !A || A && B is equivalent to !A || B warning
- Preferred to add new AutoIBRS with SEV-SNP enabled performance warning instead
of muting legacy IBRS in use vs. eIBRS messaging in the context of SNP, since
SNP users' IBRS performance varies whether they enable SNP BTB Isolation
v2: https://lore.kernel.org/kvm/20260311130611.2201214-2-kim.phillips@amd.com/
- Address Dave Hansen's comment to adhere to using the IBRS_ENHANCED
Intel feature flag also for AutoIBRS.
v1:
https://lore.kernel.org/kvm/20260224180157.725159-2-kim.phillips@amd.com/
arch/x86/kernel/cpu/bugs.c | 10 +++++++++-
arch/x86/kernel/cpu/common.c | 6 +-----
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 83f51cab0b1e..dfefbde10646 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1658,6 +1658,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
#define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
#define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
#define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
+#define SPECTRE_V2_EIBRS_SNP_PERF_MSG "WARNING: AutoIBRS mitigation selected on SEV-SNP enabled CPU, this may cause unnecessary performance loss\n"
#define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
#ifdef CONFIG_BPF_SYSCALL
@@ -2181,7 +2182,12 @@ static void __init spectre_v2_select_mitigation(void)
break;
fallthrough;
case SPECTRE_V2_CMD_FORCE:
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ /*
+ * Don't use AutoIBRS when SNP is enabled because it degrades
+ * host userspace indirect branch performance.
+ */
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) &&
+ !boot_cpu_has(X86_FEATURE_SEV_SNP)) {
spectre_v2_enabled = SPECTRE_V2_EIBRS;
break;
}
@@ -2257,6 +2263,8 @@ static void __init spectre_v2_apply_mitigation(void)
return;
case SPECTRE_V2_EIBRS:
+ if (boot_cpu_has(X86_FEATURE_SEV_SNP))
+ pr_warn(SPECTRE_V2_EIBRS_SNP_PERF_MSG);
break;
case SPECTRE_V2_IBRS:
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4e1f0c4afe3a..0cdcbbedf883 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1485,13 +1485,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
/*
* AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
* flag and protect from vendor-specific bugs via the whitelist.
- *
- * Don't use AutoIBRS when SNP is enabled because it degrades host
- * userspace indirect branch performance.
*/
if ((x86_arch_cap_msr & ARCH_CAP_IBRS_ALL) ||
- (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
- !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
+ cpu_has(c, X86_FEATURE_AUTOIBRS)) {
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
!(x86_arch_cap_msr & ARCH_CAP_PBRSB_NO))
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/6] cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, Dave Hansen, stable
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
This is to prepare to allow legacy IBRS toggling on AMD systems,
where the BTB Isolation SEV-SNP feature can use it to optimize the
quick VM exit to re-entry path.
There is no reason this wasn't allowed in the first place, therefore
adding the cc: stable and Fixes: tags.
Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v3: No changes
v2: No changes
https://lore.kernel.org/kvm/20260311130611.2201214-3-kim.phillips@amd.com/
v1: https://lore.kernel.org/kvm/20260224180157.725159-3-kim.phillips@amd.com/
arch/x86/kernel/cpu/bugs.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index dfefbde10646..eed5a72a870c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2153,11 +2153,6 @@ static void __init spectre_v2_select_mitigation(void)
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
- if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
- pr_err("IBRS selected but not Intel CPU. Switching to AUTO select\n");
- spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
- }
-
if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && !boot_cpu_has(X86_FEATURE_IBRS)) {
pr_err("IBRS selected but CPU doesn't have IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
@@ -2250,7 +2245,8 @@ static void __init spectre_v2_apply_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);
if (spectre_v2_in_ibrs_mode(spectre_v2_enabled)) {
- if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS) &&
+ spectre_v2_enabled != SPECTRE_V2_IBRS) {
msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
} else {
x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/6] KVM: SEV: Disallow setting SNP-only features for non-SNP guests via a single mask
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
As SNP-only features get added, adding them to the valid_vmsa_features mask
in __sev_guest_init() often gets neglected. Add SVM_SEV_FEAT_SNP_ONLY_MASK
to help group these common features together.
Suggested-by: Sean Christopherson <seanjc@google.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/kvm/aaWog_UjW-M3412C@google.com/
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v3: new
arch/x86/include/asm/svm.h | 2 ++
arch/x86/kvm/svm/sev.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..7e3f9d92351a 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -307,6 +307,8 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
+#define SVM_SEV_FEAT_SNP_ONLY_MASK SVM_SEV_FEAT_SECURE_TSC
+
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
struct vmcb_seg {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3f9c1aa39a0a..2b4f3c05e282 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -456,7 +456,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
return -EINVAL;
if (!snp_active)
- valid_vmsa_features &= ~SVM_SEV_FEAT_SECURE_TSC;
+ valid_vmsa_features &= ~SVM_SEV_FEAT_SNP_ONLY_MASK;
if (data->vmsa_features & ~valid_vmsa_features)
return -EINVAL;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/6] KVM: SEV: Advertise SVM_SEV_FEAT_SNP_ACTIVE
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
Allow userspace to set the flag in kvm_sev_init.flags.
KVM still needs to set the flag for backwards compatibility, but
disallowing SVM_SEV_FEAT_SNP_ACTIVE for an SNP guest is "bizarre."
Suggested-by: Sean Christopherson <seanjc@google.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/kvm/aaWog_UjW-M3412C@google.com/
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v3: new
arch/x86/include/asm/svm.h | 3 ++-
arch/x86/kvm/svm/sev.c | 8 ++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 7e3f9d92351a..4f844a72890c 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -307,7 +307,8 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
-#define SVM_SEV_FEAT_SNP_ONLY_MASK SVM_SEV_FEAT_SECURE_TSC
+#define SVM_SEV_FEAT_SNP_ONLY_MASK (SVM_SEV_FEAT_SNP_ACTIVE | \
+ SVM_SEV_FEAT_SECURE_TSC)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2b4f3c05e282..9663424c0cf0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3165,8 +3165,12 @@ void __init sev_hardware_setup(void)
cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
- if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
- sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+ if (sev_snp_enabled) {
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
+
+ if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+ }
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/6] KVM: SEV: Add support for IBPB-on-Entry
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
AMD EPYC 5th generation and above processors support IBPB-on-Entry
for SNP guests. By invoking an Indirect Branch Prediction Barrier
(IBPB) on VMRUN, old indirect branch predictions are prevented
from influencing indirect branches within the guest.
SNP guests may choose to enable IBPB-on-Entry by setting
SEV_FEATURES bit 21 (IbpbOnEntry).
Host support for IBPB on Entry is indicated by CPUID
Fn8000_001F[IbpbOnEntry], bit 31.
If supported, indicate support for IBPB on Entry in
sev_supported_vmsa_features bit 23 (IbpbOnEntry).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Cc: Sean Christopherson <seanjc@google.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
v3: Rebased on top of new SNP_ONLY_MASK etc. changes
v2: https://lore.kernel.org/kvm/20260203222405.4065706-3-kim.phillips@amd.com/
- Added Tom's Reviewed-by.
v1: https://lore.kernel.org/kvm/20260126224205.1442196-3-kim.phillips@amd.com/
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/svm.h | 4 +++-
arch/x86/kvm/svm/sev.c | 3 +++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index dbe104df339b..236411a1a86a 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -459,6 +459,7 @@
#define X86_FEATURE_ALLOWED_SEV_FEATURES (19*32+27) /* Allowed SEV Features */
#define X86_FEATURE_SVSM (19*32+28) /* "svsm" SVSM present */
#define X86_FEATURE_HV_INUSE_WR_ALLOWED (19*32+30) /* Allow Write to in-use hypervisor-owned pages */
+#define X86_FEATURE_IBPB_ON_ENTRY (19*32+31) /* SEV-SNP IBPB on VM Entry */
/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
#define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* No Nested Data Breakpoints */
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 4f844a72890c..2a2b8705b2c0 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -306,9 +306,11 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
+#define SVM_SEV_FEAT_IBPB_ON_ENTRY BIT(21)
#define SVM_SEV_FEAT_SNP_ONLY_MASK (SVM_SEV_FEAT_SNP_ACTIVE | \
- SVM_SEV_FEAT_SECURE_TSC)
+ SVM_SEV_FEAT_SECURE_TSC | \
+ SVM_SEV_FEAT_IBPB_ON_ENTRY)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9663424c0cf0..561023486253 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3170,6 +3170,9 @@ void __init sev_hardware_setup(void)
if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (cpu_feature_enabled(X86_FEATURE_IBPB_ON_ENTRY))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_IBPB_ON_ENTRY;
}
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 6/6] KVM: SEV: Add support for SNP BTB Isolation
From: Kim Phillips @ 2026-04-02 20:25 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260402202558.195005-1-kim.phillips@amd.com>
Advertise support for BTB Isolation via SEV_VMSA_FEATURES when SNP is
enabled, as all hardware that supports SNP also support BTB Isolation.
BTB Isolation is an optional feature that can be enabled by the guest to
ensure its guest Branch Target Buffers (BTBs) are not
affected by any context outside that guest.
SNP-active guests may choose to enable the Branch Target Buffer
Isolation mode through SEV_FEATURES bit 7 (BTBIsolation).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Cc: Sean Christopherson <seanjc@google.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
v3: Reworded, Rebased on top of new SNP_ONLY_MASK etc. changes
v2: https://lore.kernel.org/kvm/20260203222405.4065706-3-kim.phillips@amd.com/
- Added Tom's Reviewed-by.
v1: https://lore.kernel.org/kvm/20260126224205.1442196-3-kim.phillips@amd.com/
arch/x86/include/asm/svm.h | 2 ++
arch/x86/kvm/svm/sev.c | 7 ++++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 2a2b8705b2c0..d3a15a40a09b 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -305,10 +305,12 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
+#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
#define SVM_SEV_FEAT_IBPB_ON_ENTRY BIT(21)
#define SVM_SEV_FEAT_SNP_ONLY_MASK (SVM_SEV_FEAT_SNP_ACTIVE | \
+ SVM_SEV_FEAT_BTB_ISOLATION | \
SVM_SEV_FEAT_SECURE_TSC | \
SVM_SEV_FEAT_IBPB_ON_ENTRY)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 561023486253..733423000bc8 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3166,7 +3166,12 @@ void __init sev_hardware_setup(void)
sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
if (sev_snp_enabled) {
- sev_supported_vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
+ /*
+ * Some SNP-only features such as BTB Isolation are
+ * available on all systems that support SNP.
+ */
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE |
+ SVM_SEV_FEAT_BTB_ISOLATION;
if (tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 20:47 UTC (permalink / raw)
To: Dave Hansen
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <a14531ab-f069-41f9-8c5c-9fe6f28a9454@intel.com>
On Thu, Mar 19, 2026, Dave Hansen wrote:
> On 3/18/26 17:57, Yan Zhao wrote:
> > Remove the completely unnecessary assumption that memory mapped into a TDX
> > guest is backed by refcounted struct page memory. From KVM's point of view,
> > TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
> > have no business placing requirements on how KVM and guest_memfd manage
> > memory.
>
> I think this goes a bit too far.
>
> It's one thing to say that it's more convenient for KVM to stick with
> pfns because it's what KVM uses now. Or, that the goals of using 'struct
> page' can be accomplished other ways. It's quite another to say what
> other bits of the codebase have "business" doing.
>
> Sean, can we tone this down a _bit_ to help guide folks in the future?
I strongly disagree on this one. IMO, super low level APIs have no business
placing unnecessary requirements on callers. Requiring that the target memory
be convertible? A-ok because that's an actual requirement of the architecture.
Requiring or assuming anything about "struct page" or folios? Not ok.
This isn't a convenience thing, it's a core tenent of KVM guest memory managment.
KVM's MMUs work with PFNs, full stop. A PFN might have been acquired via GUP and
thus a refcounted struct page, but there is a hard boundary in KVM between getting
the page via GUP and installing the PFN into KVM's MMU.
KVM didn't always have a hard boundary, and it took us literally years to undo
the resulting messes. And the TDX hugepage support that was posted that pulled
information from "struct page" and/or its folio re-introduced the exact type of
flawed assumptions that we spent years purging from KVM.
So yeah, what I wrote was a strongly worded statement, but that was 100% intentional,
because I want to be crystal clear that requiring KVM to pass a struct page is a
complete non-starter for me.
> > Rip out the misguided struct page assumptions/constraints and instead have
>
> Could we maybe tone down the editorializing a bit, please? Folks can
> have honest disagreements about this stuff while not being "misguided".
FWIW, I'm not trying to say the intent or people's viewpoints were misguided, I'm
saying the code itself is misguided. AFAICT, the "struct page" stuff was added
to try to harden the TDX implementation, e.g. to guard against effective UAF of
memory that was assigned to a TD. But my viewpoint is that requiring a struct
page made the overall implemenation _less_ robust, and thus the code is misguided
because its justfication/reasoning was flawed.
> > the two SEAMCALL wrapper APIs take PFN directly. This ensures that for
> > future huge page support in S-EPT, the kernel doesn't pick up even worse
> > assumptions like "a hugepage must be contained in a single folio".
>
> I don't really understand what this is saying.
>
> Is the concern that KVM might want to set up page tables for memory that
> differ from how it was allocated? I'm a bit worried that this assumes
> something about folios that doesn't always hold.
Heh, the concern is that taking a page/folio in the SEAMCALL wrappers will lead
to assumptions that don't always hold. Specifically, the TDX hugepage support[*]
was building up assumptions that KVM would never attempt to install a hugepage
that didn't fit into a single folio:
+ if (start_idx + npages > folio_nr_pages(folio))
+ return TDX_OPERAND_INVALID;
[*] https://lore.kernel.org/all/20250807094132.4453-1-yan.y.zhao@intel.com
> I think the hugetlbfs gigantic support uses folios in at least a few
> spots today.
Yes, and the in-progress guest_memfd+HugeTLB work will also use folios. The
potential hiccup with the above folio_nr_pages() assumption is that KVM may want
to shatter folios to 4KiB granularity for tracking purposes, but still map
hugepage when memory is known to be physically contiguous.
That's where a lot of this is coming from. Taking a "struct page" is a bad
enough assumption on its own (that all TDX private memory is backed by struct page),
but even worse it's a slippery slope to even more bad assumptions (e.g. about how
guest_memfd internally manages its folios).
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Dave Hansen @ 2026-04-02 21:09 UTC (permalink / raw)
To: Sean Christopherson
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <ac7V0g2q2hN3dU5u@google.com>
On 4/2/26 13:47, Sean Christopherson wrote:
> On Thu, Mar 19, 2026, Dave Hansen wrote:
>> On 3/18/26 17:57, Yan Zhao wrote:
>>> Remove the completely unnecessary assumption that memory mapped into a TDX
>>> guest is backed by refcounted struct page memory. From KVM's point of view,
>>> TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
>>> have no business placing requirements on how KVM and guest_memfd manage
>>> memory.
>>
>> I think this goes a bit too far.
>>
>> It's one thing to say that it's more convenient for KVM to stick with
>> pfns because it's what KVM uses now. Or, that the goals of using 'struct
>> page' can be accomplished other ways. It's quite another to say what
>> other bits of the codebase have "business" doing.
>>
>> Sean, can we tone this down a _bit_ to help guide folks in the future?
>
> I strongly disagree on this one.
I think I understand the motivation now. All I'm saying is that instead
of something like:
Remove the completely unnecessary assumption that memory mapped
into a TDX guest is backed by refcounted struct page memory.
I'd rather see something along the lines of
KVM's MMUs work with PFNs. This is very much an intentional
design choice. It ensures that the KVM MMUs remains flexible
and are not too tied to the regular CPU MMUs and the kernel code
around them.
Using 'struct page' for TDX memory is not a good fit anywhere
near the KVM MMU code.
Would you disagree strongly with that kind of rewording?
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 22:11 UTC (permalink / raw)
To: Dave Hansen
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <33adf49d-4937-413e-a594-830c11b2bed0@intel.com>
On Thu, Apr 02, 2026, Dave Hansen wrote:
> On 4/2/26 13:47, Sean Christopherson wrote:
> > On Thu, Mar 19, 2026, Dave Hansen wrote:
> >> On 3/18/26 17:57, Yan Zhao wrote:
> >>> Remove the completely unnecessary assumption that memory mapped into a TDX
> >>> guest is backed by refcounted struct page memory. From KVM's point of view,
> >>> TDH_MEM_PAGE_ADD and TDH_MEM_PAGE_AUG are glorified writes to PTEs, so they
> >>> have no business placing requirements on how KVM and guest_memfd manage
> >>> memory.
> >>
> >> I think this goes a bit too far.
> >>
> >> It's one thing to say that it's more convenient for KVM to stick with
> >> pfns because it's what KVM uses now. Or, that the goals of using 'struct
> >> page' can be accomplished other ways. It's quite another to say what
> >> other bits of the codebase have "business" doing.
> >>
> >> Sean, can we tone this down a _bit_ to help guide folks in the future?
> >
> > I strongly disagree on this one.
>
> I think I understand the motivation now. All I'm saying is that instead
> of something like:
>
> Remove the completely unnecessary assumption that memory mapped
> into a TDX guest is backed by refcounted struct page memory.
>
> I'd rather see something along the lines of
>
> KVM's MMUs work with PFNs. This is very much an intentional
> design choice. It ensures that the KVM MMUs remains flexible
> and are not too tied to the regular CPU MMUs and the kernel code
> around them.
>
> Using 'struct page' for TDX memory is not a good fit anywhere
> near the KVM MMU code.
>
> Would you disagree strongly with that kind of rewording?
Not at all, works for me.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-02 23:07 UTC (permalink / raw)
To: Chang S. Bae
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <3051c067-048e-4388-8c22-1e275d8d3b5a@intel.com>
On Wed, Mar 25, 2026, Chang S. Bae wrote:
> On 3/12/2026 10:47 AM, Sean Christopherson wrote:
> > On Thu, Mar 12, 2026, Chang S. Bae wrote:
> > >
> > > However, that is sort of what-if scenarios at best. The host kernel still
> > > manages EGPR context switching through XSAVE. Saving EGPRs into regs[] would
> > > introduce an oddity to synchronize between two buffers: regs[] and
> > > gfpu->fpstate, which looks like unnecessary complexity.
>
> No, this looks ugly.
Sorry, you lost me. What looks ugly?
> If guest EGPR state is saved in vcpu->arch.regs[], the APX area there isn't
> necessary:
>
> When the KVM API exposes state in XSAVE format, the frontend can handle this
> separately. Alongside uABI <-> guest fpstate copy functions, new copy
> functions may deal with the state between uABI <-> VCPU cache.
>
> Further, one could think of exclusion as such:
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 76153dfb58c9..5404f9399eea 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -794,9 +794,10 @@ static u64 __init guest_default_mask(void)
> {
> /*
> * Exclude dynamic features, which require userspace opt-in even
> - * for KVM guests.
> + * for KVM guests, and APX as extended general-purpose register
> + * states are saved in the KVM cache separately.
> */
> - return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
> + return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_APX);
> }
>
> But this default bitmask feeds into the permission bits:
>
> fpu->guest_perm.__state_perm = guest_default_cfg.features;
> fpu->guest_perm.__state_size = guest_default_cfg.size;
>
> This policy looks clear and sensible: permission is granted only if space is
> reserved to save the state. If there is a strong desire to save memory, I
> think it should go through a more thorough review to revisit this policy.
And I'm lost again.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Sean Christopherson @ 2026-04-02 23:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kiryl Shutsemau, kvm, x86, linux-coco, linux-kernel,
Chang S . Bae
In-Reply-To: <7ec084f8-812e-42f2-8470-e416fa7ee848@redhat.com>
On Wed, Mar 11, 2026, Paolo Bonzini wrote:
> On 3/11/26 01:33, Sean Christopherson wrote:
> > Clean up KVM's register tracking and storage in preparation for landing APX,
> > which expands the maximum number of GPRs from 16 to 32.
> >
> > This is kinda sorta an RFC, as there are some very opinionated changes. I.e.
> > if you dislike something, please speak up.
> >
> > My thought is to treat R16-R31 as much like other GPRs as possible (though
> > maybe we don't need to expand regs[] as sketched out in the last patch?).
>
> The cleanups in patches 1-4 are nice.
>
> For APX specifically, in abstract it's nice to treat R16-R31 as much as
> possible as regular GPRs. On the other hand, the extra 16 regs[] entries
> would be more or less unused, the ugly switch statements wouldn't go away.
Hmm, yeah, but only if XSAVE is the source of truth for guest R16-R31.
Do we know what the compiler and/or kernel rules for using R16-R31 will be?
E.g. if C code is allowed to use R16-R31 at will, then KVM will either need to
swap R16-R31 in assembly, or annotate a pile of functions as "no_egpr" or
whatever.
At that point, my vote would be to use regs[] to track R16-R31 for KVM's purposes.
IIUC, we could largely ignore XSAVE state at runtime and just ensure R16-R31 are
copied to/from userspace as needed, same as we do for PKRU.
If R16-R31 aren't generally available for C code, then how exactly is APX going
to be used?
Understanding the usage rules for R16-R31 seems fundamental to figuring what to
do in KVM...
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Ackerley Tng @ 2026-04-02 23:23 UTC (permalink / raw)
To: Yan Zhao, seanjc, pbonzini, dave.hansen
Cc: tglx, mingo, bp, kas, x86, linux-kernel, kvm, linux-coco,
kai.huang, rick.p.edgecombe, yilun.xu, vannapurve, sagis,
binbin.wu, xiaoyao.li, isaku.yamahata
In-Reply-To: <20260319005703.8983-1-yan.y.zhao@intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
>
> [...snip...]
>
> -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> + u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> .rcx = gpa,
> .rdx = tdx_tdr_pa(td),
> - .r8 = page_to_phys(page),
> + .r8 = PFN_PHYS(pfn),
> .r9 = page_to_phys(source),
Perhaps in some future patch, are we considering also passing pfn
instead of struct page for source? Would we also update
kvm_tdx->page_add_src to be a kvm_pfn_t?
> };
> u64 ret;
>
> - tdx_clflush_page(page);
> + tdx_clflush_pfn(pfn);
> ret = seamcall_ret(TDH_MEM_PAGE_ADD, &args);
>
> *ext_err1 = args.rcx;
>
> [...snip...]
>
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 23:35 UTC (permalink / raw)
To: Ackerley Tng
Cc: Yan Zhao, pbonzini, dave.hansen, tglx, mingo, bp, kas, x86,
linux-kernel, kvm, linux-coco, kai.huang, rick.p.edgecombe,
yilun.xu, vannapurve, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <CAEvNRgGknoP=QscA-efLB0LSS03XttsdN3v0KBVROAXTknAakw@mail.gmail.com>
On Thu, Apr 02, 2026, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> >
> > [...snip...]
> >
> > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > + u64 *ext_err1, u64 *ext_err2)
> > {
> > struct tdx_module_args args = {
> > .rcx = gpa,
> > .rdx = tdx_tdr_pa(td),
> > - .r8 = page_to_phys(page),
> > + .r8 = PFN_PHYS(pfn),
> > .r9 = page_to_phys(source),
>
> Perhaps in some future patch, are we considering also passing pfn instead of
> struct page for source? Would we also update kvm_tdx->page_add_src to be a
> kvm_pfn_t?
Probably?
I assume you're asking in the context of in-place conversion, where KVM will
allow a single guest_memfd page to be both the source and the dest?
Right now, KVM requires the source page to be a GUP'able page, specifically so
that KVM can obtain a reference and ensure the page isn't freed until KVM is done
with it. If/when the source and dest are one and the same, then I don't think
we'd want to GUP the page (and there would be no need to since this would all run
while holding gmem's filemap_invalidate_lock()), at which point, yeah, passing a
"struct page" doesn't make much sense, and passing kvm_pfn_t or u64 or whatever
seems like the obvious choice.
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Edgecombe, Rick P @ 2026-04-02 23:36 UTC (permalink / raw)
To: ackerleytng@google.com, pbonzini@redhat.com, seanjc@google.com,
Zhao, Yan Y, dave.hansen@linux.intel.com
Cc: sagis@google.com, Yamahata, Isaku, x86@kernel.org, kas@kernel.org,
yilun.xu@linux.intel.com, bp@alien8.de, mingo@redhat.com,
linux-kernel@vger.kernel.org, Huang, Kai, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, tglx@kernel.org,
binbin.wu@linux.intel.com, Annapurve, Vishal
In-Reply-To: <CAEvNRgGknoP=QscA-efLB0LSS03XttsdN3v0KBVROAXTknAakw@mail.gmail.com>
On Thu, 2026-04-02 at 16:23 -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
>
> >
> > [...snip...]
> >
> > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > + u64 *ext_err1, u64 *ext_err2)
> > {
> > struct tdx_module_args args = {
> > .rcx = gpa,
> > .rdx = tdx_tdr_pa(td),
> > - .r8 = page_to_phys(page),
> > + .r8 = PFN_PHYS(pfn),
> > .r9 = page_to_phys(source),
>
> Perhaps in some future patch, are we considering also passing pfn
> instead of struct page for source? Would we also update
> kvm_tdx->page_add_src to be a kvm_pfn_t?
Can you remind me, with the new API we were going to do an in-place add right?
Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
single pfn arg. The passing of ->src_page is kind of awkward already.
Like Ira was playing around with here:
https://lore.kernel.org/kvm/20251105-tdx-init-in-place-v1-1-1196b67d0423@intel.com/
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Sean Christopherson @ 2026-04-02 23:46 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: ackerleytng@google.com, pbonzini@redhat.com, Yan Y Zhao,
dave.hansen@linux.intel.com, sagis@google.com, Isaku Yamahata,
x86@kernel.org, kas@kernel.org, yilun.xu@linux.intel.com,
bp@alien8.de, mingo@redhat.com, linux-kernel@vger.kernel.org,
Kai Huang, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Xiaoyao Li, tglx@kernel.org, binbin.wu@linux.intel.com,
Vishal Annapurve
In-Reply-To: <4f0a5d5f7b743e3872547e14d02b894cd9f60550.camel@intel.com>
On Thu, Apr 02, 2026, Rick P Edgecombe wrote:
> On Thu, 2026-04-02 at 16:23 -0700, Ackerley Tng wrote:
> > Yan Zhao <yan.y.zhao@intel.com> writes:
> >
> > >
> > > [...snip...]
> > >
> > > -u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2)
> > > +u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> > > + u64 *ext_err1, u64 *ext_err2)
> > > {
> > > struct tdx_module_args args = {
> > > .rcx = gpa,
> > > .rdx = tdx_tdr_pa(td),
> > > - .r8 = page_to_phys(page),
> > > + .r8 = PFN_PHYS(pfn),
> > > .r9 = page_to_phys(source),
> >
> > Perhaps in some future patch, are we considering also passing pfn
> > instead of struct page for source? Would we also update
> > kvm_tdx->page_add_src to be a kvm_pfn_t?
>
> Can you remind me, with the new API we were going to do an in-place add right?
> Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
> single pfn arg.
No. In-place ADD will be supported, but it won't be mandatory. Practically
speaking, we can't make it mandatory unless we're willing to completely rip out
support for per-VM attributes (or at least, per-VM PRIVATE tracking). I suppose
we could require in-place ADD when using per-gmem attributes, but I don't see
the point given that TDH_MEM_PAGE_ADD itself takes a source and dest.
^ permalink raw reply
* Re: [PATCH 1/2] x86/virt/tdx: Use PFN directly for mapping guest private memory
From: Edgecombe, Rick P @ 2026-04-02 23:53 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, mingo@redhat.com,
pbonzini@redhat.com, binbin.wu@linux.intel.com, Yamahata, Isaku,
ackerleytng@google.com, sagis@google.com, tglx@kernel.org,
bp@alien8.de, Annapurve, Vishal, yilun.xu@linux.intel.com,
x86@kernel.org
In-Reply-To: <ac7_uQc5t0uU3a1h@google.com>
On Thu, 2026-04-02 at 16:46 -0700, Sean Christopherson wrote:
> > Can you remind me, with the new API we were going to do an in-place add
> > right?
> > Then I'd wonder if we could maybe change tdh_mem_page_add() to only have a
> > single pfn arg.
>
> No. In-place ADD will be supported, but it won't be mandatory. Practically
> speaking, we can't make it mandatory unless we're willing to completely rip
> out support for per-VM attributes (or at least, per-VM PRIVATE tracking). I
> suppose we could require in-place ADD when using per-gmem attributes, but I
> don't see the point given that TDH_MEM_PAGE_ADD itself takes a source and
> dest.
Thanks. It might still be cleaner to copy clear text from the GUPed page to the
destination page and let the tdh_mem_page_add() call in the map path do it in-
place? Especially if we want to support in-place add as an option, it would make
the code more uniform.
But it sounds like we don't need to decide now.
^ permalink raw reply
* Re: [PATCH 0/7] KVM: x86: APX reg prep work
From: Chang S. Bae @ 2026-04-03 0:05 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Kiryl Shutsemau, kvm, x86, linux-coco,
linux-kernel
In-Reply-To: <ac72z1cfXnpUmkWv@google.com>
[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]
On 4/2/2026 4:07 PM, Sean Christopherson wrote:
> On Wed, Mar 25, 2026, Chang S. Bae wrote:
>> On 3/12/2026 10:47 AM, Sean Christopherson wrote:
>>> On Thu, Mar 12, 2026, Chang S. Bae wrote:
>>>>
>>>> However, that is sort of what-if scenarios at best. The host kernel still
>>>> manages EGPR context switching through XSAVE. Saving EGPRs into regs[] would
>>>> introduce an oddity to synchronize between two buffers: regs[] and
>>>> gfpu->fpstate, which looks like unnecessary complexity.
>>
>> No, this looks ugly.
>
> Sorry, you lost me. What looks ugly?
Oh, this is against my comment above. Keeping regs[] <-> guest fpstate
in sync will be unnecessarily complex without clear usage (continues below).
>> If guest EGPR state is saved in vcpu->arch.regs[], the APX area there isn't
>> necessary:
>>
>> When the KVM API exposes state in XSAVE format, the frontend can handle this
>> separately. Alongside uABI <-> guest fpstate copy functions, new copy
>> functions may deal with the state between uABI <-> VCPU cache.
>>
>> Further, one could think of exclusion as such:
>>
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index 76153dfb58c9..5404f9399eea 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -794,9 +794,10 @@ static u64 __init guest_default_mask(void)
>> {
>> /*
>> * Exclude dynamic features, which require userspace opt-in even
>> - * for KVM guests.
>> + * for KVM guests, and APX as extended general-purpose register
>> + * states are saved in the KVM cache separately.
>> */
>> - return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
>> + return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_APX);
>> }
>>
>> But this default bitmask feeds into the permission bits:
>>
>> fpu->guest_perm.__state_perm = guest_default_cfg.features;
>> fpu->guest_perm.__state_size = guest_default_cfg.size;
>>
>> This policy looks clear and sensible: permission is granted only if space is
>> reserved to save the state. If there is a strong desire to save memory, I
>> think it should go through a more thorough review to revisit this policy.
>
> And I'm lost again.
Here I made myself pursuing the approach saving/restoring EGPRs via
regs[] on VM entry/exit. Then a couple of follow-up questions:
1. What about APX area in guest fpstate?
2. How to support the state for KVM ABI?
It surely departs from the "XSAVE - the single source of truth" model.
Then,
Leave the APX area in guest fpstate unused.
Copying APX state directly between regs[] and uABI to preserve XSAVE-
based ABI like in the attached diff.
That's all I'm saying.
[-- Attachment #2: kvmapi-apx.diff --]
[-- Type: text/plain, Size: 4470 bytes --]
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index fffbf087937d..b3ab2ac827e6 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -59,6 +59,16 @@ void __init kvm_init_xstate_sizes(void)
}
}
+u32 xstate_size(unsigned int xfeature)
+{
+ return xstate_sizes[xfeature].eax;
+}
+
+u32 xstate_offset(unsigned int xfeature)
+{
+ return xstate_sizes[xfeature].ebx;
+}
+
u32 xstate_required_size(u64 xstate_bv, bool compacted)
{
u32 ret = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET;
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 039b8e6f40ba..5ace99dd152b 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -64,6 +64,8 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
void __init kvm_init_xstate_sizes(void);
u32 xstate_required_size(u64 xstate_bv, bool compacted);
+u32 xstate_size(unsigned int xfeature);
+u32 xstate_offset(unsigned int xfeature);
int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu);
int cpuid_query_maxguestphyaddr(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c1e1b3030786..1f064a32b8b7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -108,6 +108,12 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_host);
#define emul_to_vcpu(ctxt) \
((struct kvm_vcpu *)(ctxt)->vcpu)
+#ifdef CONFIG_KVM_APX
+#define VCPU_EGPRS_PTR(vcpu) &(vcpu)->arch.regs[VCPU_REGS_R16]
+#else
+#define VCPU_EGPRS_PTR(vcpu) NULL
+#endif
+
/* EFER defaults:
* - enable syscall per default because its emulated by KVM
* - enable LME and LMA per default on 64 bit KVM
@@ -5804,10 +5810,33 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
return 0;
}
+static void kvm_copy_vcpu_regs_to_uabi(struct kvm_vcpu *vcpu, struct kvm_xsave *uabi_xsave)
+{
+ union fpregs_state *xstate = (union fpregs_state *)uabi_xsave->region;
+ void *uabi_apx = (void*)uabi_xsave->region + xstate_offset(XFEATURE_APX);
+ void *vcpu_egprs = VCPU_EGPRS_PTR(vcpu);
+
+ if (!vcpu_egprs)
+ return;
-static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
- u8 *state, unsigned int size)
+ memcpy(uabi_apx, vcpu_egprs, xstate_size(XFEATURE_APX));
+ xstate->xsave.header.xfeatures |= XFEATURE_MASK_APX;
+}
+
+static void kvm_copy_uabi_to_vcpu_regs(struct kvm_vcpu *vcpu, struct kvm_xsave *uabi_xsave)
{
+ union fpregs_state *xstate = (union fpregs_state *)uabi_xsave->region;
+ void *uabi_apx = (void*)uabi_xsave->region + xstate_offset(XFEATURE_APX);
+ void *vcpu_egprs = VCPU_EGPRS_PTR(vcpu);
+
+ if (vcpu_egprs && xstate->xsave.header.xfeatures & XFEATURE_MASK_APX)
+ memcpy(vcpu_egprs, uabi_apx, xstate_size(XFEATURE_APX));
+}
+
+static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, struct kvm_xsave *guest_xsave,
+ unsigned int size)
+{
+
/*
* Only copy state for features that are enabled for the guest. The
* state itself isn't problematic, but setting bits in the header for
@@ -5826,15 +5855,23 @@ static int kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
- fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, state, size,
+ /*
+ * The generic XSAVE copy function zeros out areas not present in
+ * guest fpstate. Those not in fpstate but in somewhere else,
+ * like EGPRs, should be copied after this.
+ */
+ fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, guest_xsave->region, size,
supported_xcr0, vcpu->arch.pkru);
+
+ kvm_copy_vcpu_regs_to_uabi(vcpu, guest_xsave);
+
return 0;
}
static int kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
struct kvm_xsave *guest_xsave)
{
- return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
+ return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, guest_xsave,
sizeof(guest_xsave->region));
}
@@ -5853,6 +5890,8 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
*/
xstate->xsave.header.xfeatures &= ~vcpu->arch.guest_fpu.fpstate->xfd;
+ kvm_copy_uabi_to_vcpu_regs(vcpu, guest_xsave);
+
return fpu_copy_uabi_to_guest_fpstate(&vcpu->arch.guest_fpu,
guest_xsave->region,
kvm_caps.supported_xcr0,
@@ -6464,7 +6503,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (!u.xsave)
break;
- r = kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.buffer, size);
+ r = kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.xsave, size);
if (r < 0)
break;
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox